Skip to content

Commit

Permalink
BC-5936 - replace migration library (#4680)
Browse files Browse the repository at this point in the history
In order to get rid of mongoose and switch to mikro-orm migration,
it was necessary to upgrade mikro-orm and remove old migrations.
  • Loading branch information
virgilchiriac authored Jan 29, 2024
1 parent d2baaef commit 53c02d8
Show file tree
Hide file tree
Showing 72 changed files with 1,857 additions and 8,157 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/dependency-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ jobs:
uses: actions/dependency-review-action@v3
with:
allow-licenses: AGPL-3.0-only, LGPL-3.0, MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause, ISC, X11, 0BSD, GPL-3.0, Unlicense
# temporarily ignore dependency error for upgrade mongodb 4.9 to 4.11, remove when mikroORM is upgraded to 5.9
# temporarily ignore dependency error sprintf-js 1.0.3, remove when it gets upgraded to 1.1.3
allow-dependencies-licenses: 'pkg:npm/[email protected]'
allow-ghsas: 'GHSA-vxvm-qww3-2fh7'
19 changes: 7 additions & 12 deletions .github/workflows/migrations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ on:
pull_request:
branches: [ main ]

env:
MONGODB_VERSION: 4.4
NODE_VERSION: '18'
jobs:
migration:
runs-on: ubuntu-latest
Expand All @@ -14,21 +17,13 @@ jobs:
timeout-minutes: 5
steps:
- uses: actions/checkout@v4
- name: check all migrations are up in database seed
run: test $(grep "\"down\"" ./backup/setup/migrations.json -c) -eq 0
- name: mongodb setup
uses: supercharge/[email protected]
- name: setup
uses: actions/setup-node@v4
with:
node-version: '16'
node-version: ${{ env.NODE_VERSION }}
- run: npm ci
- run: npm run setup
- name: check migrations.json formatting
run: |
npm run migration-persist
git diff --exit-code backup/**
- name: check filesystem migrations have been added to database
run: npm run migration-list
- name: check migrations in database exist in filesystem
run: npm run migration-prune
- run: npm run setup:db:seed
- name: check no pending migrations (migration is in db)
run: test $(npx mikro-orm migration:pending | grep -c "Migration") -eq 0

This comment has been minimized.

Copy link
@Loki-Afro

Loki-Afro Feb 5, 2024

Member

do not use npx!

1 change: 0 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ COPY esbuild ./esbuild
RUN npm ci && npm cache clean --force
COPY config /schulcloud-server/config
COPY backup /schulcloud-server/backup
COPY migrations /schulcloud-server/migrations
COPY src /schulcloud-server/src
COPY apps /schulcloud-server/apps
COPY --from=git /app/serverversion /schulcloud-server/apps/server/static-assets
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ spec:
- secretRef:
name: api-secret
command: ['/bin/sh','-c']
args: ['npm run ensureIndexes && npm run migration-sync && npm run migration-prune && npm run migration up']
args: ['npm run ensureIndexes && npm run migration:up']
resources:
limits:
cpu: {{ API_CPU_LIMITS|default("2000m", true) }}
Expand All @@ -30,4 +30,4 @@ spec:
cpu: {{ API_CPU_REQUESTS|default("100m", true) }}
memory: {{ API_MEMORY_REQUESTS|default("150Mi", true) }}
restartPolicy: Never
backoffLimit: 3
backoffLimit: 5
2 changes: 1 addition & 1 deletion apps/server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ You find the whole [documentation published as GitHub Page](https://hpi-schul-cl
### preconditions

1. Have a MongoDB started, run `mongod`
2. Have some seed data in database, use `npm run setup` to reset the db and apply seed data
2. Have some seed data in database, use `npm run setup:db:seed` to reset the db and apply seed data
3. Have RabbitMQ started, run `docker run -d -p 5672:5672 -p 15672:15672 --name rabbitmq rabbitmq:3.8.9-management`. This starts RabbitMQ on port 5672 and a web admin console at localhost:15672 (use guest:guest to login).
4. Have MinIO (S3 compatible object storage), run [optional if you need files-storage module]

Expand Down
2 changes: 1 addition & 1 deletion apps/server/doc/keycloak.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ To add ErWIn-IDM identity broker feature via OpenID Connect (OIDC) Identity Prov

- Set env vars (or in your .env file) 'OIDCMOCK\_\_BASE_URL' to http://\<your-local-ip\>:4011.
- To make it work with the nuxt client set the env var HOST=http://localhost:4000
- re-trigger `npm run setup:db` and `npm run setup:idm` to reset and apply seed data.
- re-trigger `npm run setup:db:seed` and `npm run setup:idm` to reset and apply seed data.
- start the 'oidc-server-mock' as follows:

```bash
Expand Down
38 changes: 38 additions & 0 deletions apps/server/src/config/mikro-orm-cli.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import type { MikroOrmModuleSyncOptions } from '@mikro-orm/nestjs/typings';
import { ALL_ENTITIES } from '@shared/domain/entity';
import { FileEntity } from '@modules/files/entity';

Check warning on line 3 in apps/server/src/config/mikro-orm-cli.config.ts

View workflow job for this annotation

GitHub Actions / nest_lint

'@modules/files/entity' import is restricted from being used by a pattern. Do not deep import from a module
import { FileRecord } from '@modules/files-storage/entity';

Check warning on line 4 in apps/server/src/config/mikro-orm-cli.config.ts

View workflow job for this annotation

GitHub Actions / nest_lint

'@modules/files-storage/entity' import is restricted from being used by a pattern. Do not deep import from a module
import path from 'path';
import { DB_PASSWORD, DB_URL, DB_USERNAME } from './index';

const migrationsPath = path.resolve(__dirname, '..', 'migrations', 'mikro-orm');

export const mikroOrmCliConfig: MikroOrmModuleSyncOptions = {
// TODO repeats server module definitions
type: 'mongo',
clientUrl: DB_URL,
password: DB_PASSWORD,
user: DB_USERNAME,
entities: [...ALL_ENTITIES, FileEntity, FileRecord],
allowGlobalContext: true,
/*
findOneOrFailHandler: (entityName: string, where: Dictionary | IPrimaryKey) =>
new NotFoundException(`The requested ${entityName}: ${JSON.stringify(where)} has not been found.`),
*/
migrations: {
tableName: 'migrations', // name of database table with log of executed transactions
path: migrationsPath, // path to the folder with migrations
pathTs: migrationsPath, // path to the folder with TS migrations (if used, we should put path to compiled files in `path`)
glob: '!(*.d).{js,ts}', // how to match migration files (all .js and .ts files, but not .d.ts)
transactional: false, // wrap each migration in a transaction
disableForeignKeys: true, // wrap statements with `set foreign_key_checks = 0` or equivalent
allOrNothing: false, // wrap all migrations in master transaction
dropTables: false, // allow to disable table dropping
safe: false, // allow to disable table and column dropping
snapshot: true, // save snapshot when creating new migrations
emit: 'ts', // migration generation mode
// generator: TSMigrationGenerator, // migration generator, e.g. to allow custom formatting
},
};

export default mikroOrmCliConfig;
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ describe('DatabaseManagementConsole (API)', () => {
const rootCli = consoleService.getRootCli();
rootCli.exitOverride(exitFn);
const spyConsoleWriterInfo = jest.spyOn(consoleWriter, 'info');
return { spyConsoleWriterInfo };
const spyConsoleWriterError = jest.spyOn(consoleWriter, 'error');
return { spyConsoleWriterInfo, spyConsoleWriterError };
};
describe('when command not exists', () => {

describe('when command does not exist', () => {
it('should fail for unknown command', async () => {
setup();
await expect(execute(bootstrap, ['database', 'not_existing_command'])).rejects.toThrow(
Expand Down Expand Up @@ -74,6 +76,22 @@ describe('DatabaseManagementConsole (API)', () => {

expect(spyConsoleWriterInfo).toBeCalled();
});

it('should output error if command "migration" is called without flags', async () => {
const { spyConsoleWriterError } = setup();

await execute(bootstrap, ['database', 'migration']);

expect(spyConsoleWriterError).toBeCalled();
});

it('should provide command "migration"', async () => {
const { spyConsoleWriterInfo } = setup();

await execute(bootstrap, ['database', 'migration', '--up']);

expect(spyConsoleWriterInfo).toBeCalled();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('ServerConsole (API)', () => {
consoleService.resetCli();
});

it('should poduce default output when executing "console server test"', async () => {
it('should produce default output when executing "console server test"', async () => {
await execute(bootstrap, ['server', 'test']);
expect(logMock).toHaveBeenCalledWith('Schulcloud Server API');
});
Expand Down
21 changes: 4 additions & 17 deletions apps/server/src/console/console.module.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
import { Configuration } from '@hpi-schul-cloud/commons/lib';
import { ConsoleWriterModule } from '@infra/console/console-writer/console-writer.module';

Check warning on line 2 in apps/server/src/console/console.module.ts

View workflow job for this annotation

GitHub Actions / nest_lint

'@infra/console/console-writer/console-writer.module' import is restricted from being used by a pattern. Do not deep import from a module
import { KeycloakModule } from '@infra/identity-management/keycloak/keycloak.module';
import { Dictionary, IPrimaryKey } from '@mikro-orm/core';
import { MikroOrmModule } from '@mikro-orm/nestjs';
import { FilesModule } from '@modules/files';
import { FileRecord } from '@modules/files-storage/entity';
import { FileEntity } from '@modules/files/entity';
import { ManagementModule } from '@modules/management/management.module';
import { serverConfig } from '@modules/server';
import { Module, NotFoundException } from '@nestjs/common';
import { Module } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config';
import { ALL_ENTITIES } from '@shared/domain/entity';
import { DB_PASSWORD, DB_URL, DB_USERNAME, createConfigModuleOptions } from '@src/config';
import { createConfigModuleOptions } from '@src/config';
import { ConsoleModule } from 'nestjs-console';
import { ServerConsole } from './server.console';
import { mikroOrmCliConfig } from '../config/mikro-orm-cli.config';

@Module({
imports: [
Expand All @@ -23,17 +20,7 @@ import { ServerConsole } from './server.console';
FilesModule,
ConfigModule.forRoot(createConfigModuleOptions(serverConfig)),
...((Configuration.get('FEATURE_IDENTITY_MANAGEMENT_ENABLED') as boolean) ? [KeycloakModule] : []),
MikroOrmModule.forRoot({
// TODO repeats server module definitions
type: 'mongo',
clientUrl: DB_URL,
password: DB_PASSWORD,
user: DB_USERNAME,
entities: [...ALL_ENTITIES, FileEntity, FileRecord],
allowGlobalContext: true,
findOneOrFailHandler: (entityName: string, where: Dictionary | IPrimaryKey) =>
new NotFoundException(`The requested ${entityName}: ${JSON.stringify(where)} has not been found.`),
}),
MikroOrmModule.forRoot(mikroOrmCliConfig),
],
providers: [
/** add console services as providers */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,16 @@ describe('ConsoleWriterService', () => {
it('should be defined', () => {
expect(service).toBeDefined();
});

it('should log info', () => {
const spy = jest.spyOn(console, 'info').mockImplementation(() => {});
service.info('test');
expect(spy).toHaveBeenCalledWith('Info:', 'test');
});

it('should log error', () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => {});
service.error('test');
expect(spy).toHaveBeenCalledWith('Error:', 'test');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,9 @@ export class ConsoleWriterService {
// eslint-disable-next-line no-console
console.info('Info:', text);
}

error(text: string): void {
// eslint-disable-next-line no-console
console.error('Error:', text);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,74 @@ describe('DatabaseManagementService', () => {
expect(spy).toHaveBeenCalled();
});
});

describe('When call migrationUp()', () => {
const setup = () => {
orm.getMigrator().up = jest.fn();
};
it('should call migrator.up()', async () => {
setup();
await service.migrationUp();
expect(orm.getMigrator().up).toHaveBeenCalled();
});
it('should call migrator.up() with from', async () => {
setup();
const params = { from: 'foo' };
await service.migrationUp(params.from, undefined, undefined);
expect(orm.getMigrator().up).toHaveBeenCalledWith(params);
});
it('should call migrator.up() with param "to"', async () => {
setup();
const params = { to: 'foo' };
await service.migrationUp(undefined, params.to, undefined);
expect(orm.getMigrator().up).toHaveBeenCalledWith(params);
});
it('should call migrator.up() with param "only"', async () => {
setup();
const params = { only: 'foo' };
await service.migrationUp(undefined, undefined, params.only);
expect(orm.getMigrator().up).toHaveBeenCalledWith({ migrations: [params.only] });
});
it('should call migrator.up() with param "only" and ignore from and to', async () => {
setup();
const params = { only: 'foo' };
await service.migrationUp('bar', 'baz', params.only);
expect(orm.getMigrator().up).toHaveBeenCalledWith({ migrations: [params.only] });
});
});

describe('When call migrationDown()', () => {
const setup = () => {
orm.getMigrator().down = jest.fn();
};
it('should call migrator.down()', async () => {
setup();
await service.migrationDown();
expect(orm.getMigrator().down).toHaveBeenCalled();
});
it('should call migrator.down() with from', async () => {
setup();
const params = { from: 'foo' };
await service.migrationDown(params.from, undefined, undefined);
expect(orm.getMigrator().down).toHaveBeenCalledWith(params);
});
it('should call migrator.down() with param "to"', async () => {
setup();
const params = { to: 'foo' };
await service.migrationDown(undefined, params.to, undefined);
expect(orm.getMigrator().down).toHaveBeenCalledWith(params);
});
it('should call migrator.down() with param "only"', async () => {
setup();
const params = { only: 'foo' };
await service.migrationDown(undefined, undefined, params.only);
expect(orm.getMigrator().down).toHaveBeenCalledWith({ migrations: [params.only] });
});
it('should call migrator.down() with param "only" and ignore from and to', async () => {
setup();
const params = { only: 'foo' };
await service.migrationDown('bar', 'baz', params.only);
expect(orm.getMigrator().down).toHaveBeenCalledWith({ migrations: [params.only] });
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { EntityManager } from '@mikro-orm/mongodb';
import { Injectable } from '@nestjs/common';
import { BaseEntity } from '@shared/domain/entity';
import { Collection, Db } from 'mongodb';
import { MigrateOptions } from '@mikro-orm/migrations-mongodb';

@Injectable()
export class DatabaseManagementService {
Expand Down Expand Up @@ -66,4 +67,32 @@ export class DatabaseManagementService {
async syncIndexes(): Promise<void> {
return this.orm.getSchemaGenerator().ensureIndexes();
}

async migrationUp(from?: string, to?: string, only?: string): Promise<void> {
const migrator = this.orm.getMigrator();
const params = this.migrationParams(only, from, to);
await migrator.up(params);
}

async migrationDown(from?: string, to?: string, only?: string): Promise<void> {
const migrator = this.orm.getMigrator();
const params = this.migrationParams(only, from, to);

await migrator.down(params);
}

private migrationParams(only?: string, from?: string, to?: string) {
const params: MigrateOptions = {};
if (only) {
params.migrations = [only];
} else {
if (from) {
params.from = from;
}
if (to) {
params.to = to;
}
}
return params;
}
}
17 changes: 17 additions & 0 deletions apps/server/src/migrations/mikro-orm/Migration20240108145519.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Migration } from '@mikro-orm/migrations-mongodb';

/*
* cleanup old migration records from db
*/
export class Migration20240108111130 extends Migration {
async up(): Promise<void> {
const { deletedCount } = await this.getCollection('migrations').deleteMany({}, { session: this.ctx });
console.log(`removed ${deletedCount} records`);
}

// eslint-disable-next-line @typescript-eslint/require-await
async down(): Promise<void> {
// do nothing
console.error(`Migration down not implemented. You might need to restore database from backup!`);
}
}
Loading

0 comments on commit 53c02d8

Please sign in to comment.