From 603b77799b22fcb2864013fab1b6ecff517ffa59 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Wed, 6 Jun 2018 11:04:54 +0200
Subject: [PATCH] feat: refactor unpublishSafe for multiple package managers
 (#2090)

---
 lib/datasource/npm.js                         | 13 +--
 lib/workers/branch/status-checks.js           | 54 +++++--------
 .../repository/process/lookup/index.js        | 11 +--
 .../repository/process/lookup/rollback.js     |  1 -
 .../datasource/__snapshots__/npm.spec.js.snap | 79 ++++++++++++++-----
 test/datasource/npm.spec.js                   | 72 +++++++++++------
 .../lookup/__snapshots__/index.spec.js.snap   | 16 ----
 7 files changed, 132 insertions(+), 114 deletions(-)

diff --git a/lib/datasource/npm.js b/lib/datasource/npm.js
index 89720d9a0e..9ed5025541 100644
--- a/lib/datasource/npm.js
+++ b/lib/datasource/npm.js
@@ -1,5 +1,5 @@
 const is = require('@sindresorhus/is');
-
+const moment = require('moment');
 const got = require('got');
 const url = require('url');
 const ini = require('ini');
@@ -176,11 +176,12 @@ async function getDependency(name, retries = 5) {
       'renovate-config': latestVersion['renovate-config'],
     };
     Object.keys(dep.versions).forEach(version => {
-      // We don't use any of the version payload currently
-      dep.versions[version] = {
-        // fall back to arbitrary time for old npm servers
-        time: res.time ? res.time[version] : '2017-01-01T12:00:00.000Z',
-      };
+      const v = {};
+      if (res.time && res.time[version]) {
+        v.time = res.time[version];
+        v.canBeUnpublished = moment().diff(moment(v.time), 'days') === 0;
+      }
+      dep.versions[version] = v;
     });
     logger.trace({ dep }, 'dep');
     memcache[name] = dep;
diff --git a/lib/workers/branch/status-checks.js b/lib/workers/branch/status-checks.js
index b481cb356d..d5d931a1b0 100644
--- a/lib/workers/branch/status-checks.js
+++ b/lib/workers/branch/status-checks.js
@@ -1,5 +1,3 @@
-const is = require('@sindresorhus/is');
-
 module.exports = {
   setUnpublishable,
 };
@@ -8,42 +6,30 @@ async function setUnpublishable(config) {
   if (!config.unpublishSafe) {
     return;
   }
-  let canBeUnpublished;
-  for (const upgrade of config.upgrades) {
-    if (!is.undefined(upgrade.canBeUnpublished)) {
-      if (!is.undefined(canBeUnpublished)) {
-        canBeUnpublished = canBeUnpublished && upgrade.canBeUnpublished;
-      } else {
-        ({ canBeUnpublished } = upgrade);
-      }
-    }
-  }
-  if (is.undefined(canBeUnpublished)) {
-    canBeUnpublished = true;
-  }
+  const canBeUnpublished = config.upgrades.some(
+    upgrade => upgrade.canBeUnpublished
+  );
   const context = 'renovate/unpublish-safe';
   const existingState = await platform.getBranchStatusCheck(
     config.branchName,
     context
   );
-  if (config.unpublishSafe && !is.undefined(canBeUnpublished)) {
-    // Set canBeUnpublished status check
-    const state = canBeUnpublished ? 'pending' : 'success';
-    const description = canBeUnpublished
-      ? 'Packages < 24 hours old can be unpublished'
-      : 'Packages are at least 24 hours old';
-    // Check if state needs setting
-    if (existingState === state) {
-      logger.debug('Status check is already up-to-date');
-    } else {
-      logger.debug(`Updating status check state to ${state}`);
-      await platform.setBranchStatus(
-        config.branchName,
-        context,
-        description,
-        state,
-        'https://renovatebot.com/docs/configuration-reference/configuration-options#unpublishsafe'
-      );
-    }
+  // Set canBeUnpublished status check
+  const state = canBeUnpublished ? 'pending' : 'success';
+  const description = canBeUnpublished
+    ? 'Packages < 24 hours old can be unpublished'
+    : 'Packages cannot be unpublished';
+  // Check if state needs setting
+  if (existingState === state) {
+    logger.debug('Status check is already up-to-date');
+  } else {
+    logger.debug(`Updating status check state to ${state}`);
+    await platform.setBranchStatus(
+      config.branchName,
+      context,
+      description,
+      state,
+      'https://renovatebot.com/docs/configuration-reference/configuration-options#unpublishsafe'
+    );
   }
 }
diff --git a/lib/workers/repository/process/lookup/index.js b/lib/workers/repository/process/lookup/index.js
index bfd82fcd8e..45caccb2b9 100644
--- a/lib/workers/repository/process/lookup/index.js
+++ b/lib/workers/repository/process/lookup/index.js
@@ -1,5 +1,4 @@
 const versioning = require('../../../../versioning');
-const moment = require('moment');
 const { getRollbackUpdate } = require('./rollback');
 const { getRangeStrategy } = require('../../../../manager');
 const { filterVersions } = require('./filter');
@@ -100,7 +99,6 @@ async function lookupUpdates(config) {
       isPin: true,
       newValue: fromVersion,
       newMajor: getMajor(fromVersion),
-      canBeUnpublished: false,
     });
   }
   // Filter latest, unstable, etc
@@ -127,14 +125,7 @@ async function lookupUpdates(config) {
       update.isRange = true;
     }
 
-    // TODO: move canBeUnpublished to npm-specific
-    const version = dependency.versions[toVersion];
-    const elapsed =
-      version && version.time
-        ? moment().diff(moment(version.time), 'days')
-        : 999;
-    update.canBeUnpublished = elapsed === 0;
-    // end TODO
+    update.canBeUnpublished = dependency.versions[toVersion].canBeUnpublished;
 
     const bucket = getBucket(config, update);
     if (buckets[bucket]) {
diff --git a/lib/workers/repository/process/lookup/rollback.js b/lib/workers/repository/process/lookup/rollback.js
index 086d4a454b..c62cef0a3c 100644
--- a/lib/workers/repository/process/lookup/rollback.js
+++ b/lib/workers/repository/process/lookup/rollback.js
@@ -45,6 +45,5 @@ function getRollbackUpdate(config, versions) {
     newValue,
     newMajor: getMajor(toVersion),
     semanticCommitType: 'fix',
-    canBeUnpublished: false,
   };
 }
diff --git a/test/datasource/__snapshots__/npm.spec.js.snap b/test/datasource/__snapshots__/npm.spec.js.snap
index aa56f81055..5de6c2a098 100644
--- a/test/datasource/__snapshots__/npm.spec.js.snap
+++ b/test/datasource/__snapshots__/npm.spec.js.snap
@@ -9,10 +9,15 @@ Object {
   "latestVersion": "0.0.1",
   "name": undefined,
   "renovate-config": undefined,
-  "repositoryUrl": undefined,
+  "repositoryUrl": "https://github.com/renovateapp/dummy",
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
+    },
+    "0.0.2": Object {
+      "canBeUnpublished": false,
+      "time": "2018-05-07T07:21:53+02:00",
     },
   },
 }
@@ -30,13 +35,18 @@ Object {
   "repositoryUrl": "https://github.com/renovateapp/dummy",
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
+    },
+    "0.0.2": Object {
+      "canBeUnpublished": false,
+      "time": "2018-05-07T07:21:53+02:00",
     },
   },
 }
 `;
 
-exports[`api/npm should replace any environment variable in npmrc 1`] = `
+exports[`api/npm should handle no time 1`] = `
 Object {
   "dist-tags": Object {
     "latest": "0.0.1",
@@ -45,18 +55,18 @@ Object {
   "latestVersion": "0.0.1",
   "name": undefined,
   "renovate-config": undefined,
-  "repositoryUrl": undefined,
+  "repositoryUrl": "https://github.com/renovateapp/dummy",
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
     },
+    "0.0.2": Object {},
   },
 }
 `;
 
-exports[`api/npm should retry when 408 or 5xx 1`] = `null`;
-
-exports[`api/npm should send an authorization header if provided 1`] = `
+exports[`api/npm should replace any environment variable in npmrc 1`] = `
 Object {
   "dist-tags": Object {
     "latest": "0.0.1",
@@ -65,16 +75,23 @@ Object {
   "latestVersion": "0.0.1",
   "name": undefined,
   "renovate-config": undefined,
-  "repositoryUrl": undefined,
+  "repositoryUrl": "https://github.com/renovateapp/dummy",
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
+    },
+    "0.0.2": Object {
+      "canBeUnpublished": false,
+      "time": "2018-05-07T07:21:53+02:00",
     },
   },
 }
 `;
 
-exports[`api/npm should use NPM_TOKEN if provided 1`] = `
+exports[`api/npm should retry when 408 or 5xx 1`] = `null`;
+
+exports[`api/npm should send an authorization header if provided 1`] = `
 Object {
   "dist-tags": Object {
     "latest": "0.0.1",
@@ -83,16 +100,21 @@ Object {
   "latestVersion": "0.0.1",
   "name": undefined,
   "renovate-config": undefined,
-  "repositoryUrl": undefined,
+  "repositoryUrl": "https://github.com/renovateapp/dummy",
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
+    },
+    "0.0.2": Object {
+      "canBeUnpublished": false,
+      "time": "2018-05-07T07:21:53+02:00",
     },
   },
 }
 `;
 
-exports[`api/npm should use default registry if missing from npmrc 1`] = `
+exports[`api/npm should use NPM_TOKEN if provided 1`] = `
 Object {
   "dist-tags": Object {
     "latest": "0.0.1",
@@ -101,16 +123,21 @@ Object {
   "latestVersion": "0.0.1",
   "name": undefined,
   "renovate-config": undefined,
-  "repositoryUrl": undefined,
+  "repositoryUrl": "https://github.com/renovateapp/dummy",
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
+    },
+    "0.0.2": Object {
+      "canBeUnpublished": false,
+      "time": "2018-05-07T07:21:53+02:00",
     },
   },
 }
 `;
 
-exports[`api/npm should use dummy time if missing 1`] = `
+exports[`api/npm should use default registry if missing from npmrc 1`] = `
 Object {
   "dist-tags": Object {
     "latest": "0.0.1",
@@ -119,10 +146,15 @@ Object {
   "latestVersion": "0.0.1",
   "name": undefined,
   "renovate-config": undefined,
-  "repositoryUrl": undefined,
+  "repositoryUrl": "https://github.com/renovateapp/dummy",
   "versions": Object {
     "0.0.1": Object {
-      "time": "2017-01-01T12:00:00.000Z",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
+    },
+    "0.0.2": Object {
+      "canBeUnpublished": false,
+      "time": "2018-05-07T07:21:53+02:00",
     },
   },
 }
@@ -140,7 +172,12 @@ Object {
   "repositoryUrl": "https://google.com",
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "canBeUnpublished": false,
+      "time": "2018-05-06T07:21:53+02:00",
+    },
+    "0.0.2": Object {
+      "canBeUnpublished": false,
+      "time": "2018-05-07T07:21:53+02:00",
     },
   },
 }
diff --git a/test/datasource/npm.spec.js b/test/datasource/npm.spec.js
index 3caca56167..70b1e52b2f 100644
--- a/test/datasource/npm.spec.js
+++ b/test/datasource/npm.spec.js
@@ -1,33 +1,39 @@
 const npm = require('../../lib/datasource/npm');
 const registryAuthToken = require('registry-auth-token');
 const nock = require('nock');
+const moment = require('moment');
 
 jest.mock('registry-auth-token');
 jest.mock('delay');
 
-const npmResponse = {
-  versions: {
-    '0.0.1': {
-      foo: 1,
-    },
-  },
-  repository: {
-    type: 'git',
-    url: 'git://github.com/renovateapp/dummy.git',
-  },
-  'dist-tags': {
-    latest: '0.0.1',
-  },
-  time: {
-    '0.0.1': '',
-  },
-};
+let npmResponse;
 
 describe('api/npm', () => {
   delete process.env.NPM_TOKEN;
   beforeEach(() => {
     jest.resetAllMocks();
     npm.resetCache();
+    npmResponse = {
+      versions: {
+        '0.0.1': {
+          foo: 1,
+        },
+        '0.0.2': {
+          foo: 2,
+        },
+      },
+      repository: {
+        type: 'git',
+        url: 'git://github.com/renovateapp/dummy.git',
+      },
+      'dist-tags': {
+        latest: '0.0.1',
+      },
+      time: {
+        '0.0.1': '2018-05-06T07:21:53+02:00',
+        '0.0.2': '2018-05-07T07:21:53+02:00',
+      },
+    };
   });
   it('should return null for no versions', async () => {
     const missingVersions = { ...npmResponse };
@@ -47,6 +53,29 @@ describe('api/npm', () => {
       .reply(200, npmResponse);
     const res = await npm.getDependency('foobar');
     expect(res).toMatchSnapshot();
+    expect(res.versions['0.0.1'].canBeUnpublished).toBe(false);
+    expect(res.versions['0.0.2'].canBeUnpublished).toBe(false);
+  });
+  it('should handle no time', async () => {
+    delete npmResponse.time['0.0.2'];
+    nock('https://registry.npmjs.org')
+      .get('/foobar')
+      .reply(200, npmResponse);
+    const res = await npm.getDependency('foobar');
+    expect(res).toMatchSnapshot();
+    expect(res.versions['0.0.1'].canBeUnpublished).toBe(false);
+    expect(res.versions['0.0.2'].canBeUnpublished).toBeUndefined();
+  });
+  it('should return canBeUnpublished=true', async () => {
+    npmResponse.time['0.0.2'] = moment()
+      .subtract(6, 'hours')
+      .format();
+    nock('https://registry.npmjs.org')
+      .get('/foobar')
+      .reply(200, npmResponse);
+    const res = await npm.getDependency('foobar');
+    expect(res.versions['0.0.1'].canBeUnpublished).toBe(false);
+    expect(res.versions['0.0.2'].canBeUnpublished).toBe(true);
   });
   it('should use homepage', async () => {
     const npmResponseHomepage = { ...npmResponse };
@@ -184,15 +213,6 @@ describe('api/npm', () => {
     const res = await npm.getDependency('foobar');
     expect(res).toMatchSnapshot();
   });
-  it('should use dummy time if missing', async () => {
-    const noTimeResponse = { ...npmResponse };
-    delete noTimeResponse.time;
-    nock('https://registry.npmjs.org')
-      .get('/foobar')
-      .reply(200, noTimeResponse);
-    const res = await npm.getDependency('foobar');
-    expect(res).toMatchSnapshot();
-  });
   it('should cache package info from npm', async () => {
     npm.setNpmrc('//registry.npmjs.org/:_authToken=abcdefghijklmnopqrstuvwxyz');
     nock('https://registry.npmjs.org')
diff --git a/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap b/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap
index a7b4477288..ec2bdc5dd6 100644
--- a/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap
+++ b/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap
@@ -3,7 +3,6 @@
 exports[`manager/npm/lookup .lookupUpdates() disables major release separation (major) 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.4.4",
@@ -41,7 +40,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() handles PEP440 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.9.4",
@@ -110,7 +108,6 @@ exports[`manager/npm/lookup .lookupUpdates() handles unknown purl 1`] = `Array [
 exports[`manager/npm/lookup .lookupUpdates() ignores pinning for ranges when other upgrade exists 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.9.7",
@@ -133,7 +130,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() pins minor ranged versions 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 1,
     "newValue": "1.4.1",
@@ -200,7 +196,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() returns both updates if automerging minor 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.4.4",
@@ -298,7 +293,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() returns only one update if automerging major 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.4.4",
@@ -321,7 +315,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() returns only one update if grouping 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.4.4",
@@ -430,7 +423,6 @@ exports[`manager/npm/lookup .lookupUpdates() returns rollback for pinned version
 Array [
   Object {
     "branchName": "{{{branchPrefix}}}rollback-{{{depNameSanitized}}}-{{{newMajor}}}.x",
-    "canBeUnpublished": false,
     "commitMessageAction": "Roll back",
     "isRollback": true,
     "newMajor": 0,
@@ -456,7 +448,6 @@ exports[`manager/npm/lookup .lookupUpdates() returns rollback for ranged version
 Array [
   Object {
     "branchName": "{{{branchPrefix}}}rollback-{{{depNameSanitized}}}-{{{newMajor}}}.x",
-    "canBeUnpublished": false,
     "commitMessageAction": "Roll back",
     "isRollback": true,
     "newMajor": 0,
@@ -500,7 +491,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() should downgrade from missing versions 1`] = `
 Object {
   "branchName": "{{{branchPrefix}}}rollback-{{{depNameSanitized}}}-{{{newMajor}}}.x",
-  "canBeUnpublished": false,
   "commitMessageAction": "Roll back",
   "isRollback": true,
   "newMajor": 1,
@@ -654,7 +644,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() supports minor and major upgrades for ranged versions 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.4.4",
@@ -687,7 +676,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() supports minor and major upgrades for tilde ranges 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 0,
     "newValue": "0.4.4",
@@ -752,7 +740,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() upgrades .x minor ranges 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 1,
     "newValue": "1.3.0",
@@ -995,7 +982,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() upgrades minor ranged versions 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 1,
     "newValue": "1.0.1",
@@ -1104,7 +1090,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() upgrades tilde ranges 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 1,
     "newValue": "1.3.0",
@@ -1143,7 +1128,6 @@ Array [
 exports[`manager/npm/lookup .lookupUpdates() uses the locked version for pinning 1`] = `
 Array [
   Object {
-    "canBeUnpublished": false,
     "isPin": true,
     "newMajor": 1,
     "newValue": "1.0.0",
-- 
GitLab