Optimize number of times cache is being cleared in ItemsService updateBatch (#16453)

* improve cache clearing for updateBatch

* add service level data-is-array check

Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>

api/src/services/items.test.ts

@@ -1,16 +1,40 @@
 import { Query } from '@directus/shared/types';
 import knex, { Knex } from 'knex';
 import { getTracker, MockClient, Tracker } from 'knex-mock-client';
+import { cloneDeep } from 'lodash';
+import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
 import { ItemsService } from '../../src/services';
+import { InvalidPayloadException } from '../exceptions';
 import { sqlFieldFormatter, sqlFieldList } from '../__utils__/items-utils';
 import { systemSchema, userSchema } from '../__utils__/schemas';
-import { cloneDeep } from 'lodash';
-import { describe, beforeAll, afterEach, it, expect, vi } from 'vitest';
+
+vi.mock('../env', async () => {
+	const actual = (await vi.importActual('../env')) as { default: Record<string, any> };
+
+	return {
+		default: {
+			...actual.default,
+			CACHE_AUTO_PURGE: true,
+		},
+	};
+});
 
 vi.mock('../../src/database/index', () => {
-	return { getDatabaseClient: vi.fn().mockReturnValue('postgres') };
+	return { default: vi.fn(), getDatabaseClient: vi.fn().mockReturnValue('postgres') };
+});
+
+vi.mock('../cache', () => {
+	return {
+		getCache: vi.fn().mockReturnValue({
+			cache: {
+				clear: vi.fn(),
+			},
+			systemCache: {
+				clear: vi.fn(),
+			},
+		}),
+	};
 });
-vi.mock('../../src/database/index');
 
 describe('Integration Tests', () => {
 	let db: Knex;
@@ -971,4 +995,64 @@ describe('Integration Tests', () => {
 			}
 		);
 	});
+
+	describe('updateBatch', () => {
+		const items = [
+			{ id: '6107c897-9182-40f7-b22e-4f044d1258d2', name: 'Item 1' },
+			{ id: '6e7d4a2c-e62f-43b4-a343-9196f4b1783f', name: 'Item 2' },
+		];
+
+		it.each(Object.keys(schemas))('%s batch update should only clear cache once', async (schema) => {
+			const table = schemas[schema].tables[0];
+			schemas[schema].accountability = null;
+
+			tracker.on.select(table).response(items);
+			tracker.on.update(table).response(items);
+
+			const itemsService = new ItemsService(table, {
+				knex: db,
+				accountability: {
+					role: 'admin',
+					admin: true,
+				},
+				schema: schemas[schema].schema,
+			});
+
+			const itemsServiceCacheClearSpy = vi.spyOn(itemsService.cache, 'clear' as never).mockResolvedValue(() => vi.fn());
+
+			await itemsService.updateBatch(items);
+
+			expect(itemsServiceCacheClearSpy).toHaveBeenCalledOnce();
+		});
+
+		it.each(Object.keys(schemas))(
+			'%s batch update should throw InvalidPayloadException when passing non-array data',
+			async (schema) => {
+				const table = schemas[schema].tables[0];
+				schemas[schema].accountability = null;
+
+				tracker.on.select(table).response({});
+				tracker.on.update(table).response({});
+
+				const itemsService = new ItemsService(table, {
+					knex: db,
+					accountability: {
+						role: 'admin',
+						admin: true,
+					},
+					schema: schemas[schema].schema,
+				});
+
+				expect.assertions(2);
+
+				try {
+					// intentional `as any` to test non-array data on runtime
+					await itemsService.updateBatch(items[0] as any);
+				} catch (err) {
+					expect(err.message).toBe(`Input should be an array of items.`);
+					expect(err).toBeInstanceOf(InvalidPayloadException);
+				}
+			}
+		);
+	});
 });

api/src/services/items.ts

@@ -453,22 +453,33 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
 	 * Update multiple items in a single transaction
 	 */
 	async updateBatch(data: Partial<Item>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
+		if (!Array.isArray(data)) {
+			throw new InvalidPayloadException('Input should be an array of items.');
+		}
+
 		const primaryKeyField = this.schema.collections[this.collection].primary;
 
 		const keys: PrimaryKey[] = [];
 
-		await this.knex.transaction(async (trx) => {
-			const service = new ItemsService(this.collection, {
-				accountability: this.accountability,
-				knex: trx,
-				schema: this.schema,
-			});
+		try {
+			await this.knex.transaction(async (trx) => {
+				const service = new ItemsService(this.collection, {
+					accountability: this.accountability,
+					knex: trx,
+					schema: this.schema,
+				});
 
-			for (const item of data) {
-				if (!item[primaryKeyField]) throw new InvalidPayloadException(`Item in update misses primary key.`);
-				keys.push(await service.updateOne(item[primaryKeyField]!, omit(item, primaryKeyField), opts));
+				for (const item of data) {
+					if (!item[primaryKeyField]) throw new InvalidPayloadException(`Item in update misses primary key.`);
+					const combinedOpts = Object.assign({ autoPurgeCache: false }, opts);
+					keys.push(await service.updateOne(item[primaryKeyField]!, omit(item, primaryKeyField), combinedOpts));
+				}
+			});
+		} finally {
+			if (this.cache && env.CACHE_AUTO_PURGE && opts?.autoPurgeCache !== false) {
+				await this.cache.clear();
 			}
-		});
+		}
 
 		return keys;
 	}