From 41d1c6f2b8cc0fd4785482c5038ddf84ef4f1c9c Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 15 Apr 2022 19:54:26 -0700 Subject: [PATCH] begin work on emberjs/rfcs#793 --- .../explicit-polymorphism-test.js | 192 ++++++++++++++++++ packages/model/addon/-private/model.js | 46 +++-- .../system/relationships/relationship-meta.ts | 4 +- .../addon/-private/graph/-edge-definition.ts | 21 +- .../operations/replace-related-record.ts | 8 +- 5 files changed, 250 insertions(+), 21 deletions(-) create mode 100644 packages/-ember-data/tests/integration/relationships/explicit-polymorphism-test.js diff --git a/packages/-ember-data/tests/integration/relationships/explicit-polymorphism-test.js b/packages/-ember-data/tests/integration/relationships/explicit-polymorphism-test.js new file mode 100644 index 00000000000..2616041c416 --- /dev/null +++ b/packages/-ember-data/tests/integration/relationships/explicit-polymorphism-test.js @@ -0,0 +1,192 @@ +import { module, test } from 'qunit'; + +import { setupTest } from 'ember-qunit'; + +import Model, { attr, belongsTo } from '@ember-data/model'; +import { recordIdentifierFor } from '@ember-data/store'; + +class FrameworkClass { + constructor(args) { + Object.assign(this, args); + } + + static create(args) { + return new this(args); + } +} + +module('Integration | Relationships | Explicit Polymorphism', function (hooks) { + setupTest(hooks); + + test('We can fetch a polymorphic belongsTo relationship with a null inverse', async function (assert) { + const { owner } = this; + const store = owner.lookup('service:store'); + + owner.register( + 'model:tag', + class extends Model { + @attr name; + @belongsTo('taggable', { async: false, inverse: null, polymorphic: true }) tagged; + } + ); + owner.register( + 'model:comment', + class extends Model { + @attr name; + } + ); + owner.register( + 'model:post', + class extends Model { + @attr name; + } + ); + + owner.register( + 'adapter:application', + class extends FrameworkClass { + findRecord(store, schema, id, snapshot) { + return { + data: { + type: 'comment', + id, + attributes: { + name: 'My Comment', + }, + }, + }; + } + } + ); + owner.register( + 'serializer:application', + class extends FrameworkClass { + normalizeResponse(_, __, data) { + return data; + } + } + ); + + const tag = store.push({ + data: { + type: 'tag', + id: '1', + attributes: { name: 'My Tag' }, + relationships: { + tagged: { + // we expect the store to not error on push for this unknown model name + data: { type: 'comment', id: '1' }, + }, + }, + }, + }); + + assert.strictEqual(tag.name, 'My Tag', 'We pushed the Tag'); + assert.strictEqual(tag.belongsTo('tagged').id(), '1', 'we have the data for the relationship'); + + await store.findRecord('comment', '1'); + + assert.strictEqual(tag.tagged.id, '1', 'we have the loaded comment'); + assert.strictEqual(tag.tagged.name, 'My Comment', 'Comment is correct'); + assert.strictEqual(tag.tagged.constructor.modelName, 'comment', 'model name is correct'); + const identifier = recordIdentifierFor(tag.tagged); + assert.strictEqual(identifier.type, 'comment', 'identifier type is correct'); + + // update the value + const post = store.push({ + data: { + type: 'post', + id: '1', + attributes: { name: 'My Post' }, + }, + }); + tag.tagged = post; + assert.strictEqual(tag.tagged.id, '1', 'we have the loaded post'); + assert.strictEqual(tag.tagged.name, 'My Post', 'Post is correct'); + assert.strictEqual(tag.tagged.constructor.modelName, 'post', 'model name is correct'); + const identifier2 = recordIdentifierFor(tag.tagged); + assert.strictEqual(identifier2.type, 'post', 'identifier type is correct'); + }); + + test('We can fetch a polymorphic belongsTo relationship with a specified inverse', async function (assert) { + const { owner } = this; + const store = owner.lookup('service:store'); + + owner.register( + 'model:tag', + class extends Model { + @attr name; + @belongsTo('taggable', { async: false, inverse: 'tag', polymorphic: true }) tagged; + } + ); + owner.register( + 'model:comment', + class extends Model { + @attr name; + @belongsTo('tag', { async: false, inverse: 'tagged', as: 'taggable' }) tag; + } + ); + owner.register( + 'model:post', + class extends Model { + @attr name; + @belongsTo('tag', { async: false, inverse: 'tagged', as: 'taggable' }) tag; + } + ); + + owner.register( + 'adapter:application', + class extends FrameworkClass { + findRecord(store, schema, id, snapshot) { + return { + data: { + type: 'comment', + id, + attributes: { + name: 'My Comment', + }, + }, + }; + } + } + ); + owner.register( + 'serializer:application', + class extends FrameworkClass { + normalizeResponse(_, __, data) { + return data; + } + } + ); + + const post = store.push({ + data: { + type: 'post', + id: '1', + attributes: { name: 'My Post' }, + relationships: { + tag: { + // we expect the store to not error on push for this unknown model name + data: { type: 'tag', id: '1' }, + }, + }, + }, + included: [ + { + type: 'tag', + id: '1', + attributes: { name: 'My Tag' }, + relationships: { + tagged: { + // we expect the store to not error on push for this unknown model name + data: { type: 'post', id: '1' }, + }, + }, + }, + ], + }); + const tag = store.peekRecord('tag', '1'); + + assert.strictEqual(post.tag, tag, 'we have a tag'); + }); +}); diff --git a/packages/model/addon/-private/model.js b/packages/model/addon/-private/model.js index 14b27a276dc..30fe3d98fe1 100644 --- a/packages/model/addon/-private/model.js +++ b/packages/model/addon/-private/model.js @@ -1204,6 +1204,9 @@ class Model extends EmberObject { @return {Model} the type of the relationship, or undefined */ static typeForRelationship(name, store) { + if (!store.getSchemaDefinitionService().doesTypeExist(name)) { + return null; + } let relationship = this.relationshipsByName.get(name); return relationship && store.modelFor(relationship.type); } @@ -1261,11 +1264,13 @@ class Model extends EmberObject { //Calculate the inverse, ignoring the cache static _findInverseFor(name, store) { let inverseType = this.typeForRelationship(name, store); - if (!inverseType) { + let propertyMeta = this.metaForProperty(name); + + if (!inverseType && !propertyMeta.options.polymorphic) { + // TODO still error? return null; } - let propertyMeta = this.metaForProperty(name); //If inverse is manually specified to be null, like `comments: hasMany('message', { inverse: null })` let options = propertyMeta.options; if (options.inverse === null) { @@ -1275,23 +1280,25 @@ class Model extends EmberObject { let inverseName, inverseKind, inverse, inverseOptions; //If inverse is specified manually, return the inverse - if (options.inverse) { + if ((!options.polymorphic || inverseType) && options.inverse) { inverseName = options.inverse; - inverse = inverseType.relationshipsByName.get(inverseName); + inverse = inverseType && inverseType.relationshipsByName.get(inverseName); - assert( - "We found no inverse relationships by the name of '" + - inverseName + - "' on the '" + - inverseType.modelName + - "' model. This is most likely due to a missing attribute on your model definition.", - !isNone(inverse) - ); + if (!options.polymorphic) { + assert( + "We found no inverse relationships by the name of '" + + inverseName + + "' on the '" + + inverseType.modelName + + "' model. This is most likely due to a missing attribute on your model definition.", + !isNone(inverse) + ); + } // TODO probably just return the whole inverse here inverseKind = inverse.kind; inverseOptions = inverse.options; - } else { + } else if (!options.inverse) { //No inverse was specified manually, we need to use a heuristic to guess one if (propertyMeta.type === propertyMeta.parentModelName) { warn( @@ -1347,10 +1354,15 @@ class Model extends EmberObject { inverseOptions = possibleRelationships[0].options; } - assert( - `The ${inverseType.modelName}:${inverseName} relationship declares 'inverse: null', but it was resolved as the inverse for ${this.modelName}:${name}.`, - !inverseOptions || inverseOptions.inverse !== null - ); + if (inverseOptions?.polymorphic) { + // validate + assert(`these should match`, !!options.as && inverse.type === options.as); + } else { + assert( + `The ${inverseType.modelName}:${inverseName} relationship declares 'inverse: null', but it was resolved as the inverse for ${this.modelName}:${name}.`, + !inverseOptions || inverseOptions.inverse !== null + ); + } return { type: inverseType, diff --git a/packages/model/addon/-private/system/relationships/relationship-meta.ts b/packages/model/addon/-private/system/relationships/relationship-meta.ts index 2349d3dbbaa..1ebf3e77ee7 100644 --- a/packages/model/addon/-private/system/relationships/relationship-meta.ts +++ b/packages/model/addon/-private/system/relationships/relationship-meta.ts @@ -88,7 +88,9 @@ export class RelationshipDefinition implements RelationshipSchema { if (shouldFindInverse(this.meta)) { inverse = modelClass.inverseFor(this.key, store); - } else if (DEBUG) { + } + // TODO make this error again for the non-polymorphic case + if (DEBUG && !this.options.polymorphic) { modelClass.typeForRelationship(this.key, store); } diff --git a/packages/record-data/addon/-private/graph/-edge-definition.ts b/packages/record-data/addon/-private/graph/-edge-definition.ts index 8c301c55da0..5ae7c77f45a 100644 --- a/packages/record-data/addon/-private/graph/-edge-definition.ts +++ b/packages/record-data/addon/-private/graph/-edge-definition.ts @@ -171,13 +171,30 @@ export function upgradeDefinition( // CASE: Inverse is explicitly null if (definition.inverseKey === null) { + // TODO probably dont need this assertion if polymorphic assert(`Expected the inverse model to exist`, storeWrapper._store.modelFor(inverseType)); inverseDefinition = null; } else { inverseKey = storeWrapper.inverseForRelationship(type, propertyName); - // CASE: Inverse resolves to null - if (!inverseKey) { + // 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 + // concrete type as the baseType, so we must construct and artificial + // placeholder + if (!inverseKey && definition.isPolymorphic && definition.inverseKey) { + inverseDefinition = { + kind: 'belongsTo', // this must be updated when we find the first belongsTo or hasMany definition that matches + key: definition.inverseKey, + type: type, + isAsync: false, // this must be updated when we find the first belongsTo or hasMany definition that matches + isImplicit: false, + isCollection: false, // this must be updated when we find the first belongsTo or hasMany definition that matches + isPolymorphic: false, + isInitialized: false, // tracks whether we have seen the other side at least once + }; + + // CASE: Inverse resolves to null + } else if (!inverseKey) { inverseDefinition = null; } else { // CASE: We have an explicit inverse or were able to resolve one diff --git a/packages/record-data/addon/-private/graph/operations/replace-related-record.ts b/packages/record-data/addon/-private/graph/operations/replace-related-record.ts index 48e12ddc8bb..a4a7d35e606 100644 --- a/packages/record-data/addon/-private/graph/operations/replace-related-record.ts +++ b/packages/record-data/addon/-private/graph/operations/replace-related-record.ts @@ -98,7 +98,13 @@ export default function replaceRelatedRecord(graph: Graph, op: ReplaceRelatedRec if (op.value) { if (definition.type !== op.value.type) { - assertPolymorphicType(relationship.identifier, definition, op.value, graph.store); + if (!definition.isPolymorphic) { + // TODO this should now handle the deprecation warning if isPolymorphic is not set + // but the record does turn out to be polymorphic + // this should still assert if the user is relying on legacy inheritance/mixins to + // provide polymorphic behavior and has not yet added the polymorphic flags + assertPolymorphicType(relationship.identifier, definition, op.value, graph.store); + } graph.registerPolymorphicType(definition.type, op.value.type); } addToInverse(graph, op.value, definition.inverseKey, op.record, isRemote);