Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: refactor IdentityCache to make resource more opaque #8736

Merged
merged 9 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,6 @@ module.exports = {
'packages/store/src/-private/store-service.ts',
'packages/store/src/-private/utils/coerce-id.ts',
'packages/store/src/-private/index.ts',
'packages/store/src/-private/caches/identifier-cache.ts',
'packages/serializer/src/index.ts',
'@types/@ember/runloop/index.d.ts',
'@types/@ember/polyfills/index.d.ts',
'tests/graph/tests/integration/graph/polymorphism/implicit-keys-test.ts',
Expand Down
2 changes: 1 addition & 1 deletion ember-data-types/q/identifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export type StableRecordIdentifier = StableExistingRecordIdentifier | StableNewR
*/
export interface GenerationMethod {
(data: ImmutableRequestInfo, bucket: 'document'): string | null;
(data: ResourceData | { type: string }, bucket: 'record'): string;
(data: unknown | { type: string }, bucket: 'record'): string;
(data: unknown, bucket: IdentifierBucket): string | null;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/debug/addon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export default class extends DataAdapter {
}];
const discoveredTypes = typesMapFor(store);

Object.keys(store.identifierCache._cache.types).forEach((type) => {
Object.keys(store.identifierCache._cache.resourcesByType).forEach((type) => {
discoveredTypes.set(type, false);
});

Expand Down
10 changes: 5 additions & 5 deletions packages/graph/src/-private/graph/-edge-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ export function upgradeDefinition(
const polymorphicLookup = graph._potentialPolymorphicTypes;

const { type } = identifier;
let cached = expandingGet<EdgeDefinition | null>(cache, type, propertyName);
let cached = /*#__NOINLINE__*/ expandingGet<EdgeDefinition | null>(cache, type, propertyName);

// CASE: We have a cached resolution (null if no relationship exists)
if (cached !== undefined) {
Expand All @@ -357,7 +357,7 @@ export function upgradeDefinition(
for (let i = 0; i < altTypes.length; i++) {
const _cached = expandingGet<EdgeDefinition | null>(cache, altTypes[i], propertyName);
if (_cached) {
expandingSet<EdgeDefinition | null>(cache, type, propertyName, _cached);
/*#__NOINLINE__*/ expandingSet<EdgeDefinition | null>(cache, type, propertyName, _cached);
_cached.rhs_modelNames.push(type);
return _cached;
}
Expand All @@ -371,7 +371,7 @@ export function upgradeDefinition(
cache[type]![propertyName] = null;
return null;
}
const definition = upgradeMeta(meta);
const definition = /*#__NOINLINE__*/ upgradeMeta(meta);

let inverseDefinition;
let inverseKey;
Expand All @@ -383,7 +383,7 @@ export function upgradeDefinition(
assert(`Expected the inverse model to exist`, getStore(storeWrapper).modelFor(inverseType));
inverseDefinition = null;
} else {
inverseKey = inverseForRelationship(getStore(storeWrapper), identifier, propertyName);
inverseKey = /*#__NOINLINE__*/ inverseForRelationship(getStore(storeWrapper), identifier, propertyName);

// CASE: If we are polymorphic, and we declared an inverse that is non-null
// we must assume that the lack of inverseKey means that there is no
Expand Down Expand Up @@ -423,7 +423,7 @@ export function upgradeDefinition(
// CASE: We have no inverse
if (!inverseDefinition) {
// polish off meta
inverseKey = implicitKeyFor(type, propertyName);
inverseKey = /*#__NOINLINE__*/ implicitKeyFor(type, propertyName);
inverseDefinition = {
kind: 'implicit',
key: inverseKey,
Expand Down
46 changes: 27 additions & 19 deletions packages/graph/src/-private/graph/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class Graph {

let relationship = relationships[propertyName];
if (!relationship) {
const info = upgradeDefinition(this, identifier, propertyName);
const info = /*#__NOINLINE__*/ upgradeDefinition(this, identifier, propertyName);
assert(`Could not determine relationship information for ${identifier.type}.${propertyName}`, info !== null);

if (info.rhs_definition?.kind === 'implicit') {
Expand All @@ -122,7 +122,9 @@ export class Graph {
// this.registerPolymorphicType(info.rhs_baseModelName, identifier.type);
}

const meta = isLHS(info, identifier.type, propertyName) ? info.lhs_definition : info.rhs_definition!;
const meta = /*#__NOINLINE__*/ isLHS(info, identifier.type, propertyName)
? info.lhs_definition
: info.rhs_definition!;

if (meta.kind !== 'implicit') {
const Klass = meta.kind === 'hasMany' ? ManyRelationship : BelongsToRelationship;
Expand Down Expand Up @@ -227,8 +229,8 @@ export class Graph {
if (!rel) {
return;
}
destroyRelationship(this, rel, silenceNotifications);
if (isImplicit(rel)) {
/*#__NOINLINE__*/ destroyRelationship(this, rel, silenceNotifications);
if (/*#__NOINLINE__*/ isImplicit(rel)) {
// @ts-expect-error
relationships[key] = undefined;
}
Expand Down Expand Up @@ -293,7 +295,7 @@ export class Graph {
case 'mergeIdentifiers': {
const relationships = this.identifiers.get(op.record);
if (relationships) {
mergeIdentifier(this, op, relationships);
/*#__NOINLINE__*/ mergeIdentifier(this, op, relationships);
}
break;
}
Expand All @@ -304,7 +306,7 @@ export class Graph {
// TODO add deprecations/assertion here for duplicates
assertValidRelationshipPayload(this, op);
}
updateRelationshipOperation(this, op);
/*#__NOINLINE__*/ updateRelationshipOperation(this, op);
break;
case 'deleteRecord': {
assert(`Can only perform the operation deleteRelationship on remote state`, isRemote);
Expand All @@ -320,23 +322,23 @@ export class Graph {
// works together with the has check
// @ts-expect-error
relationships[key] = undefined;
removeCompletelyFromInverse(this, rel);
/*#__NOINLINE__*/ removeCompletelyFromInverse(this, rel);
});
this.identifiers.delete(identifier);
}
break;
}
case 'replaceRelatedRecord':
replaceRelatedRecord(this, op, isRemote);
/*#__NOINLINE__*/ replaceRelatedRecord(this, op, isRemote);
break;
case 'addToRelatedRecords':
addToRelatedRecords(this, op, isRemote);
/*#__NOINLINE__*/ addToRelatedRecords(this, op, isRemote);
break;
case 'removeFromRelatedRecords':
removeFromRelatedRecords(this, op, isRemote);
/*#__NOINLINE__*/ removeFromRelatedRecords(this, op, isRemote);
break;
case 'replaceRelatedRecords':
replaceRelatedRecords(this, op, isRemote);
/*#__NOINLINE__*/ replaceRelatedRecords(this, op, isRemote);
break;
default:
assert(`No local relationship update operation exists for '${op.op}'`);
Expand Down Expand Up @@ -410,7 +412,7 @@ export class Graph {
this._willSyncLocal = false;
let updated = this._updatedRelationships;
this._updatedRelationships = new Set();
updated.forEach((rel) => syncRemoteToLocal(this, rel));
updated.forEach((rel) => /*#__NOINLINE__*/ syncRemoteToLocal(this, rel));
}

destroy() {
Expand Down Expand Up @@ -446,7 +448,7 @@ export class Graph {
function destroyRelationship(graph: Graph, rel: RelationshipEdge, silenceNotifications?: boolean) {
if (isImplicit(rel)) {
if (graph.isReleasable(rel.identifier)) {
removeCompletelyFromInverse(graph, rel);
/*#__NOINLINE__*/ removeCompletelyFromInverse(graph, rel);
}
return;
}
Expand All @@ -455,14 +457,20 @@ function destroyRelationship(graph: Graph, rel: RelationshipEdge, silenceNotific
const { inverseKey } = rel.definition;

if (!rel.definition.inverseIsImplicit) {
forAllRelatedIdentifiers(rel, (inverseIdentifer: StableRecordIdentifier) =>
notifyInverseOfDematerialization(graph, inverseIdentifer, inverseKey, identifier, silenceNotifications)
/*#__NOINLINE__*/ forAllRelatedIdentifiers(rel, (inverseIdentifer: StableRecordIdentifier) =>
/*#__NOINLINE__*/ notifyInverseOfDematerialization(
graph,
inverseIdentifer,
inverseKey,
identifier,
silenceNotifications
)
);
}

if (!rel.definition.inverseIsImplicit && !rel.definition.inverseIsAsync) {
rel.state.isStale = true;
clearRelationship(rel);
/*#__NOINLINE__*/ clearRelationship(rel);

// necessary to clear relationships in the ui from dematerialized records
// hasMany is managed by Model which calls `retreiveLatest` after
Expand All @@ -473,7 +481,7 @@ function destroyRelationship(graph: Graph, rel: RelationshipEdge, silenceNotific
// leave the ui relationship populated since the record is destroyed and
// internally we've fully cleaned up.
if (!rel.definition.isAsync && !silenceNotifications) {
notifyChange(graph, rel.identifier, rel.definition.key);
/*#__NOINLINE__*/ notifyChange(graph, rel.identifier, rel.definition.key);
}
}
}
Expand All @@ -495,7 +503,7 @@ function notifyInverseOfDematerialization(
// For remote members, it is possible that inverseRecordData has already been associated to
// to another record. For such cases, do not dematerialize the inverseRecordData
if (!isBelongsTo(relationship) || !relationship.localState || identifier === relationship.localState) {
removeDematerializedInverse(
/*#__NOINLINE__*/ removeDematerializedInverse(
graph,
relationship as BelongsToRelationship | ManyRelationship,
identifier,
Expand Down Expand Up @@ -558,7 +566,7 @@ function removeDematerializedInverse(
// cache.
// if the record being unloaded only exists on the client, we similarly
// treat it as a client side delete
removeIdentifierCompletelyFromRelationship(graph, relationship, inverseIdentifier);
/*#__NOINLINE__*/ removeIdentifierCompletelyFromRelationship(graph, relationship, inverseIdentifier);
} else {
relationship.state.hasDematerializedInverse = true;
}
Expand Down
31 changes: 17 additions & 14 deletions packages/json-api/src/-private/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { LOG_MUTATIONS, LOG_OPERATIONS } from '@ember-data/debugging';
import { DEBUG } from '@ember-data/env';
import { graphFor, peekGraph } from '@ember-data/graph/-private';
import type { LocalRelationshipOperation } from '@ember-data/graph/-private/graph/-operations';
import type { Graph } from '@ember-data/graph/-private/graph/graph';
import type { ImplicitRelationship } from '@ember-data/graph/-private/graph/index';
import type BelongsToRelationship from '@ember-data/graph/-private/relationships/state/belongs-to';
import type ManyRelationship from '@ember-data/graph/-private/relationships/state/has-many';
Expand Down Expand Up @@ -118,11 +119,13 @@ export default class JSONAPICache implements Cache {
declare __cache: Map<StableRecordIdentifier, CachedResource>;
declare __destroyedCache: Map<StableRecordIdentifier, CachedResource>;
declare __documents: Map<string, StructuredDocument<ResourceDocument>>;
declare __graph: Graph;

constructor(storeWrapper: CacheCapabilitiesManager) {
this.version = '2';
this.__storeWrapper = storeWrapper;
this.__cache = new Map();
this.__graph = graphFor(storeWrapper);
this.__destroyedCache = new Map();
this.__documents = new Map();
}
Expand Down Expand Up @@ -285,7 +288,7 @@ export default class JSONAPICache implements Cache {
this.__cache.set(op.value, cache);
this.__cache.delete(op.record);
}
graphFor(this.__storeWrapper).update(op, true);
this.__graph.update(op, true);
}
}

Expand All @@ -308,7 +311,7 @@ export default class JSONAPICache implements Cache {
console.log(`EmberData | Mutation - update ${mutation.op}`, mutation);
}
}
graphFor(this.__storeWrapper).update(mutation, false);
this.__graph.update(mutation, false);
}

/**
Expand Down Expand Up @@ -357,8 +360,7 @@ export default class JSONAPICache implements Cache {
const attributes = Object.assign({}, peeked.remoteAttrs, peeked.inflightAttrs, peeked.localAttrs);
const relationships = {};

const graph = graphFor(this.__storeWrapper);
const rels = graph.identifiers.get(identifier);
const rels = this.__graph.identifiers.get(identifier);
if (rels) {
Object.keys(rels).forEach((key) => {
const rel = rels[key]!;
Expand Down Expand Up @@ -418,8 +420,8 @@ export default class JSONAPICache implements Cache {
const existed = !!peeked;
const cached = peeked || this._createCache(identifier);

const isLoading = _isLoading(peeked, this.__storeWrapper, identifier) || !recordIsLoaded(peeked);
let isUpdate = !_isEmpty(peeked) && !isLoading;
const isLoading = /*#__NOINLINE__*/ _isLoading(peeked, this.__storeWrapper, identifier) || !recordIsLoaded(peeked);
let isUpdate = /*#__NOINLINE__*/ !_isEmpty(peeked) && !isLoading;

if (LOG_OPERATIONS) {
try {
Expand Down Expand Up @@ -458,7 +460,7 @@ export default class JSONAPICache implements Cache {
}

if (data.relationships) {
setupRelationships(this.__storeWrapper, identifier, data);
setupRelationships(this.__graph, this.__storeWrapper, identifier, data);
}

if (changedKeys && changedKeys.length) {
Expand Down Expand Up @@ -612,7 +614,7 @@ export default class JSONAPICache implements Cache {
const storeWrapper = this.__storeWrapper;
let attributeDefs = storeWrapper.getSchemaDefinitionService().attributesDefinitionFor(identifier);
let relationshipDefs = storeWrapper.getSchemaDefinitionService().relationshipsDefinitionFor(identifier);
const graph = graphFor(storeWrapper);
const graph = this.__graph;
let propertyNames = Object.keys(options);

for (let i = 0; i < propertyNames.length; i++) {
Expand Down Expand Up @@ -713,7 +715,7 @@ export default class JSONAPICache implements Cache {

const cached = this.__peek(identifier, false);
if (cached.isDeleted) {
graphFor(this.__storeWrapper).push({
this.__graph.push({
op: 'deleteRecord',
record: identifier,
isNew: false,
Expand Down Expand Up @@ -750,7 +752,7 @@ export default class JSONAPICache implements Cache {
);

if (data.relationships) {
setupRelationships(this.__storeWrapper, identifier, data);
setupRelationships(this.__graph, this.__storeWrapper, identifier, data);
}
newCanonicalAttributes = data.attributes;
}
Expand Down Expand Up @@ -995,7 +997,7 @@ export default class JSONAPICache implements Cache {
}

if (cached.isNew) {
graphFor(this.__storeWrapper).push({
this.__graph.push({
op: 'deleteRecord',
record: identifier,
isNew: true,
Expand Down Expand Up @@ -1033,7 +1035,7 @@ export default class JSONAPICache implements Cache {
identifier: StableRecordIdentifier,
field: string
): SingleResourceRelationship | CollectionResourceRelationship {
return (graphFor(this.__storeWrapper).get(identifier, field) as BelongsToRelationship | ManyRelationship).getData();
return (this.__graph.get(identifier, field) as BelongsToRelationship | ManyRelationship).getData();
}

// Resource State
Expand All @@ -1055,7 +1057,7 @@ export default class JSONAPICache implements Cache {
cached.isDeleted = isDeleted;
if (cached.isNew) {
// TODO can we delete this since we will do this in unload?
graphFor(this.__storeWrapper).push({
this.__graph.push({
op: 'deleteRecord',
record: identifier,
isNew: true,
Expand Down Expand Up @@ -1329,6 +1331,7 @@ function _isLoading(
}

function setupRelationships(
graph: Graph,
storeWrapper: CacheCapabilitiesManager,
identifier: StableRecordIdentifier,
data: JsonApiResource
Expand All @@ -1347,7 +1350,7 @@ function setupRelationships(
continue;
}

graphFor(storeWrapper).push({
graph.push({
op: 'updateRelationship',
record: identifier,
field: relationshipName,
Expand Down
Loading