Skip to content

Commit

Permalink
Consolidate validation code into a single location (#6139)
Browse files Browse the repository at this point in the history
  • Loading branch information
timleslie authored Jul 19, 2021
1 parent 3a9636c commit 587a8d0
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 97 deletions.
5 changes: 5 additions & 0 deletions .changeset/spicy-pillows-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-next/keystone': patch
---

Refactored mutation validation handling into a single location.
61 changes: 8 additions & 53 deletions packages/keystone/src/lib/core/mutations/create-update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import {
resolveRelateToOneForUpdateInput,
} from './nested-mutation-one-input-resolvers';
import { applyAccessControlForCreate, getAccessControlledItemForUpdate } from './access-control';
import { runSideEffectOnlyHook, validationHook } from './hooks';
import { runSideEffectOnlyHook } from './hooks';
import { validateUpdateCreate } from './validation';

export class NestedMutationState {
#afterChanges: (() => void | Promise<void>)[] = [];
Expand Down Expand Up @@ -224,62 +225,16 @@ async function resolveInputForCreateOrUpdate(
existingItem
);

// Check isRequired
await validationHook(list.listKey, operation, originalInput, addValidationError => {
for (const [fieldKey, field] of Object.entries(list.fields)) {
// yes, this is a massive hack, it's just to make image and file fields work well enough
let val = resolvedData[fieldKey];
if (field.dbField.kind === 'multi') {
if (Object.values(resolvedData[fieldKey]).every(x => x === null)) {
val = null;
}
if (Object.values(resolvedData[fieldKey]).every(x => x === undefined)) {
val = undefined;
}
}
if (
field.__legacy?.isRequired &&
((operation === 'create' && val == null) || (operation === 'update' && val === null))
) {
addValidationError(
`Required field "${fieldKey}" is null or undefined.`,
{ resolvedData, operation, originalInput },
{}
);
}
}
});
const { listKey } = list;
const hookArgs = { context, listKey, operation, originalInput, resolvedData, existingItem };

// Field validation hooks
const args = {
context,
listKey: list.listKey,
operation,
originalInput,
resolvedData,
existingItem,
};
await validationHook(list.listKey, operation, originalInput, async addValidationError => {
await promiseAllRejectWithAllErrors(
Object.entries(list.fields).map(async ([fieldKey, field]) => {
await field.hooks.validateInput?.({
...args,
addValidationError,
fieldPath: fieldKey,
});
})
);
});

// List validation hooks
await validationHook(list.listKey, operation, originalInput, async addValidationError => {
await list.hooks.validateInput?.({ ...args, addValidationError });
});
// Apply all validation checks
await validateUpdateCreate({ list, hookArgs });

// Run beforeChange hooks
const originalInputKeys = new Set(Object.keys(originalInput));
const shouldCallFieldLevelSideEffectHook = (fieldKey: string) => originalInputKeys.has(fieldKey);
await runSideEffectOnlyHook(list, 'beforeChange', args, shouldCallFieldLevelSideEffectHook);
await runSideEffectOnlyHook(list, 'beforeChange', hookArgs, shouldCallFieldLevelSideEffectHook);

// Return the full resolved input (ready for prisma level operation),
// and the afterChange hook to be applied
Expand All @@ -290,7 +245,7 @@ async function resolveInputForCreateOrUpdate(
await runSideEffectOnlyHook(
list,
'afterChange',
{ ...args, updatedItem, existingItem },
{ ...hookArgs, updatedItem, existingItem },
shouldCallFieldLevelSideEffectHook
);
},
Expand Down
19 changes: 5 additions & 14 deletions packages/keystone/src/lib/core/mutations/delete.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { KeystoneContext, DatabaseProvider } from '@keystone-next/types';
import pLimit from 'p-limit';
import { InitialisedList } from '../types-for-lists';
import { getPrismaModelForList, promiseAllRejectWithAllErrors } from '../utils';
import { getPrismaModelForList } from '../utils';
import { resolveUniqueWhereInput, UniqueInputFilter } from '../where-inputs';
import { getAccessControlledItemForDelete } from './access-control';
import { runSideEffectOnlyHook, validationHook } from './hooks';
import { runSideEffectOnlyHook } from './hooks';
import { validateDelete } from './validation';

export function deleteMany(
{ where }: { where: UniqueInputFilter[] },
Expand Down Expand Up @@ -59,20 +60,10 @@ async function processDelete(
uniqueWhere
);

// Field validation
const hookArgs = { operation: 'delete' as const, listKey: list.listKey, context, existingItem };
await validationHook(list.listKey, 'delete', undefined, async addValidationError => {
await promiseAllRejectWithAllErrors(
Object.entries(list.fields).map(async ([fieldPath, field]) => {
await field.hooks.validateDelete?.({ ...hookArgs, addValidationError, fieldPath });
})
);
});

// List validation
await validationHook(list.listKey, 'delete', undefined, async addValidationError => {
await list.hooks.validateDelete?.({ ...hookArgs, addValidationError });
});
// Apply all validation checks
await validateDelete({ list, hookArgs });

// Before delete
await runSideEffectOnlyHook(list, 'beforeDelete', hookArgs, () => true);
Expand Down
30 changes: 0 additions & 30 deletions packages/keystone/src/lib/core/mutations/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,5 @@
import { ValidationFailureError } from '../graphql-errors';
import { promiseAllRejectWithAllErrors } from '../utils';

type ValidationError = { msg: string; data: {}; internalData: {} };

type AddValidationError = (msg: string, data?: {}, internalData?: {}) => void;

export async function validationHook(
listKey: string,
operation: 'create' | 'update' | 'delete',
originalInput: Record<string, string> | undefined,
validationHook: (addValidationError: AddValidationError) => void | Promise<void>
) {
const errors: ValidationError[] = [];

await validationHook((msg, data = {}, internalData = {}) => {
errors.push({ msg, data, internalData });
});

if (errors.length) {
throw new ValidationFailureError({
data: {
messages: errors.map(e => e.msg),
errors: errors.map(e => e.data),
listKey,
operation,
},
internalData: { errors: errors.map(e => e.internalData), data: originalInput },
});
}
}

export async function runSideEffectOnlyHook<
HookName extends string,
List extends {
Expand Down
109 changes: 109 additions & 0 deletions packages/keystone/src/lib/core/mutations/validation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { ValidationFailureError } from '../graphql-errors';
import { InitialisedList } from '../types-for-lists';
import { promiseAllRejectWithAllErrors } from '../utils';

type ValidationError = { msg: string; data: {}; internalData: {} };

type AddValidationError = (msg: string, data?: {}, internalData?: {}) => void;

export async function validationHook(
listKey: string,
operation: 'create' | 'update' | 'delete',
originalInput: Record<string, string> | undefined,
validationHook: (addValidationError: AddValidationError) => void | Promise<void>
) {
const errors: ValidationError[] = [];

await validationHook((msg, data = {}, internalData = {}) => {
errors.push({ msg, data, internalData });
});

if (errors.length) {
throw new ValidationFailureError({
data: {
messages: errors.map(e => e.msg),
errors: errors.map(e => e.data),
listKey,
operation,
},
internalData: { errors: errors.map(e => e.internalData), data: originalInput },
});
}
}

type UpdateCreateHookArgs = Parameters<
Exclude<InitialisedList['hooks']['validateInput'], undefined>
>[0];
export async function validateUpdateCreate({
list,
hookArgs,
}: {
list: InitialisedList;
hookArgs: Omit<UpdateCreateHookArgs, 'addValidationError'>;
}) {
const { operation, resolvedData, originalInput } = hookArgs;
// Check isRequired
await validationHook(list.listKey, operation, originalInput, addValidationError => {
for (const [fieldKey, field] of Object.entries(list.fields)) {
// yes, this is a massive hack, it's just to make image and file fields work well enough
let val = resolvedData[fieldKey];
if (field.dbField.kind === 'multi') {
if (Object.values(resolvedData[fieldKey]).every(x => x === null)) {
val = null;
}
if (Object.values(resolvedData[fieldKey]).every(x => x === undefined)) {
val = undefined;
}
}
if (
field.__legacy?.isRequired &&
((operation === 'create' && val == null) || (operation === 'update' && val === null))
) {
addValidationError(
`Required field "${fieldKey}" is null or undefined.`,
{ resolvedData, operation, originalInput },
{}
);
}
}
});

// Field validation hooks
await validationHook(list.listKey, operation, originalInput, async addValidationError => {
await promiseAllRejectWithAllErrors(
Object.entries(list.fields).map(async ([fieldPath, field]) => {
// @ts-ignore
await field.hooks.validateInput?.({ ...hookArgs, addValidationError, fieldPath });
})
);
});

// List validation hooks
await validationHook(list.listKey, operation, originalInput, async addValidationError => {
// @ts-ignore
await list.hooks.validateInput?.({ ...hookArgs, addValidationError });
});
}

type DeleteHookArgs = Parameters<Exclude<InitialisedList['hooks']['validateDelete'], undefined>>[0];
export async function validateDelete({
list,
hookArgs,
}: {
list: InitialisedList;
hookArgs: Omit<DeleteHookArgs, 'addValidationError'>;
}) {
// Field validation
await validationHook(list.listKey, 'delete', undefined, async addValidationError => {
await promiseAllRejectWithAllErrors(
Object.entries(list.fields).map(async ([fieldPath, field]) => {
await field.hooks.validateDelete?.({ ...hookArgs, addValidationError, fieldPath });
})
);
});

// List validation
await validationHook(list.listKey, 'delete', undefined, async addValidationError => {
await list.hooks.validateDelete?.({ ...hookArgs, addValidationError });
});
}

1 comment on commit 587a8d0

@vercel
Copy link

@vercel vercel bot commented on 587a8d0 Jul 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.