Skip to content

Commit

Permalink
fix: backports polymorphism and app-config propagation fixes (#8636)
Browse files Browse the repository at this point in the history
* work on backporting polymophism fixes

* fix tests

* fix: ensure deprecation configs are threaded to each package
  • Loading branch information
runspired authored Jun 29, 2023
1 parent 899d847 commit de6b66c
Show file tree
Hide file tree
Showing 25 changed files with 964 additions and 139 deletions.
4 changes: 2 additions & 2 deletions ember-data-types/q/schema-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ export interface SchemaService {
* Additionally the following options properties are optional. See [Polymorphic Relationships](https://rfcs.emberjs.com/id/0793-polymporphic-relations-without-inheritance)
*
* - `options.polymorphic` a boolean representing whether multiple resource types
* can be used to satisfy this relationship.
* can be used to satisfy this relationship.
* - `options.as` a string representing the abstract type that the concrete side of
* a relationship must specify when fulfilling a polymorphic inverse.
* a relationship must specify when fulfilling a polymorphic inverse.
*
* For example, the following Model using @ember-data/model would generate this relationships
* definition by default:
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter/addon-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ module.exports = {
const ownConfig = this.options['@embroider/macros'].setOwnConfig;
ownConfig.compatWith = hostOptions.compatWith || null;
ownConfig.debug = debugOptions;
ownConfig.deprecations = Object.assign(DEPRECATIONS, ownConfig.deprecations || {});
ownConfig.deprecations = Object.assign(DEPRECATIONS, ownConfig.deprecations || {}, hostOptions.deprecations || {});
ownConfig.features = Object.assign({}, FEATURES);
ownConfig.includeDataAdapter = includeDataAdapter;
ownConfig.packages = MACRO_PACKAGE_FLAGS;
Expand Down
2 changes: 1 addition & 1 deletion packages/graph/addon-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ module.exports = {
const ownConfig = this.options['@embroider/macros'].setOwnConfig;
ownConfig.compatWith = hostOptions.compatWith || null;
ownConfig.debug = debugOptions;
ownConfig.deprecations = Object.assign(DEPRECATIONS, ownConfig.deprecations || {}, hostOptions || {});
ownConfig.deprecations = Object.assign(DEPRECATIONS, ownConfig.deprecations || {}, hostOptions.deprecations || {});
ownConfig.features = Object.assign({}, FEATURES);
ownConfig.includeDataAdapter = includeDataAdapter;
ownConfig.packages = MACRO_PACKAGE_FLAGS;
Expand Down
188 changes: 182 additions & 6 deletions packages/graph/src/-private/debug/assert-polymorphic-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { DEPRECATE_NON_EXPLICIT_POLYMORPHISM } from '@ember-data/deprecations';
`record.relationshipFor(key)`.
*/
let assertPolymorphicType;
let assertInheritedSchema;

if (DEBUG) {
let checkPolymorphic = function checkPolymorphic(modelClass, addedModelClass) {
Expand All @@ -26,10 +27,134 @@ if (DEBUG) {
modelClass.__mixin.detect(Object.getPrototypeOf(addedModelClass).PrototypeMixin)
);
}

return addedModelClass.prototype instanceof modelClass || modelClass.detect(addedModelClass);
};

function validateSchema(definition, meta) {
const errors = new Map();

if (definition.inverseKey !== meta.name) {
errors.set('name', ` <---- should be '${definition.inverseKey}'`);
}
if (definition.inverseType !== meta.type) {
errors.set('type', ` <---- should be '${definition.inverseType}'`);
}
if (definition.inverseKind !== meta.kind) {
errors.set('type', ` <---- should be '${definition.inverseKind}'`);
}
if (definition.inverseIsAsync !== meta.options.async) {
errors.set('async', ` <---- should be ${definition.inverseIsAsync}`);
}
if (definition.inverseIsPolymorphic && definition.inverseIsPolymorphic !== meta.options.polymorphic) {
errors.set('polymorphic', ` <---- should be ${definition.inverseIsPolymorphic}`);
}
if (definition.key !== meta.options.inverse) {
errors.set('inverse', ` <---- should be '${definition.key}'`);
}
if (definition.type !== meta.options.as) {
errors.set('as', ` <---- should be '${definition.type}'`);
}

return errors;
}

function expectedSchema(definition) {
return printSchema({
name: definition.inverseKey,
type: definition.inverseType,
kind: definition.inverseKind,
options: {
as: definition.type,
async: definition.inverseIsAsync,
polymorphic: definition.inverseIsPolymorphic || false,
inverse: definition.key
}
});
}

function printSchema(config, errors) {
return `
\`\`\`
{
${config.name}: {
name: '${config.name}',${errors?.get('name') || ''}
type: '${config.type}',${errors?.get('type') || ''}
kind: '${config.kind}',${errors?.get('kind') || ''}
options: {
as: '${config.options.as}',${errors?.get('as') || ''}
async: ${config.options.async},${errors?.get('async') || ''}
polymorphic: ${config.options.polymorphic},${errors?.get('polymorphic') || ''}
inverse: '${config.options.inverse}'${errors?.get('inverse') || ''}
}
}
}
\`\`\`
`
}

function metaFrom(definition) {
return {
name: definition.key,
type: definition.type,
kind: definition.kind,
options: {
async: definition.isAsync,
polymorphic: definition.isPolymorphic,
inverse: definition.inverseKey
}
};
}
function inverseMetaFrom(definition) {
return {
name: definition.inverseKey,
type: definition.inverseType,
kind: definition.inverseKind,
options: {
as: definition.isPolymorphic ? definition.type : undefined,
async: definition.inverseIsAsync,
polymorphic: definition.inverseIsPolymorphic,
inverse: definition.key
}
};
}
function inverseDefinition(definition) {
return {
key: definition.inverseKey,
type: definition.inverseType,
kind: definition.inverseKind,
isAsync: definition.inverseIsAsync,
isPolymorphic: true,
inverseKey: definition.key,
inverseType: definition.type,
inverseKind: definition.kind,
inverseIsAsync: definition.isAsync,
inverseIsPolymorphic: definition.isPolymorphic
};
}
function definitionWithPolymorphic(definition) {
return Object.assign({}, definition, { inverseIsPolymorphic: true });
}

assertInheritedSchema = function assertInheritedSchema(definition, type) {
let meta1 = metaFrom(definition);
let meta2 = inverseMetaFrom(definition);
let errors1 = validateSchema(inverseDefinition(definition), meta1);
let errors2 = validateSchema(definitionWithPolymorphic(definition), meta2);

if (errors2.size === 0 && errors1.size > 0) {
throw new Error(`The schema for the relationship '${type}.${definition.key}' is not configured to satisfy '${definition.inverseType}' and thus cannot utilize the '${definition.inverseType}.${definition.key}' relationship to connect with '${definition.type}.${definition.inverseKey}'\n\nIf using this relationship in a polymorphic manner is desired, the relationships schema definition for '${type}' should include:${printSchema(meta1, errors1)}`);

} else if (errors1.size > 0) {
throw new Error(`The schema for the relationship '${type}.${definition.key}' is not configured to satisfy '${definition.inverseType}' and thus cannot utilize the '${definition.inverseType}.${definition.key}' relationship to connect with '${definition.type}.${definition.inverseKey}'\n\nIf using this relationship in a polymorphic manner is desired, the relationships schema definition for '${type}' should include:${printSchema(meta1, errors1)} and the relationships schema definition for '${definition.type}' should include:${printSchema(meta2, errors2)}`);

} else if (errors2.size > 0) {
throw new Error(`The schema for the relationship '${type}.${definition.key}' satisfies '${definition.inverseType}' but cannot utilize the '${definition.inverseType}.${definition.key}' relationship to connect with '${definition.type}.${definition.inverseKey}' because that relationship is not polymorphic.\n\nIf using this relationship in a polymorphic manner is desired, the relationships schema definition for '${definition.type}' should include:${printSchema(meta2, errors2)}`);

}
}

assertPolymorphicType = function assertPolymorphicType(parentIdentifier, parentDefinition, addedIdentifier, store) {
let asserted = false;

Expand All @@ -40,12 +165,63 @@ if (DEBUG) {
let meta = store.getSchemaDefinitionService().relationshipsDefinitionFor(addedIdentifier)[
parentDefinition.inverseKey
];
if (meta?.options?.as) {
asserted = true;
if (DEPRECATE_NON_EXPLICIT_POLYMORPHISM) {
if (meta?.options?.as) {
asserted = true;
assert(`No '${parentDefinition.inverseKey}' field exists on '${addedIdentifier.type}'. To use this type in the polymorphic relationship '${parentDefinition.inverseType}.${parentDefinition.key}' the relationships schema definition for ${addedIdentifier.type} should include:${expectedSchema(parentDefinition)}`, meta);
assert(
`You should not specify both options.as and options.inverse as null on ${addedIdentifier.type}.${parentDefinition.inverseKey}, as if there is no inverse field there is no abstract type to conform to. You may have intended for this relationship to be polymorphic, or you may have mistakenly set inverse to null.`,
!(meta.options.inverse === null && meta?.options.as?.length > 0)
);
let errors = validateSchema(parentDefinition, meta);
assert(
`The schema for the relationship '${parentDefinition.inverseKey}' on '${addedIdentifier.type}' type does not correctly implement '${parentDefinition.type}' and thus cannot be assigned to the '${parentDefinition.key}' relationship in '${parentIdentifier.type}'. If using this record in this polymorphic relationship is desired, correct the errors in the schema shown below:${printSchema(meta, errors)}`,
errors.size === 0,
);
}
} else {
assert(`No '${parentDefinition.inverseKey}' field exists on '${addedIdentifier.type}'. To use this type in the polymorphic relationship '${parentDefinition.inverseType}.${parentDefinition.key}' the relationships schema definition for ${addedIdentifier.type} should include:${expectedSchema(parentDefinition)}`, meta);
assert(
`The schema for the relationship '${parentDefinition.inverseKey}' on '${addedIdentifier.type}' type does not implement '${parentDefinition.type}' and thus cannot be assigned to the '${parentDefinition.key}' relationship in '${parentIdentifier.type}'. The definition should specify 'as: "${parentDefinition.type}"' in options.`,
meta.options.as === parentDefinition.type
`You should not specify both options.as and options.inverse as null on ${addedIdentifier.type}.${parentDefinition.inverseKey}, as if there is no inverse field there is no abstract type to conform to. You may have intended for this relationship to be polymorphic, or you may have mistakenly set inverse to null.`,
!(meta.options.inverse === null && meta?.options.as?.length > 0)
);
let errors = validateSchema(parentDefinition, meta);
assert(
`The schema for the relationship '${parentDefinition.inverseKey}' on '${addedIdentifier.type}' type does not correctly implement '${parentDefinition.type}' and thus cannot be assigned to the '${parentDefinition.key}' relationship in '${parentIdentifier.type}'. If using this record in this polymorphic relationship is desired, correct the errors in the schema shown below:${printSchema(meta, errors)}`,
errors.size === 0,
);
}

} else if (addedIdentifier.type !== parentDefinition.type) {
// if we are not polymorphic
// then the addedIdentifier.type must be the same as the parentDefinition.type
let meta = store.getSchemaDefinitionService().relationshipsDefinitionFor(addedIdentifier)[
parentDefinition.inverseKey
];

if (!DEPRECATE_NON_EXPLICIT_POLYMORPHISM) {
if (meta?.options.as === parentDefinition.type) {
// inverse is likely polymorphic but missing the polymorphic flag
let meta = store.getSchemaDefinitionService().relationshipsDefinitionFor({ type: parentDefinition.inverseType })[
parentDefinition.key
];
let errors = validateSchema(definitionWithPolymorphic(inverseDefinition(parentDefinition)), meta);
assert(`The '<${addedIdentifier.type}>.${parentDefinition.inverseKey}' relationship cannot be used polymorphically because '<${parentDefinition.inverseType}>.${parentDefinition.key} is not a polymorphic relationship. To use this relationship in a polymorphic manner, fix the following schema issues on the relationships schema for '${parentDefinition.inverseType}':${printSchema(meta, errors)}`);
} else {
assert(`The '${addedIdentifier.type}' type does not implement '${parentDefinition.type}' and thus cannot be assigned to the '${parentDefinition.key}' relationship in '${parentIdentifier.type}'. If this relationship should be polymorphic, mark ${parentDefinition.inverseType}.${parentDefinition.key} as \`polymorphic: true\` and ${addedIdentifier.type}.${parentDefinition.inverseKey} as implementing it via \`as: '${parentDefinition.type}'\`.`);
}
} else if (meta?.options?.as?.length > 0) {
asserted = true;
if (meta?.options.as === parentDefinition.type) {
// inverse is likely polymorphic but missing the polymorphic flag
let meta = store.getSchemaDefinitionService().relationshipsDefinitionFor({ type: parentDefinition.inverseType })[
parentDefinition.key
];
let errors = validateSchema(definitionWithPolymorphic(inverseDefinition(parentDefinition)), meta);
assert(`The '<${addedIdentifier.type}>.${parentDefinition.inverseKey}' relationship cannot be used polymorphically because '<${parentDefinition.inverseType}>.${parentDefinition.key} is not a polymorphic relationship. To use this relationship in a polymorphic manner, fix the following schema issues on the relationships schema for '${parentDefinition.inverseType}':${printSchema(meta, errors)}`);
} else {
assert(`The '${addedIdentifier.type}' type does not implement '${parentDefinition.type}' and thus cannot be assigned to the '${parentDefinition.key}' relationship in '${parentIdentifier.type}'. If this relationship should be polymorphic, mark ${parentDefinition.inverseType}.${parentDefinition.key} as \`polymorphic: true\` and ${addedIdentifier.type}.${parentDefinition.inverseKey} as implementing it via \`as: '${parentDefinition.type}'\`.`);
}
}
}

Expand All @@ -68,4 +244,4 @@ if (DEBUG) {
};
}

export { assertPolymorphicType };
export { assertPolymorphicType, assertInheritedSchema };
Loading

0 comments on commit de6b66c

Please sign in to comment.