From 7bd322d585b0893561b3ffb3c5ad47b2764c18bd Mon Sep 17 00:00:00 2001 From: Nathaniel Tucker Date: Sun, 23 Jun 2024 12:54:31 +0100 Subject: [PATCH] enhance: Entity runs validate after marking circular reference point (#3133) --- .changeset/wet-mirrors-visit.md | 9 +++++++ packages/endpoint/src/schemas/EntitySchema.ts | 7 ++--- .../__snapshots__/index.test.js.snap | 2 ++ .../normalizr/src/__tests__/index.test.js | 14 ++++++++++ packages/normalizr/src/normalize.ts | 26 +++++++++---------- 5 files changed, 42 insertions(+), 16 deletions(-) create mode 100644 .changeset/wet-mirrors-visit.md diff --git a/.changeset/wet-mirrors-visit.md b/.changeset/wet-mirrors-visit.md new file mode 100644 index 000000000000..433b8baea7af --- /dev/null +++ b/.changeset/wet-mirrors-visit.md @@ -0,0 +1,9 @@ +--- +'@data-client/endpoint': patch +--- + +Validate after marking cirucular reference loops + +This should not change any behavior as validate should be deterministic so if it fails +it will fail again and failure measure throwing which exits the whole stack. +This improves code grouping. (And possibly cache locality improvement - though didn't check.) \ No newline at end of file diff --git a/packages/endpoint/src/schemas/EntitySchema.ts b/packages/endpoint/src/schemas/EntitySchema.ts index dc55e8ab9e2a..0e015a5a2bee 100644 --- a/packages/endpoint/src/schemas/EntitySchema.ts +++ b/packages/endpoint/src/schemas/EntitySchema.ts @@ -300,8 +300,9 @@ export default function EntitySchema( } else { id = `${id}`; } - const entityType = this.key; + /* Circular reference short-circuiter */ + const entityType = this.key; if (!(entityType in visitedEntities)) { visitedEntities[entityType] = {}; } @@ -313,11 +314,11 @@ export default function EntitySchema( ) { return id; } + visitedEntities[entityType][id].push(input); + const errorMessage = this.validate(processedEntity); throwValidationError(errorMessage); - visitedEntities[entityType][id].push(input); - Object.keys(this.schema).forEach(key => { if (Object.hasOwn(processedEntity, key)) { processedEntity[key] = visit( diff --git a/packages/normalizr/src/__tests__/__snapshots__/index.test.js.snap b/packages/normalizr/src/__tests__/__snapshots__/index.test.js.snap index 09ea21c8f28d..d2e5c8a3a717 100644 --- a/packages/normalizr/src/__tests__/__snapshots__/index.test.js.snap +++ b/packages/normalizr/src/__tests__/__snapshots__/index.test.js.snap @@ -538,6 +538,8 @@ exports[`normalize normalizes entities with circular references 1`] = ` } `; +exports[`normalize normalizes entities with circular references that fails validation 1`] = `"this always fails"`; + exports[`normalize normalizes nested entities 1`] = ` { "entities": { diff --git a/packages/normalizr/src/__tests__/index.test.js b/packages/normalizr/src/__tests__/index.test.js index 18bb740686a7..6247d83d2323 100644 --- a/packages/normalizr/src/__tests__/index.test.js +++ b/packages/normalizr/src/__tests__/index.test.js @@ -216,6 +216,20 @@ describe('normalize', () => { expect(normalize(input, User)).toMatchSnapshot(); }); + test('normalizes entities with circular references that fails validation', () => { + class User extends IDEntity { + static validate(processedEntity) { + return 'this always fails'; + } + } + User.schema = { friends: [User] }; + + const input = { id: '123', friends: [] }; + input.friends.push(input); + + expect(() => normalize(input, User)).toThrowErrorMatchingSnapshot(); + }); + test('normalizes nested entities', () => { class User extends IDEntity {} class Comment extends IDEntity { diff --git a/packages/normalizr/src/normalize.ts b/packages/normalizr/src/normalize.ts index c801feffdef7..86d431f52e26 100644 --- a/packages/normalizr/src/normalize.ts +++ b/packages/normalizr/src/normalize.ts @@ -59,9 +59,9 @@ const addEntities = ( newEntities: Record, newIndexes: Record, - storeEntities: Record, - storeIndexes: Record, - storeEntityMeta: { + entitiesCopy: Record, + indexesCopy: Record, + entityMetaCopy: { [entityKey: string]: { [pk: string]: { date: number; @@ -78,8 +78,8 @@ const addEntities = if (!(schemaKey in newEntities)) { newEntities[schemaKey] = {}; // we will be editing these, so we need to clone them first - storeEntities[schemaKey] = { ...storeEntities[schemaKey] }; - storeEntityMeta[schemaKey] = { ...storeEntityMeta[schemaKey] }; + entitiesCopy[schemaKey] = { ...entitiesCopy[schemaKey] }; + entityMetaCopy[schemaKey] = { ...entityMetaCopy[schemaKey] }; } const existingEntity = newEntities[schemaKey][id]; @@ -89,21 +89,21 @@ const addEntities = processedEntity, ); } else { - const inStoreEntity = storeEntities[schemaKey][id]; + const inStoreEntity = entitiesCopy[schemaKey][id]; let inStoreMeta: { date: number; expiresAt: number; fetchedAt: number; }; // this case we already have this entity in store - if (inStoreEntity && (inStoreMeta = storeEntityMeta[schemaKey][id])) { + if (inStoreEntity && (inStoreMeta = entityMetaCopy[schemaKey][id])) { newEntities[schemaKey][id] = schema.mergeWithStore( inStoreMeta, actionMeta, inStoreEntity, processedEntity, ); - storeEntityMeta[schemaKey][id] = schema.mergeMetaWithStore( + entityMetaCopy[schemaKey][id] = schema.mergeMetaWithStore( inStoreMeta, actionMeta, inStoreEntity, @@ -111,7 +111,7 @@ const addEntities = ); } else { newEntities[schemaKey][id] = processedEntity; - storeEntityMeta[schemaKey][id] = actionMeta; + entityMetaCopy[schemaKey][id] = actionMeta; } } @@ -119,19 +119,19 @@ const addEntities = if (schema.indexes) { if (!(schemaKey in newIndexes)) { newIndexes[schemaKey] = {}; - storeIndexes[schemaKey] = { ...storeIndexes[schemaKey] }; + indexesCopy[schemaKey] = { ...indexesCopy[schemaKey] }; } handleIndexes( id, schema.indexes, newIndexes[schemaKey], - storeIndexes[schemaKey], + indexesCopy[schemaKey], newEntities[schemaKey][id], - storeEntities[schemaKey], + entitiesCopy[schemaKey], ); } // set this after index updates so we know what indexes to remove from - storeEntities[schemaKey][id] = newEntities[schemaKey][id]; + entitiesCopy[schemaKey][id] = newEntities[schemaKey][id]; }; function handleIndexes(