From de5e712305b1658f33bf4029de580021b0deb6c0 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Thu, 19 Sep 2019 12:36:25 -0400 Subject: [PATCH 1/2] when looking for latest fastly VCL to clone, use only active VCLs --- bin/configure-fastly.js | 2 +- bin/lib/fastly-extended.js | 13 ++++++-- test/unit/lib/fastly-extended.test.js | 44 +++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 test/unit/lib/fastly-extended.test.js diff --git a/bin/configure-fastly.js b/bin/configure-fastly.js index c6f76411d..c3eb4cca2 100644 --- a/bin/configure-fastly.js +++ b/bin/configure-fastly.js @@ -24,7 +24,7 @@ var routes = routeJson.map(function (route) { async.auto({ version: function (cb) { - fastly.getLatestVersion(function (err, response) { + fastly.getLatestActiveVersion(function (err, response) { if (err) return cb(err); // Validate latest version before continuing if (response.active || response.locked) { diff --git a/bin/lib/fastly-extended.js b/bin/lib/fastly-extended.js index 0e4467221..fbad130d5 100644 --- a/bin/lib/fastly-extended.js +++ b/bin/lib/fastly-extended.js @@ -24,11 +24,11 @@ module.exports = function (apiKey, serviceId) { }; /* - * getLatestVersion: Get the most recent version for the configured service + * getLatestActiveVersion: Get the most recent version for the configured service * * @param {callback} Callback with signature *err, latestVersion) */ - fastly.getLatestVersion = function (cb) { + fastly.getLatestActiveVersion = function (cb) { if (!this.serviceId) { return cb('Failed to get latest version. No serviceId configured'); } @@ -39,7 +39,14 @@ module.exports = function (apiKey, serviceId) { } var latestVersion = versions.reduce(function (lateVersion, version) { if (!lateVersion) return version; - if (version.number > lateVersion.number) return version; + // if versions we're comparing are both active, or both inactive, prefer + // whichever has a higher version number + if (lateVersion.active === version.active) { + if (version.number > lateVersion.number) return version; + return lateVersion; + } + // if only one of the versions is active, prefer that one + if (version.active === true) return version; return lateVersion; }); return cb(null, latestVersion); diff --git a/test/unit/lib/fastly-extended.test.js b/test/unit/lib/fastly-extended.test.js new file mode 100644 index 000000000..edc5e78f1 --- /dev/null +++ b/test/unit/lib/fastly-extended.test.js @@ -0,0 +1,44 @@ +describe('fastly library', () => { + test('getLatestActiveVersion returns largest active VCL number', done => { + + const mockedFastlyRequest = jest.fn((method, url, cb) => { + cb(null, [ + { + number: 4, + active: false + }, + { + number: 1, + active: true + }, + { + number: 2, + active: true + }, + { + number: 3, + active: false + } + ]); + }); + + jest.mock('fastly', () => (() => ({ + request: mockedFastlyRequest + }))); + + const fastlyExtended = require('../../../bin/lib/fastly-extended'); // eslint-disable-line global-require + const fastlyInstance = fastlyExtended('api_key', 'service_id'); + + fastlyInstance.getLatestActiveVersion((err, response) => { + expect(err).toBe(null); + expect(response).toEqual({ + number: 2, + active: true + }); + expect(mockedFastlyRequest).toHaveBeenCalledWith( + 'GET', '/service/service_id/version', expect.any(Function) + ); + done(); + }); + }); +}); From 88abed34e9785333ad2581d283a80cf53bbd27dd Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Thu, 26 Sep 2019 10:49:12 -0400 Subject: [PATCH 2/2] revised fastly tests, added more --- bin/lib/fastly-extended.js | 21 ++-- test/unit/lib/fastly-extended.test.js | 134 ++++++++++++++++++++++++-- 2 files changed, 134 insertions(+), 21 deletions(-) diff --git a/bin/lib/fastly-extended.js b/bin/lib/fastly-extended.js index fbad130d5..ffac40acc 100644 --- a/bin/lib/fastly-extended.js +++ b/bin/lib/fastly-extended.js @@ -37,18 +37,15 @@ module.exports = function (apiKey, serviceId) { if (err) { return cb('Failed to fetch versions: ' + err); } - var latestVersion = versions.reduce(function (lateVersion, version) { - if (!lateVersion) return version; - // if versions we're comparing are both active, or both inactive, prefer - // whichever has a higher version number - if (lateVersion.active === version.active) { - if (version.number > lateVersion.number) return version; - return lateVersion; - } - // if only one of the versions is active, prefer that one - if (version.active === true) return version; - return lateVersion; - }); + var latestVersion = versions.reduce((latestActiveSoFar, cur) => { + // if one of [latestActiveSoFar, cur] is active and the other isn't, + // return whichever is active. If both are not active, return + // latestActiveSoFar. + if (!cur || !cur.active) return latestActiveSoFar; + if (!latestActiveSoFar || !latestActiveSoFar.active) return cur; + // when both are active, prefer whichever has a higher version number. + return (cur.number > latestActiveSoFar.number) ? cur : latestActiveSoFar; + }, null); return cb(null, latestVersion); }); }; diff --git a/test/unit/lib/fastly-extended.test.js b/test/unit/lib/fastly-extended.test.js index edc5e78f1..451902c16 100644 --- a/test/unit/lib/fastly-extended.test.js +++ b/test/unit/lib/fastly-extended.test.js @@ -1,7 +1,50 @@ describe('fastly library', () => { - test('getLatestActiveVersion returns largest active VCL number', done => { + let mockedFastlyRequest = {}; - const mockedFastlyRequest = jest.fn((method, url, cb) => { + jest.mock('fastly', () => (() => ({ + request: mockedFastlyRequest + }))); + const fastlyExtended = require('../../../bin/lib/fastly-extended'); // eslint-disable-line global-require + + test('getLatestActiveVersion returns largest active VCL number, ' + + 'when called with VCLs in sequential order', done => { + mockedFastlyRequest = jest.fn((method, url, cb) => { + cb(null, [ + { + number: 1, + active: false + }, + { + number: 2, + active: false + }, + { + number: 3, + active: true + }, + { + number: 4, + active: false + } + ]); + }); + const fastlyInstance = fastlyExtended('api_key', 'service_id'); + + fastlyInstance.getLatestActiveVersion((err, response) => { + expect(err).toBe(null); + expect(response).toEqual({ + number: 3, + active: true + }); + expect(mockedFastlyRequest).toHaveBeenCalledWith( + 'GET', '/service/service_id/version', expect.any(Function) + ); + done(); + }); + }); + + test('getLatestActiveVersion returns largest active VCL number, when called with VCLs in mixed up order', done => { + mockedFastlyRequest = jest.fn((method, url, cb) => { cb(null, [ { number: 4, @@ -9,7 +52,7 @@ describe('fastly library', () => { }, { number: 1, - active: true + active: false }, { number: 2, @@ -21,12 +64,6 @@ describe('fastly library', () => { } ]); }); - - jest.mock('fastly', () => (() => ({ - request: mockedFastlyRequest - }))); - - const fastlyExtended = require('../../../bin/lib/fastly-extended'); // eslint-disable-line global-require const fastlyInstance = fastlyExtended('api_key', 'service_id'); fastlyInstance.getLatestActiveVersion((err, response) => { @@ -41,4 +78,83 @@ describe('fastly library', () => { done(); }); }); + + test('getLatestActiveVersion returns null, when none of the VCL versions are active', done => { + mockedFastlyRequest = jest.fn((method, url, cb) => { + cb(null, [ + { + number: 4, + active: false + }, + { + number: 1, + active: false + }, + { + number: 2, + active: false + }, + { + number: 3, + active: false + } + ]); + }); + const fastlyInstance = fastlyExtended('api_key', 'service_id'); + + fastlyInstance.getLatestActiveVersion((err, response) => { + expect(err).toBe(null); + expect(response).toEqual(null); + expect(mockedFastlyRequest).toHaveBeenCalledWith( + 'GET', '/service/service_id/version', expect.any(Function) + ); + done(); + }); + }); + + test('getLatestActiveVersion returns largest active VCL number, ' + + 'when called with a single active VCL', done => { + mockedFastlyRequest = jest.fn((method, url, cb) => { + cb(null, [ + { + number: 1, + active: true + } + ]); + }); + const fastlyInstance = fastlyExtended('api_key', 'service_id'); + + fastlyInstance.getLatestActiveVersion((err, response) => { + expect(err).toBe(null); + expect(response).toEqual({ + number: 1, + active: true + }); + expect(mockedFastlyRequest).toHaveBeenCalledWith( + 'GET', '/service/service_id/version', expect.any(Function) + ); + done(); + }); + }); + + test('getLatestActiveVersion returns null, when called with a single inactive VCL', done => { + mockedFastlyRequest = jest.fn((method, url, cb) => { + cb(null, [ + { + number: 1, + active: false + } + ]); + }); + const fastlyInstance = fastlyExtended('api_key', 'service_id'); + + fastlyInstance.getLatestActiveVersion((err, response) => { + expect(err).toBe(null); + expect(response).toEqual(null); + expect(mockedFastlyRequest).toHaveBeenCalledWith( + 'GET', '/service/service_id/version', expect.any(Function) + ); + done(); + }); + }); });