From f1448e2e73444aea7253d590c235592b91d1341d Mon Sep 17 00:00:00 2001 From: Azri Kahar <42867097+azrikahar@users.noreply.github.com> Date: Wed, 9 Nov 2022 23:49:44 +0800 Subject: [PATCH] fix numeric precision and scale default value (#16368) --- api/src/services/fields.test.ts | 356 ++++++++++++++++++++++++++++++++ api/src/services/fields.ts | 2 +- 2 files changed, 357 insertions(+), 1 deletion(-) create mode 100644 api/src/services/fields.test.ts diff --git a/api/src/services/fields.test.ts b/api/src/services/fields.test.ts new file mode 100644 index 0000000000000..d8ea13d1a8ec2 --- /dev/null +++ b/api/src/services/fields.test.ts @@ -0,0 +1,356 @@ +import { Field } from '@directus/shared/types'; +import knex, { Knex } from 'knex'; +import { getTracker, MockClient, Tracker } from 'knex-mock-client'; +import { FieldsService } from '.'; +import { InvalidPayloadException } from '../exceptions'; + +jest.mock('../../src/database/index', () => { + return { + __esModule: true, + default: jest.fn(), + getDatabaseClient: jest.fn().mockReturnValue('postgres'), + getSchemaInspector: jest.fn(), + }; +}); + +describe('Integration Tests', () => { + let db: jest.Mocked; + let tracker: Tracker; + + beforeAll(() => { + db = knex({ client: MockClient }) as jest.Mocked; + tracker = getTracker(); + }); + + afterEach(() => { + tracker.reset(); + }); + + describe('Services / Fields', () => { + let service: FieldsService; + + beforeEach(() => { + service = new FieldsService({ + schema: { collections: {}, relations: [] }, + }); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('addColumnToTable', () => { + let knexCreateTableBuilderSpy: jest.SpyInstance; + + it.each(['alias', 'unknown'])('%s fields should be skipped', async (type) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + + const promise = db.schema.alterTable(testCollection, (table) => { + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as Field); + }); + + await expect(promise).resolves.not.toThrow(); + }); + + it('illegal fields should be throw InvalidPayloadException', async () => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + const type = 'mystery'; + + expect.assertions(2); // to ensure both assertions in the catch block are reached + + try { + await db.schema.alterTable(testCollection, (table) => { + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as any); + }); + } catch (err: any) { + expect(err.message).toBe(`Illegal type passed: "${type}"`); + expect(err).toBeInstanceOf(InvalidPayloadException); + } + }); + + it.each([ + { type: 'integer', method: 'increments' }, + { type: 'bigInteger', method: 'bigIncrements' }, + ])('auto increment $type fields should use $method()', async ({ type, method }) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" .*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable(testCollection, (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, method as keyof Knex.CreateTableBuilder); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + has_auto_increment: true, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField); + }); + + it.each([10, undefined])('string fields should use string() with %j max length', async (maxLength) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + const type = 'string'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" .*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable(testCollection, (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, type as keyof Knex.CreateTableBuilder); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + max_length: maxLength, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField, maxLength); + }); + + it.each(['float', 'decimal'])( + 'precision and scale for %s fields should fallback to default value', + async (type) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" ${type}.*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable(testCollection, (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, type as keyof Knex.CreateTableBuilder); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField, 10, 5); + } + ); + + it.each(['float', 'decimal'])( + 'zero precision or scale for %s fields should not fallback to default value', + async (type) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" ${type}.*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable('test_collection', (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, type as keyof Knex.CreateTableBuilder); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + numeric_precision: 0, + numeric_scale: 0, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField, 0, 0); + } + ); + + it('csv fields should use string()', async () => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + const type = 'csv'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" .*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable(testCollection, (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, 'string'); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField); + }); + + it('hash fields should use string() with length 255', async () => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + const type = 'hash'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" .*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable(testCollection, (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, 'string'); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField, 255); + }); + + it.each([ + { type: 'dateTime', useTz: false }, + { type: 'timestamp', useTz: true }, + ])('$type fields should use $type() with { useTz: $useTz } option', async ({ type, useTz }) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" .*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable(testCollection, (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, type as keyof Knex.CreateTableBuilder); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField, { useTz }); + }); + + it.each(['geometry', 'geometry.Point'])('%s fields should use this.helpers.st.createColumn()', async (type) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" .*`); + tracker.on.any(regex).response({}); + + const thisHelpersStCreateColumnSpy = jest.spyOn(service.helpers.st, 'createColumn'); + + await db.schema.alterTable(testCollection, (table) => { + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as Field); + }); + + expect(thisHelpersStCreateColumnSpy).toHaveBeenCalled(); + }); + + // the rest of KNEX_TYPES except the ones above + it.each([ + { type: 'boolean' }, + { type: 'date' }, + { type: 'json' }, + { type: 'text' }, + { type: 'time' }, + { type: 'binary' }, + { type: 'uuid' }, + ])('$type fields should use $type()', async ({ type }) => { + const testCollection = 'test_collection'; + const testField = 'test_field'; + + const regex = new RegExp(`alter table "${testCollection}" add column "${testField}" .*`); + tracker.on.any(regex).response({}); + + await db.schema.alterTable(testCollection, (table) => { + knexCreateTableBuilderSpy = jest.spyOn(table, type as keyof Knex.CreateTableBuilder); + + service.addColumnToTable(table, { + collection: testCollection, + field: testField, + type, + schema: { + name: testField, + table: testCollection, + data_type: type, + }, + meta: {}, + } as Field); + }); + + expect(knexCreateTableBuilderSpy).toHaveBeenCalledWith(testField); + }); + }); + }); +}); diff --git a/api/src/services/fields.ts b/api/src/services/fields.ts index 50f7406a9c88c..679d93cce7f47 100644 --- a/api/src/services/fields.ts +++ b/api/src/services/fields.ts @@ -592,7 +592,7 @@ export class FieldsService { column = table.string(field.field, field.schema?.max_length ?? undefined); } else if (['float', 'decimal'].includes(field.type)) { const type = field.type as 'float' | 'decimal'; - column = table[type](field.field, field.schema?.numeric_precision || 10, field.schema?.numeric_scale || 5); + column = table[type](field.field, field.schema?.numeric_precision ?? 10, field.schema?.numeric_scale ?? 5); } else if (field.type === 'csv') { column = table.string(field.field); } else if (field.type === 'hash') {