From bb4bfd3d41fdd415f68e6e13f0d4a7e8a0093933 Mon Sep 17 00:00:00 2001 From: wangsijie Date: Fri, 23 Sep 2022 17:16:57 +0800 Subject: [PATCH] feat(core,schemas): use timestamp to version migrations --- packages/core/package.json | 2 - packages/core/src/env-set/index.ts | 3 +- packages/core/src/migration/constants.ts | 4 +- packages/core/src/migration/index.test.ts | 67 ++++++++++--------- packages/core/src/migration/index.ts | 41 +++++------- packages/core/src/migration/utils.test.ts | 26 +++---- packages/core/src/migration/utils.ts | 18 ++--- packages/schemas/migrations/README.md | 4 +- ...next-1663923211-machine-to-machine-app.ts} | 0 packages/schemas/migrations/types.ts | 6 +- pnpm-lock.yaml | 12 +--- 11 files changed, 77 insertions(+), 106 deletions(-) rename packages/schemas/migrations/{next.ts => next-1663923211-machine-to-machine-app.ts} (100%) diff --git a/packages/core/package.json b/packages/core/package.json index 92e818b79..638c0073b 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -57,7 +57,6 @@ "query-string": "^7.0.1", "rimraf": "^3.0.2", "roarr": "^7.11.0", - "semver": "^7.3.7", "slonik": "^30.0.0", "slonik-interceptor-preset": "^1.2.10", "slonik-sql-tag-raw": "^1.1.4", @@ -86,7 +85,6 @@ "@types/node": "^16.3.1", "@types/oidc-provider": "^7.11.1", "@types/rimraf": "^3.0.2", - "@types/semver": "^7.3.12", "@types/supertest": "^2.0.11", "@types/tar": "^6.1.2", "copyfiles": "^2.4.1", diff --git a/packages/core/src/env-set/index.ts b/packages/core/src/env-set/index.ts index 3383417e5..5962675a7 100644 --- a/packages/core/src/env-set/index.ts +++ b/packages/core/src/env-set/index.ts @@ -82,7 +82,8 @@ function createEnvSet() { pool = await createPoolByEnv(values.isTest); await addConnectors(values.connectorDirectory); - if (pool) { + // FIXME: @sijie temparaly disable migration for integration test + if (pool && !values.isIntegrationTest) { await checkMigrationState(pool); } }, diff --git a/packages/core/src/migration/constants.ts b/packages/core/src/migration/constants.ts index 7cd50d399..b97984707 100644 --- a/packages/core/src/migration/constants.ts +++ b/packages/core/src/migration/constants.ts @@ -1,3 +1,3 @@ -export const databaseVersionKey = 'databaseVersion'; +export const migrationStateKey = 'migrationState'; export const logtoConfigsTableFilePath = 'node_modules/@logto/schemas/tables/logto_configs.sql'; -export const migrationFilesDirectory = 'node_modules/@logto/schemas/lib/migrations'; +export const migrationFilesDirectory = 'node_modules/@logto/schemas/migrations'; diff --git a/packages/core/src/migration/index.test.ts b/packages/core/src/migration/index.test.ts index f1d87a33d..dae4792ce 100644 --- a/packages/core/src/migration/index.test.ts +++ b/packages/core/src/migration/index.test.ts @@ -5,15 +5,14 @@ import { convertToIdentifiers } from '@/database/utils'; import { QueryType, expectSqlAssert } from '@/utils/test-utils'; import * as functions from '.'; -import { databaseVersionKey } from './constants'; +import { migrationStateKey } from './constants'; const mockQuery: jest.MockedFunction = jest.fn(); const { createLogtoConfigsTable, - getCurrentDatabaseVersion, isLogtoConfigsTableExists, - updateDatabaseVersion, - getMigrationFiles, + updateDatabaseTimestamp, + getCurrentDatabaseTimestamp, getUndeployedMigrations, } = functions; const pool = createMockPool({ @@ -22,6 +21,7 @@ const pool = createMockPool({ }, }); const { table, fields } = convertToIdentifiers(LogtoConfigs); +const timestamp = 1_663_923_776; describe('isLogtoConfigsTableExists()', () => { it('generates "select exists" sql and query for result', async () => { @@ -45,11 +45,11 @@ describe('isLogtoConfigsTableExists()', () => { }); }); -describe('getCurrentDatabaseVersion()', () => { +describe('getCurrentDatabaseTimestamp()', () => { it('returns null if query failed (table not found)', async () => { mockQuery.mockRejectedValueOnce(new Error('table not found')); - await expect(getCurrentDatabaseVersion(pool)).resolves.toBeNull(); + await expect(getCurrentDatabaseTimestamp(pool)).resolves.toBeNull(); }); it('returns null if the row is not found', async () => { @@ -59,12 +59,12 @@ describe('getCurrentDatabaseVersion()', () => { mockQuery.mockImplementationOnce(async (sql, values) => { expectSqlAssert(sql, expectSql.sql); - expect(values).toEqual([databaseVersionKey]); + expect(values).toEqual([migrationStateKey]); return createMockQueryResult([]); }); - await expect(getCurrentDatabaseVersion(pool)).resolves.toBeNull(); + await expect(getCurrentDatabaseTimestamp(pool)).resolves.toBeNull(); }); it('returns null if the value is in bad format', async () => { @@ -74,29 +74,28 @@ describe('getCurrentDatabaseVersion()', () => { mockQuery.mockImplementationOnce(async (sql, values) => { expectSqlAssert(sql, expectSql.sql); - expect(values).toEqual([databaseVersionKey]); + expect(values).toEqual([migrationStateKey]); - return createMockQueryResult([{ value: 'some_version' }]); + return createMockQueryResult([{ value: 'some_value' }]); }); - await expect(getCurrentDatabaseVersion(pool)).resolves.toBeNull(); + await expect(getCurrentDatabaseTimestamp(pool)).resolves.toBeNull(); }); - it('returns the version from database', async () => { + it('returns the timestamp from database', async () => { const expectSql = sql` select * from ${table} where ${fields.key}=$1 `; - const version = 'version'; mockQuery.mockImplementationOnce(async (sql, values) => { expectSqlAssert(sql, expectSql.sql); - expect(values).toEqual([databaseVersionKey]); + expect(values).toEqual([migrationStateKey]); // @ts-expect-error createMockQueryResult doesn't support jsonb - return createMockQueryResult([{ value: { version, updatedAt: 'now' } }]); + return createMockQueryResult([{ value: { timestamp, updatedAt: 'now' } }]); }); - await expect(getCurrentDatabaseVersion(pool)).resolves.toEqual(version); + await expect(getCurrentDatabaseTimestamp(pool)).resolves.toEqual(timestamp); }); }); @@ -113,13 +112,12 @@ describe('createLogtoConfigsTable()', () => { }); }); -describe('updateDatabaseVersion()', () => { +describe('updateDatabaseTimestamp()', () => { const expectSql = sql` insert into ${table} (${fields.key}, ${fields.value}) values ($1, $2) on conflict (${fields.key}) do update set ${fields.value}=excluded.${fields.value} `; - const version = 'version'; const updatedAt = '2022-09-21T06:32:46.583Z'; beforeAll(() => { @@ -143,20 +141,20 @@ describe('updateDatabaseVersion()', () => { .mockImplementationOnce(jest.fn()); jest.spyOn(functions, 'isLogtoConfigsTableExists').mockResolvedValueOnce(false); - await updateDatabaseVersion(pool, version); + await updateDatabaseTimestamp(pool, timestamp); expect(mockCreateLogtoConfigsTable).toHaveBeenCalled(); }); - it('sends upsert sql with version and updatedAt', async () => { + it('sends upsert sql with timestamp and updatedAt', async () => { mockQuery.mockImplementationOnce(async (sql, values) => { expectSqlAssert(sql, expectSql.sql); - expect(values).toEqual([databaseVersionKey, JSON.stringify({ version, updatedAt })]); + expect(values).toEqual([migrationStateKey, JSON.stringify({ timestamp, updatedAt })]); return createMockQueryResult([]); }); jest.spyOn(functions, 'isLogtoConfigsTableExists').mockResolvedValueOnce(true); - await updateDatabaseVersion(pool, version); + await updateDatabaseTimestamp(pool, timestamp); }); }); @@ -164,22 +162,29 @@ describe('getUndeployedMigrations()', () => { beforeEach(() => { jest .spyOn(functions, 'getMigrationFiles') - .mockResolvedValueOnce(['1.0.0.js', '1.0.2.js', '1.0.1.js']); + .mockResolvedValueOnce([ + '1.0.0-1663923770-a.js', + '1.0.0-1663923772-c.js', + '1.0.0-1663923771-b.js', + ]); }); - it('returns all files with right order if database version is null', async () => { - jest.spyOn(functions, 'getCurrentDatabaseVersion').mockResolvedValueOnce(null); + it('returns all files with right order if database migration timestamp is null', async () => { + jest.spyOn(functions, 'getCurrentDatabaseTimestamp').mockResolvedValueOnce(null); await expect(getUndeployedMigrations(pool)).resolves.toEqual([ - '1.0.0.js', - '1.0.1.js', - '1.0.2.js', + '1.0.0-1663923770-a.js', + '1.0.0-1663923771-b.js', + '1.0.0-1663923772-c.js', ]); }); - it('returns files whose version is greater then database version', async () => { - jest.spyOn(functions, 'getCurrentDatabaseVersion').mockResolvedValueOnce('1.0.0'); + it('returns files whose timestamp is greater then database timstamp', async () => { + jest.spyOn(functions, 'getCurrentDatabaseTimestamp').mockResolvedValueOnce(1_663_923_770); - await expect(getUndeployedMigrations(pool)).resolves.toEqual(['1.0.1.js', '1.0.2.js']); + await expect(getUndeployedMigrations(pool)).resolves.toEqual([ + '1.0.0-1663923771-b.js', + '1.0.0-1663923772-c.js', + ]); }); }); diff --git a/packages/core/src/migration/index.ts b/packages/core/src/migration/index.ts index 5edc78cd9..671a7e91a 100644 --- a/packages/core/src/migration/index.ts +++ b/packages/core/src/migration/index.ts @@ -4,9 +4,9 @@ import path from 'path'; import { LogtoConfig, LogtoConfigs } from '@logto/schemas'; import { - DatabaseVersion, - databaseVersionGuard, MigrationScript, + MigrationState, + migrationStateGuard, } from '@logto/schemas/migrations/types'; import { conditionalString } from '@silverhand/essentials'; import chalk from 'chalk'; @@ -15,12 +15,8 @@ import { raw } from 'slonik-sql-tag-raw'; import { convertToIdentifiers } from '@/database/utils'; -import { - databaseVersionKey, - logtoConfigsTableFilePath, - migrationFilesDirectory, -} from './constants'; -import { compareVersion, getVersionFromFileName, migrationFileNameRegex } from './utils'; +import { logtoConfigsTableFilePath, migrationFilesDirectory, migrationStateKey } from './constants'; +import { getTimestampFromFileName, migrationFileNameRegex } from './utils'; const { table, fields } = convertToIdentifiers(LogtoConfigs); @@ -37,14 +33,14 @@ export const isLogtoConfigsTableExists = async (pool: DatabasePool) => { return exists; }; -export const getCurrentDatabaseVersion = async (pool: DatabasePool) => { +export const getCurrentDatabaseTimestamp = async (pool: DatabasePool) => { try { const query = await pool.maybeOne( - sql`select * from ${table} where ${fields.key}=${databaseVersionKey}` + sql`select * from ${table} where ${fields.key}=${migrationStateKey}` ); - const databaseVersion = databaseVersionGuard.parse(query?.value); + const { timestamp } = migrationStateGuard.parse(query?.value); - return databaseVersion.version; + return timestamp; } catch { return null; } @@ -55,20 +51,20 @@ export const createLogtoConfigsTable = async (pool: DatabasePool) => { await pool.query(sql`${raw(tableQuery)}`); }; -export const updateDatabaseVersion = async (pool: DatabasePool, version: string) => { +export const updateDatabaseTimestamp = async (pool: DatabasePool, timestamp: number) => { if (!(await isLogtoConfigsTableExists(pool))) { await createLogtoConfigsTable(pool); } - const value: DatabaseVersion = { - version, + const value: MigrationState = { + timestamp, updatedAt: new Date().toISOString(), }; await pool.query( sql` insert into ${table} (${fields.key}, ${fields.value}) - values (${databaseVersionKey}, ${JSON.stringify(value)}) + values (${migrationStateKey}, ${JSON.stringify(value)}) on conflict (${fields.key}) do update set ${fields.value}=excluded.${fields.value} ` ); @@ -86,18 +82,13 @@ export const getMigrationFiles = async () => { }; export const getUndeployedMigrations = async (pool: DatabasePool) => { - const databaseVersion = await getCurrentDatabaseVersion(pool); + const databaseTimestamp = await getCurrentDatabaseTimestamp(pool); const files = await getMigrationFiles(); return files - .filter( - (file) => - !databaseVersion || compareVersion(getVersionFromFileName(file), databaseVersion) > 0 - ) + .filter((file) => !databaseTimestamp || getTimestampFromFileName(file) > databaseTimestamp) .slice() - .sort((file1, file2) => - compareVersion(getVersionFromFileName(file1), getVersionFromFileName(file2)) - ); + .sort((file1, file2) => getTimestampFromFileName(file1) - getTimestampFromFileName(file2)); }; const importMigration = async (file: string): Promise => { @@ -127,7 +118,7 @@ const runMigration = async (pool: DatabasePool, file: string) => { throw error; } - await updateDatabaseVersion(pool, getVersionFromFileName(file)); + await updateDatabaseTimestamp(pool, getTimestampFromFileName(file)); console.log(`${chalk.blue('[migration]')} run ${file} succeeded.`); }; diff --git a/packages/core/src/migration/utils.test.ts b/packages/core/src/migration/utils.test.ts index f6fdc42a7..4239f79d6 100644 --- a/packages/core/src/migration/utils.test.ts +++ b/packages/core/src/migration/utils.test.ts @@ -1,25 +1,15 @@ -import { compareVersion, getVersionFromFileName } from './utils'; +import { getTimestampFromFileName } from './utils'; -describe('compareVersion', () => { - it('should return 1 for 1.0.0 and 1.0.0-beta.9', () => { - expect(compareVersion('1.0.0', '1.0.0-beta.9')).toBe(1); +describe('getTimestampFromFileName()', () => { + it('should get for 1.0.0-1663923211.js', () => { + expect(getTimestampFromFileName('1.0.0-1663923211.js')).toEqual(1_663_923_211); }); - it('should return 1 for 1.0.0-beta.10 and 1.0.0-beta.9', () => { - expect(compareVersion('1.0.0-beta.10', '1.0.0-beta.9')).toBe(1); + it('should get for 1.0.0-1663923211-user-table.js', () => { + expect(getTimestampFromFileName('1.0.0-1663923211-user-table.js')).toEqual(1_663_923_211); }); - it('should return 1 for 1.0.0 and 0.0.8', () => { - expect(compareVersion('1.0.0', '0.0.8')).toBe(1); - }); -}); - -describe('getVersionFromFileName', () => { - it('should get version for 1.0.2.js', () => { - expect(getVersionFromFileName('1.0.2.js')).toEqual('1.0.2'); - }); - - it('should throw for next.js', () => { - expect(() => getVersionFromFileName('next.js')).toThrowError(); + it('should throw for 166392321.js', () => { + expect(() => getTimestampFromFileName('166392321.js')).toThrowError(); }); }); diff --git a/packages/core/src/migration/utils.ts b/packages/core/src/migration/utils.ts index eaead2784..f6b8f6eeb 100644 --- a/packages/core/src/migration/utils.ts +++ b/packages/core/src/migration/utils.ts @@ -1,21 +1,11 @@ -import semver from 'semver'; +export const migrationFileNameRegex = /-(\d{10,11})-?.*\.js$/; -export const migrationFileNameRegex = /^(((?!next).)*)\.js$/; - -export const getVersionFromFileName = (fileName: string) => { +export const getTimestampFromFileName = (fileName: string) => { const match = migrationFileNameRegex.exec(fileName); if (!match?.[1]) { - throw new Error(`Can not find version name: ${fileName}`); + throw new Error(`Can not get timestamp: ${fileName}`); } - return match[1]; -}; - -export const compareVersion = (version1: string, version2: string) => { - if (semver.eq(version1, version2)) { - return 0; - } - - return semver.gt(version1, version2) ? 1 : -1; + return Number(match[1]); }; diff --git a/packages/schemas/migrations/README.md b/packages/schemas/migrations/README.md index c156795cc..367bf6180 100644 --- a/packages/schemas/migrations/README.md +++ b/packages/schemas/migrations/README.md @@ -4,10 +4,12 @@ The folder for all migration files. ## Format -The migration files are named in the format of `.ts` where `version` is this npm package's version number. +The migration files are named in the format of `--name.js` where `` is the unix timestamp of when the migration was created and `name` is the name of the migration, `version` is this npm package's version number. As for development, the `version` is "next" until the package is released. +Note that, you SHOULD NOT change the content of the migration files after they are created. If you need to change the migration, you should create a new migration file with the new content. + ## Typing ```ts diff --git a/packages/schemas/migrations/next.ts b/packages/schemas/migrations/next-1663923211-machine-to-machine-app.ts similarity index 100% rename from packages/schemas/migrations/next.ts rename to packages/schemas/migrations/next-1663923211-machine-to-machine-app.ts diff --git a/packages/schemas/migrations/types.ts b/packages/schemas/migrations/types.ts index f750275d7..7cea0844d 100644 --- a/packages/schemas/migrations/types.ts +++ b/packages/schemas/migrations/types.ts @@ -1,12 +1,12 @@ import { DatabaseTransactionConnection } from 'slonik'; import { z } from 'zod'; -export const databaseVersionGuard = z.object({ - version: z.string(), +export const migrationStateGuard = z.object({ + timestamp: z.number(), updatedAt: z.string().optional(), }); -export type DatabaseVersion = z.infer; +export type MigrationState = z.infer; export type MigrationScript = { up: (connection: DatabaseTransactionConnection) => Promise; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c50850936..234cbcfc2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -174,7 +174,6 @@ importers: '@types/node': ^16.3.1 '@types/oidc-provider': ^7.11.1 '@types/rimraf': ^3.0.2 - '@types/semver': ^7.3.12 '@types/supertest': ^2.0.11 '@types/tar': ^6.1.2 chalk: ^4 @@ -217,7 +216,6 @@ importers: query-string: ^7.0.1 rimraf: ^3.0.2 roarr: ^7.11.0 - semver: ^7.3.7 slonik: ^30.0.0 slonik-interceptor-preset: ^1.2.10 slonik-sql-tag-raw: ^1.1.4 @@ -263,7 +261,6 @@ importers: query-string: 7.0.1 rimraf: 3.0.2 roarr: 7.11.0 - semver: 7.3.7 slonik: 30.1.2 slonik-interceptor-preset: 1.2.10 slonik-sql-tag-raw: 1.1.4_roarr@7.11.0+slonik@30.1.2 @@ -291,7 +288,6 @@ importers: '@types/node': 16.11.12 '@types/oidc-provider': 7.11.1 '@types/rimraf': 3.0.2 - '@types/semver': 7.3.12 '@types/supertest': 2.0.11 '@types/tar': 6.1.2 copyfiles: 2.4.1 @@ -4557,10 +4553,6 @@ packages: resolution: {integrity: sha512-hppQEBDmlwhFAXKJX2KnWLYu5yMfi91yazPb2l+lbJiwW+wdo1gNeRA+3RgNSO39WYX2euey41KEwnqesU2Jew==} dev: true - /@types/semver/7.3.12: - resolution: {integrity: sha512-WwA1MW0++RfXmCr12xeYOOC5baSC9mSb0ZqCquFzKhcoF4TvHu5MKOuXsncgZcpVFhB1pXd5hZmM0ryAoCp12A==} - dev: true - /@types/serve-static/1.13.10: resolution: {integrity: sha512-nCkHGI4w7ZgAdNkrEu0bv+4xNV/XDqW+DydknebMOQwkpDGx8G+HTlj7R7ABI8i8nKxVw0wtKPi1D+lPOkh4YQ==} dependencies: @@ -5917,8 +5909,8 @@ packages: engines: {node: '>=10'} hasBin: true dependencies: - JSONStream: 1.3.5 is-text-path: 1.0.1 + JSONStream: 1.3.5 lodash: 4.17.21 meow: 8.1.2 split2: 3.2.2 @@ -10337,6 +10329,7 @@ packages: engines: {node: '>=10'} dependencies: yallist: 4.0.0 + dev: true /lru-cache/7.10.1: resolution: {integrity: sha512-BQuhQxPuRl79J5zSXRP+uNzPOyZw2oFI9JLRQ80XswSvg21KMKNtQza9eF42rfI/3Z40RvzBdXgziEkudzjo8A==} @@ -13678,6 +13671,7 @@ packages: hasBin: true dependencies: lru-cache: 6.0.0 + dev: true /serialize-error/7.0.1: resolution: {integrity: sha512-8I8TjW5KMOKsZQTvoxjuSIa7foAwPWGOts+6o7sgjz41/qMD9VQHEDxi6PBvK2l0MXUmqZyNpUK+T2tQaaElvw==}