From bbcf76e030af231e93e0bfb4da13dabdc7ec0296 Mon Sep 17 00:00:00 2001 From: Brainslug Date: Thu, 15 Dec 2022 06:34:31 +0100 Subject: [PATCH] enforce uppercase UUIDs for MS SQL (#16691) * add forced uppercase for UUIDs in MS SQL * test whether mssql returned uuid is uppercase * use once just for slight better correctness * once is actually not enough Co-authored-by: Azri Kahar <42867097+azrikahar@users.noreply.github.com> Co-authored-by: ian --- .../database/helpers/schema/dialects/mssql.ts | 7 +++++ api/src/database/helpers/schema/index.ts | 2 +- api/src/database/helpers/schema/types.ts | 4 +++ api/src/services/items.test.ts | 27 +++++++++++++++++-- api/src/services/items.ts | 8 +++++- 5 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 api/src/database/helpers/schema/dialects/mssql.ts diff --git a/api/src/database/helpers/schema/dialects/mssql.ts b/api/src/database/helpers/schema/dialects/mssql.ts new file mode 100644 index 0000000000..caf967d0e5 --- /dev/null +++ b/api/src/database/helpers/schema/dialects/mssql.ts @@ -0,0 +1,7 @@ +import { SchemaHelper } from '../types'; + +export class SchemaHelperMSSQL extends SchemaHelper { + formatUUID(uuid: string): string { + return uuid.toUpperCase(); + } +} diff --git a/api/src/database/helpers/schema/index.ts b/api/src/database/helpers/schema/index.ts index d1c2c7d3c9..d38554adfe 100644 --- a/api/src/database/helpers/schema/index.ts +++ b/api/src/database/helpers/schema/index.ts @@ -4,4 +4,4 @@ export { SchemaHelperDefault as redshift } from './dialects/default'; export { SchemaHelperOracle as oracle } from './dialects/oracle'; export { SchemaHelperSQLite as sqlite } from './dialects/sqlite'; export { SchemaHelperDefault as mysql } from './dialects/default'; -export { SchemaHelperDefault as mssql } from './dialects/default'; +export { SchemaHelperMSSQL as mssql } from './dialects/mssql'; diff --git a/api/src/database/helpers/schema/types.ts b/api/src/database/helpers/schema/types.ts index eb40090f54..2fff855b20 100644 --- a/api/src/database/helpers/schema/types.ts +++ b/api/src/database/helpers/schema/types.ts @@ -96,4 +96,8 @@ export abstract class SchemaHelper extends DatabaseHelper { // reference issue #14873 return existingName; } + + formatUUID(uuid: string): string { + return uuid; // no-op by defaut + } } diff --git a/api/src/services/items.test.ts b/api/src/services/items.test.ts index 5c7c75d011..70f0fac9f6 100644 --- a/api/src/services/items.test.ts +++ b/api/src/services/items.test.ts @@ -2,11 +2,12 @@ import { NestedDeepQuery } 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, MockedFunction } from 'vitest'; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi, MockedFunction } 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 { getDatabaseClient } from '../../src/database/index'; vi.mock('../env', async () => { const actual = (await vi.importActual('../env')) as { default: Record }; @@ -21,7 +22,7 @@ vi.mock('../env', async () => { vi.mock('../../src/database/index', () => ({ default: vi.fn(), - getDatabaseClient: vi.fn().mockReturnValue('postgres'), + getDatabaseClient: vi.fn(), })); vi.mock('../cache', () => ({ @@ -49,6 +50,10 @@ describe('Integration Tests', () => { tracker = getTracker(); }); + beforeEach(() => { + vi.mocked(getDatabaseClient).mockReturnValue('postgres'); + }); + afterEach(() => { tracker.reset(); }); @@ -80,6 +85,24 @@ describe('Integration Tests', () => { expect(response).toBe(item.id); } ); + + it(`the returned UUID primary key for MS SQL should be uppercase`, async () => { + vi.mocked(getDatabaseClient).mockReturnValue('mssql'); + + const table = schemas.system.tables[0]; + + const itemsService = new ItemsService(table, { + knex: db, + accountability: { role: 'admin', admin: true }, + schema: schemas.system.schema, + }); + + tracker.on.insert(table).responseOnce(item); + + const response = await itemsService.createOne(item, { emitEvents: false }); + + expect(response).toBe(item.id.toUpperCase()); + }); }); describe('readOne', () => { diff --git a/api/src/services/items.ts b/api/src/services/items.ts index 479276d233..cc58ed84ac 100644 --- a/api/src/services/items.ts +++ b/api/src/services/items.ts @@ -4,6 +4,7 @@ import { Knex } from 'knex'; import { assign, clone, cloneDeep, omit, pick, without } from 'lodash'; import { getCache } from '../cache'; import getDatabase from '../database'; +import { getHelpers } from '../database/helpers'; import runAST from '../database/run-ast'; import emitter from '../emitter'; import env from '../env'; @@ -145,7 +146,12 @@ export class ItemsService implements AbstractSer .then((result) => result[0]); const returnedKey = typeof result === 'object' ? result[primaryKeyField] : result; - primaryKey = primaryKey ?? returnedKey; + + if (this.schema.collections[this.collection].fields[primaryKeyField].type === 'uuid') { + primaryKey = getHelpers(trx).schema.formatUUID(primaryKey ?? returnedKey); + } else { + primaryKey = primaryKey ?? returnedKey; + } } catch (err: any) { throw await translateDatabaseError(err); }