diff --git a/src/core/dataloader/creators/base/data.loader.creator.base.options.ts b/src/core/dataloader/creators/base/data.loader.creator.base.options.ts index db3a86f881..eed7bbc588 100644 --- a/src/core/dataloader/creators/base/data.loader.creator.base.options.ts +++ b/src/core/dataloader/creators/base/data.loader.creator.base.options.ts @@ -29,10 +29,28 @@ export interface DataLoaderCreatorBaseOptions { */ cache?: boolean; /*** - * What to return when resolving the unresolved result for a key. - * The default behaviour is to return an error - set to true to return NULL instead. - * This is useful when an a result is expected to be null and it's not an - * exceptional case. + * Overwrites the default behaviour of what to return when resolving the unresolved result for a key. + * This is useful when the result is expected to be null, and it's not an exceptional case. + * --- + * The default behaviour is determined by the Loader decorator on the dataloader`s creation. + * If the underlying graphql field, for which the dataloader is created, is nullable - the result can be resolved to null. + *
+ * Example: + * ``` + * @ResolveField(() => ICallout, { + * nullable: true, + * description: 'The Callout that was published.', + * }) + * public callout( + * @Parent() { payload }: InAppNotificationCalloutPublished, + * @Loader(CalloutLoaderCreator) loader: ILoader + * ) { + * return loader.load(payload.calloutID); + * } + * ``` + * The `callout` is decorated as nullable, so the dataloader will auto-resolve to `null` if the result is not found. + *
+ * You can override this behaviour by setting the option to `false`. That way the problematic values will always be resolved to errors. */ resolveToNull?: boolean; } diff --git a/src/core/dataloader/decorators/data.loader.decorator.ts b/src/core/dataloader/decorators/data.loader.decorator.ts index 7ca8858bd1..7a1c8e2a80 100644 --- a/src/core/dataloader/decorators/data.loader.decorator.ts +++ b/src/core/dataloader/decorators/data.loader.decorator.ts @@ -1,14 +1,13 @@ +import { isNonNullType } from 'graphql/type'; +import { GraphQLResolveInfo } from 'graphql/type/definition'; import { createParamDecorator, ExecutionContext, Type } from '@nestjs/common'; -import { APP_INTERCEPTOR } from '@nestjs/core'; import { GqlExecutionContext } from '@nestjs/graphql'; -import { DataLoaderInterceptorNotProvided } from '@common/exceptions/data-loader'; import { DATA_LOADER_CTX_INJECT_TOKEN } from '../data.loader.inject.token'; -import { DataLoaderInterceptor } from '../interceptors'; import { DataLoaderCreator, DataLoaderCreatorOptions } from '../creators/base'; export function Loader( creatorRef: Type>, - options?: DataLoaderCreatorOptions + options: DataLoaderCreatorOptions = {} ): ParameterDecorator { return createParamDecorator( ( @@ -17,11 +16,16 @@ export function Loader( ) => { const ctx = GqlExecutionContext.create(context).getContext(); + // as the default behaviour we resolve to null if the field is nullable + if (options.resolveToNull === undefined) { + const info: GraphQLResolveInfo = context.getArgByIndex(3); - if (!ctx[DATA_LOADER_CTX_INJECT_TOKEN]) { - throw new DataLoaderInterceptorNotProvided( - `You must provide ${DataLoaderInterceptor.name} globally with the ${APP_INTERCEPTOR} injector token` - ); + if (info) { + const fieldName = info.fieldName; + const field = info.parentType.getFields()[fieldName]; + + options.resolveToNull = !isNonNullType(field.type); + } } return ctx[DATA_LOADER_CTX_INJECT_TOKEN].get(innerCreatorRef, options); diff --git a/src/core/dataloader/utils/createTypedRelationLoader.ts b/src/core/dataloader/utils/createTypedRelationLoader.ts index 54b0af7f3f..1d8b135065 100644 --- a/src/core/dataloader/utils/createTypedRelationLoader.ts +++ b/src/core/dataloader/utils/createTypedRelationLoader.ts @@ -34,9 +34,9 @@ export const createTypedRelationDataLoader = < : fields : { id: true }; - return new DataLoader( + return new DataLoader( keys => - findByBatchIds( + findByBatchIds( manager, parentClassRef, keys as string[], diff --git a/src/core/dataloader/utils/createTypedSimpleLoader.ts b/src/core/dataloader/utils/createTypedSimpleLoader.ts index ba7c972dfb..4da8568cb4 100644 --- a/src/core/dataloader/utils/createTypedSimpleLoader.ts +++ b/src/core/dataloader/utils/createTypedSimpleLoader.ts @@ -24,7 +24,7 @@ export const createTypedSimpleDataLoader = ( : fields : undefined; - return new DataLoader( + return new DataLoader( keys => findByBatchIdsSimple(manager, classRef, keys as string[], { ...restOptions, diff --git a/src/core/dataloader/utils/findByBatchIds.ts b/src/core/dataloader/utils/findByBatchIds.ts index 16be6ee55f..18cb71ac77 100644 --- a/src/core/dataloader/utils/findByBatchIds.ts +++ b/src/core/dataloader/utils/findByBatchIds.ts @@ -7,18 +7,18 @@ import { import { Type } from '@nestjs/common'; import { EntityNotFoundException } from '@common/exceptions'; import { LogContext } from '@common/enums'; -import { FindByBatchIdsOptions } from '../utils'; +import { FindByBatchIdsOptions, sorOutputByKeys } from '../utils'; export const findByBatchIds = async < TParent extends { id: string } & { [key: string]: any }, // todo better type - TResult + TResult, >( manager: EntityManager, classRef: Type, ids: string[], relations: FindOptionsRelations, options?: FindByBatchIdsOptions -): Promise<(TResult | Error)[] | never> => { +): Promise<(TResult | null | EntityNotFoundException)[] | never> => { if (!ids.length) { return []; } @@ -33,7 +33,7 @@ export const findByBatchIds = async < const { select, limit } = options ?? {}; - const results = await manager.find(classRef, { + const unsortedResults = await manager.find(classRef, { take: limit, where: { id: In(ids), @@ -41,6 +41,7 @@ export const findByBatchIds = async < relations: relations, select: select, }); + const sortedResults = sorOutputByKeys(unsortedResults, ids); const topLevelRelation = relationKeys[0]; @@ -48,16 +49,21 @@ export const findByBatchIds = async < options?.getResult?.(result) ?? result[topLevelRelation]; // return empty object because DataLoader does not allow to return NULL values // handle the value when the result is returned - const resolveUnresolvedForKey = options?.resolveToNull - ? () => ({} as TResult) - : (key: string) => - new EntityNotFoundException( - `Could not load relation '${topLevelRelation}' for ${classRef.name}: ${key}`, - LogContext.DATA_LOADER + const resolveUnresolvedForKey = (key: string) => { + return options?.resolveToNull + ? null + : new EntityNotFoundException( + `Could not load relation '${topLevelRelation}' for ${classRef.name} for the given key`, + LogContext.DATA_LOADER, + { id: key } ); + }; const resultsById = new Map( - results.map<[string, TResult]>(result => [result.id, getRelation(result)]) + sortedResults.map<[string, TResult]>(result => [ + result.id, + getRelation(result), + ]) ); // ensure the result length matches the input length return ids.map(id => resultsById.get(id) ?? resolveUnresolvedForKey(id)); diff --git a/src/core/dataloader/utils/findByBatchIdsSimple.ts b/src/core/dataloader/utils/findByBatchIdsSimple.ts index 6ca4c37108..052d59da4c 100644 --- a/src/core/dataloader/utils/findByBatchIdsSimple.ts +++ b/src/core/dataloader/utils/findByBatchIdsSimple.ts @@ -9,7 +9,7 @@ export const findByBatchIdsSimple = async ( classRef: Type, ids: string[], options?: FindByBatchIdsOptions -): Promise<(TResult | Error)[] | never> => { +): Promise<(TResult | null | EntityNotFoundException)[] | never> => { if (!ids.length) { return []; } @@ -25,13 +25,15 @@ export const findByBatchIdsSimple = async ( }); // return empty object because DataLoader does not allow to return NULL values // handle the value when the result is returned - const resolveUnresolvedForKey = options?.resolveToNull - ? () => ({} as TResult) - : (key: string) => - new EntityNotFoundException( - `Could not load resource for '${key}'`, - LogContext.DATA_LOADER + const resolveUnresolvedForKey = (key: string) => { + return options?.resolveToNull + ? null + : new EntityNotFoundException( + `Could not find ${classRef.name} for the given key`, + LogContext.DATA_LOADER, + { id: key } ); + }; const resultsById = new Map( results.map<[string, TResult]>(result => [result.id, result]) diff --git a/src/core/dataloader/utils/index.ts b/src/core/dataloader/utils/index.ts index d9151be7db..649f02438d 100644 --- a/src/core/dataloader/utils/index.ts +++ b/src/core/dataloader/utils/index.ts @@ -5,3 +5,4 @@ export * from './createTypedSimpleLoader'; export * from './createTypedBatchLoader'; export * from './selectOptionsFromFields'; export * from './find.by.batch.options'; +export * from './sort.output.by.keys'; diff --git a/src/domain/collaboration/callout/callout.resolver.fields.ts b/src/domain/collaboration/callout/callout.resolver.fields.ts index f553315cd2..f0c3ecbdd7 100644 --- a/src/domain/collaboration/callout/callout.resolver.fields.ts +++ b/src/domain/collaboration/callout/callout.resolver.fields.ts @@ -126,7 +126,7 @@ export class CalloutResolverFields { }) async publishedBy( @Parent() callout: ICallout, - @Loader(UserLoaderCreator, { resolveToNull: true }) loader: ILoader + @Loader(UserLoaderCreator) loader: ILoader ): Promise { const publishedBy = callout.publishedBy; if (!publishedBy) { @@ -154,31 +154,13 @@ export class CalloutResolverFields { }) async createdBy( @Parent() callout: ICallout, - @Loader(UserLoaderCreator, { resolveToNull: true }) loader: ILoader + @Loader(UserLoaderCreator) loader: ILoader ): Promise { const createdBy = callout.createdBy; if (!createdBy) { return null; } - try { - return await loader - .load(createdBy) - // empty object is result because DataLoader does not allow to return NULL values - // handle the value when the result is returned - .then(x => { - return !Object.keys(x).length ? null : x; - }); - } catch (e: unknown) { - if (e instanceof EntityNotFoundException) { - this.logger?.warn( - `createdBy '${createdBy}' unable to be resolved when resolving callout '${callout.id}'`, - LogContext.COLLABORATION - ); - return null; - } else { - throw e; - } - } + return loader.load(createdBy); } } diff --git a/src/domain/common/whiteboard/whiteboard.resolver.fields.ts b/src/domain/common/whiteboard/whiteboard.resolver.fields.ts index 8e2a395b74..8777169f5e 100644 --- a/src/domain/common/whiteboard/whiteboard.resolver.fields.ts +++ b/src/domain/common/whiteboard/whiteboard.resolver.fields.ts @@ -40,7 +40,7 @@ export class WhiteboardResolverFields { }) async createdBy( @Parent() whiteboard: IWhiteboard, - @Loader(UserLoaderCreator, { resolveToNull: true }) loader: ILoader + @Loader(UserLoaderCreator) loader: ILoader ): Promise { const createdBy = whiteboard.createdBy; if (!createdBy) { @@ -51,25 +51,7 @@ export class WhiteboardResolverFields { return null; } - try { - return await loader - .load(createdBy) - // empty object is result because DataLoader does not allow to return NULL values - // handle the value when the result is returned - .then(x => { - return !Object.keys(x).length ? null : x; - }); - } catch (e: unknown) { - if (e instanceof EntityNotFoundException) { - this.logger?.warn( - `createdBy '${createdBy}' unable to be resolved when resolving whiteboard '${whiteboard.id}'`, - LogContext.COLLABORATION - ); - return null; - } else { - throw e; - } - } + return loader.load(createdBy); } @UseGuards(GraphqlGuard)