From bdcea9f5f12c541398458d2a57c9d03480a7b47c Mon Sep 17 00:00:00 2001 From: Juan Castano Date: Fri, 10 Jan 2025 15:24:09 +1100 Subject: [PATCH 1/9] =?UTF-8?q?Set=20common=20loader=20key=20in=20BaseLoad?= =?UTF-8?q?er=20getBaseRelatedIdLoader=20Set=20common=20loader=20key=20so?= =?UTF-8?q?=20we=20can=20properly=20batch=20related=20entities=20and=20mak?= =?UTF-8?q?e=20one=20call=20to=20findByRelatedId=20with=20all=20the=20batc?= =?UTF-8?q?hed=20keys.=20```=20const=20loaderKey=20=3D=20`${gqlTypeName}-$?= =?UTF-8?q?{relatedField}-${JSON.stringify(=20=09=09filter=20=09)}`;=20/*?= =?UTF-8?q?=20gqlTypeName-fieldname-filterObject=20*/=20```=20=F0=9F=91=86?= =?UTF-8?q?=20The=20above=20code=20causes=20the=20`loaderKey`=20to=20be=20?= =?UTF-8?q?unique=20for=20each=20related=20entity=20instead=20of=20having?= =?UTF-8?q?=20a=20common=20key.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` const loaderKey = `${gqlTypeName}-${relatedField}`; /* gqlTypeName-fieldname */ ``` This makes a common key per entity and field --- src/packages/core/src/base-loader.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/packages/core/src/base-loader.ts b/src/packages/core/src/base-loader.ts index 49f872a39..0af9a0edc 100644 --- a/src/packages/core/src/base-loader.ts +++ b/src/packages/core/src/base-loader.ts @@ -127,9 +127,7 @@ export const getBaseRelatedIdLoader = ({ filter?: Filter; }) => { const gqlTypeName = getGqlEntityName(gqlEntityType); - const loaderKey = `${gqlTypeName}-${relatedField}-${JSON.stringify( - filter - )}`; /* gqlTypeName-fieldname-filterObject */ + const loaderKey = `${gqlTypeName}-${relatedField}`; /* gqlTypeName-fieldname */ if (!keyStore[loaderKey]) { const entity = graphweaverMetadata.getEntityByName(gqlTypeName); From 370a82341195d542f4e65af3ed0e38265b5009b7 Mon Sep 17 00:00:00 2001 From: Juan Castano Date: Mon, 13 Jan 2025 12:48:25 +1100 Subject: [PATCH 2/9] added `populateWhere` to findByRelatedId --- src/packages/core/src/base-loader.ts | 3 +++ src/packages/mikroorm/src/provider/provider.ts | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/packages/core/src/base-loader.ts b/src/packages/core/src/base-loader.ts index 0af9a0edc..eb315baa4 100644 --- a/src/packages/core/src/base-loader.ts +++ b/src/packages/core/src/base-loader.ts @@ -127,6 +127,9 @@ export const getBaseRelatedIdLoader = ({ filter?: Filter; }) => { const gqlTypeName = getGqlEntityName(gqlEntityType); + // const loaderKey = `${gqlTypeName}-${relatedField}-${JSON.stringify( + // filter + // )}`; /* gqlTypeName-fieldname */ const loaderKey = `${gqlTypeName}-${relatedField}`; /* gqlTypeName-fieldname */ if (!keyStore[loaderKey]) { diff --git a/src/packages/mikroorm/src/provider/provider.ts b/src/packages/mikroorm/src/provider/provider.ts index 180e96f2c..ddc7efb9a 100644 --- a/src/packages/mikroorm/src/provider/provider.ts +++ b/src/packages/mikroorm/src/provider/provider.ts @@ -407,7 +407,9 @@ export class MikroBackendProvider implements BackendProvider { const populate = [relatedField as AutoPath]; const result = await this.database.em.find(entity, queryFilter, { + flags: [QueryFlag.DISTINCT], populate, + populateWhere: { [relatedField]: { $in: relatedFieldIds } }, }); return result as D[]; From 5d7e34258a93ab7c720f2c49c06ed7115b296083 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 13 Jan 2025 14:28:55 +1100 Subject: [PATCH 3/9] Constrain the loaded entity set on relationship joins. --- src/packages/mikroorm/src/provider/provider.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/packages/mikroorm/src/provider/provider.ts b/src/packages/mikroorm/src/provider/provider.ts index ddc7efb9a..3e5a7cc9f 100644 --- a/src/packages/mikroorm/src/provider/provider.ts +++ b/src/packages/mikroorm/src/provider/provider.ts @@ -17,7 +17,7 @@ import { graphweaverMetadata, } from '@exogee/graphweaver'; import { logger } from '@exogee/logger'; -import { Reference, RequestContext, sql } from '@mikro-orm/core'; +import { LoadStrategy, Reference, RequestContext, sql } from '@mikro-orm/core'; import { AutoPath, PopulateHint } from '@mikro-orm/postgresql'; import { pluginManager, apolloPluginManager } from '@exogee/graphweaver-server'; @@ -407,8 +407,17 @@ export class MikroBackendProvider implements BackendProvider { const populate = [relatedField as AutoPath]; const result = await this.database.em.find(entity, queryFilter, { + // We only need one result per entity. flags: [QueryFlag.DISTINCT], + + // We do want to populate the relation, however, see below. populate, + + // We'd love to use the default joined loading strategy, but it doesn't work with the + // populateWhere option. + strategy: LoadStrategy.SELECT_IN, + + // This tells MikroORM we only need to load the related entities if they match the filter specified above. populateWhere: { [relatedField]: { $in: relatedFieldIds } }, }); From 13484b7d01cce003a96f01f4a4cf9250707326db Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 13 Jan 2025 15:18:37 +1100 Subject: [PATCH 4/9] Fix for other relationship loading type. --- src/packages/core/src/base-loader.ts | 3 --- src/packages/core/src/resolvers.ts | 2 +- .../mikroorm/src/provider/provider.ts | 21 ++++++++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/packages/core/src/base-loader.ts b/src/packages/core/src/base-loader.ts index eb315baa4..0af9a0edc 100644 --- a/src/packages/core/src/base-loader.ts +++ b/src/packages/core/src/base-loader.ts @@ -127,9 +127,6 @@ export const getBaseRelatedIdLoader = ({ filter?: Filter; }) => { const gqlTypeName = getGqlEntityName(gqlEntityType); - // const loaderKey = `${gqlTypeName}-${relatedField}-${JSON.stringify( - // filter - // )}`; /* gqlTypeName-fieldname */ const loaderKey = `${gqlTypeName}-${relatedField}`; /* gqlTypeName-fieldname */ if (!keyStore[loaderKey]) { diff --git a/src/packages/core/src/resolvers.ts b/src/packages/core/src/resolvers.ts index ea3d68a7d..8a150625b 100644 --- a/src/packages/core/src/resolvers.ts +++ b/src/packages/core/src/resolvers.ts @@ -695,7 +695,7 @@ const _listRelationshipField = async ( gqlEntityType, relatedField: field.relationshipInfo.relatedField as keyof D & string, id: String(source[sourcePrimaryKeyField]), - filter: relatedEntityFilter as Filter, + filter, }); } else if (idValue) { logger.trace('Loading with loadOne'); diff --git a/src/packages/mikroorm/src/provider/provider.ts b/src/packages/mikroorm/src/provider/provider.ts index 3e5a7cc9f..23d0e3e3a 100644 --- a/src/packages/mikroorm/src/provider/provider.ts +++ b/src/packages/mikroorm/src/provider/provider.ts @@ -401,24 +401,29 @@ export class MikroBackendProvider implements BackendProvider { trace?: TraceOptions ): Promise { trace?.span.updateName(`Mikro-Orm - findByRelatedId ${this.entityType.name}`); - const queryFilter = { - $and: [{ [relatedField]: { $in: relatedFieldIds } }, ...[gqlToMikro(filter) ?? []]], - }; - const populate = [relatedField as AutoPath]; + // Any is the actual type from MikroORM, sorry folks. + let queryFilter: any = { [relatedField]: { $in: relatedFieldIds } }; + + if (filter) { + // Since the user has supplied a filter, we need to and it in. + queryFilter = { + $and: [queryFilter, ...[gqlToMikro(filter)]], + }; + } + const result = await this.database.em.find(entity, queryFilter, { // We only need one result per entity. flags: [QueryFlag.DISTINCT], // We do want to populate the relation, however, see below. - populate, + populate: [relatedField as AutoPath], - // We'd love to use the default joined loading strategy, but it doesn't work with the - // populateWhere option. + // We'd love to use the default joined loading strategy, but it doesn't work with the populateWhere option. strategy: LoadStrategy.SELECT_IN, // This tells MikroORM we only need to load the related entities if they match the filter specified above. - populateWhere: { [relatedField]: { $in: relatedFieldIds } }, + populateWhere: queryFilter, }); return result as D[]; From e0aafc2d99e2cd26bb67d59f8faece1efd80374f Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 13 Jan 2025 15:40:56 +1100 Subject: [PATCH 5/9] Restore filter separation on base loaders. --- src/packages/core/src/base-loader.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/packages/core/src/base-loader.ts b/src/packages/core/src/base-loader.ts index 0af9a0edc..49f872a39 100644 --- a/src/packages/core/src/base-loader.ts +++ b/src/packages/core/src/base-loader.ts @@ -127,7 +127,9 @@ export const getBaseRelatedIdLoader = ({ filter?: Filter; }) => { const gqlTypeName = getGqlEntityName(gqlEntityType); - const loaderKey = `${gqlTypeName}-${relatedField}`; /* gqlTypeName-fieldname */ + const loaderKey = `${gqlTypeName}-${relatedField}-${JSON.stringify( + filter + )}`; /* gqlTypeName-fieldname-filterObject */ if (!keyStore[loaderKey]) { const entity = graphweaverMetadata.getEntityByName(gqlTypeName); From 8a103b72118a21219232f9fa0aab803e7e634b67 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 13 Jan 2025 15:47:13 +1100 Subject: [PATCH 6/9] Restoring old style for less change in the diff. --- src/packages/mikroorm/src/provider/provider.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/packages/mikroorm/src/provider/provider.ts b/src/packages/mikroorm/src/provider/provider.ts index 23d0e3e3a..0f63afadd 100644 --- a/src/packages/mikroorm/src/provider/provider.ts +++ b/src/packages/mikroorm/src/provider/provider.ts @@ -411,13 +411,13 @@ export class MikroBackendProvider implements BackendProvider { $and: [queryFilter, ...[gqlToMikro(filter)]], }; } - + const populate = [relatedField as AutoPath]; const result = await this.database.em.find(entity, queryFilter, { // We only need one result per entity. flags: [QueryFlag.DISTINCT], // We do want to populate the relation, however, see below. - populate: [relatedField as AutoPath], + populate, // We'd love to use the default joined loading strategy, but it doesn't work with the populateWhere option. strategy: LoadStrategy.SELECT_IN, From e9d8a76aad86186b783ac2870d33e928f95924b0 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 13 Jan 2025 15:48:00 +1100 Subject: [PATCH 7/9] Better formatting. --- src/packages/mikroorm/src/provider/provider.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/packages/mikroorm/src/provider/provider.ts b/src/packages/mikroorm/src/provider/provider.ts index 0f63afadd..853b3c86f 100644 --- a/src/packages/mikroorm/src/provider/provider.ts +++ b/src/packages/mikroorm/src/provider/provider.ts @@ -411,6 +411,7 @@ export class MikroBackendProvider implements BackendProvider { $and: [queryFilter, ...[gqlToMikro(filter)]], }; } + const populate = [relatedField as AutoPath]; const result = await this.database.em.find(entity, queryFilter, { // We only need one result per entity. From bcf140016cbaac1300292272a885d04812af7b02 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 13 Jan 2025 16:02:20 +1100 Subject: [PATCH 8/9] Actually infer works just fine now that we're using the other strategy. --- src/packages/mikroorm/src/provider/provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/packages/mikroorm/src/provider/provider.ts b/src/packages/mikroorm/src/provider/provider.ts index 853b3c86f..9a00fa973 100644 --- a/src/packages/mikroorm/src/provider/provider.ts +++ b/src/packages/mikroorm/src/provider/provider.ts @@ -424,7 +424,7 @@ export class MikroBackendProvider implements BackendProvider { strategy: LoadStrategy.SELECT_IN, // This tells MikroORM we only need to load the related entities if they match the filter specified above. - populateWhere: queryFilter, + populateWhere: PopulateHint.INFER, }); return result as D[]; From 1957fd7fbef93800723948c16833d0f74957b3ec Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 13 Jan 2025 16:30:09 +1100 Subject: [PATCH 9/9] Support relationship filtering for relationships that call loadOne. --- src/packages/core/src/base-loader.ts | 17 +++++++++++++---- src/packages/core/src/resolvers.ts | 1 + 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/packages/core/src/base-loader.ts b/src/packages/core/src/base-loader.ts index 49f872a39..3fc7f8490 100644 --- a/src/packages/core/src/base-loader.ts +++ b/src/packages/core/src/base-loader.ts @@ -20,6 +20,7 @@ type LoaderMap = { [key: string]: DataLoader }; type LoadOneOptions = { gqlEntityType: { new (...args: any[]): G }; id: string; + filter?: Filter; }; type LoadByRelatedIdOptions = { @@ -39,12 +40,14 @@ const getGqlEntityName = (gqlEntityType: any) => { const getBaseLoadOneLoader = ({ gqlEntityType, + filter, keyStore, }: { gqlEntityType: { new (...args: any[]): G; }; keyStore: LoaderMap; + filter?: Filter; }) => { const gqlTypeName = getGqlEntityName(gqlEntityType); if (!keyStore[gqlTypeName]) { @@ -59,11 +62,17 @@ const getBaseLoadOneLoader = ({ ); const primaryKeyField = graphweaverMetadata.primaryKeyFieldForEntity(entity) as keyof D; - const listFilter = { + let listFilter = { [`${String(primaryKeyField)}_in`]: keys, // Note: Typecast here shouldn't be necessary, but FilterEntity doesn't like this. } as Filter; + if (filter) { + listFilter = { + _and: [listFilter, filter], + } as Filter; + } + const backendFilter = isTransformableGraphQLEntityClass(entity.target) && entity.target.toBackendEntityFilter ? entity.target.toBackendEntityFilter(listFilter) @@ -214,9 +223,9 @@ export class BaseLoader { private loadOneLoaderMap: LoaderMap = {}; private relatedIdLoaderMap: LoaderMap = {}; - public loadOne({ gqlEntityType, id }: LoadOneOptions) { - const loader = getBaseLoadOneLoader({ gqlEntityType, keyStore: this.loadOneLoaderMap }); - return loader.load(id); + public loadOne(args: LoadOneOptions) { + const loader = getBaseLoadOneLoader({ ...args, keyStore: this.loadOneLoaderMap }); + return loader.load(args.id); } public loadByRelatedId(args: LoadByRelatedIdOptions) { diff --git a/src/packages/core/src/resolvers.ts b/src/packages/core/src/resolvers.ts index 8a150625b..28e2011fa 100644 --- a/src/packages/core/src/resolvers.ts +++ b/src/packages/core/src/resolvers.ts @@ -705,6 +705,7 @@ const _listRelationshipField = async ( BaseLoaders.loadOne({ gqlEntityType, id: String(id), + filter, }) ) );