Propagate mutation options for schema apply (#16562)

* Propagate mutation options for schema apply

* Fix unit test

* Add bypassCache flag and remove flushCaches calls

* remove accountability option from getSchema

Co-authored-by: Azri Kahar <42867097+azrikahar@users.noreply.github.com>

api/src/cli/commands/schema/apply.ts

@@ -4,7 +4,6 @@ import { promises as fs } from 'fs';
 import inquirer from 'inquirer';
 import { load as loadYaml } from 'js-yaml';
 import path from 'path';
-import { flushCaches } from '../../../cache';
 import getDatabase, { isInstalled, validateDatabaseConnection } from '../../../database';
 import logger from '../../../logger';
 import { Snapshot } from '../../../types';
@@ -19,8 +18,6 @@ export async function apply(snapshotPath: string, options?: { yes: boolean; dryR
 
 	await validateDatabaseConnection(database);
 
-	await flushCaches();
-
 	if ((await isInstalled()) === false) {
 		logger.error(`Directus isn't installed on this database. Please run "directus bootstrap" first.`);
 		database.destroy();

api/src/cli/commands/schema/snapshot.ts

@@ -5,14 +5,11 @@ import { constants as fsConstants, promises as fs } from 'fs';
 import path from 'path';
 import inquirer from 'inquirer';
 import { dump as toYaml } from 'js-yaml';
-import { flushCaches } from '../../../cache';
 
 export async function snapshot(
 	snapshotPath?: string,
 	options?: { yes: boolean; format: 'json' | 'yaml' }
 ): Promise<void> {
-	await flushCaches();
-
 	const database = getDatabase();
 
 	try {

api/src/middleware/schema.ts

@@ -3,7 +3,7 @@ import asyncHandler from '../utils/async-handler';
 import { getSchema } from '../utils/get-schema';
 
 const schema: RequestHandler = asyncHandler(async (req, res, next) => {
-	req.schema = await getSchema({ accountability: req.accountability });
+	req.schema = await getSchema();
 	return next();
 });
 

api/src/services/collections.ts

@@ -173,7 +173,7 @@ export class CollectionsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;
@@ -222,7 +222,7 @@ export class CollectionsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;
@@ -405,7 +405,7 @@ export class CollectionsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;
@@ -461,7 +461,7 @@ export class CollectionsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;
@@ -511,7 +511,7 @@ export class CollectionsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;
@@ -640,7 +640,7 @@ export class CollectionsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;
@@ -688,7 +688,7 @@ export class CollectionsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;

api/src/services/fields.ts

@@ -242,7 +242,8 @@ export class FieldsService {
 	async createField(
 		collection: string,
 		field: Partial<Field> & { field: string; type: Type | null },
-		table?: Knex.CreateTableBuilder // allows collection creation to
+		table?: Knex.CreateTableBuilder, // allows collection creation to
+		opts?: MutationOptions
 	): Promise<void> {
 		if (this.accountability && this.accountability.admin !== true) {
 			throw new ForbiddenException();
@@ -310,7 +311,7 @@ export class FieldsService {
 					);
 				}
 
-				nestedActionEvents.push({
+				const actionEvent = {
 					event: 'fields.create',
 					meta: {
 						payload: hookAdjustedField,
@@ -322,29 +323,39 @@ export class FieldsService {
 						schema: this.schema,
 						accountability: this.accountability,
 					},
-				});
+				};
+
+				if (opts?.bypassEmitAction) {
+					opts.bypassEmitAction(actionEvent);
+				} else {
+					nestedActionEvents.push(actionEvent);
+				}
 			});
 		} finally {
 			if (runPostColumnChange) {
 				await this.helpers.schema.postColumnChange();
 			}
 
-			if (this.cache && env.CACHE_AUTO_PURGE) {
+			if (this.cache && env.CACHE_AUTO_PURGE && opts?.autoPurgeCache !== false) {
 				await this.cache.clear();
 			}
 
-			await clearSystemCache();
+			if (opts?.autoPurgeSystemCache !== false) {
+				await clearSystemCache();
+			}
 
-			const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
+				const updatedSchema = await getSchema();
 
-			for (const nestedActionEvent of nestedActionEvents) {
-				nestedActionEvent.context.schema = updatedSchema;
-				emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				for (const nestedActionEvent of nestedActionEvents) {
+					nestedActionEvent.context.schema = updatedSchema;
+					emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				}
 			}
 		}
 	}
 
-	async updateField(collection: string, field: RawField): Promise<string> {
+	async updateField(collection: string, field: RawField, opts?: MutationOptions): Promise<string> {
 		if (this.accountability && this.accountability.admin !== true) {
 			throw new ForbiddenException();
 		}
@@ -418,7 +429,7 @@ export class FieldsService {
 				}
 			}
 
-			nestedActionEvents.push({
+			const actionEvent = {
 				event: 'fields.update',
 				meta: {
 					payload: hookAdjustedField,
@@ -430,7 +441,13 @@ export class FieldsService {
 					schema: this.schema,
 					accountability: this.accountability,
 				},
-			});
+			};
+
+			if (opts?.bypassEmitAction) {
+				opts.bypassEmitAction(actionEvent);
+			} else {
+				nestedActionEvents.push(actionEvent);
+			}
 
 			return field.field;
 		} finally {
@@ -438,17 +455,21 @@ export class FieldsService {
 				await this.helpers.schema.postColumnChange();
 			}
 
-			if (this.cache && env.CACHE_AUTO_PURGE) {
+			if (this.cache && env.CACHE_AUTO_PURGE && opts?.autoPurgeCache !== false) {
 				await this.cache.clear();
 			}
 
-			await clearSystemCache();
+			if (opts?.autoPurgeSystemCache !== false) {
+				await clearSystemCache();
+			}
 
-			const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
+				const updatedSchema = await getSchema();
 
-			for (const nestedActionEvent of nestedActionEvents) {
-				nestedActionEvent.context.schema = updatedSchema;
-				emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				for (const nestedActionEvent of nestedActionEvents) {
+					nestedActionEvent.context.schema = updatedSchema;
+					emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				}
 			}
 		}
 	}
@@ -609,7 +630,7 @@ export class FieldsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;

api/src/services/relations.ts

@@ -125,7 +125,7 @@ export class RelationsService {
 	/**
 	 * Create a new relationship / foreign key constraint
 	 */
-	async createOne(relation: Partial<Relation>): Promise<void> {
+	async createOne(relation: Partial<Relation>, opts?: MutationOptions): Promise<void> {
 		if (this.accountability && this.accountability.admin !== true) {
 			throw new ForbiddenException();
 		}
@@ -208,7 +208,8 @@ export class RelationsService {
 				});
 
 				await relationsItemService.createOne(metaRow, {
-					bypassEmitAction: (params) => nestedActionEvents.push(params),
+					bypassEmitAction: (params) =>
+						opts?.bypassEmitAction ? opts.bypassEmitAction(params) : nestedActionEvents.push(params),
 				});
 			});
 		} finally {
@@ -216,13 +217,17 @@ export class RelationsService {
 				await this.helpers.schema.postColumnChange();
 			}
 
-			await clearSystemCache();
+			if (opts?.autoPurgeSystemCache !== false) {
+				await clearSystemCache();
+			}
 
-			const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
+				const updatedSchema = await getSchema();
 
-			for (const nestedActionEvent of nestedActionEvents) {
-				nestedActionEvent.context.schema = updatedSchema;
-				emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				for (const nestedActionEvent of nestedActionEvents) {
+					nestedActionEvent.context.schema = updatedSchema;
+					emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				}
 			}
 		}
 	}
@@ -232,7 +237,12 @@ export class RelationsService {
 	 *
 	 * Note: You can update anything under meta, but only the `on_delete` trigger under schema
 	 */
-	async updateOne(collection: string, field: string, relation: Partial<Relation>): Promise<void> {
+	async updateOne(
+		collection: string,
+		field: string,
+		relation: Partial<Relation>,
+		opts?: MutationOptions
+	): Promise<void> {
 		if (this.accountability && this.accountability.admin !== true) {
 			throw new ForbiddenException();
 		}
@@ -298,7 +308,8 @@ export class RelationsService {
 				if (relation.meta) {
 					if (existingRelation?.meta) {
 						await relationsItemService.updateOne(existingRelation.meta.id, relation.meta, {
-							bypassEmitAction: (params) => nestedActionEvents.push(params),
+							bypassEmitAction: (params) =>
+								opts?.bypassEmitAction ? opts.bypassEmitAction(params) : nestedActionEvents.push(params),
 						});
 					} else {
 						await relationsItemService.createOne(
@@ -309,7 +320,8 @@ export class RelationsService {
 								one_collection: existingRelation.related_collection || null,
 							},
 							{
-								bypassEmitAction: (params) => nestedActionEvents.push(params),
+								bypassEmitAction: (params) =>
+									opts?.bypassEmitAction ? opts.bypassEmitAction(params) : nestedActionEvents.push(params),
 							}
 						);
 					}
@@ -320,13 +332,17 @@ export class RelationsService {
 				await this.helpers.schema.postColumnChange();
 			}
 
-			await clearSystemCache();
+			if (opts?.autoPurgeSystemCache !== false) {
+				await clearSystemCache();
+			}
 
-			const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
+				const updatedSchema = await getSchema();
 
-			for (const nestedActionEvent of nestedActionEvents) {
-				nestedActionEvent.context.schema = updatedSchema;
-				emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				for (const nestedActionEvent of nestedActionEvents) {
+					nestedActionEvent.context.schema = updatedSchema;
+					emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+				}
 			}
 		}
 	}
@@ -405,7 +421,7 @@ export class RelationsService {
 			}
 
 			if (opts?.emitEvents !== false && nestedActionEvents.length > 0) {
-				const updatedSchema = await getSchema({ accountability: this.accountability || undefined });
+				const updatedSchema = await getSchema();
 
 				for (const nestedActionEvent of nestedActionEvents) {
 					nestedActionEvent.context.schema = updatedSchema;

api/src/utils/apply-snapshot.test.ts

@@ -20,6 +20,11 @@ describe('applySnapshot', () => {
 	let db: MockedFunction<Knex>;
 	let tracker: Tracker;
 
+	const mutationOptions = {
+		autoPurgeSystemCache: false,
+		bypassEmitAction: expect.any(Function),
+	};
+
 	beforeEach(() => {
 		db = vi.mocked(knex({ client: Client_PG }));
 		tracker = getTracker();
@@ -112,7 +117,7 @@ describe('applySnapshot', () => {
 			});
 
 			expect(createOneCollectionSpy).toHaveBeenCalledTimes(1);
-			expect(createOneCollectionSpy).toHaveBeenCalledWith(expected);
+			expect(createOneCollectionSpy).toHaveBeenCalledWith(expected, mutationOptions);
 
 			// There should be no fields left to create
 			// they will get filtered in createCollections
@@ -263,8 +268,8 @@ describe('applySnapshot', () => {
 			});
 
 			expect(createOneCollectionSpy).toHaveBeenCalledTimes(2);
-			expect(createOneCollectionSpy).toHaveBeenCalledWith(expected);
-			expect(createOneCollectionSpy).toHaveBeenCalledWith(expected2);
+			expect(createOneCollectionSpy).toHaveBeenCalledWith(expected, mutationOptions);
+			expect(createOneCollectionSpy).toHaveBeenCalledWith(expected2, mutationOptions);
 
 			// There should be no fields left to create
 			// they will get filtered in createCollections

api/src/utils/apply-snapshot.ts

@@ -5,11 +5,12 @@ import { merge, set } from 'lodash';
 import getDatabase from '../database';
 import logger from '../logger';
 import { CollectionsService, FieldsService, RelationsService } from '../services';
-import { Collection, Snapshot, SnapshotDiff, SnapshotField } from '../types';
+import { ActionEventParams, Collection, MutationOptions, Snapshot, SnapshotDiff, SnapshotField } from '../types';
 import { getSchema } from './get-schema';
 import { getSnapshot } from './get-snapshot';
 import { getSnapshotDiff } from './get-snapshot-diff';
 import { getCache } from '../cache';
+import emitter from '../emitter';
 
 type CollectionDelta = {
 	collection: string;
@@ -21,12 +22,18 @@ export async function applySnapshot(
 	options?: { database?: Knex; schema?: SchemaOverview; current?: Snapshot; diff?: SnapshotDiff }
 ): Promise<void> {
 	const database = options?.database ?? getDatabase();
-	const schema = options?.schema ?? (await getSchema({ database }));
+	const schema = options?.schema ?? (await getSchema({ database, bypassCache: true }));
 	const { systemCache } = getCache();
 
 	const current = options?.current ?? (await getSnapshot({ database, schema }));
 	const snapshotDiff = options?.diff ?? getSnapshotDiff(current, snapshot);
 
+	const nestedActionEvents: ActionEventParams[] = [];
+	const mutationOptions: MutationOptions = {
+		autoPurgeSystemCache: false,
+		bypassEmitAction: (params) => nestedActionEvents.push(params),
+	};
+
 	await database.transaction(async (trx) => {
 		const collectionsService = new CollectionsService({ knex: trx, schema });
 
@@ -64,10 +71,13 @@ export async function applySnapshot(
 						});
 
 					try {
-						await collectionsService.createOne({
-							...diff[0].rhs,
-							fields,
-						});
+						await collectionsService.createOne(
+							{
+								...diff[0].rhs,
+								fields,
+							},
+							mutationOptions
+						);
 					} catch (err: any) {
 						logger.error(`Failed to create collection "${collection}"`);
 						throw err;
@@ -93,7 +103,7 @@ export async function applySnapshot(
 
 						for (const relation of relations) {
 							try {
-								await relationsService.deleteOne(relation.collection, relation.field);
+								await relationsService.deleteOne(relation.collection, relation.field, mutationOptions);
 							} catch (err) {
 								logger.error(
 									`Failed to delete collection "${collection}" due to relation "${relation.collection}.${relation.field}"`
@@ -111,7 +121,7 @@ export async function applySnapshot(
 					await deleteCollections(getNestedCollectionsToDelete(collection));
 
 					try {
-						await collectionsService.deleteOne(collection);
+						await collectionsService.deleteOne(collection, mutationOptions);
 					} catch (err) {
 						logger.error(`Failed to delete collection "${collection}"`);
 						throw err;
@@ -168,7 +178,7 @@ export async function applySnapshot(
 
 				if (newValues) {
 					try {
-						await collectionsService.updateOne(collection, newValues);
+						await collectionsService.updateOne(collection, newValues, mutationOptions);
 					} catch (err) {
 						logger.error(`Failed to update collection "${collection}"`);
 						throw err;
@@ -177,12 +187,15 @@ export async function applySnapshot(
 			}
 		}
 
-		const fieldsService = new FieldsService({ knex: trx, schema: await getSchema({ database: trx }) });
+		const fieldsService = new FieldsService({
+			knex: trx,
+			schema: await getSchema({ database: trx, bypassCache: true }),
+		});
 
 		for (const { collection, field, diff } of snapshotDiff.fields) {
 			if (diff?.[0].kind === 'N' && !isNestedMetaUpdate(diff?.[0])) {
 				try {
-					await fieldsService.createField(collection, (diff[0] as DiffNew<Field>).rhs);
+					await fieldsService.createField(collection, (diff[0] as DiffNew<Field>).rhs, undefined, mutationOptions);
 				} catch (err) {
 					logger.error(`Failed to create field "${collection}.${field}"`);
 					throw err;
@@ -196,9 +209,13 @@ export async function applySnapshot(
 
 				if (newValues) {
 					try {
-						await fieldsService.updateField(collection, {
-							...newValues,
-						});
+						await fieldsService.updateField(
+							collection,
+							{
+								...newValues,
+							},
+							mutationOptions
+						);
 					} catch (err) {
 						logger.error(`Failed to update field "${collection}.${field}"`);
 						throw err;
@@ -208,7 +225,7 @@ export async function applySnapshot(
 
 			if (diff?.[0].kind === 'D' && !isNestedMetaUpdate(diff?.[0])) {
 				try {
-					await fieldsService.deleteField(collection, field);
+					await fieldsService.deleteField(collection, field, mutationOptions);
 				} catch (err) {
 					logger.error(`Failed to delete field "${collection}.${field}"`);
 					throw err;
@@ -222,7 +239,10 @@ export async function applySnapshot(
 			}
 		}
 
-		const relationsService = new RelationsService({ knex: trx, schema: await getSchema({ database: trx }) });
+		const relationsService = new RelationsService({
+			knex: trx,
+			schema: await getSchema({ database: trx, bypassCache: true }),
+		});
 
 		for (const { collection, field, diff } of snapshotDiff.relations) {
 			const structure = {};
@@ -233,7 +253,7 @@ export async function applySnapshot(
 
 			if (diff?.[0].kind === 'N') {
 				try {
-					await relationsService.createOne((diff[0] as DiffNew<Relation>).rhs);
+					await relationsService.createOne((diff[0] as DiffNew<Relation>).rhs, mutationOptions);
 				} catch (err) {
 					logger.error(`Failed to create relation "${collection}.${field}"`);
 					throw err;
@@ -247,7 +267,7 @@ export async function applySnapshot(
 
 				if (newValues) {
 					try {
-						await relationsService.updateOne(collection, field, newValues);
+						await relationsService.updateOne(collection, field, newValues, mutationOptions);
 					} catch (err) {
 						logger.error(`Failed to update relation "${collection}.${field}"`);
 						throw err;
@@ -257,7 +277,7 @@ export async function applySnapshot(
 
 			if (diff?.[0].kind === 'D') {
 				try {
-					await relationsService.deleteOne(collection, field);
+					await relationsService.deleteOne(collection, field, mutationOptions);
 				} catch (err) {
 					logger.error(`Failed to delete relation "${collection}.${field}"`);
 					throw err;
@@ -267,6 +287,15 @@ export async function applySnapshot(
 	});
 
 	await systemCache?.clear();
+
+	if (nestedActionEvents.length > 0) {
+		const updatedSchema = await getSchema({ database, bypassCache: true });
+
+		for (const nestedActionEvent of nestedActionEvents) {
+			nestedActionEvent.context.schema = updatedSchema;
+			emitter.emitAction(nestedActionEvent.event, nestedActionEvent.meta, nestedActionEvent.context);
+		}
+	}
 }
 
 export function isNestedMetaUpdate(diff: Diff<SnapshotField | undefined>): boolean {

api/src/utils/get-schema.ts

@@ -1,5 +1,5 @@
 import SchemaInspector from '@directus/schema';
-import { Accountability, Filter, SchemaOverview } from '@directus/shared/types';
+import { Filter, SchemaOverview } from '@directus/shared/types';
 import { parseJSON, toArray } from '@directus/shared/utils';
 import { Knex } from 'knex';
 import { mapValues } from 'lodash';
@@ -15,15 +15,20 @@ import getDefaultValue from './get-default-value';
 import getLocalType from './get-local-type';
 
 export async function getSchema(options?: {
-	accountability?: Accountability;
 	database?: Knex;
+
+	/**
+	 * To bypass any cached schema if bypassCache is enabled.
+	 * Used to ensure schema snapshot/apply is not using outdated schema
+	 */
+	bypassCache?: boolean;
 }): Promise<SchemaOverview> {
 	const database = options?.database || getDatabase();
 	const schemaInspector = SchemaInspector(database);
 
 	let result: SchemaOverview;
 
-	if (env.CACHE_SCHEMA !== false) {
+	if (!options?.bypassCache && env.CACHE_SCHEMA !== false) {
 		let cachedSchema;
 
 		try {

api/src/utils/get-snapshot.ts

@@ -9,7 +9,7 @@ import { SchemaOverview } from '@directus/shared/types';
 
 export async function getSnapshot(options?: { database?: Knex; schema?: SchemaOverview }): Promise<Snapshot> {
 	const database = options?.database ?? getDatabase();
-	const schema = options?.schema ?? (await getSchema({ database }));
+	const schema = options?.schema ?? (await getSchema({ database, bypassCache: true }));
 
 	const collectionsService = new CollectionsService({ knex: database, schema });
 	const fieldsService = new FieldsService({ knex: database, schema });