diff --git a/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts b/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts index a6f3a3157a07..7ccbde417af6 100644 --- a/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts +++ b/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts @@ -372,6 +372,47 @@ export function hasManyInclusionResolverAcceptance( ); }); + it('tolerates duplicate and missing source key values when resolving inclusion', async () => { + // Regression test: the hasMany inclusion resolver used to pass its + // raw, unfiltered source-id list by reference straight to + // findByForeignKeys(), which hands that same array on to the + // connector inside an `{inq: [...]}` where clause. A connector/query + // layer that sanitizes an `inq` array in place (stripping falsy + // values before running the query, as the in-memory connector does) + // then mutates that shared array out from under the caller, shrinking + // the very array the resolver still needed - unmodified - to zip + // results back onto each original entity. That silently truncated + // and misaligned the resolver's return value whenever any source + // entity's key was undefined (e.g. excluded by a fields filter) or + // duplicated another entity's. belongsTo/referencesMany already pass + // a fresh, deduplicated, filtered array (never the original + // reference) before querying; hasMany now does the same. + const thor = await customerRepo.create({name: 'Thor'}); + const thorOrder = await orderRepo.create({ + customerId: thor.id, + description: "Thor's Mjolnir", + }); + + const resolver = customerRepo.inclusionResolvers.get('orders')!; + const result = await resolver( + [ + thor, + thor, // duplicate id + {name: 'no id'} as unknown as Customer, // id is undefined + ], + 'orders', + ); + + // The entity with no source key has no related entities; after JSON + // serialization that slot is `null` (JSON has no `undefined`). The key + // point is that the result stays length-3 and aligned with the input. + expect(toJSON(result)).to.deepEqual([ + [toJSON(thorOrder)], + [toJSON(thorOrder)], + null, + ]); + }); + it('throws error if the target repository does not have the registered resolver', async () => { const customer = await customerRepo.create({name: 'customer'}); await orderRepo.create({ diff --git a/packages/repository-tests/src/crud/relations/acceptance/has-many-through-inclusion-resolver.acceptance.ts b/packages/repository-tests/src/crud/relations/acceptance/has-many-through-inclusion-resolver.acceptance.ts index 7227b97e87a6..57afa5d83d0f 100644 --- a/packages/repository-tests/src/crud/relations/acceptance/has-many-through-inclusion-resolver.acceptance.ts +++ b/packages/repository-tests/src/crud/relations/acceptance/has-many-through-inclusion-resolver.acceptance.ts @@ -108,6 +108,48 @@ export function hasManyThroughInclusionResolverAcceptance( ]); }); + it('tolerates duplicate and missing source key values when resolving inclusion', async () => { + // Regression test: the hasManyThrough inclusion resolver used to + // pass its raw, unfiltered source-id list by reference straight to + // findByForeignKeys(), which hands that same array on to the + // connector inside an `{inq: [...]}` where clause. A connector/ + // query layer that sanitizes an `inq` array in place (stripping + // falsy values before running the query, as the in-memory + // connector does) then mutates that shared array out from under + // the caller, shrinking the very array the resolver still needed - + // unmodified - to zip through-results back onto each original + // entity via flattenTargetsOfOneToManyRelation. That silently + // truncated and misaligned the resolver's return value whenever + // any source entity's key was undefined (e.g. excluded by a fields + // filter) or duplicated another entity's. hasMany/hasOne already + // pass a fresh, deduplicated, filtered array (never the original + // reference) before querying; hasManyThrough now does the same. + const zelda = await customerRepo.create({name: 'Zelda'}); + const zeldaCart = await customerRepo + .cartItems(zelda.id) + .create({description: 'crown'}); + + const resolver = customerRepo.inclusionResolvers.get('cartItems')!; + const result = await resolver( + [ + zelda, + zelda, // duplicate id + {name: 'no id'} as unknown as Customer, // id is undefined + ], + 'cartItems', + ); + + // The entity with no source key has no related entities; the key + // point is that the result stays length-3 and aligned with the + // input, rather than truncated/misaligned by the in-place mutation + // this regression guards against. + expect(toJSON(result)).to.deepEqual([ + [toJSON(zeldaCart)], + [toJSON(zeldaCart)], + null, + ]); + }); + it('returns multiple model instances including related instances', async () => { const link = await customerRepo.create({name: 'Link'}); const sword = await customerRepo diff --git a/packages/repository-tests/src/crud/relations/acceptance/has-one.inclusion-resolver.acceptance.ts b/packages/repository-tests/src/crud/relations/acceptance/has-one.inclusion-resolver.acceptance.ts index 9381d7e565f3..a5bc7b08e5ac 100644 --- a/packages/repository-tests/src/crud/relations/acceptance/has-one.inclusion-resolver.acceptance.ts +++ b/packages/repository-tests/src/crud/relations/acceptance/has-one.inclusion-resolver.acceptance.ts @@ -160,6 +160,50 @@ export function hasOneInclusionResolverAcceptance( expect(toJSON(result)).to.deepEqual(toJSON(expected)); }); + it('tolerates duplicate and missing source key values when resolving inclusion', async () => { + // Regression test: the hasOne inclusion resolver used to pass its raw, + // unfiltered source-id list by reference straight to + // findByForeignKeys(), which hands that same array on to the + // connector inside an `{inq: [...]}` where clause. A connector/query + // layer that sanitizes an `inq` array in place (stripping falsy + // values before running the query, as the in-memory connector does) + // then mutates that shared array out from under the caller, shrinking + // the very array the resolver still needed - unmodified - to zip + // results back onto each original entity. That silently truncated + // and misaligned the resolver's return value whenever any source + // entity's key was undefined (e.g. excluded by a fields filter) or + // duplicated another entity's. belongsTo/referencesMany already pass + // a fresh, deduplicated, filtered array (never the original + // reference) before querying; hasOne now does the same. + const thor = await customerRepo.create({name: 'Thor'}); + const thorAddress = await addressRepo.create({ + street: 'home of Thor Rd.', + city: 'Thrudheim', + province: 'Asgard', + zipcode: '8200', + customerId: thor.id, + }); + + const resolver = customerRepo.inclusionResolvers.get('address')!; + const result = await resolver( + [ + thor, + thor, // duplicate id + {name: 'no id'} as unknown as Customer, // id is undefined + ], + 'address', + ); + + // The entity with no source key has no related entity; after JSON + // serialization that slot is `null` (JSON has no `undefined`). The key + // point is that the result stays length-3 and aligned with the input. + expect(toJSON(result)).to.deepEqual([ + toJSON(thorAddress), + toJSON(thorAddress), + null, + ]); + }); + it('throws error if the target repository does not have the registered resolver', async () => { const customer = await customerRepo.create({name: 'customer'}); await addressRepo.create({ diff --git a/packages/repository/src/relations/has-many/has-many-through.inclusion-resolver.ts b/packages/repository/src/relations/has-many/has-many-through.inclusion-resolver.ts index b89c1f2dd4a8..f5d831305ff3 100644 --- a/packages/repository/src/relations/has-many/has-many-through.inclusion-resolver.ts +++ b/packages/repository/src/relations/has-many/has-many-through.inclusion-resolver.ts @@ -12,6 +12,7 @@ import {Entity} from '../../model'; import {EntityCrudRepository} from '../../repositories'; import { StringKeyOf, + deduplicate, findByForeignKeys, flattenTargetsOfOneToManyRelation, } from '../relation.helpers'; @@ -100,7 +101,11 @@ export function createHasManyThroughInclusionResolver< const throughFound = await findByForeignKeys( throughRepo, throughKeyFrom, - sourceIds, + // Dedup/filter before querying: findByForeignKeys() passes this array + // by reference into an `inq` filter, and some connectors mutate it in + // place, corrupting the `sourceIds` this resolver still needs below. + // See the PR description for the full failure mode. + deduplicate(sourceIds).filter(e => e), {}, // scope will be applied at the target level options, ); diff --git a/packages/repository/src/relations/has-many/has-many.inclusion-resolver.ts b/packages/repository/src/relations/has-many/has-many.inclusion-resolver.ts index 5c41091a1243..b0e2331080a9 100644 --- a/packages/repository/src/relations/has-many/has-many.inclusion-resolver.ts +++ b/packages/repository/src/relations/has-many/has-many.inclusion-resolver.ts @@ -9,6 +9,7 @@ import {AnyObject, Options} from '../../common-types'; import {Entity} from '../../model'; import {EntityCrudRepository} from '../../repositories'; import { + deduplicate, findByForeignKeys, flattenTargetsOfOneToManyRelation, StringKeyOf, @@ -69,7 +70,11 @@ export function createHasManyInclusionResolver< const targetsFound = await findByForeignKeys( targetRepo, targetKey, - sourceIds, + // Dedup/filter before querying: findByForeignKeys() passes this array + // by reference into an `inq` filter, and some connectors mutate it in + // place, corrupting the `sourceIds` this resolver still needs below. + // See the PR description for the full failure mode. + deduplicate(sourceIds).filter(e => e), scope, options, ); diff --git a/packages/repository/src/relations/has-one/has-one.inclusion-resolver.ts b/packages/repository/src/relations/has-one/has-one.inclusion-resolver.ts index a74640205fca..d836afeed1a1 100644 --- a/packages/repository/src/relations/has-one/has-one.inclusion-resolver.ts +++ b/packages/repository/src/relations/has-one/has-one.inclusion-resolver.ts @@ -9,6 +9,7 @@ import {AnyObject, Options} from '../../common-types'; import {Entity} from '../../model'; import {EntityCrudRepository} from '../../repositories'; import { + deduplicate, findByForeignKeys, flattenTargetsOfOneToOneRelation, StringKeyOf, @@ -110,7 +111,11 @@ export function createHasOneInclusionResolver< const targetsFound = await findByForeignKeys( targetRepo, targetKey, - sourceIdsCategorized[k], + // Dedup/filter before querying: findByForeignKeys() passes this + // array by reference into an `inq` filter, and some connectors + // mutate it in place, corrupting the array this resolver still + // needs below. See the PR description for the full failure mode. + deduplicate(sourceIdsCategorized[k]).filter(e => e), scope, {...options, polymorphicType: k}, );