From 68f3332e976537e5b38a85ca2a0fd18fe92af965 Mon Sep 17 00:00:00 2001 From: Svetoslav Date: Wed, 11 Dec 2024 15:55:30 +0200 Subject: [PATCH 1/2] dataloader can now resolve to null for nullable fields --- .../creators/base/data.loader.creator.ts | 5 ++- src/core/dataloader/creators/base/index.ts | 1 + .../callout.loader.creator.ts | 20 +++++++---- .../community.type.loader.creator.ts | 17 ++++++---- .../contributor.loader.creator.ts | 20 +++++++---- .../space.loader.creator.ts | 20 +++++++---- .../utils/createTypedBatchLoader.ts | 34 ++++++++++++------- .../in.app.notification.callout.published.ts | 4 +-- ...n.app.notification.community.new.member.ts | 2 +- .../dto/in.app.notification.user.mentioned.ts | 6 ++-- ...ation.callout.published.resolver.fields.ts | 10 +++--- ...on.community.new.member.resolver.fields.ts | 10 +++--- 12 files changed, 93 insertions(+), 56 deletions(-) diff --git a/src/core/dataloader/creators/base/data.loader.creator.ts b/src/core/dataloader/creators/base/data.loader.creator.ts index 5aab3c26d7..796f958c5c 100644 --- a/src/core/dataloader/creators/base/data.loader.creator.ts +++ b/src/core/dataloader/creators/base/data.loader.creator.ts @@ -1,6 +1,9 @@ import { ILoader } from '../../loader.interface'; import { DataLoaderCreatorOptions } from './data.loader.creator.options'; +import { EntityNotFoundException } from '@common/exceptions'; export interface DataLoaderCreator { - create(options?: DataLoaderCreatorOptions): ILoader; + create( + options?: DataLoaderCreatorOptions + ): ILoader; } diff --git a/src/core/dataloader/creators/base/index.ts b/src/core/dataloader/creators/base/index.ts index 9b5d359156..70715bd036 100644 --- a/src/core/dataloader/creators/base/index.ts +++ b/src/core/dataloader/creators/base/index.ts @@ -1,5 +1,6 @@ export * from './data.loader.creator'; +export * from './data.loader.creator.base.options'; export * from './data.loader.creator.options'; export * from './data.loader.creator.limit.options'; export * from './data.loader.creator.pagination.options'; diff --git a/src/core/dataloader/creators/loader.creators/in-app-notification/callout.loader.creator.ts b/src/core/dataloader/creators/loader.creators/in-app-notification/callout.loader.creator.ts index 90da35b295..c13ef7a72c 100644 --- a/src/core/dataloader/creators/loader.creators/in-app-notification/callout.loader.creator.ts +++ b/src/core/dataloader/creators/loader.creators/in-app-notification/callout.loader.creator.ts @@ -1,21 +1,27 @@ import { EntityManager, In } from 'typeorm'; import { Injectable } from '@nestjs/common'; import { InjectEntityManager } from '@nestjs/typeorm'; -import { DataLoaderCreator } from '@core/dataloader/creators/base'; +import { + DataLoaderCreator, + DataLoaderCreatorBaseOptions, +} from '@core/dataloader/creators/base'; import { createBatchLoader } from '@core/dataloader/utils'; import { ILoader } from '@core/dataloader/loader.interface'; import { Callout, ICallout } from '@domain/collaboration/callout'; +import { EntityNotFoundException } from '@common/exceptions'; @Injectable() export class CalloutLoaderCreator implements DataLoaderCreator { constructor(@InjectEntityManager() private manager: EntityManager) {} - public create(): ILoader { - return createBatchLoader( - this.constructor.name, - Callout.name, - this.calloutInBatch - ); + public create( + options?: DataLoaderCreatorBaseOptions + ): ILoader { + return createBatchLoader(this.calloutInBatch, { + name: this.constructor.name, + loadedTypeName: Callout.name, + resolveToNull: options?.resolveToNull, + }); } private calloutInBatch = ( diff --git a/src/core/dataloader/creators/loader.creators/in-app-notification/community.type.loader.creator.ts b/src/core/dataloader/creators/loader.creators/in-app-notification/community.type.loader.creator.ts index 3cfa64f361..3911c5c04e 100644 --- a/src/core/dataloader/creators/loader.creators/in-app-notification/community.type.loader.creator.ts +++ b/src/core/dataloader/creators/loader.creators/in-app-notification/community.type.loader.creator.ts @@ -2,7 +2,10 @@ import { EntityManager } from 'typeorm'; import { Injectable } from '@nestjs/common'; import { InjectEntityManager } from '@nestjs/typeorm'; import { CommunityContributorType } from '@common/enums/community.contributor.type'; -import { DataLoaderCreator } from '@core/dataloader/creators/base'; +import { + DataLoaderCreator, + DataLoaderCreatorBaseOptions, +} from '@core/dataloader/creators/base'; import { createBatchLoader } from '@core/dataloader/utils'; import { User } from '@domain/community/user/user.entity'; import { Organization } from '@domain/community/organization'; @@ -14,12 +17,12 @@ export class CommunityTypeLoaderCreator { constructor(@InjectEntityManager() private manager: EntityManager) {} - create() { - return createBatchLoader( - this.constructor.name, - 'CommunityContributorType', - this.communityTypeInBatch - ); + create(options?: DataLoaderCreatorBaseOptions) { + return createBatchLoader(this.communityTypeInBatch, { + name: this.constructor.name, + loadedTypeName: 'CommunityContributorType', + resolveToNull: options?.resolveToNull, + }); } private async communityTypeInBatch( diff --git a/src/core/dataloader/creators/loader.creators/in-app-notification/contributor.loader.creator.ts b/src/core/dataloader/creators/loader.creators/in-app-notification/contributor.loader.creator.ts index 8a5677f308..56ad013a28 100644 --- a/src/core/dataloader/creators/loader.creators/in-app-notification/contributor.loader.creator.ts +++ b/src/core/dataloader/creators/loader.creators/in-app-notification/contributor.loader.creator.ts @@ -1,7 +1,10 @@ import { EntityManager, In } from 'typeorm'; import { Injectable } from '@nestjs/common'; import { InjectEntityManager } from '@nestjs/typeorm'; -import { DataLoaderCreator } from '@core/dataloader/creators/base'; +import { + DataLoaderCreator, + DataLoaderCreatorBaseOptions, +} from '@core/dataloader/creators/base'; import { createBatchLoader } from '@core/dataloader/utils'; import { ILoader } from '@core/dataloader/loader.interface'; import { IContributor } from '@domain/community/contributor/contributor.interface'; @@ -9,6 +12,7 @@ import { User } from '@domain/community/user/user.entity'; import { Organization } from '@domain/community/organization'; import { VirtualContributor } from '@domain/community/virtual-contributor/virtual.contributor.entity'; import { IContributorBase } from '@domain/community/contributor'; +import { EntityNotFoundException } from '@common/exceptions'; @Injectable() export class ContributorLoaderCreator @@ -16,12 +20,14 @@ export class ContributorLoaderCreator { constructor(@InjectEntityManager() private manager: EntityManager) {} - public create(): ILoader { - return createBatchLoader( - this.constructor.name, - 'Contributor', - this.contributorsInBatch - ); + public create( + options?: DataLoaderCreatorBaseOptions + ): ILoader { + return createBatchLoader(this.contributorsInBatch, { + name: this.constructor.name, + loadedTypeName: 'Contributor', + resolveToNull: options?.resolveToNull, + }); } private contributorsInBatch = async ( diff --git a/src/core/dataloader/creators/loader.creators/in-app-notification/space.loader.creator.ts b/src/core/dataloader/creators/loader.creators/in-app-notification/space.loader.creator.ts index f8138d7fe3..f74994853a 100644 --- a/src/core/dataloader/creators/loader.creators/in-app-notification/space.loader.creator.ts +++ b/src/core/dataloader/creators/loader.creators/in-app-notification/space.loader.creator.ts @@ -1,22 +1,28 @@ import { EntityManager, In } from 'typeorm'; import { Injectable } from '@nestjs/common'; import { InjectEntityManager } from '@nestjs/typeorm'; -import { DataLoaderCreator } from '@core/dataloader/creators/base'; +import { + DataLoaderCreator, + DataLoaderCreatorBaseOptions, +} from '@core/dataloader/creators/base'; import { createBatchLoader } from '@core/dataloader/utils'; import { ILoader } from '@core/dataloader/loader.interface'; import { ISpace } from '@domain/space/space/space.interface'; import { Space } from '@domain/space/space/space.entity'; +import { EntityNotFoundException } from '@common/exceptions'; @Injectable() export class SpaceLoaderCreator implements DataLoaderCreator { constructor(@InjectEntityManager() private manager: EntityManager) {} - public create(): ILoader { - return createBatchLoader( - this.constructor.name, - Space.name, - this.spaceInBatch - ); + public create( + options?: DataLoaderCreatorBaseOptions + ): ILoader { + return createBatchLoader(this.spaceInBatch, { + name: this.constructor.name, + loadedTypeName: Space.name, + resolveToNull: options?.resolveToNull, + }); } private spaceInBatch = (keys: ReadonlyArray): Promise => { diff --git a/src/core/dataloader/utils/createTypedBatchLoader.ts b/src/core/dataloader/utils/createTypedBatchLoader.ts index 27291f450e..18f716671d 100644 --- a/src/core/dataloader/utils/createTypedBatchLoader.ts +++ b/src/core/dataloader/utils/createTypedBatchLoader.ts @@ -3,17 +3,22 @@ import { EntityNotFoundException } from '@common/exceptions'; import { LogContext } from '@common/enums'; import { ILoader } from '../loader.interface'; import { sorOutputByKeys } from '@core/dataloader/utils/sort.output.by.keys'; +import { DataLoaderCreatorBaseOptions } from '@core/dataloader/creators/base/data.loader.creator.base.options'; export const createBatchLoader = ( - name: string, // for debugging purposes - loadedTypeName: string, // for debugging purposes - batchLoadFn: (keys: ReadonlyArray) => Promise -): ILoader => { - // the data loader returns an array the MUST match the input length + batchLoadFn: (keys: ReadonlyArray) => Promise, + options?: { + name: string; // for debugging purposes + loadedTypeName: string; // for debugging purposes + } & Pick, 'resolveToNull'> +): ILoader => { + // the data loader returns an array the MUST match the input length AND input key order // the provided batch function does not necessarily complete this requirement - // so we create a wrapper function that executes the batch function and ensure the output length + // so we create a wrapper function that executes the batch function and ensure the output length and order // by either returning the original output (if the length matches) or filling the missing values with errors - const loadAndEnsureOutputLengthAndOrder = async (keys: readonly string[]) => { + const loadAndEnsureOutputLengthAndOrder = async ( + keys: readonly string[] + ): Promise<(TResult | null | Error)[]> => { const unsortedOutput = await batchLoadFn(keys); const sortedOutput = sorOutputByKeys(unsortedOutput, keys); if (sortedOutput.length == keys.length) { @@ -29,16 +34,19 @@ export const createBatchLoader = ( key => resultsById.get(key) ?? resolveUnresolvedForKey(key) ); }; + const { name, loadedTypeName, resolveToNull } = options ?? {}; // a function to resolve an unresolved entity for a given key (e.g. if not found, etc.) const resolveUnresolvedForKey = (key: string) => { - return new EntityNotFoundException( - `Could not find ${loadedTypeName} for the given key`, - LogContext.DATA_LOADER, - { id: key } - ); + return resolveToNull + ? null + : new EntityNotFoundException( + `Could not find ${loadedTypeName} for the given key`, + LogContext.DATA_LOADER, + { id: key } + ); }; - return new DataLoader( + return new DataLoader( keys => loadAndEnsureOutputLengthAndOrder(keys), { cache: true, diff --git a/src/domain/in-app-notification-reader/dto/in.app.notification.callout.published.ts b/src/domain/in-app-notification-reader/dto/in.app.notification.callout.published.ts index 4f70924bd1..7302d708a4 100644 --- a/src/domain/in-app-notification-reader/dto/in.app.notification.callout.published.ts +++ b/src/domain/in-app-notification-reader/dto/in.app.notification.callout.published.ts @@ -15,6 +15,6 @@ export abstract class InAppNotificationCalloutPublished extends InAppNotificatio type!: NotificationEventType.COLLABORATION_CALLOUT_PUBLISHED; payload!: InAppNotificationCalloutPublishedPayload; // fields resolved by a concrete resolver - callout!: ICallout; - space!: ISpace; + callout?: ICallout; + space?: ISpace; } diff --git a/src/domain/in-app-notification-reader/dto/in.app.notification.community.new.member.ts b/src/domain/in-app-notification-reader/dto/in.app.notification.community.new.member.ts index 0f8b797c75..2878eebec9 100644 --- a/src/domain/in-app-notification-reader/dto/in.app.notification.community.new.member.ts +++ b/src/domain/in-app-notification-reader/dto/in.app.notification.community.new.member.ts @@ -18,7 +18,7 @@ export class InAppNotificationCommunityNewMember extends InAppNotificationBase() type!: NotificationEventType.COMMUNITY_NEW_MEMBER; payload!: InAppNotificationCommunityNewMemberPayload; // fields resolved by a concrete resolver - contributorType?: CommunityContributorType; + contributorType!: CommunityContributorType; actor?: IContributor; space?: ISpace; } diff --git a/src/domain/in-app-notification-reader/dto/in.app.notification.user.mentioned.ts b/src/domain/in-app-notification-reader/dto/in.app.notification.user.mentioned.ts index 6109d447de..bdff6cc1ff 100644 --- a/src/domain/in-app-notification-reader/dto/in.app.notification.user.mentioned.ts +++ b/src/domain/in-app-notification-reader/dto/in.app.notification.user.mentioned.ts @@ -17,7 +17,7 @@ export class InAppNotificationUserMentioned extends InAppNotificationBase() { type!: NotificationEventType.COMMUNICATION_USER_MENTION; payload!: InAppNotificationContributorMentionedPayload; // fields resolved by a concrete resolver - contributorType?: CommunityContributorType; - comment?: string; - commentUrl?: string; + contributorType!: CommunityContributorType; + comment!: string; + commentUrl!: string; } diff --git a/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.callout.published.resolver.fields.ts b/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.callout.published.resolver.fields.ts index f1a532302c..5c2e322745 100644 --- a/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.callout.published.resolver.fields.ts +++ b/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.callout.published.resolver.fields.ts @@ -12,23 +12,25 @@ import { ILoader } from '@core/dataloader/loader.interface'; @Resolver(() => InAppNotificationCalloutPublished) export class InAppNotificationCalloutPublishedResolverFields { @ResolveField(() => ICallout, { - nullable: false, + nullable: true, description: 'The Callout that was published.', }) public callout( @Parent() { payload }: InAppNotificationCalloutPublished, - @Loader(CalloutLoaderCreator) loader: ILoader + @Loader(CalloutLoaderCreator, { resolveToNull: true }) + loader: ILoader ) { return loader.load(payload.calloutID); } @ResolveField(() => ISpace, { - nullable: false, + nullable: true, description: 'Where the callout is located.', }) public space( @Parent() { payload }: InAppNotificationCalloutPublished, - @Loader(SpaceLoaderCreator) loader: ILoader + @Loader(SpaceLoaderCreator, { resolveToNull: true }) + loader: ILoader ) { return loader.load(payload.spaceID); } diff --git a/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.community.new.member.resolver.fields.ts b/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.community.new.member.resolver.fields.ts index 088a848f3c..c2811212c1 100644 --- a/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.community.new.member.resolver.fields.ts +++ b/src/domain/in-app-notification-reader/field-resolvers/in.app.notification.community.new.member.resolver.fields.ts @@ -21,24 +21,26 @@ export class InAppNotificationCommunityNewMemberResolverFields { } @ResolveField(() => IContributor, { - nullable: false, + nullable: true, description: 'The Contributor that joined.', }) // todo: rename? public actor( @Parent() { payload }: InAppNotificationCommunityNewMember, - @Loader(ContributorLoaderCreator) loader: ILoader + @Loader(ContributorLoaderCreator, { resolveToNull: true }) + loader: ILoader ) { return loader.load(payload.newMemberID); } @ResolveField(() => ISpace, { - nullable: false, + nullable: true, description: 'The Space that was joined.', }) public space( @Parent() { payload }: InAppNotificationCommunityNewMember, - @Loader(SpaceLoaderCreator) loader: ILoader + @Loader(SpaceLoaderCreator, { resolveToNull: true }) + loader: ILoader ) { return loader.load(payload.spaceID); } From b676314121bcd672edcd5f29fa70afa0a9e20fff Mon Sep 17 00:00:00 2001 From: Svetoslav Date: Wed, 11 Dec 2024 17:40:47 +0200 Subject: [PATCH 2/2] proper return type --- src/core/dataloader/utils/createTypedRelationLoader.ts | 5 +++-- src/core/dataloader/utils/createTypedSimpleLoader.ts | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/dataloader/utils/createTypedRelationLoader.ts b/src/core/dataloader/utils/createTypedRelationLoader.ts index 1c580b0719..54b0af7f3f 100644 --- a/src/core/dataloader/utils/createTypedRelationLoader.ts +++ b/src/core/dataloader/utils/createTypedRelationLoader.ts @@ -5,6 +5,7 @@ import { FindOptionsSelect, } from 'typeorm'; import { Type } from '@nestjs/common'; +import { EntityNotFoundException } from '@common/exceptions'; import { DataLoaderCreatorOptions } from '../creators/base'; import { ILoader } from '../loader.interface'; import { findByBatchIds } from './findByBatchIds'; @@ -12,14 +13,14 @@ import { selectOptionsFromFields } from './selectOptionsFromFields'; export const createTypedRelationDataLoader = < TParent extends { id: string } & { [key: string]: any }, // todo better type, - TResult + TResult, >( manager: EntityManager, parentClassRef: Type, relations: FindOptionsRelations, name: string, options?: DataLoaderCreatorOptions -): ILoader => { +): ILoader => { const { fields, ...restOptions } = options ?? {}; const topRelation = Object.keys(relations)[0]; diff --git a/src/core/dataloader/utils/createTypedSimpleLoader.ts b/src/core/dataloader/utils/createTypedSimpleLoader.ts index 6d2ea5c21a..ba7c972dfb 100644 --- a/src/core/dataloader/utils/createTypedSimpleLoader.ts +++ b/src/core/dataloader/utils/createTypedSimpleLoader.ts @@ -1,6 +1,7 @@ import DataLoader from 'dataloader'; import { EntityManager, FindOptionsSelect } from 'typeorm'; import { Type } from '@nestjs/common'; +import { EntityNotFoundException } from '@common/exceptions'; import { DataLoaderCreatorOptions } from '../creators/base'; import { ILoader } from '../loader.interface'; import { selectOptionsFromFields } from './selectOptionsFromFields'; @@ -11,7 +12,7 @@ export const createTypedSimpleDataLoader = ( classRef: Type, name: string, options?: DataLoaderCreatorOptions -): ILoader => { +): ILoader => { const { fields, ...restOptions } = options ?? {}; // if fields ia specified, select specific fields, otherwise select all fields const selectOptions = fields