From a50e5aa34c9dc92926969ef89f2543d86402b673 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Tue, 8 Feb 2022 09:10:09 +0100 Subject: [PATCH] feat(npm): optimize remediation to detect already updated branches --- .../update/locked-dependency/index.spec.ts | 26 +++++++++++- .../package-lock/get-locked.spec.ts | 5 +++ .../package-lock/get-locked.ts | 4 +- .../locked-dependency/package-lock/index.ts | 41 +++++++++++++++++-- lib/manager/types.ts | 1 + lib/workers/branch/get-updated.ts | 1 + 6 files changed, 71 insertions(+), 7 deletions(-) diff --git a/lib/manager/npm/update/locked-dependency/index.spec.ts b/lib/manager/npm/update/locked-dependency/index.spec.ts index 9909f810f159b4..db50a395d846de 100644 --- a/lib/manager/npm/update/locked-dependency/index.spec.ts +++ b/lib/manager/npm/update/locked-dependency/index.spec.ts @@ -120,13 +120,37 @@ describe('manager/npm/update/locked-dependency/index', () => { const packageLock = JSON.parse(res.files['package-lock.json']); expect(packageLock.dependencies.express.version).toBe('4.1.0'); }); - it('returns if already remediated', async () => { + it('returns already-updated if already remediated exactly', async () => { config.depName = 'mime'; config.currentVersion = '1.2.10'; config.newVersion = '1.2.11'; const res = await updateLockedDependency(config); expect(res.status).toBe('already-updated'); }); + it('returns already-updated if already remediated higher', async () => { + config.depName = 'mime'; + config.currentVersion = '1.2.9'; + config.newVersion = '1.2.10'; + config.allowHigherOrRemoved = true; + const res = await updateLockedDependency(config); + expect(res.status).toBe('already-updated'); + }); + it('returns already-updated if not found', async () => { + config.depName = 'notfound'; + config.currentVersion = '1.2.9'; + config.newVersion = '1.2.10'; + config.allowHigherOrRemoved = true; + const res = await updateLockedDependency(config); + expect(res.status).toBe('already-updated'); + }); + it('returns update-failed if other, lower version found', async () => { + config.depName = 'mime'; + config.currentVersion = '1.2.5'; + config.newVersion = '1.2.15'; + config.allowHigherOrRemoved = true; + const res = await updateLockedDependency(config); + expect(res.status).toBe('update-failed'); + }); it('remediates mime', async () => { config.depName = 'mime'; config.currentVersion = '1.2.11'; diff --git a/lib/manager/npm/update/locked-dependency/package-lock/get-locked.spec.ts b/lib/manager/npm/update/locked-dependency/package-lock/get-locked.spec.ts index 619975e72429de..3c3e3c303665ca 100644 --- a/lib/manager/npm/update/locked-dependency/package-lock/get-locked.spec.ts +++ b/lib/manager/npm/update/locked-dependency/package-lock/get-locked.spec.ts @@ -36,6 +36,11 @@ describe('manager/npm/update/locked-dependency/package-lock/get-locked', () => { }, ]); }); + it('finds any version', () => { + expect(getLockedDependencies(packageLockJson, 'send', null)).toHaveLength( + 2 + ); + }); it('finds bundled dependency', () => { expect( getLockedDependencies(bundledPackageLockJson, 'ansi-regex', '3.0.0') diff --git a/lib/manager/npm/update/locked-dependency/package-lock/get-locked.ts b/lib/manager/npm/update/locked-dependency/package-lock/get-locked.ts index d726174ad7e752..82acfdea1a36da 100644 --- a/lib/manager/npm/update/locked-dependency/package-lock/get-locked.ts +++ b/lib/manager/npm/update/locked-dependency/package-lock/get-locked.ts @@ -5,7 +5,7 @@ import type { PackageLockDependency, PackageLockOrEntry } from './types'; export function getLockedDependencies( entry: PackageLockOrEntry, depName: string, - currentVersion: string, + currentVersion: string | null, bundled = false ): PackageLockDependency[] { let res: PackageLockDependency[] = []; @@ -15,7 +15,7 @@ export function getLockedDependencies( return []; } const dep = dependencies[depName]; - if (dep?.version === currentVersion) { + if (dep && (currentVersion === null || dep?.version === currentVersion)) { if (bundled || entry.bundled) { dep.bundled = true; } diff --git a/lib/manager/npm/update/locked-dependency/package-lock/index.ts b/lib/manager/npm/update/locked-dependency/package-lock/index.ts index d910f84b00141d..d54dae102306f6 100644 --- a/lib/manager/npm/update/locked-dependency/package-lock/index.ts +++ b/lib/manager/npm/update/locked-dependency/package-lock/index.ts @@ -22,6 +22,7 @@ export async function updateLockedDependency( lockFile, lockFileContent, allowParentUpdates = true, + allowHigherOrRemoved = false, } = config; logger.debug( `npm.updateLockedDependency: ${depName}@${currentVersion} -> ${newVersion} [${lockFile}]` @@ -66,10 +67,42 @@ export async function updateLockedDependency( ); status = 'already-updated'; } else { - logger.debug( - `${depName}@${currentVersion} not found in ${lockFile} - cannot update` - ); - status = 'update-failed'; + if (allowHigherOrRemoved) { + // it's acceptable if the package is no longer present + const anyVersionLocked = getLockedDependencies( + packageLockJson, + depName, + null + ); + if (anyVersionLocked.length) { + if ( + anyVersionLocked.every((dep) => + semver.isGreaterThan(dep.version, newVersion) + ) + ) { + logger.debug( + `${depName} found in ${lockFile} with higher version - looks like it's already updated` + ); + status = 'already-updated'; + } else { + logger.debug( + { anyVersionLocked }, + `Found alternative versions of qs` + ); + status = 'update-failed'; + } + } else { + logger.debug( + `${depName} not found in ${lockFile} - looks like it's already removed` + ); + status = 'already-updated'; + } + } else { + logger.debug( + `${depName}@${currentVersion} not found in ${lockFile} - cannot update` + ); + status = 'update-failed'; + } } // Don't return {} if we're a parent update or else the whole update will fail // istanbul ignore if: too hard to replicate diff --git a/lib/manager/types.ts b/lib/manager/types.ts index 05857baa5ef038..29bf3e04ceccb7 100644 --- a/lib/manager/types.ts +++ b/lib/manager/types.ts @@ -229,6 +229,7 @@ export interface UpdateLockedConfig { currentVersion?: string; newVersion?: string; allowParentUpdates?: boolean; + allowHigherOrRemoved?: boolean; } export interface UpdateLockedResult { diff --git a/lib/workers/branch/get-updated.ts b/lib/workers/branch/get-updated.ts index 2a596f47588de9..2eebdd09b0d49d 100644 --- a/lib/workers/branch/get-updated.ts +++ b/lib/workers/branch/get-updated.ts @@ -75,6 +75,7 @@ export async function getUpdatedPackageFiles( packageFileContent, lockFileContent, allowParentUpdates: true, + allowHigherOrRemoved: true, }); if (reuseExistingBranch && status !== 'already-updated') { logger.debug(