Skip to content

Commit

Permalink
begin work on emberjs/rfcs#793
Browse files Browse the repository at this point in the history
  • Loading branch information
runspired committed Apr 16, 2022
1 parent cfd8268 commit 41d1c6f
Show file tree
Hide file tree
Showing 5 changed files with 250 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -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');
});
});
46 changes: 29 additions & 17 deletions packages/model/addon/-private/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
21 changes: 19 additions & 2 deletions packages/record-data/addon/-private/graph/-edge-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 41d1c6f

Please sign in to comment.