From cc57f3e713dac16ea3a4dc2ef8713041b41a03e2 Mon Sep 17 00:00:00 2001 From: rijkvanzanten Date: Wed, 29 Jul 2020 17:34:08 -0400 Subject: [PATCH] Fix creating new collections --- api/src/routes/collections.ts | 48 ++++----- api/src/services/collections.ts | 184 +++++++++++--------------------- api/src/services/fields.ts | 78 ++++++++------ api/src/services/items.ts | 2 +- api/src/types/collection.ts | 3 + 5 files changed, 130 insertions(+), 185 deletions(-) diff --git a/api/src/routes/collections.ts b/api/src/routes/collections.ts index 6d04a2deea..0b3c003062 100644 --- a/api/src/routes/collections.ts +++ b/api/src/routes/collections.ts @@ -1,38 +1,23 @@ import { Router } from 'express'; import asyncHandler from 'express-async-handler'; import sanitizeQuery from '../middleware/sanitize-query'; -import * as CollectionsService from '../services/collections'; +import CollectionsService from '../services/collections'; import { schemaInspector } from '../database'; -import { InvalidPayloadException, CollectionNotFoundException } from '../exceptions'; -import Joi from 'joi'; +import { CollectionNotFoundException } from '../exceptions'; import useCollection from '../middleware/use-collection'; const router = Router(); -const fieldSchema = Joi.object({ - field: Joi.string().required(), - datatype: Joi.string().required(), - note: Joi.string().required(), - primary_key: Joi.boolean(), - auto_increment: Joi.boolean(), -}); - -const collectionSchema = Joi.object({ - collection: Joi.string().required(), - fields: Joi.array().items(fieldSchema).min(1).unique().required(), - note: Joi.string(), -}); - router.post( '/', useCollection('directus_collections'), asyncHandler(async (req, res) => { - const { error } = collectionSchema.validate(req.body); - if (error) throw new InvalidPayloadException(error.message); + const collectionsService = new CollectionsService({ accountability: req.accountability }); - const createdCollection = await CollectionsService.create(req.body, req.accountability); + const collectionKey = await collectionsService.create(req.body); + const record = await collectionsService.readByKey(collectionKey); - res.json({ data: createdCollection || null }); + res.json({ data: record || null }); }) ); @@ -41,10 +26,9 @@ router.get( useCollection('directus_collections'), sanitizeQuery, asyncHandler(async (req, res) => { - const collections = await CollectionsService.readAll( - req.sanitizedQuery, - req.accountability - ); + const collectionsService = new CollectionsService({ accountability: req.accountability }); + const collections = await collectionsService.readByQuery(req.sanitizedQuery); + res.json({ data: collections || null }); }) ); @@ -54,15 +38,17 @@ router.get( useCollection('directus_collections'), sanitizeQuery, asyncHandler(async (req, res) => { + /** @todo move this validation to CollectionsService methods */ const exists = await schemaInspector.hasTable(req.params.collection); - if (exists === false) throw new CollectionNotFoundException(req.params.collection); - const collection = await CollectionsService.readOne( + const collectionsService = new CollectionsService({ accountability: req.accountability }); + + const collection = await collectionsService.readByKey( req.params.collection, - req.sanitizedQuery, - req.accountability + req.sanitizedQuery ); + res.json({ data: collection || null }); }) ); @@ -71,11 +57,13 @@ router.delete( '/:collection', useCollection('directus_collections'), asyncHandler(async (req, res) => { + /** @todo move this validation to CollectionsService methods */ if ((await schemaInspector.hasTable(req.params.collection)) === false) { throw new CollectionNotFoundException(req.params.collection); } - await CollectionsService.deleteCollection(req.params.collection, req.accountability); + const collectionsService = new CollectionsService({ accountability: req.accountability }); + await collectionsService.delete(req.params.collection); res.end(); }) diff --git a/api/src/services/collections.ts b/api/src/services/collections.ts index 3c6f333da8..21e4f7f527 100644 --- a/api/src/services/collections.ts +++ b/api/src/services/collections.ts @@ -1,135 +1,77 @@ import database, { schemaInspector } from '../database'; -import ItemsService from '../services/items'; import { Query } from '../types/query'; -import { ColumnBuilder } from 'knex'; -import { Accountability } from '../types/accountability'; +import { AbstractServiceOptions, Accountability, Collection } from '../types'; +import Knex from 'knex'; +import ItemsService from '../services/items'; +import FieldsService from '../services/fields'; +import { omit } from 'lodash'; -/** - * @TODO turn this into a class - */ +export default class CollectionsService extends ItemsService { + knex: Knex; + accountability: Accountability | null; + itemsService: ItemsService; + fieldsService: FieldsService; -/** @Todo properly type this */ -export const create = async (payload: any, accountability?: Accountability) => { - const itemsService = new ItemsService('directus_collections', { accountability }); + constructor(options?: AbstractServiceOptions) { + super('directus_collections', options); - await database.schema.createTable(payload.collection, (table) => { - if (payload.note) { - table.comment(payload.note); + this.knex = options?.knex || database; + this.accountability = options?.accountability || null; + this.itemsService = new ItemsService('directus_collections', options); + this.fieldsService = new FieldsService(options); + } + + create(data: Partial[]): Promise; + create(data: Partial): Promise; + async create(data: Partial | Partial[]): Promise { + const payloads = Array.isArray(data) ? data : [data]; + + // We'll create the fields separately. We don't want them to be inserted relationally + const payloadsWithoutFields = payloads.map((payload) => omit(payload, 'fields')); + + await this.itemsService.create(payloadsWithoutFields); + + for (const payload of payloads) { + // @TODO add basic validation to ensure all used fields are provided before attempting to save + await this.knex.schema.createTable(payload.collection!, async (table) => { + for (const field of payload.fields!) { + await this.fieldsService.createField(payload.collection!, field, table); + } + }); } - /** @todo move this into fields service */ - - /** @todo adhere to new nested structure system vs database */ - - payload.fields?.forEach((field: any) => { - let column: ColumnBuilder; - - if (field.auto_increment) { - column = table.increments(field.field); - } else { - const datatype = field.length - ? `${field.datatype}(${field.length})` - : field.datatype; - column = table.specificType(field.field, datatype); - - // increments() also sets primary key - if (field.primary_key) { - column.primary(); - } - } - - if (field.note) { - column.comment(field.note); - } - }); - }); - - const primaryKey = await itemsService.create({ - collection: payload.collection, - hidden: payload.hidden || false, - singleton: payload.singleton || false, - icon: payload.icon || null, - note: payload.note || null, - translation: payload.translation || null, - }); + const collectionNames = payloads.map((payload) => payload.collection!); + return Array.isArray(data) ? collectionNames : collectionNames[0]; + } /** - * @TODO make this flexible and based on payload + * @todo + * update w/ nested fields */ - await database('directus_fields').insert( - payload.fields.map((field: any) => ({ - collection: payload.collection, - field: field.field, - locked: false, - required: false, - readonly: false, - hidden_detail: false, - hidden_browse: false, - })) - ); - return await itemsService.readByKey(primaryKey); -}; + delete(collection: string): Promise; + delete(collections: string[]): Promise; + async delete(collection: string | string[]): Promise { + const collections = Array.isArray(collection) ? collection : [collection]; -export const readAll = async (query: Query, accountability?: Accountability) => { - const itemsService = new ItemsService('directus_collections', { accountability }); + /** + * @todo check permissions manually + * this.itemsService.delete does the permissions check, but we have to delete the records from fields/relations first + * to adhere to the foreign key constraints + */ - const [tables, collections] = await Promise.all([ - schemaInspector.tableInfo(), - itemsService.readByQuery(query), - ]); - - const data = tables.map((table) => { - const collectionInfo = collections.find((collection) => { - return collection.collection === table.name; - }); - - return { - collection: table.name, - note: table.comment, - hidden: collectionInfo?.hidden || false, - singleton: collectionInfo?.singleton || false, - icon: collectionInfo?.icon || null, - translation: collectionInfo?.translation || null, - }; - }); - - return data; -}; - -export const readOne = async ( - collection: string, - query: Query, - accountability?: Accountability -) => { - const itemsService = new ItemsService('directus_collections', { accountability }); - - const [table, collectionInfo] = await Promise.all([ - schemaInspector.tableInfo(collection), - itemsService.readByQuery(query), - ]); - - return { - collection: table.name, - note: table.comment, - hidden: collectionInfo[0]?.hidden || false, - singleton: collectionInfo[0]?.singleton || false, - icon: collectionInfo[0]?.icon || null, - translation: collectionInfo[0]?.translation || null, - }; -}; - -export const deleteCollection = async (collection: string, accountability?: Accountability) => { - const itemsService = new ItemsService('directus_collections', { accountability }); - - await Promise.all([ - database.schema.dropTable(collection), - itemsService.delete(collection), - database.delete().from('directus_fields').where({ collection }), - database + await this.knex('directus_fields').delete().whereIn('collection', collections); + await this.knex('directus_relations') .delete() - .from('directus_relations') - .where({ many_collection: collection }) - .orWhere({ one_collection: collection }), - ]); -}; + .whereIn('many_collection', collections) + .orWhereIn('one_collection', collections); + + await this.itemsService.delete(collection as any); + + for (const collectionName of collections) { + await this.knex.schema.dropTable(collectionName); + } + + return collection; + } +} diff --git a/api/src/services/fields.ts b/api/src/services/fields.ts index f09f4d5795..3ca50a4f2c 100644 --- a/api/src/services/fields.ts +++ b/api/src/services/fields.ts @@ -7,7 +7,7 @@ import { ColumnBuilder } from 'knex'; import getLocalType from '../utils/get-local-type'; import { types } from '../types'; import { FieldNotFoundException } from '../exceptions'; -import Knex from 'knex'; +import Knex, { CreateTableBuilder } from 'knex'; type RawField = Partial & { field: string; type: typeof types[number] }; @@ -88,53 +88,65 @@ export default class FieldsService { async createField( collection: string, field: Partial & { field: string; type: typeof types[number] }, - accountability?: Accountability + table?: CreateTableBuilder // allows collection creation to ) { - const itemsService = new ItemsService('directus_fields', { accountability }); - /** * @todo * Check if table / directus_fields row already exists */ if (field.database) { - await database.schema.alterTable(collection, (table) => { - let column: ColumnBuilder; - - if (!field.database) return; - - if (field.type === 'string') { - column = table.string( - field.field, - field.database.max_length !== null ? field.database.max_length : undefined - ); - } else if (['float', 'decimal'].includes(field.type)) { - const type = field.type as 'float' | 'decimal'; - /** @todo add precision and scale support */ - column = table[type](field.field /* precision, scale */); - } else { - column = table[field.type](field.field); - } - - if (field.database.default_value) { - column.defaultTo(field.database.default_value); - } - - if (field.database.is_nullable && field.database.is_nullable === true) { - column.nullable(); - } else { - column.notNullable(); - } - }); + if (table) { + addColumnToTable(table); + } else { + await database.schema.alterTable(collection, (table) => { + addColumnToTable(table); + }); + } } if (field.system) { - await itemsService.create({ + await this.service.create({ ...field.system, collection: collection, field: field.field, }); } + + function addColumnToTable(table: CreateTableBuilder) { + let column: ColumnBuilder; + + if (!field.database) return; + + if (field.database.has_auto_increment) { + column = table.increments(field.field); + } else if (field.type === 'string') { + column = table.string( + field.field, + field.database.max_length !== null ? field.database.max_length : undefined + ); + } else if (['float', 'decimal'].includes(field.type)) { + const type = field.type as 'float' | 'decimal'; + /** @todo add precision and scale support */ + column = table[type](field.field /* precision, scale */); + } else { + column = table[field.type](field.field); + } + + if (field.database.default_value) { + column.defaultTo(field.database.default_value); + } + + if (field.database.is_nullable && field.database.is_nullable === true) { + column.nullable(); + } else { + column.notNullable(); + } + + if (field.database.is_primary_key) { + column.primary(); + } + } } /** @todo research how to make this happen in SQLite / Redshift */ diff --git a/api/src/services/items.ts b/api/src/services/items.ts index 0f3f506f9e..929aba51cc 100644 --- a/api/src/services/items.ts +++ b/api/src/services/items.ts @@ -79,7 +79,7 @@ export default class ItemsService implements AbstractService { const primaryKeys: PrimaryKey[] = await trx .insert(payloadsWithoutAliases) .into(this.collection) - .returning('id'); + .returning(primaryKeyField); if (this.accountability) { const activityRecords = primaryKeys.map((key) => ({ diff --git a/api/src/types/collection.ts b/api/src/types/collection.ts index 929320fd72..999d8ed5af 100644 --- a/api/src/types/collection.ts +++ b/api/src/types/collection.ts @@ -1,3 +1,5 @@ +import { Field } from './field'; + export type Collection = { collection: string; note: string | null; @@ -5,4 +7,5 @@ export type Collection = { single: boolean; icon: string | null; translation: Record; + fields?: Field[]; };