From a24fa72373307c8bd0172473e8a440a34506aea9 Mon Sep 17 00:00:00 2001 From: rijkvanzanten Date: Fri, 17 Jul 2020 14:42:12 -0400 Subject: [PATCH] Add batch type overrides + Item type --- src/routes/files.ts | 7 +- src/routes/items.ts | 82 ++++++++------------ src/services/activity.ts | 7 +- src/services/files.ts | 9 ++- src/services/folders.ts | 6 +- src/services/items.ts | 144 +++++++++++++++++++----------------- src/services/payload.ts | 18 ++--- src/services/permissions.ts | 7 +- src/services/presets.ts | 6 +- src/services/relations.ts | 6 +- src/services/revisions.ts | 4 +- src/services/roles.ts | 6 +- src/services/settings.ts | 4 +- src/services/users.ts | 6 +- src/services/webhooks.ts | 6 +- src/types/express.d.ts | 3 +- src/types/index.ts | 1 + src/types/items.ts | 6 ++ 18 files changed, 159 insertions(+), 169 deletions(-) create mode 100644 src/types/items.ts diff --git a/src/routes/files.ts b/src/routes/files.ts index 8e5afd2556..86421e88d1 100644 --- a/src/routes/files.ts +++ b/src/routes/files.ts @@ -6,6 +6,7 @@ import * as FilesService from '../services/files'; import logger from '../logger'; import { InvalidPayloadException } from '../exceptions'; import useCollection from '../middleware/use-collection'; +import { Item } from '../types'; const router = express.Router(); @@ -14,7 +15,7 @@ router.use(useCollection('directus_files')); const multipartHandler = (operation: 'create' | 'update') => asyncHandler(async (req, res, next) => { const busboy = new Busboy({ headers: req.headers }); - const savedFiles: Record = []; + const savedFiles: Item[] = []; /** * The order of the fields in multipart/form-data is important. We require that all fields @@ -23,7 +24,7 @@ const multipartHandler = (operation: 'create' | 'update') => */ let disk: string; - let payload: Record = {}; + let payload: Partial = {}; busboy.on('field', (fieldname, val) => { if (fieldname === 'storage') { @@ -134,7 +135,7 @@ router.patch( '/:pk', sanitizeQuery, asyncHandler(async (req, res, next) => { - let file: Record; + let file: Item; if (req.is('multipart/form-data')) { file = await multipartHandler('update')(req, res, next); diff --git a/src/routes/items.ts b/src/routes/items.ts index 729f5cd8c9..f0316f5630 100644 --- a/src/routes/items.ts +++ b/src/routes/items.ts @@ -26,29 +26,16 @@ router.post( userAgent: req.get('user-agent'), }; - const isBatch = Array.isArray(req.body); + const primaryKey = await ItemsService.createItem(req.collection, req.body, accountability); - if (isBatch) { - const body: Record[] = req.body; - const primaryKeys = await ItemsService.createItem(req.collection, body, accountability); - const items = await ItemsService.readItem( - req.collection, - primaryKeys, - req.sanitizedQuery, - accountability - ); - res.json({ data: items || null }); - } else { - const body: Record = req.body; - const primaryKey = await ItemsService.createItem(req.collection, body, accountability); - const item = await ItemsService.readItem( - req.collection, - primaryKey, - req.sanitizedQuery, - accountability - ); - res.json({ data: item || null }); - } + const result = await ItemsService.readItem( + req.collection, + primaryKey, + req.sanitizedQuery, + accountability + ); + + res.json({ data: result || null }); }) ); @@ -132,39 +119,30 @@ router.patch( throw new RouteNotFoundException(req.path); } - const primaryKey = req.params.pk; - const isBatch = primaryKey.includes(','); + const accountability: Accountability = { + user: req.user, + role: req.role, + admin: req.admin, + ip: req.ip, + userAgent: req.get('user-agent'), + }; - if (isBatch) { - const primaryKeys = primaryKey.split(','); - const items = await Promise.all(primaryKeys.map(updateItem)); - return res.json({ data: items || null }); - } else { - const item = await updateItem(primaryKey); - return res.json({ data: item || null }); - } + const primaryKey = req.params.pk.includes(',') ? req.params.pk.split(',') : req.params.pk; + const updatedPrimaryKey = await ItemsService.updateItem( + req.collection, + primaryKey, + req.body, + accountability + ); - async function updateItem(pk: string | number) { - const primaryKey = await ItemsService.updateItem(req.collection, pk, req.body, { - role: req.role, - admin: req.admin, - ip: req.ip, - userAgent: req.get('user-agent'), - user: req.user, - }); + const result = await ItemsService.readItem( + req.collection, + updatedPrimaryKey, + req.sanitizedQuery, + accountability + ); - const item = await ItemsService.readItem( - req.collection, - primaryKey, - req.sanitizedQuery, - { - role: req.role, - admin: req.admin, - } - ); - - return item; - } + res.json({ data: result || null }); }) ); diff --git a/src/services/activity.ts b/src/services/activity.ts index 28b875066d..5257f9c557 100644 --- a/src/services/activity.ts +++ b/src/services/activity.ts @@ -1,6 +1,5 @@ -import { Query } from '../types/query'; import * as ItemsService from './items'; -import { Accountability } from '../types'; +import { Accountability, Item, Query } from '../types'; export enum Action { CREATE = 'create', @@ -12,7 +11,7 @@ export enum Action { AUTHENTICATE = 'authenticate', } -export const createActivity = async (data: Record) => { +export const createActivity = async (data: Partial) => { return await ItemsService.createItem('directus_activity', data); }; @@ -30,7 +29,7 @@ export const readActivity = async ( export const updateActivity = async ( pk: string | number, - data: Record, + data: Partial, accountability: Accountability ) => { return await ItemsService.updateItem('directus_activity', pk, data, accountability); diff --git a/src/services/files.ts b/src/services/files.ts index 9062c46dd7..95b97ab035 100644 --- a/src/services/files.ts +++ b/src/services/files.ts @@ -9,17 +9,17 @@ import parseEXIF from 'exif-reader'; import parseIPTC from '../utils/parse-iptc'; import path from 'path'; import { v4 as uuidv4 } from 'uuid'; -import { Accountability } from '../types'; +import { Accountability, Item } from '../types'; import { Readable } from 'stream'; export const createFile = async ( - data: Record, + data: Partial, stream: NodeJS.ReadableStream, accountability: Accountability ) => { const id = uuidv4(); - const payload: Record = { + const payload: Partial = { ...data, id, }; @@ -70,7 +70,7 @@ export const readFile = async ( export const updateFile = async ( pk: string | number, - data: Record, + data: Partial, accountability: Accountability, stream?: NodeJS.ReadableStream ) => { @@ -98,6 +98,7 @@ export const updateFile = async ( }; export const deleteFile = async (pk: string, accountability: Accountability) => { + /** @todo use ItemsService */ const file = await database .select('storage', 'filename_disk') .from('directus_files') diff --git a/src/services/folders.ts b/src/services/folders.ts index 66887d0432..6d8c39541a 100644 --- a/src/services/folders.ts +++ b/src/services/folders.ts @@ -1,8 +1,8 @@ import * as ItemsService from './items'; -import { Accountability, Query } from '../types'; +import { Accountability, Query, Item } from '../types'; export const createFolder = async ( - data: Record, + data: Partial, accountability: Accountability ): Promise => { return (await ItemsService.createItem('directus_folders', data, accountability)) as string; @@ -18,7 +18,7 @@ export const readFolder = async (pk: string, query: Query, accountability?: Acco export const updateFolder = async ( pk: string, - data: Record, + data: Partial, accountability: Accountability ): Promise => { return (await ItemsService.updateItem('directus_folders', pk, data, accountability)) as string; diff --git a/src/services/items.ts b/src/services/items.ts index 62eb7f9e8b..1c57a687e8 100644 --- a/src/services/items.ts +++ b/src/services/items.ts @@ -2,21 +2,26 @@ import database, { schemaInspector } from '../database'; import { Query } from '../types/query'; import runAST from '../database/run-ast'; import getASTFromQuery from '../utils/get-ast-from-query'; -import { Accountability, Operation } from '../types'; +import { Accountability, Operation, Item } from '../types'; import * as PayloadService from './payload'; import * as PermissionsService from './permissions'; import * as ActivityService from './activity'; import * as RevisionsService from './revisions'; -import { pick } from 'lodash'; +import { pick, clone } from 'lodash'; import logger from '../logger'; +/** + * @todo + * have this support passing in a knex instance, so we can hook it up to the same TX instance + * as batch insert / update + */ async function saveActivityAndRevision( action: ActivityService.Action, collection: string, item: string, - payload: Record, + payload: Partial, accountability: Accountability ) { const activityID = await ActivityService.createActivity({ @@ -43,17 +48,17 @@ async function saveActivityAndRevision( export async function createItem( collection: string, - data: Record[], + data: Partial[], accountability?: Accountability ): Promise<(string | number)[]>; export async function createItem( collection: string, - data: Record, + data: Partial, accountability?: Accountability ): Promise; export async function createItem( collection: string, - data: Record | Record[], + data: Partial | Partial[], accountability?: Accountability ): Promise { const isBatch = Array.isArray(data); @@ -62,7 +67,7 @@ export async function createItem( let payloads = isBatch ? data : [data]; const primaryKeys: (string | number)[] = await Promise.all( - payloads.map(async (payload: Record) => { + payloads.map(async (payload: Partial) => { if (accountability && accountability.admin === false) { payload = await PermissionsService.processValues( 'create', @@ -116,7 +121,7 @@ export async function createItem( }); } -export const readItems = async >( +export const readItems = async >( collection: string, query: Query, accountability?: Accountability @@ -131,27 +136,13 @@ export const readItems = async >( return await PayloadService.processValues('read', collection, records); }; -export async function readItem>( +export const readItem = async ( collection: string, - pk: number | string, - query?: Query, - accountability?: Accountability, - operation?: Operation -): Promise; -export async function readItem>( - collection: string, - pk: (number | string)[], - query?: Query, - accountability?: Accountability, - operation?: Operation -): Promise; -export async function readItem>( - collection: string, - pk: number | string | (number | string)[], + pk: T, query: Query = {}, accountability?: Accountability, operation?: Operation -): Promise { +): Promise : Partial[]> => { // We allow overriding the operation, so we can use the item read logic to validate permissions // for update and delete as well operation = operation || 'read'; @@ -190,56 +181,71 @@ export async function readItem>( const records = await runAST(ast); const processedRecords = await PayloadService.processValues('read', collection, records); return isBatch ? processedRecords : processedRecords[0]; -} +}; -export const updateItem = async ( +export const updateItem = async ( collection: string, - pk: number | string, - data: Record, + pk: T, + data: Partial, accountability?: Accountability -): Promise => { - let payload = data; +): Promise => { + const primaryKeys: any[] = Array.isArray(pk) ? pk : [pk]; - if (accountability && accountability.admin === false) { - await PermissionsService.checkAccess('update', collection, pk, accountability.role); + const updatedPrimaryKeys = database.transaction(async (tx) => { + let payload = clone(data); - payload = await PermissionsService.processValues( - 'validate', - collection, - accountability.role, - data + return await Promise.all( + primaryKeys.map(async (key) => { + if (accountability && accountability.admin === false) { + await PermissionsService.checkAccess( + 'update', + collection, + key, + accountability.role + ); + + payload = await PermissionsService.processValues( + 'validate', + collection, + accountability.role, + data + ); + } + + payload = await PayloadService.processValues('update', collection, payload); + payload = await PayloadService.processM2O(collection, payload); + + const primaryKeyField = await schemaInspector.primary(collection); + + // Only insert the values that actually save to an existing column. This ensures we ignore aliases etc + const columns = await schemaInspector.columns(collection); + + const payloadWithoutAlias = pick( + payload, + columns.map(({ column }) => column) + ); + + await tx(collection) + .update(payloadWithoutAlias) + .where({ [primaryKeyField]: key }); + + if (accountability) { + // Don't await this. It can run async in the background + saveActivityAndRevision( + ActivityService.Action.UPDATE, + collection, + String(pk), + payloadWithoutAlias, + accountability + ).catch((err) => logger.error(err)); + } + + return pk; + }) ); - } + }); - payload = await PayloadService.processValues('update', collection, payload); - payload = await PayloadService.processM2O(collection, payload); - - const primaryKeyField = await schemaInspector.primary(collection); - - // Only insert the values that actually save to an existing column. This ensures we ignore aliases etc - const columns = await schemaInspector.columns(collection); - - const payloadWithoutAlias = pick( - payload, - columns.map(({ column }) => column) - ); - - await database(collection) - .update(payloadWithoutAlias) - .where({ [primaryKeyField]: pk }); - - if (accountability) { - // Don't await this. It can run async in the background - saveActivityAndRevision( - ActivityService.Action.UPDATE, - collection, - String(pk), - payloadWithoutAlias, - accountability - ).catch((err) => logger.error(err)); - } - - return pk; + return Array.isArray(pk) ? updatedPrimaryKeys : updatedPrimaryKeys[0]; }; export const deleteItem = async ( @@ -293,7 +299,7 @@ export const readSingleton = async ( export const upsertSingleton = async ( collection: string, - data: Record, + data: Partial, accountability: Accountability ) => { const primaryKeyField = await schemaInspector.primary(collection); diff --git a/src/services/payload.ts b/src/services/payload.ts index 40859409c3..b969fd2640 100644 --- a/src/services/payload.ts +++ b/src/services/payload.ts @@ -15,11 +15,7 @@ import * as ItemsService from './items'; type Operation = 'create' | 'read' | 'update'; type Transformers = { - [type: string]: ( - operation: Operation, - value: any, - payload: Record - ) => Promise; + [type: string]: (operation: Operation, value: any, payload: Partial) => Promise; }; /** @@ -72,7 +68,7 @@ const transformers: Transformers = { export const processValues = async ( operation: Operation, collection: string, - payload: Record | Record[] + payload: Partial | Partial[] ) => { let processedPayload = clone(payload); @@ -113,7 +109,7 @@ export const processValues = async ( async function processField( field: Pick, - payload: Record, + payload: Partial, operation: Operation ) { if (transformers.hasOwnProperty(field.special)) { @@ -126,7 +122,7 @@ async function processField( /** * Recursively checks for nested relational items, and saves them bottom up, to ensure we have IDs etc ready */ -export const processM2O = async (collection: string, payload: Record) => { +export const processM2O = async (collection: string, payload: Partial) => { const payloadClone = clone(payload); const relations = await database @@ -145,7 +141,7 @@ export const processM2O = async (collection: string, payload: Record { - const relatedRecord: Record = payloadClone[relation.field_many]; + const relatedRecord: Partial = payloadClone[relation.field_many]; const hasPrimaryKey = relatedRecord.hasOwnProperty(relation.primary_one); let relatedPrimaryKey: string | number; @@ -172,7 +168,7 @@ export const processM2O = async (collection: string, payload: Record) => { +export const processO2M = async (collection: string, payload: Partial) => { const payloadClone = clone(payload); const relations = await database @@ -194,7 +190,7 @@ export const processO2M = async (collection: string, payload: Record, index: number) => { + relatedRecords.map(async (relatedRecord: Partial, index: number) => { relatedRecord[relation.field_many] = payloadClone[relation.primary_one]; const hasPrimaryKey = relatedRecord.hasOwnProperty(relation.primary_many); diff --git a/src/services/permissions.ts b/src/services/permissions.ts index 55210a88dd..3486bd622d 100644 --- a/src/services/permissions.ts +++ b/src/services/permissions.ts @@ -6,6 +6,7 @@ import { Query, Permission, Operation, + Item, } from '../types'; import * as ItemsService from './items'; import database from '../database'; @@ -14,7 +15,7 @@ import { uniq } from 'lodash'; import generateJoi from '../utils/generate-joi'; export const createPermission = async ( - data: Record, + data: Partial, accountability: Accountability ): Promise => { return (await ItemsService.createItem('directus_permissions', data, accountability)) as number; @@ -30,7 +31,7 @@ export const readPermission = async (pk: number, query: Query, accountability?: export const updatePermission = async ( pk: number, - data: Record, + data: Partial, accountability: Accountability ): Promise => { return (await ItemsService.updateItem( @@ -183,7 +184,7 @@ export const processValues = async ( operation: Operation, collection: string, role: string | null, - data: Record + data: Partial ) => { const permission = await database .select('*') diff --git a/src/services/presets.ts b/src/services/presets.ts index 86a7bbff86..ae9a288620 100644 --- a/src/services/presets.ts +++ b/src/services/presets.ts @@ -1,10 +1,10 @@ -import { Accountability, Query } from '../types'; +import { Accountability, Query, Item } from '../types'; import * as ItemsService from './items'; /** @todo check if we want to save activity for collection presets */ export const createCollectionPreset = async ( - data: Record, + data: Partial, accountability: Accountability ) => { return await ItemsService.createItem('directus_presets', data, accountability); @@ -24,7 +24,7 @@ export const readCollectionPreset = async ( export const updateCollectionPreset = async ( pk: string | number, - data: Record, + data: Partial, accountability: Accountability ) => { return await ItemsService.updateItem('directus_presets', pk, data, accountability); diff --git a/src/services/relations.ts b/src/services/relations.ts index 1a9c0dbfc5..fcff021b74 100644 --- a/src/services/relations.ts +++ b/src/services/relations.ts @@ -1,7 +1,7 @@ -import { Accountability, Query } from '../types'; +import { Accountability, Query, Item } from '../types'; import * as ItemsService from './items'; -export const createRelation = async (data: Record, accountability: Accountability) => { +export const createRelation = async (data: Partial, accountability: Accountability) => { return await ItemsService.createItem('directus_relations', data, accountability); }; @@ -19,7 +19,7 @@ export const readRelation = async ( export const updateRelation = async ( pk: string | number, - data: Record, + data: Partial, accountability: Accountability ) => { return await ItemsService.updateItem('directus_relations', pk, data, accountability); diff --git a/src/services/revisions.ts b/src/services/revisions.ts index 0d6a7b4538..f0d8b0ed5e 100644 --- a/src/services/revisions.ts +++ b/src/services/revisions.ts @@ -1,7 +1,7 @@ import * as ItemsService from './items'; -import { Accountability, Query } from '../types'; +import { Accountability, Query, Item } from '../types'; -export const createRevision = async (data: Record) => { +export const createRevision = async (data: Partial) => { return await ItemsService.createItem('directus_revisions', data); }; diff --git a/src/services/roles.ts b/src/services/roles.ts index 5558421424..1bdd2c631b 100644 --- a/src/services/roles.ts +++ b/src/services/roles.ts @@ -1,7 +1,7 @@ -import { Accountability, Query } from '../types'; +import { Accountability, Query, Item } from '../types'; import * as ItemsService from './items'; -export const createRole = async (data: Record, accountability: Accountability) => { +export const createRole = async (data: Partial, accountability: Accountability) => { return await ItemsService.createItem('directus_roles', data, accountability); }; @@ -19,7 +19,7 @@ export const readRole = async ( export const updateRole = async ( pk: string | number, - data: Record, + data: Partial, accountability: Accountability ) => { return await ItemsService.updateItem('directus_roles', pk, data, accountability); diff --git a/src/services/settings.ts b/src/services/settings.ts index decb16dffa..60023c737f 100644 --- a/src/services/settings.ts +++ b/src/services/settings.ts @@ -1,4 +1,4 @@ -import { Query } from '../types/query'; +import { Query, Item } from '../types/query'; import * as ItemsService from './items'; import { Accountability } from '../types'; @@ -6,6 +6,6 @@ export const readSettings = async (query: Query, accountability?: Accountability return await ItemsService.readSingleton('directus_settings', query, accountability); }; -export const updateSettings = async (data: Record, accountability: Accountability) => { +export const updateSettings = async (data: Partial, accountability: Accountability) => { return await ItemsService.upsertSingleton('directus_settings', data, accountability); }; diff --git a/src/services/users.ts b/src/services/users.ts index d504ff7c9b..a31f31b0bf 100644 --- a/src/services/users.ts +++ b/src/services/users.ts @@ -4,9 +4,9 @@ import { sendInviteMail } from '../mail'; import database from '../database'; import argon2 from 'argon2'; import { InvalidPayloadException } from '../exceptions'; -import { Accountability, Query } from '../types'; +import { Accountability, Query, Item } from '../types'; -export const createUser = async (data: Record, accountability: Accountability) => { +export const createUser = async (data: Partial, accountability: Accountability) => { return await ItemsService.createItem('directus_users', data, accountability); }; @@ -24,7 +24,7 @@ export const readUser = async ( export const updateUser = async ( pk: string | number, - data: Record, + data: Partial, accountability: Accountability ) => { /** diff --git a/src/services/webhooks.ts b/src/services/webhooks.ts index 5e7476b0ed..9d6985c10d 100644 --- a/src/services/webhooks.ts +++ b/src/services/webhooks.ts @@ -1,7 +1,7 @@ -import { Accountability, Query } from '../types'; +import { Accountability, Query, Item } from '../types'; import * as ItemsService from './items'; -export const createWebhook = async (data: Record, accountability: Accountability) => { +export const createWebhook = async (data: Partial, accountability: Accountability) => { return await ItemsService.createItem('directus_webhooks', data, accountability); }; @@ -19,7 +19,7 @@ export const readWebhook = async ( export const updateWebhook = async ( pk: string | number, - data: Record, + data: Partial, accountability: Accountability ) => { return await ItemsService.updateItem('directus_webhooks', pk, data, accountability); diff --git a/src/types/express.d.ts b/src/types/express.d.ts index 33db09d58f..0cb8e75b6e 100644 --- a/src/types/express.d.ts +++ b/src/types/express.d.ts @@ -3,6 +3,7 @@ */ import { Permission } from './permissions'; +import { Query } from './query'; export {}; @@ -14,7 +15,7 @@ declare global { role: string | null; admin: boolean; collection?: string; - sanitizedQuery?: Record; + sanitizedQuery?: Query; single?: boolean; permissions?: Permission; } diff --git a/src/types/index.ts b/src/types/index.ts index 796d30f5b1..50fd7341d5 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -9,3 +9,4 @@ export * from './permissions'; export * from './query'; export * from './relation'; export * from './sessions'; +export * from './items'; diff --git a/src/types/items.ts b/src/types/items.ts new file mode 100644 index 0000000000..19aa2e514c --- /dev/null +++ b/src/types/items.ts @@ -0,0 +1,6 @@ +/** + * I know this looks a little silly, but it allows us to explicitly differentiate between when we're + * expecting an item vs any other generic object. + */ + +export type Item = Record;