Skip to content

Commit

Permalink
[BUGFIX canary]: Remove the last Symbols (#6428)
Browse files Browse the repository at this point in the history
* [BUGIFX canary] Replace IS_IDENTIFIER Symbol..

..by IDENTIFIERS WeakMap

* [BUGFIX canary]: Switch Symbol to symbol

* Relocate IDENTIFIERS

* Add unmarkStableIdentifier() to remove..

identifier from the WeakMap when it is removed from the identifier
cache.

* [TESTS] Add cache integration test
  • Loading branch information
dcyriller authored and runspired committed Sep 13, 2019
1 parent 0f59603 commit 88dcf5f
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 11 deletions.
55 changes: 55 additions & 0 deletions packages/-ember-data/tests/integration/identifiers/cache-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { IDENTIFIERS } from '@ember-data/canary-features';
import Store from '@ember-data/store';
import { identifierCacheFor } from '@ember-data/store/-private';

if (IDENTIFIERS) {
module('Integration | Identifiers - cache', function(hooks) {
setupTest(hooks);
let store, cache;

hooks.beforeEach(function() {
this.owner.register(`service:store`, Store);
store = this.owner.lookup('service:store');
cache = identifierCacheFor(store);
});

module('getOrCreateRecordIdentifier()', function() {
test('creates a new resource identifier if forgetRecordIdentifier() has been called on the existing identifier', async function(assert) {
const runspiredHash = {
type: 'person',
id: '1',
attributes: {
name: 'runspired',
},
};
const identifier = cache.getOrCreateRecordIdentifier(runspiredHash);

cache.forgetRecordIdentifier(identifier);

const regeneratedIdentifier = cache.getOrCreateRecordIdentifier(runspiredHash);

assert.notEqual(identifier, regeneratedIdentifier, 'a record get a new identifier if identifier get forgotten');
});

test('returns the existing identifier when called with an identifier', async function(assert) {
const houseHash = {
type: 'house',
id: '1',
attributes: {
name: 'Moomin',
},
};
const cache = identifierCacheFor(store);
const identifier = cache.getOrCreateRecordIdentifier(houseHash);

assert.equal(
identifier,
cache.getOrCreateRecordIdentifier(identifier),
'getOrCreateRecordIdentifier() return identifier'
);
});
});
});
}
8 changes: 4 additions & 4 deletions packages/store/addon/-private/identifiers/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Dict } from '../ts-interfaces/utils';
import { ResourceIdentifierObject, ExistingResourceObject } from '../ts-interfaces/ember-data-json-api';
import {
StableRecordIdentifier,
IS_IDENTIFIER,
DEBUG_CLIENT_ORIGINATED,
DEBUG_IDENTIFIER_BUCKET,
GenerationMethod,
Expand All @@ -16,7 +15,7 @@ import {
import coerceId from '../system/coerce-id';
import uuidv4 from './utils/uuid-v4';
import normalizeModelName from '../system/normalize-model-name';
import isStableIdentifier from './is-stable-identifier';
import isStableIdentifier, { markStableIdentifier, unmarkStableIdentifier } from './is-stable-identifier';
import isNonEmptyString from '../utils/is-non-empty-string';
import Store from '../system/ds-model-store';
import CoreStore from '../system/core-store';
Expand Down Expand Up @@ -385,6 +384,7 @@ export class IdentifierCache {
let index = keyOptions._allIdentifiers.indexOf(identifier);
keyOptions._allIdentifiers.splice(index, 1);

unmarkStableIdentifier(identifierObject);
this._forget(identifier, 'record');
}

Expand Down Expand Up @@ -416,17 +416,16 @@ function makeStableRecordIdentifier(
clientOriginated: boolean = false
): Readonly<StableRecordIdentifier> {
let recordIdentifier = {
[IS_IDENTIFIER]: true as const,
lid,
id,
type,
};
markStableIdentifier(recordIdentifier);

if (DEBUG) {
// we enforce immutability in dev
// but preserve our ability to do controlled updates to the reference
let wrapper = Object.freeze({
[IS_IDENTIFIER]: true as const,
[DEBUG_CLIENT_ORIGINATED]: clientOriginated,
[DEBUG_IDENTIFIER_BUCKET]: bucket,
get lid() {
Expand All @@ -443,6 +442,7 @@ function makeStableRecordIdentifier(
return `${clientOriginated ? '[CLIENT_ORIGINATED] ' : ''}${type}:${id} (${lid})`;
},
});
markStableIdentifier(wrapper);
DEBUG_MAP.set(wrapper, recordIdentifier);
return wrapper;
}
Expand Down
16 changes: 13 additions & 3 deletions packages/store/addon/-private/identifiers/is-stable-identifier.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import { StableRecordIdentifier, IS_IDENTIFIER } from '../ts-interfaces/identifier';
import { StableRecordIdentifier } from '../ts-interfaces/identifier';

/**
@module @ember-data/store
*/

export default function isStableIdentifier(identifier: any): identifier is StableRecordIdentifier {
return identifier[IS_IDENTIFIER] === true;
const IDENTIFIERS = new WeakMap();

export default function isStableIdentifier(identifier: Object): identifier is StableRecordIdentifier {
return IDENTIFIERS.has(identifier);
}

export function markStableIdentifier(identifier: Object) {
IDENTIFIERS.set(identifier, 'is-identifier');
}

export function unmarkStableIdentifier(identifier: Object) {
IDENTIFIERS.delete(identifier);
}
7 changes: 3 additions & 4 deletions packages/store/addon/-private/ts-interfaces/identifier.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/**
@module @ember-data/store
*/
export const IS_IDENTIFIER = Symbol('is-identifier');
import { symbol } from '../ts-interfaces/utils/symbol';

// provided for additional debuggability
export const DEBUG_CLIENT_ORIGINATED = Symbol('record-originated-on-client');
export const DEBUG_IDENTIFIER_BUCKET = Symbol('identifier-bucket');
export const DEBUG_CLIENT_ORIGINATED: unique symbol = symbol('record-originated-on-client');
export const DEBUG_IDENTIFIER_BUCKET: unique symbol = symbol('identifier-bucket');

export interface Identifier {
lid: string;
Expand All @@ -32,7 +32,6 @@ export interface RecordIdentifier extends Identifier {
* @internal
*/
export interface StableIdentifier extends Identifier {
[IS_IDENTIFIER]: true;
[DEBUG_IDENTIFIER_BUCKET]?: string;
}

Expand Down

0 comments on commit 88dcf5f

Please sign in to comment.