From a4249f9afeab0fd00f5fe56408fab86375beef9f Mon Sep 17 00:00:00 2001
From: Maksim <m.v.sharipov@gmail.com>
Date: Sat, 11 Dec 2021 14:56:51 +0100
Subject: [PATCH] refactor(migrations): extend abstract migration (#12963)
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
---
lib/config/migration.ts | 2 +-
.../migrations/base/abstract-migration.ts | 45 ++++++++++++++-----
.../base/remove-property-migration.ts | 12 +++++
.../base/rename-property-migration.ts | 12 ++---
.../custom/binary-source-migration.spec.ts | 7 ++-
.../custom/binary-source-migration.ts | 11 ++---
.../custom/go-mod-tidy-migration.spec.ts | 22 +++++----
.../custom/go-mod-tidy-migration.ts | 17 ++++---
.../ignore-node-modules-migration.spec.ts | 5 ++-
.../custom/ignore-node-modules-migration.ts | 11 ++---
.../required-status-checks-migration.spec.ts | 8 ++--
.../required-status-checks-migration.ts | 11 ++---
.../custom/trust-level-migration.spec.ts | 22 +++++----
.../custom/trust-level-migration.ts | 18 +++-----
.../migrations/migrations-service.spec.ts | 19 +++++---
lib/config/migrations/migrations-service.ts | 36 ++++++++-------
lib/config/migrations/types.ts | 10 ++++-
17 files changed, 163 insertions(+), 105 deletions(-)
diff --git a/lib/config/migration.ts b/lib/config/migration.ts
index 4219e35676..c2c4015e06 100644
--- a/lib/config/migration.ts
+++ b/lib/config/migration.ts
@@ -33,7 +33,7 @@ export function migrateConfig(
optionTypes[option.name] = option.type;
});
}
- const newConfig = MigrationsService.run(config);
+ const newConfig = MigrationsService.run(config).migratedConfig;
const migratedConfig = clone(newConfig) as MigratedRenovateConfig;
const depTypes = [
'dependencies',
diff --git a/lib/config/migrations/base/abstract-migration.ts b/lib/config/migrations/base/abstract-migration.ts
index 56d6fb7548..9d47be887b 100644
--- a/lib/config/migrations/base/abstract-migration.ts
+++ b/lib/config/migrations/base/abstract-migration.ts
@@ -1,24 +1,49 @@
+import is from '@sindresorhus/is';
import type { RenovateConfig } from '../../types';
import type { Migration } from '../types';
export abstract class AbstractMigration implements Migration {
- readonly propertyName: string;
+ abstract readonly propertyName: string;
- protected readonly originalConfig: RenovateConfig;
+ private readonly originalConfig: RenovateConfig;
- protected readonly migratedConfig: RenovateConfig;
+ private readonly migratedConfig: RenovateConfig;
- constructor(
- propertyName: string,
- originalConfig: RenovateConfig,
- migratedConfig: RenovateConfig
- ) {
- this.propertyName = propertyName;
+ constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {
this.originalConfig = originalConfig;
this.migratedConfig = migratedConfig;
}
- abstract run(): void;
+ abstract run(value: unknown): void;
+
+ protected get<Key extends keyof RenovateConfig>(
+ key: Key
+ ): RenovateConfig[Key] {
+ return this.migratedConfig[key] ?? this.originalConfig[key];
+ }
+
+ protected setSafely<Key extends keyof RenovateConfig>(
+ key: Key,
+ value: RenovateConfig[Key]
+ ): void {
+ if (
+ is.nullOrUndefined(this.originalConfig[key]) &&
+ is.nullOrUndefined(this.migratedConfig[key])
+ ) {
+ this.migratedConfig[key] = value;
+ }
+ }
+
+ protected setHard<Key extends keyof RenovateConfig>(
+ key: Key,
+ value: RenovateConfig[Key]
+ ): void {
+ this.migratedConfig[key] = value;
+ }
+
+ protected rewrite(value: unknown): void {
+ this.setHard(this.propertyName, value);
+ }
protected delete(property: string): void {
delete this.migratedConfig[property];
diff --git a/lib/config/migrations/base/remove-property-migration.ts b/lib/config/migrations/base/remove-property-migration.ts
index 0b2ad24969..21c8b61e0b 100644
--- a/lib/config/migrations/base/remove-property-migration.ts
+++ b/lib/config/migrations/base/remove-property-migration.ts
@@ -1,6 +1,18 @@
+import type { RenovateConfig } from '../../types';
import { AbstractMigration } from './abstract-migration';
export class RemovePropertyMigration extends AbstractMigration {
+ readonly propertyName: string;
+
+ constructor(
+ propertyName: string,
+ originalConfig: RenovateConfig,
+ migratedConfig: RenovateConfig
+ ) {
+ super(originalConfig, migratedConfig);
+ this.propertyName = propertyName;
+ }
+
override run(): void {
this.delete(this.propertyName);
}
diff --git a/lib/config/migrations/base/rename-property-migration.ts b/lib/config/migrations/base/rename-property-migration.ts
index c317634be4..d883dd5069 100644
--- a/lib/config/migrations/base/rename-property-migration.ts
+++ b/lib/config/migrations/base/rename-property-migration.ts
@@ -2,7 +2,9 @@ import type { RenovateConfig } from '../../types';
import { AbstractMigration } from './abstract-migration';
export class RenamePropertyMigration extends AbstractMigration {
- protected readonly newPropertyName: string;
+ readonly propertyName: string;
+
+ private readonly newPropertyName: string;
constructor(
deprecatedPropertyName: string,
@@ -10,14 +12,14 @@ export class RenamePropertyMigration extends AbstractMigration {
originalConfig: RenovateConfig,
migratedConfig: RenovateConfig
) {
- super(deprecatedPropertyName, originalConfig, migratedConfig);
+ super(originalConfig, migratedConfig);
+ this.propertyName = deprecatedPropertyName;
this.newPropertyName = newPropertyName;
}
- override run(): void {
+ override run(value): void {
this.delete(this.propertyName);
- this.migratedConfig[this.newPropertyName] =
- this.originalConfig[this.propertyName];
+ this.setSafely(this.newPropertyName, value);
}
}
diff --git a/lib/config/migrations/custom/binary-source-migration.spec.ts b/lib/config/migrations/custom/binary-source-migration.spec.ts
index 22ef956ba4..5471ca20d0 100644
--- a/lib/config/migrations/custom/binary-source-migration.spec.ts
+++ b/lib/config/migrations/custom/binary-source-migration.spec.ts
@@ -2,10 +2,13 @@ import { MigrationsService } from '../migrations-service';
describe('config/migrations/custom/binary-source-migration', () => {
it('should migrate "auto" to "global"', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
binarySource: 'auto',
});
- expect(migratedConfig.binarySource).toBe('global');
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({
+ binarySource: 'global',
+ });
});
});
diff --git a/lib/config/migrations/custom/binary-source-migration.ts b/lib/config/migrations/custom/binary-source-migration.ts
index 1452897133..81337609e6 100644
--- a/lib/config/migrations/custom/binary-source-migration.ts
+++ b/lib/config/migrations/custom/binary-source-migration.ts
@@ -1,14 +1,11 @@
-import type { RenovateConfig } from '../../types';
import { AbstractMigration } from '../base/abstract-migration';
export class BinarySourceMigration extends AbstractMigration {
- constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {
- super('binarySource', originalConfig, migratedConfig);
- }
+ readonly propertyName = 'binarySource';
- override run(): void {
- if (this.originalConfig.binarySource === 'auto') {
- this.migratedConfig.binarySource = 'global';
+ override run(value): void {
+ if (value === 'auto') {
+ this.rewrite('global');
}
}
}
diff --git a/lib/config/migrations/custom/go-mod-tidy-migration.spec.ts b/lib/config/migrations/custom/go-mod-tidy-migration.spec.ts
index f0c7be274f..4f342710dc 100644
--- a/lib/config/migrations/custom/go-mod-tidy-migration.spec.ts
+++ b/lib/config/migrations/custom/go-mod-tidy-migration.spec.ts
@@ -2,30 +2,34 @@ import { MigrationsService } from '../migrations-service';
describe('config/migrations/custom/go-mod-tidy-migration', () => {
it('should add postUpdateOptions option when true', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
gomodTidy: true,
postUpdateOptions: ['test'],
});
- expect(migratedConfig).not.toHaveProperty('gomodTidy');
- expect(migratedConfig.postUpdateOptions).toEqual(['test', 'gomodTidy']);
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({
+ postUpdateOptions: ['test', 'gomodTidy'],
+ });
});
it('should handle case when postUpdateOptions is not defined ', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
gomodTidy: true,
});
- expect(migratedConfig).not.toHaveProperty('gomodTidy');
- expect(migratedConfig.postUpdateOptions).toEqual(['gomodTidy']);
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({
+ postUpdateOptions: ['gomodTidy'],
+ });
});
it('should only remove when false', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
gomodTidy: false,
});
- expect(migratedConfig).not.toHaveProperty('gomodTidy');
- expect(migratedConfig).not.toHaveProperty('postUpdateOptions');
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({});
});
});
diff --git a/lib/config/migrations/custom/go-mod-tidy-migration.ts b/lib/config/migrations/custom/go-mod-tidy-migration.ts
index b655489da2..9f3c984e36 100644
--- a/lib/config/migrations/custom/go-mod-tidy-migration.ts
+++ b/lib/config/migrations/custom/go-mod-tidy-migration.ts
@@ -1,19 +1,18 @@
-import type { RenovateConfig } from '../../types';
import { AbstractMigration } from '../base/abstract-migration';
export class GoModTidyMigration extends AbstractMigration {
- constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {
- super('gomodTidy', originalConfig, migratedConfig);
- }
+ readonly propertyName = 'gomodTidy';
- override run(): void {
- const { gomodTidy, postUpdateOptions } = this.originalConfig;
+ override run(value): void {
+ const postUpdateOptions = this.get('postUpdateOptions');
this.delete(this.propertyName);
- if (gomodTidy) {
- this.migratedConfig.postUpdateOptions ??= postUpdateOptions ?? [];
- this.migratedConfig.postUpdateOptions.push('gomodTidy');
+ if (value) {
+ const newPostUpdateOptions = Array.isArray(postUpdateOptions)
+ ? postUpdateOptions.concat(['gomodTidy'])
+ : ['gomodTidy'];
+ this.setHard('postUpdateOptions', newPostUpdateOptions);
}
}
}
diff --git a/lib/config/migrations/custom/ignore-node-modules-migration.spec.ts b/lib/config/migrations/custom/ignore-node-modules-migration.spec.ts
index 4f2d4d000a..2be6ae247a 100644
--- a/lib/config/migrations/custom/ignore-node-modules-migration.spec.ts
+++ b/lib/config/migrations/custom/ignore-node-modules-migration.spec.ts
@@ -2,10 +2,11 @@ import { MigrationsService } from '../migrations-service';
describe('config/migrations/custom/ignore-node-modules-migration', () => {
it('should migrate to ignorePaths', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
ignoreNodeModules: true,
});
- expect(migratedConfig.ignorePaths).toEqual(['node_modules/']);
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({ ignorePaths: ['node_modules/'] });
});
});
diff --git a/lib/config/migrations/custom/ignore-node-modules-migration.ts b/lib/config/migrations/custom/ignore-node-modules-migration.ts
index 976b53df3c..79a1c77e75 100644
--- a/lib/config/migrations/custom/ignore-node-modules-migration.ts
+++ b/lib/config/migrations/custom/ignore-node-modules-migration.ts
@@ -1,16 +1,11 @@
-import type { RenovateConfig } from '../../types';
import { AbstractMigration } from '../base/abstract-migration';
export class IgnoreNodeModulesMigration extends AbstractMigration {
- constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {
- super('ignoreNodeModules', originalConfig, migratedConfig);
- }
+ readonly propertyName = 'ignoreNodeModules';
- override run(): void {
+ override run(value): void {
this.delete(this.propertyName);
- this.migratedConfig.ignorePaths = this.originalConfig.ignoreNodeModules
- ? ['node_modules/']
- : [];
+ this.setSafely('ignorePaths', value ? ['node_modules/'] : []);
}
}
diff --git a/lib/config/migrations/custom/required-status-checks-migration.spec.ts b/lib/config/migrations/custom/required-status-checks-migration.spec.ts
index 2fdd678208..5aba3b883c 100644
--- a/lib/config/migrations/custom/required-status-checks-migration.spec.ts
+++ b/lib/config/migrations/custom/required-status-checks-migration.spec.ts
@@ -2,11 +2,13 @@ import { MigrationsService } from '../migrations-service';
describe('config/migrations/custom/required-status-checks-migration', () => {
it('should migrate requiredStatusChecks=null to ignoreTests=true', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
requiredStatusChecks: null,
});
- expect(migratedConfig).not.toHaveProperty('requiredStatusChecks');
- expect(migratedConfig.ignoreTests).toBeTrue();
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({
+ ignoreTests: true,
+ });
});
});
diff --git a/lib/config/migrations/custom/required-status-checks-migration.ts b/lib/config/migrations/custom/required-status-checks-migration.ts
index cd06869c20..a6fb5c3034 100644
--- a/lib/config/migrations/custom/required-status-checks-migration.ts
+++ b/lib/config/migrations/custom/required-status-checks-migration.ts
@@ -1,16 +1,13 @@
-import type { RenovateConfig } from '../../types';
import { AbstractMigration } from '../base/abstract-migration';
export class RequiredStatusChecksMigration extends AbstractMigration {
- constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {
- super('requiredStatusChecks', originalConfig, migratedConfig);
- }
+ readonly propertyName = 'requiredStatusChecks';
- override run(): void {
+ override run(value): void {
this.delete(this.propertyName);
- if (this.originalConfig.requiredStatusChecks === null) {
- this.migratedConfig.ignoreTests = true;
+ if (value === null) {
+ this.setSafely('ignoreTests', true);
}
}
}
diff --git a/lib/config/migrations/custom/trust-level-migration.spec.ts b/lib/config/migrations/custom/trust-level-migration.spec.ts
index 5fac9753ea..51ac5e070e 100644
--- a/lib/config/migrations/custom/trust-level-migration.spec.ts
+++ b/lib/config/migrations/custom/trust-level-migration.spec.ts
@@ -2,25 +2,31 @@ import { MigrationsService } from '../migrations-service';
describe('config/migrations/custom/trust-level-migration', () => {
it('should handle hight level', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
trustLevel: 'high',
});
- expect(migratedConfig.allowCustomCrateRegistries).toBeTrue();
- expect(migratedConfig.allowScripts).toBeTrue();
- expect(migratedConfig.exposeAllEnv).toBeTrue();
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({
+ allowCustomCrateRegistries: true,
+ allowScripts: true,
+ exposeAllEnv: true,
+ });
});
it('should not rewrite provided properties', () => {
- const migratedConfig = MigrationsService.run({
+ const { isMigrated, migratedConfig } = MigrationsService.run({
allowCustomCrateRegistries: false,
allowScripts: false,
exposeAllEnv: false,
trustLevel: 'high',
});
- expect(migratedConfig.allowCustomCrateRegistries).toBeFalse();
- expect(migratedConfig.allowScripts).toBeFalse();
- expect(migratedConfig.exposeAllEnv).toBeFalse();
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({
+ allowCustomCrateRegistries: false,
+ allowScripts: false,
+ exposeAllEnv: false,
+ });
});
});
diff --git a/lib/config/migrations/custom/trust-level-migration.ts b/lib/config/migrations/custom/trust-level-migration.ts
index b9e09077f4..95948940cf 100644
--- a/lib/config/migrations/custom/trust-level-migration.ts
+++ b/lib/config/migrations/custom/trust-level-migration.ts
@@ -1,21 +1,15 @@
-import type { RenovateConfig } from '../../types';
import { AbstractMigration } from '../base/abstract-migration';
export class TrustLevelMigration extends AbstractMigration {
- constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {
- super('trustLevel', originalConfig, migratedConfig);
- }
+ readonly propertyName = 'trustLevel';
- override run(): void {
+ override run(value): void {
this.delete(this.propertyName);
- if (this.originalConfig.trustLevel === 'high') {
- this.migratedConfig.allowCustomCrateRegistries =
- this.originalConfig.allowCustomCrateRegistries ?? true;
- this.migratedConfig.allowScripts =
- this.originalConfig.allowScripts ?? true;
- this.migratedConfig.exposeAllEnv =
- this.originalConfig.exposeAllEnv ?? true;
+ if (value === 'high') {
+ this.setSafely('allowCustomCrateRegistries', true);
+ this.setSafely('allowScripts', true);
+ this.setSafely('exposeAllEnv', true);
}
}
}
diff --git a/lib/config/migrations/migrations-service.spec.ts b/lib/config/migrations/migrations-service.spec.ts
index 222da9a9ca..ae7ba803c0 100644
--- a/lib/config/migrations/migrations-service.spec.ts
+++ b/lib/config/migrations/migrations-service.spec.ts
@@ -8,8 +8,10 @@ describe('config/migrations/migrations-service', () => {
[property]: 'test',
};
- const migratedConfig = MigrationsService.run(originalConfig);
- expect(migratedConfig).not.toHaveProperty(property);
+ const { isMigrated, migratedConfig } =
+ MigrationsService.run(originalConfig);
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({});
}
});
@@ -22,9 +24,12 @@ describe('config/migrations/migrations-service', () => {
[oldPropertyName]: 'test',
};
- const migratedConfig = MigrationsService.run(originalConfig);
- expect(migratedConfig).not.toHaveProperty(oldPropertyName);
- expect(migratedConfig[newPropertyName]).toBe('test');
+ const { isMigrated, migratedConfig } =
+ MigrationsService.run(originalConfig);
+ expect(isMigrated).toBeTrue();
+ expect(migratedConfig).toEqual({
+ [newPropertyName]: 'test',
+ });
}
});
@@ -34,12 +39,14 @@ describe('config/migrations/migrations-service', () => {
versionScheme: 'test',
excludedPackageNames: ['test'],
};
- const migratedConfig = MigrationsService.run(originalConfig);
+ const { isMigrated, migratedConfig } =
+ MigrationsService.run(originalConfig);
const mappedProperties = Object.keys(originalConfig).map((property) =>
MigrationsService.renamedProperties.get(property)
);
+ expect(isMigrated).toBeTrue();
expect(mappedProperties).toEqual(Object.keys(migratedConfig));
});
});
diff --git a/lib/config/migrations/migrations-service.ts b/lib/config/migrations/migrations-service.ts
index d1cee89acb..a0d67ed42d 100644
--- a/lib/config/migrations/migrations-service.ts
+++ b/lib/config/migrations/migrations-service.ts
@@ -1,4 +1,5 @@
-import type { RenovateConfig } from '../types';
+import { dequal } from 'dequal';
+import type { MigratedConfig, RenovateConfig } from '../types';
import { RemovePropertyMigration } from './base/remove-property-migration';
import { RenamePropertyMigration } from './base/rename-property-migration';
import { BinarySourceMigration } from './custom/binary-source-migration';
@@ -6,7 +7,7 @@ import { GoModTidyMigration } from './custom/go-mod-tidy-migration';
import { IgnoreNodeModulesMigration } from './custom/ignore-node-modules-migration';
import { RequiredStatusChecksMigration } from './custom/required-status-checks-migration';
import { TrustLevelMigration } from './custom/trust-level-migration';
-import type { Migration } from './types';
+import type { Migration, MigrationConstructor } from './types';
export class MigrationsService {
static readonly removedProperties: ReadonlySet<string> = new Set([
@@ -34,7 +35,15 @@ export class MigrationsService {
['versionScheme', 'versioning'],
]);
- static run(originalConfig: RenovateConfig): RenovateConfig {
+ static readonly customMigrations: ReadonlyArray<MigrationConstructor> = [
+ BinarySourceMigration,
+ GoModTidyMigration,
+ IgnoreNodeModulesMigration,
+ RequiredStatusChecksMigration,
+ TrustLevelMigration,
+ ];
+
+ static run(originalConfig: RenovateConfig): MigratedConfig {
const migratedConfig: RenovateConfig = {};
const migrations = MigrationsService.getMigrations(
originalConfig,
@@ -44,16 +53,19 @@ export class MigrationsService {
for (const [key, value] of Object.entries(originalConfig)) {
migratedConfig[key] ??= value;
const migration = migrations.find((item) => item.propertyName === key);
- migration?.run();
+ migration?.run(value);
}
- return migratedConfig;
+ return {
+ isMigrated: !dequal(originalConfig, migratedConfig),
+ migratedConfig,
+ };
}
private static getMigrations(
originalConfig: RenovateConfig,
migratedConfig: RenovateConfig
- ): Migration[] {
+ ): ReadonlyArray<Migration> {
const migrations: Migration[] = [];
for (const propertyName of MigrationsService.removedProperties) {
@@ -80,15 +92,9 @@ export class MigrationsService {
);
}
- migrations.push(new BinarySourceMigration(originalConfig, migratedConfig));
- migrations.push(
- new IgnoreNodeModulesMigration(originalConfig, migratedConfig)
- );
- migrations.push(
- new RequiredStatusChecksMigration(originalConfig, migratedConfig)
- );
- migrations.push(new TrustLevelMigration(originalConfig, migratedConfig));
- migrations.push(new GoModTidyMigration(originalConfig, migratedConfig));
+ for (const CustomMigration of this.customMigrations) {
+ migrations.push(new CustomMigration(originalConfig, migratedConfig));
+ }
return migrations;
}
diff --git a/lib/config/migrations/types.ts b/lib/config/migrations/types.ts
index 93f2035ed6..206b294863 100644
--- a/lib/config/migrations/types.ts
+++ b/lib/config/migrations/types.ts
@@ -1,4 +1,12 @@
+import type { RenovateConfig } from './../types';
+export interface MigrationConstructor {
+ new (
+ originalConfig: RenovateConfig,
+ migratedConfig: RenovateConfig
+ ): Migration;
+}
+
export interface Migration {
readonly propertyName: string;
- run(): void;
+ run(value: unknown): void;
}
--
GitLab