From 70c8d2adbe00858c7c64e18cfe0191b0cc44c984 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Sat, 2 Jul 2022 19:58:36 +0530 Subject: [PATCH 1/9] use class-based migration for packageRules --- lib/config/migration.ts | 45 -------------- .../custom/package-rules-migration.spec.ts | 50 ++++++++++++++++ .../custom/package-rules-migration.ts | 58 +++++++++++++++++++ lib/config/migrations/migrations-service.ts | 2 + 4 files changed, 110 insertions(+), 45 deletions(-) create mode 100644 lib/config/migrations/custom/package-rules-migration.spec.ts create mode 100644 lib/config/migrations/custom/package-rules-migration.ts diff --git a/lib/config/migration.ts b/lib/config/migration.ts index 571e9ab585fe0c..e4e0aaed67f628 100644 --- a/lib/config/migration.ts +++ b/lib/config/migration.ts @@ -8,7 +8,6 @@ import { getOptions } from './options'; import type { MigratedConfig, MigratedRenovateConfig, - PackageRule, RenovateConfig, RenovateOptions, } from './types'; @@ -202,50 +201,6 @@ export function migrateConfig(config: RenovateConfig): MigratedConfig { } } } - if (is.array(migratedConfig.packageRules)) { - const newRules: PackageRule[] = []; - const renameMap = { - paths: 'matchPaths', - languages: 'matchLanguages', - baseBranchList: 'matchBaseBranches', - managers: 'matchManagers', - datasources: 'matchDatasources', - depTypeList: 'matchDepTypes', - packageNames: 'matchPackageNames', - packagePatterns: 'matchPackagePatterns', - sourceUrlPrefixes: 'matchSourceUrlPrefixes', - updateTypes: 'matchUpdateTypes', - } as const; - for (const packageRule of migratedConfig.packageRules) { - const newRuleObj = {} as PackageRule; - for (const [oldKey, ruleVal] of Object.entries(packageRule)) { - const key = renameMap[oldKey as keyof typeof renameMap] ?? oldKey; - // TODO: fix types #7154 - newRuleObj[key] = ruleVal as never; - } - newRules.push(newRuleObj); - } - migratedConfig.packageRules = newRules; - } - // Migrate nested packageRules - if (is.nonEmptyArray(migratedConfig.packageRules)) { - const existingRules = migratedConfig.packageRules; - migratedConfig.packageRules = []; - for (const packageRule of existingRules) { - if (is.array(packageRule.packageRules)) { - logger.debug('Flattening nested packageRules'); - // merge each subrule and add to the parent list - for (const subrule of packageRule.packageRules) { - // TODO: fix types #7154 - const combinedRule = mergeChildConfig(packageRule, subrule as any); - delete combinedRule.packageRules; - migratedConfig.packageRules.push(combinedRule); - } - } else { - migratedConfig.packageRules.push(packageRule); - } - } - } if (is.nonEmptyArray(migratedConfig.matchManagers)) { if (migratedConfig.matchManagers.includes('gradle-lite')) { if (!migratedConfig.matchManagers.includes('gradle')) { diff --git a/lib/config/migrations/custom/package-rules-migration.spec.ts b/lib/config/migrations/custom/package-rules-migration.spec.ts new file mode 100644 index 00000000000000..8ba50ffd8dd6d3 --- /dev/null +++ b/lib/config/migrations/custom/package-rules-migration.spec.ts @@ -0,0 +1,50 @@ +import type { RenovateConfig } from '../../types'; +import { MigrationsService } from '../migrations-service'; + +describe('config/migrations/custom/package-rules-migration', () => { + it('should migrate value to object', () => { + const res = { + packageRules: [ + { + matchPaths: [], + labels: ['linting'], + matchBaseBranches: [], + matchLanguages: [], + matchManagers: [], + matchDatasources: [], + matchDepTypes: [], + addLabels: [], + matchPackageNames: [], + matchPackagePatterns: [], + matchSourceUrlPrefixes: [], + matchUpdateTypes: [], + }, + ], + }; + const originalConfig: RenovateConfig = { + packageRules: [ + { + paths: [], + labels: ['linting'], + baseBranchList: [], + languages: [], + managers: [], + datasources: [], + depTypeList: [], + addLabels: [], + packageNames: [], + packagePatterns: [], + sourceUrlPrefixes: [], + updateTypes: [], + }, + ], + }; + const migratedPackageRules = + MigrationsService.run(originalConfig).packageRules; + + const mappedProperties = Object.keys(migratedPackageRules![0]); + const expectedMappedProperties = Object.keys(res.packageRules[0]); + + expect(expectedMappedProperties).toEqual(mappedProperties); + }); +}); diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts new file mode 100644 index 00000000000000..2cf6e9d7efca69 --- /dev/null +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -0,0 +1,58 @@ +import is from '@sindresorhus/is'; +import { logger } from '../../../logger'; +import type { PackageRule } from '../../types'; +import { mergeChildConfig } from '../../utils'; +import { AbstractMigration } from '../base/abstract-migration'; + +const renameMap = { + paths: 'matchPaths', + languages: 'matchLanguages', + baseBranchList: 'matchBaseBranches', + managers: 'matchManagers', + datasources: 'matchDatasources', + depTypeList: 'matchDepTypes', + packageNames: 'matchPackageNames', + packagePatterns: 'matchPackagePatterns', + sourceUrlPrefixes: 'matchSourceUrlPrefixes', + updateTypes: 'matchUpdateTypes', +}; + +export class PackageRulesMigration extends AbstractMigration { + override readonly propertyName = 'packageRules'; + + override run(value: PackageRule[] | null): void { + let packageRules = this.get('packageRules') as PackageRule[]; + packageRules = Array.isArray(packageRules) ? [...packageRules] : []; + + if (is.plainObject(value)) { + packageRules.push(value); + } + + packageRules = packageRules.map((packageRule) => { + const newPackageRule: PackageRule = {}; + + for (const [key, value] of Object.entries(packageRule)) { + newPackageRule[renameMap[key as keyof typeof renameMap] ?? key] = value; + } + + return newPackageRule; + }); + + packageRules = packageRules.flatMap((packageRule) => { + if (Array.isArray(packageRule.packageRules)) { + const subrules: PackageRule[] = []; + logger.debug('Flattening nested packageRules'); + + for (const subrule of packageRule.packageRules) { + const combinedRule = mergeChildConfig(packageRule, subrule); + delete combinedRule.packageRules; + subrules.push(combinedRule); + } + return subrules; + } + return packageRule; + }) as PackageRule[]; + + this.rewrite(packageRules); + } +} diff --git a/lib/config/migrations/migrations-service.ts b/lib/config/migrations/migrations-service.ts index 60eb6f7b27671f..42a9e97509a29d 100644 --- a/lib/config/migrations/migrations-service.ts +++ b/lib/config/migrations/migrations-service.ts @@ -25,6 +25,7 @@ import { IgnoreNpmrcFileMigration } from './custom/ignore-npmrc-file-migration'; import { MatchStringsMigration } from './custom/match-strings-migration'; import { PackageNameMigration } from './custom/package-name-migration'; import { PackagePatternMigration } from './custom/package-pattern-migration'; +import { PackageRulesMigration } from './custom/package-rules-migration'; import { PackagesMigration } from './custom/packages-migration'; import { PathRulesMigration } from './custom/path-rules-migration'; import { PinVersionsMigration } from './custom/pin-versions-migration'; @@ -119,6 +120,7 @@ export class MigrationsService { VersionStrategyMigration, DryRunMigration, RequireConfigMigration, + PackageRulesMigration, ]; static run(originalConfig: RenovateConfig): RenovateConfig { From 2d58c3cf0da3bdbbbe590df5747659444e4f958b Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Sun, 3 Jul 2022 16:05:23 +0530 Subject: [PATCH 2/9] fix errors --- lib/config/migration.ts | 19 +++++++++++++++++++ .../custom/package-rules-migration.ts | 17 ----------------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/config/migration.ts b/lib/config/migration.ts index e4e0aaed67f628..c8a3cc64f2964f 100644 --- a/lib/config/migration.ts +++ b/lib/config/migration.ts @@ -201,6 +201,25 @@ export function migrateConfig(config: RenovateConfig): MigratedConfig { } } } + // Migrate nested packageRules + if (is.nonEmptyArray(migratedConfig.packageRules)) { + const existingRules = migratedConfig.packageRules; + migratedConfig.packageRules = []; + for (const packageRule of existingRules) { + if (is.array(packageRule.packageRules)) { + logger.debug('Flattening nested packageRules'); + // merge each subrule and add to the parent list + for (const subrule of packageRule.packageRules) { + // TODO: fix types #7154 + const combinedRule = mergeChildConfig(packageRule, subrule as any); + delete combinedRule.packageRules; + migratedConfig.packageRules.push(combinedRule); + } + } else { + migratedConfig.packageRules.push(packageRule); + } + } + } if (is.nonEmptyArray(migratedConfig.matchManagers)) { if (migratedConfig.matchManagers.includes('gradle-lite')) { if (!migratedConfig.matchManagers.includes('gradle')) { diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts index 2cf6e9d7efca69..12f0e618dc1843 100644 --- a/lib/config/migrations/custom/package-rules-migration.ts +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -1,7 +1,5 @@ import is from '@sindresorhus/is'; -import { logger } from '../../../logger'; import type { PackageRule } from '../../types'; -import { mergeChildConfig } from '../../utils'; import { AbstractMigration } from '../base/abstract-migration'; const renameMap = { @@ -38,21 +36,6 @@ export class PackageRulesMigration extends AbstractMigration { return newPackageRule; }); - packageRules = packageRules.flatMap((packageRule) => { - if (Array.isArray(packageRule.packageRules)) { - const subrules: PackageRule[] = []; - logger.debug('Flattening nested packageRules'); - - for (const subrule of packageRule.packageRules) { - const combinedRule = mergeChildConfig(packageRule, subrule); - delete combinedRule.packageRules; - subrules.push(combinedRule); - } - return subrules; - } - return packageRule; - }) as PackageRule[]; - this.rewrite(packageRules); } } From 7d59c6440f78499faed974de1157a158599c379c Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Mon, 4 Jul 2022 18:45:04 +0530 Subject: [PATCH 3/9] refactor: remove unnecessaru if statement --- .../custom/package-rules-migration.spec.ts | 50 +++++++++++-------- .../custom/package-rules-migration.ts | 7 +-- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/lib/config/migrations/custom/package-rules-migration.spec.ts b/lib/config/migrations/custom/package-rules-migration.spec.ts index 8ba50ffd8dd6d3..21285fbf9b0114 100644 --- a/lib/config/migrations/custom/package-rules-migration.spec.ts +++ b/lib/config/migrations/custom/package-rules-migration.spec.ts @@ -1,26 +1,9 @@ import type { RenovateConfig } from '../../types'; import { MigrationsService } from '../migrations-service'; +import { PackageRulesMigration, renameMap } from './package-rules-migration'; describe('config/migrations/custom/package-rules-migration', () => { - it('should migrate value to object', () => { - const res = { - packageRules: [ - { - matchPaths: [], - labels: ['linting'], - matchBaseBranches: [], - matchLanguages: [], - matchManagers: [], - matchDatasources: [], - matchDepTypes: [], - addLabels: [], - matchPackageNames: [], - matchPackagePatterns: [], - matchSourceUrlPrefixes: [], - matchUpdateTypes: [], - }, - ], - }; + it('should preserve config order', () => { const originalConfig: RenovateConfig = { packageRules: [ { @@ -43,8 +26,35 @@ describe('config/migrations/custom/package-rules-migration', () => { MigrationsService.run(originalConfig).packageRules; const mappedProperties = Object.keys(migratedPackageRules![0]); - const expectedMappedProperties = Object.keys(res.packageRules[0]); + const expectedMappedProperties = Object.keys( + originalConfig.packageRules![0] + ).map((key) => renameMap[key as keyof typeof renameMap] ?? key); expect(expectedMappedProperties).toEqual(mappedProperties); }); + + it('should not migrate nested packageRules', () => { + expect(PackageRulesMigration).toMigrate( + { + packageRules: [ + { + paths: [], + packgageRules: { + languages: ['javascript'], + }, + }, + ], + }, + { + packageRules: [ + { + matchPaths: [], + packgageRules: { + languages: ['javascript'], + }, + }, + ], + } + ); + }); }); diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts index 12f0e618dc1843..5655a1dfe1c3db 100644 --- a/lib/config/migrations/custom/package-rules-migration.ts +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -1,8 +1,7 @@ -import is from '@sindresorhus/is'; import type { PackageRule } from '../../types'; import { AbstractMigration } from '../base/abstract-migration'; -const renameMap = { +export const renameMap = { paths: 'matchPaths', languages: 'matchLanguages', baseBranchList: 'matchBaseBranches', @@ -22,10 +21,6 @@ export class PackageRulesMigration extends AbstractMigration { let packageRules = this.get('packageRules') as PackageRule[]; packageRules = Array.isArray(packageRules) ? [...packageRules] : []; - if (is.plainObject(value)) { - packageRules.push(value); - } - packageRules = packageRules.map((packageRule) => { const newPackageRule: PackageRule = {}; From 3784ae721a0ac98a18bf71d1e3a1519ef30d4c4b Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Wed, 6 Jul 2022 10:47:35 +0530 Subject: [PATCH 4/9] refactor: use argument --- lib/config/migrations/custom/package-rules-migration.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts index 5655a1dfe1c3db..5370c6fd741929 100644 --- a/lib/config/migrations/custom/package-rules-migration.ts +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -18,14 +18,14 @@ export class PackageRulesMigration extends AbstractMigration { override readonly propertyName = 'packageRules'; override run(value: PackageRule[] | null): void { - let packageRules = this.get('packageRules') as PackageRule[]; + let packageRules = value; packageRules = Array.isArray(packageRules) ? [...packageRules] : []; packageRules = packageRules.map((packageRule) => { const newPackageRule: PackageRule = {}; - for (const [key, value] of Object.entries(packageRule)) { - newPackageRule[renameMap[key as keyof typeof renameMap] ?? key] = value; + for (const [key, val] of Object.entries(packageRule)) { + newPackageRule[renameMap[key as keyof typeof renameMap] ?? key] = val; } return newPackageRule; From f7246cbd1338d6b715156031f4090f7d4ea06a28 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Wed, 6 Jul 2022 18:58:45 +0530 Subject: [PATCH 5/9] refactor: apply suggestions --- .../custom/package-rules-migration.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts index 5370c6fd741929..b42783b7e798cf 100644 --- a/lib/config/migrations/custom/package-rules-migration.ts +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -13,23 +13,23 @@ export const renameMap = { sourceUrlPrefixes: 'matchSourceUrlPrefixes', updateTypes: 'matchUpdateTypes', }; +type renameMapKeyType = keyof typeof renameMap; +function renameKeys(packageRule: PackageRule): PackageRule { + const newPackageRule: PackageRule = {}; + for (const [key, val] of Object.entries(packageRule)) { + newPackageRule[renameMap[key as renameMapKeyType] ?? key] = val; + } + return newPackageRule; +} export class PackageRulesMigration extends AbstractMigration { override readonly propertyName = 'packageRules'; - override run(value: PackageRule[] | null): void { - let packageRules = value; + override run(value: unknown): void { + let packageRules = (this.get('packageRules') as PackageRule[]) ?? []; packageRules = Array.isArray(packageRules) ? [...packageRules] : []; - packageRules = packageRules.map((packageRule) => { - const newPackageRule: PackageRule = {}; - - for (const [key, val] of Object.entries(packageRule)) { - newPackageRule[renameMap[key as keyof typeof renameMap] ?? key] = val; - } - - return newPackageRule; - }); + packageRules = packageRules.map(renameKeys); this.rewrite(packageRules); } From 11bed49cf851f5a4b7717b648f06c5b2c0e35a5f Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Thu, 7 Jul 2022 11:37:02 +0530 Subject: [PATCH 6/9] Update lib/config/migrations/custom/package-rules-migration.ts Co-authored-by: Michael Kriese --- lib/config/migrations/custom/package-rules-migration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts index b42783b7e798cf..cc502e2a9b53d5 100644 --- a/lib/config/migrations/custom/package-rules-migration.ts +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -13,7 +13,7 @@ export const renameMap = { sourceUrlPrefixes: 'matchSourceUrlPrefixes', updateTypes: 'matchUpdateTypes', }; -type renameMapKeyType = keyof typeof renameMap; +type RenameMapKey = keyof typeof renameMap; function renameKeys(packageRule: PackageRule): PackageRule { const newPackageRule: PackageRule = {}; From bbb37ad858bcf76944a149d65edd09da70a26f3e Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Thu, 7 Jul 2022 11:37:08 +0530 Subject: [PATCH 7/9] Update lib/config/migrations/custom/package-rules-migration.ts Co-authored-by: Michael Kriese --- lib/config/migrations/custom/package-rules-migration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts index cc502e2a9b53d5..5425e2c7e13d98 100644 --- a/lib/config/migrations/custom/package-rules-migration.ts +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -18,7 +18,7 @@ type RenameMapKey = keyof typeof renameMap; function renameKeys(packageRule: PackageRule): PackageRule { const newPackageRule: PackageRule = {}; for (const [key, val] of Object.entries(packageRule)) { - newPackageRule[renameMap[key as renameMapKeyType] ?? key] = val; + newPackageRule[renameMap[key as RenameMapKey] ?? key] = val; } return newPackageRule; } From 45a36e3d0fcb7d0f58137b17dec586f650c45df3 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Fri, 8 Jul 2022 18:20:47 +0530 Subject: [PATCH 8/9] refactor: import type PackageRule --- lib/config/migration.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/config/migration.ts b/lib/config/migration.ts index fd35c0b6ac7aed..7d1f4b16718893 100644 --- a/lib/config/migration.ts +++ b/lib/config/migration.ts @@ -8,6 +8,7 @@ import { getOptions } from './options'; import type { MigratedConfig, MigratedRenovateConfig, + PackageRule, RenovateConfig, RenovateOptions, } from './types'; From 6a337f4a646ef4008317ba4b1f12244fa09b4d63 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Sun, 10 Jul 2022 22:46:21 +0530 Subject: [PATCH 9/9] Update lib/config/migrations/custom/package-rules-migration.ts Co-authored-by: Michael Kriese --- lib/config/migrations/custom/package-rules-migration.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/config/migrations/custom/package-rules-migration.ts b/lib/config/migrations/custom/package-rules-migration.ts index 5425e2c7e13d98..e3be4c4c910ac0 100644 --- a/lib/config/migrations/custom/package-rules-migration.ts +++ b/lib/config/migrations/custom/package-rules-migration.ts @@ -22,6 +22,7 @@ function renameKeys(packageRule: PackageRule): PackageRule { } return newPackageRule; } + export class PackageRulesMigration extends AbstractMigration { override readonly propertyName = 'packageRules';