From c18eb70a28d61317007091ae765585adc7cd37f6 Mon Sep 17 00:00:00 2001 From: Erik Erikson Date: Fri, 2 Feb 2018 19:29:19 -0800 Subject: [PATCH 1/6] Eliminate Deadlocks, Report Hung Promises (#4687) Add a "PromiseTracker" to the Variables class. It provides tracking and reporting of pending variable resolutions during service populations. Decompose populateObject and populateProperty into smaller pieces that are easier to understand. Additionally, remove the immediate recursive resolution of variables, recursing only at the Object or Property entry point (when changes have been made). This avoids a problem where the resolved value, prior to being replaced in the containing object or value refers to itself and thereby is waiting on its own resolution before it will resolve itself. Return the given value in warnIfNotFound so that it can chain. --- lib/classes/Variables.js | 466 +++++++---- lib/classes/Variables.test.js | 1435 ++++++++++++--------------------- 2 files changed, 851 insertions(+), 1050 deletions(-) diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js index 738a0347c..a6842c4b3 100644 --- a/lib/classes/Variables.js +++ b/lib/classes/Variables.js @@ -8,12 +8,59 @@ const BbPromise = require('bluebird'); const os = require('os'); const fse = require('../utils/fs/fse'); -class Variables { +class PromiseTracker { + constructor() { + this.promiseList = []; + this.promiseMap = {}; + this.startTime = Date.now(); + } + start() { + this.interval = setInterval(this.report.bind(this), 2500); + } + report() { + const delta = Date.now() - this.startTime; + logWarning('################################################################################'); + logWarning(`# ${delta}: ${this.getSettled().length} of ${ + this.getAll().length} promises have settled`); + const pending = this.getPending(); + logWarning(`# ${delta}: ${pending.length} unsettled promises:`); + pending.forEach((promise) => { + logWarning(`# ${delta}: ${promise.waitList}`); + }); + logWarning('################################################################################'); + } + stop() { + clearInterval(this.interval); + } + add(variable, prms, specifier) { + const promise = prms; + promise.waitList = `${variable} waited on by: ${specifier}`; + promise.state = 'pending'; + promise.then( + (value) => { promise.state = 'resolved'; return Promise.resolve(value); }, + (error) => { promise.state = 'rejected'; return Promise.reject(error); }); + this.promiseList.push(promise); + this.promiseMap[variable] = promise; + return promise; + } + contains(variable) { + return variable in this.promiseMap; + } + get(variable, specifier) { + const promise = this.promiseMap[variable]; + promise.waitList += ` ${specifier}`; + return promise; + } + getPending() { return this.promiseList.filter(p => (p.state === 'pending')); } + getSettled() { return this.promiseList.filter(p => (p.state !== 'pending')); } + getAll() { return this.promiseList; } +} +class Variables { constructor(serverless) { this.serverless = serverless; this.service = this.serverless.service; - this.cache = {}; + this.tracker = new PromiseTracker(); this.overwriteSyntax = RegExp(/,/g); this.fileRefSyntax = RegExp(/^file\((~?[a-zA-Z0-9._\-/]+?)\)/g); @@ -29,6 +76,9 @@ class Variables { loadVariableSyntax() { this.variableSyntax = RegExp(this.service.provider.variableSyntax, 'g'); } + // ############# + // ## SERVICE ## + // ############# /** * Populate all variables in the service, conviently remove and restore the service attributes * that confuse the population methods. @@ -42,14 +92,117 @@ class Variables { // store const variableSyntaxProperty = this.service.provider.variableSyntax; // remove - this.service.provider.variableSyntax = true; // matches itself - this.serverless.service.serverless = null; - return this.populateObject(this.service).then(() => { - // restore - this.service.provider.variableSyntax = variableSyntaxProperty; - this.serverless.service.serverless = this.serverless; - return BbPromise.resolve(this.service); - }); + this.service.provider.variableSyntax = undefined; // otherwise matches itself + this.service.serverless = undefined; + this.tracker.start(); + return this.populateObject(this.service) + .finally(() => { + // restore + this.tracker.stop(); + this.service.serverless = this.serverless; + this.service.provider.variableSyntax = variableSyntaxProperty; + }) + .then(() => this.service); + } + // ############ + // ## OBJECT ## + // ############ + /** + * The declaration of a terminal property. This declaration includes the path and value of the + * property. + * Example Input: + * { + * foo: { + * bar: 'baz' + * } + * } + * Example Result: + * [ + * { + * path: ['foo', 'bar'] + * value: 'baz + * } + * ] + * @typedef {Object} TerminalProperty + * @property {String[]} path The path to the terminal property + * @property {Date|RegEx|String} The value of the terminal property + */ + /** + * Generate an array of objects noting the terminal properties of the given root object and their + * paths + * @param root The object to generate a terminal property path/value set for + * @param current The current part of the given root that terminal properties are being sought + * within + * @param [context] An array containing the path to the current object root (intended for internal + * use) + * @param [results] An array of current results (intended for internal use) + * @returns {TerminalProperty[]} The terminal properties of the given root object, with the path + * and value of each + */ + getProperties(root, atRoot, current, cntxt, rslts) { + let context = cntxt; + if (!context) { + context = []; + } + let results = rslts; + if (!results) { + results = []; + } + const addContext = (value, key) => + this.getProperties(root, false, value, context.concat(key), results); + if ( + _.isArray(current) + ) { + _.map(current, addContext); + } else if ( + _.isObject(current) && + !_.isDate(current) && + !_.isRegExp(current) && + !_.isFunction(current) + ) { + if (atRoot || current !== root) { + _.mapValues(current, addContext); + } + } else { + results.push({ path: context, value: current }); + } + return results; + } + + /** + * @typedef {TerminalProperty} TerminalPropertyPopulated + * @property {Object} populated The populated value of the value at the path + */ + /** + * Populate the given terminal properties, returning promises to do so + * @param properties The terminal properties to populate + * @returns {Promise[]} The promises that will resolve to the + * populated values of the given terminal properties + */ + populateProperties(properties) { + return _.map(properties, property => + this.populateValue(property.value, false) + .then(populated => _.assign({}, property, { populated }))); + } + /** + * Assign the populated values back to the target object + * @param target The object to which the given populated terminal properties should be applied + * @param populations The fully populated terminal properties + * @returns {Promise} resolving with the number of changes that were applied to the given + * target + */ + assignProperties(target, populations) { // eslint-disable-line class-methods-use-this + return BbPromise.all(populations) + .then((results) => { + let changes = 0; + results.forEach((result) => { + if (result.value !== result.populated) { + changes += 1; + _.set(target, result.path, result.populated); + } + }); + return BbPromise.resolve(changes); + }); } /** * Populate the variables in the given object. @@ -57,78 +210,104 @@ class Variables { * @returns {Promise.|*} A promise resolving to the in-place populated object. */ populateObject(objectToPopulate) { - // Map terminal values of given root (i.e. for every leaf value...) - const forEachLeaf = (root, context, callback) => { - const addContext = (value, key) => forEachLeaf(value, context.concat(key), callback); - if ( - _.isArray(root) - ) { - return _.map(root, addContext); - } else if ( - _.isObject(root) && - !_.isDate(root) && - !_.isRegExp(root) && - !_.isFunction(root) - ) { - return _.extend({}, root, _.mapValues(root, addContext)); - } - return callback(root, context); - }; - // For every leaf value... - const pendingLeaves = []; - forEachLeaf( - objectToPopulate, - [], - (leafValue, leafPath) => { - if (typeof leafValue === 'string') { - pendingLeaves.push(this - .populateProperty(leafValue, true) - .then(leafValuePopulated => _.set(objectToPopulate, leafPath, leafValuePopulated)) - ); + const leaves = this.getProperties(objectToPopulate, true, objectToPopulate); + const populations = this.populateProperties(leaves); + return this.assignProperties(objectToPopulate, populations) + .then((changes) => { + if (changes) { + return this.populateObject(objectToPopulate); } - } - ); - return BbPromise.all(pendingLeaves).then(() => objectToPopulate); + return objectToPopulate; + }); + } + // ############## + // ## PROPERTY ## + // ############## + /** + * @typedef {Object} MatchResult + * @property {String} match The original property value that matched the variable syntax + * @property {String} variable The cleaned variable string that specifies the origin for the + * property value + */ + /** + * Get matches against the configured variable syntax + * @param property The property value to attempt extracting matches from + * @returns {Object|String|MatchResult[]} The given property or the identified matches + */ + getMatches(property) { + if (typeof property !== 'string') { + return property; + } + const matches = property.match(this.variableSyntax); + if (!matches || !matches.length) { + return property; + } + return _.map(matches, match => ({ + match, + variable: match.replace(this.variableSyntax, (context, contents) => contents.trim()) + .replace(/\s/g, ''), + })); } /** - * Populate variables, in-place if specified, in the given property value. - * @param propertyToPopulate The property to populate (only strings with variables are altered). - * @param populateInPlace Whether to deeply clone the given property prior to population. - * @returns {Promise.|*} A promise resolving to the populated result. + * Populate the given matches, returning an array of Promises which will resolve to the populated + * values of the given matches + * @param {MatchResult[]} matches The matches to populate + * @returns {Promise[]} Promises for the eventual populated values of the given matches */ - populateProperty(propertyToPopulate, populateInPlace) { - let property = propertyToPopulate; - if (!populateInPlace) { - property = _.cloneDeep(propertyToPopulate); + populateMatches(matches) { + return _.map(matches, (match) => { + if (match.variable.match(this.overwriteSyntax)) { + return this.overwrite(match.variable); + } + return this.getValueFromSource(match.variable, match.match); + }); + } + /** + * Render the given matches and their associated results to the given value + * @param value The value into which to render the given results + * @param matches The matches on the given value where the results are to be rendered + * @param results The results that are to be rendered to the given value + * @returns {*} The populated value with the given results rendered according to the given matches + */ + renderMatches(value, matches, results) { + let result = value; + for (let i = 0; i < matches.length; i += 1) { + this.warnIfNotFound(matches[i].match, results[i]); + result = this.populateVariable(result, matches[i].match, results[i]); } - if ( - typeof property !== 'string' || - !property.match(this.variableSyntax) - ) { + return result; + } + /** + * Populate the given value, recursively if root is true + * @param valueToPopulate The value to populate variables within + * @param root Whether the caller is the root populator and thereby whether to recursively + * populate + * @returns {PromiseLike} A promise that resolves to the populated value, recursively if root + * is true + */ + populateValue(valueToPopulate, root) { + const property = _.cloneDeep(valueToPopulate); + const matches = this.getMatches(property); + if (!_.isArray(matches)) { return BbPromise.resolve(property); } - const pendingMatches = []; - property.match(this.variableSyntax).forEach((matchedString) => { - const variableString = matchedString - .replace(this.variableSyntax, (match, varName) => varName.trim()) - .replace(/\s/g, ''); - - let pendingMatch; - if (variableString.match(this.overwriteSyntax)) { - pendingMatch = this.overwrite(variableString); - } else { - pendingMatch = this.getValueFromSource(variableString); - } - pendingMatches.push(pendingMatch.then(matchedValue => { - this.warnIfNotFound(variableString, matchedValue); - return this.populateVariable(property, matchedString, matchedValue) - .then((populatedProperty) => { - property = populatedProperty; - }); - })); - }); - return BbPromise.all(pendingMatches) - .then(() => this.populateProperty(property, true)); + const populations = this.populateMatches(matches); + return BbPromise.all(populations) + .then(results => this.renderMatches(property, matches, results)) + .then((result) => { + if (root && matches.length) { + return this.populateValue(result); + } + return result; + }); + } + /** + * Populate variables in the given property. + * @param propertyToPopulate The property to populate (replace variables with their values). + * @returns {Promise.|*} A promise resolving to the populated result. + */ + populateProperty(propertyToPopulate) { + return this.populateValue(propertyToPopulate, true); } /** * Populate a given property, given the matched string to replace and the value to replace the @@ -155,8 +334,11 @@ class Variables { ].join(''); throw new this.serverless.classes.Error(errorMessage); } - return BbPromise.resolve(property); + return property; } + // ############### + // ## VARIABLES ## + // ############### /** * Overwrite the given variable string, resolve each variable and resolve to the first valid * value. @@ -167,8 +349,7 @@ class Variables { overwrite(variableStringsString) { const variableStrings = variableStringsString.split(','); const variableValues = variableStrings.map(variableString => - this.getValueFromSource(variableString) - ); + this.getValueFromSource(variableString, variableStringsString)); const validValue = value => ( value !== null && typeof value !== 'undefined' && @@ -176,53 +357,48 @@ class Variables { ); return BbPromise.all(variableValues) .then(values => // find and resolve first valid value, undefined if none - BbPromise.resolve(values.find(validValue)) - ); + BbPromise.resolve(values.find(validValue))); } /** * Given any variable string, return the value it should be populated with. * @param variableString The variable string to retrieve a value for. * @returns {Promise.|*} A promise resolving to the given variables value. */ - getValueFromSource(variableString) { - if (!(variableString in this.cache)) { - let value; + getValueFromSource(variableString, propertyString) { + let ret; + if (this.tracker.contains(variableString)) { + ret = this.tracker.get(variableString, propertyString); + } else { if (variableString.match(this.envRefSyntax)) { - value = this.getValueFromEnv(variableString); + ret = this.getValueFromEnv(variableString); } else if (variableString.match(this.optRefSyntax)) { - value = this.getValueFromOptions(variableString); + ret = this.getValueFromOptions(variableString); } else if (variableString.match(this.selfRefSyntax)) { - value = this.getValueFromSelf(variableString); + ret = this.getValueFromSelf(variableString); } else if (variableString.match(this.fileRefSyntax)) { - value = this.getValueFromFile(variableString); + ret = this.getValueFromFile(variableString); } else if (variableString.match(this.cfRefSyntax)) { - value = this.getValueFromCf(variableString); + ret = this.getValueFromCf(variableString); } else if (variableString.match(this.s3RefSyntax)) { - value = this.getValueFromS3(variableString); + ret = this.getValueFromS3(variableString); } else if (variableString.match(this.stringRefSyntax)) { - value = this.getValueFromString(variableString); + ret = this.getValueFromString(variableString); } else if (variableString.match(this.ssmRefSyntax)) { - value = this.getValueFromSsm(variableString); + ret = this.getValueFromSsm(variableString); } else { const errorMessage = [ `Invalid variable reference syntax for variable ${variableString}.`, ' You can only reference env vars, options, & files.', ' You can check our docs for more info.', ].join(''); - throw new this.serverless.classes.Error(errorMessage); + ret = BbPromise.reject(new this.serverless.classes.Error(errorMessage)); } - this.cache[variableString] = BbPromise.resolve(value) - .then(variableValue => { - if (_.isObject(variableValue) && variableValue !== this.service) { - return this.populateObject(variableValue); - } - return variableValue; - }); + this.tracker.add(variableString, ret, propertyString); } - return this.cache[variableString]; + return ret; } - getValueFromEnv(variableString) { + getValueFromEnv(variableString) { // eslint-disable-line class-methods-use-this const requestedEnvVar = variableString.split(':')[1]; let valueToPopulate; if (requestedEnvVar !== '' || '' in process.env) { @@ -233,7 +409,7 @@ class Variables { return BbPromise.resolve(valueToPopulate); } - getValueFromString(variableString) { + getValueFromString(variableString) { // eslint-disable-line class-methods-use-this const valueToPopulate = variableString.replace(/^['"]|['"]$/g, ''); return BbPromise.resolve(valueToPopulate); } @@ -262,13 +438,13 @@ class Variables { .replace('~', os.homedir()); let referencedFileFullPath = (path.isAbsolute(referencedFileRelativePath) ? - referencedFileRelativePath : - path.join(this.serverless.config.servicePath, referencedFileRelativePath)); + referencedFileRelativePath : + path.join(this.serverless.config.servicePath, referencedFileRelativePath)); // Get real path to handle potential symlinks (but don't fatal error) referencedFileFullPath = fse.existsSync(referencedFileFullPath) ? - fse.realpathSync(referencedFileFullPath) : - referencedFileFullPath; + fse.realpathSync(referencedFileFullPath) : + referencedFileFullPath; let fileExtension = referencedFileRelativePath.split('.'); fileExtension = fileExtension[fileExtension.length - 1]; @@ -281,7 +457,8 @@ class Variables { // Process JS files if (fileExtension === 'js') { - const jsFile = require(referencedFileFullPath); // eslint-disable-line global-require + // eslint-disable-next-line global-require, import/no-dynamic-require + const jsFile = require(referencedFileFullPath); const variableArray = variableString.split(':'); let returnValueFunction; if (variableArray[1]) { @@ -293,29 +470,28 @@ class Variables { } if (typeof returnValueFunction !== 'function') { - throw new this.serverless.classes - .Error([ - 'Invalid variable syntax when referencing', - ` file "${referencedFileRelativePath}".`, - ' Check if your javascript is exporting a function that returns a value.', - ].join('')); + const errorMessage = [ + 'Invalid variable syntax when referencing', + ` file "${referencedFileRelativePath}".`, + ' Check if your javascript is exporting a function that returns a value.', + ].join(''); + return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); } valueToPopulate = returnValueFunction.call(jsFile); - return BbPromise.resolve(valueToPopulate).then(valueToPopulateResolved => { + return BbPromise.resolve(valueToPopulate).then((valueToPopulateResolved) => { let deepProperties = variableString.replace(matchedFileRefString, ''); deepProperties = deepProperties.slice(1).split('.'); deepProperties.splice(0, 1); return this.getDeepValue(deepProperties, valueToPopulateResolved) - .then(deepValueToPopulateResolved => { + .then((deepValueToPopulateResolved) => { if (typeof deepValueToPopulateResolved === 'undefined') { const errorMessage = [ 'Invalid variable syntax when referencing', ` file "${referencedFileRelativePath}".`, ' Check if your javascript is returning the correct data.', ].join(''); - throw new this.serverless.classes - .Error(errorMessage); + return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); } return BbPromise.resolve(deepValueToPopulateResolved); }); @@ -334,8 +510,7 @@ class Variables { ` file "${referencedFileRelativePath}" sub properties`, ' Please use ":" to reference sub properties.', ].join(''); - throw new this.serverless.classes - .Error(errorMessage); + return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); } deepProperties = deepProperties.slice(1).split('.'); return this.getDeepValue(deepProperties, valueToPopulate); @@ -352,9 +527,8 @@ class Variables { .request('CloudFormation', 'describeStacks', { StackName: stackName }, - { useCache: true } // Use request cache - ) - .then(result => { + { useCache: true })// Use request cache + .then((result) => { const outputs = result.Stacks[0].Outputs; const output = outputs.find(x => x.OutputKey === outputLogicalId); @@ -364,11 +538,9 @@ class Variables { ` Stack name: "${stackName}"`, ` Requested variable: "${outputLogicalId}".`, ].join(''); - throw new this.serverless.classes - .Error(errorMessage); + return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); } - - return output.OutputValue; + return BbPromise.resolve(output.OutputValue); }); } @@ -376,47 +548,41 @@ class Variables { const groups = variableString.match(this.s3RefSyntax); const bucket = groups[1]; const key = groups[2]; - return this.serverless.getProvider('aws') - .request('S3', + return this.serverless.getProvider('aws').request( + 'S3', 'getObject', { Bucket: bucket, Key: key, }, - { useCache: true } // Use request cache - ) - .then( - response => response.Body.toString(), - err => { + { useCache: true }) // Use request cache + .then(response => BbPromise.resolve(response.Body.toString())) + .catch((err) => { const errorMessage = `Error getting value for ${variableString}. ${err.message}`; - throw new this.serverless.classes.Error(errorMessage); - } - ); + return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); + }); } getValueFromSsm(variableString) { const groups = variableString.match(this.ssmRefSyntax); const param = groups[1]; const decrypt = (groups[2] === 'true'); - return this.serverless.getProvider('aws') - .request('SSM', + return this.serverless.getProvider('aws').request( + 'SSM', 'getParameter', { Name: param, WithDecryption: decrypt, }, - { useCache: true } // Use request cache - ) - .then( - response => BbPromise.resolve(response.Parameter.Value), - err => { + { useCache: true }) // Use request cache + .then(response => BbPromise.resolve(response.Parameter.Value)) + .catch((err) => { const expectedErrorMessage = `Parameter ${param} not found.`; if (err.message !== expectedErrorMessage) { - throw new this.serverless.classes.Error(err.message); + return BbPromise.reject(new this.serverless.classes.Error(err.message)); } return BbPromise.resolve(undefined); - } - ); + }); } getDeepValue(deepProperties, valueToPopulate) { @@ -429,7 +595,7 @@ class Variables { } if (typeof computedValueToPopulate === 'string' && computedValueToPopulate.match(this.variableSyntax)) { - return this.populateProperty(computedValueToPopulate, true); + return this.populateValue(computedValueToPopulate, false); } return BbPromise.resolve(computedValueToPopulate); }, valueToPopulate); @@ -453,10 +619,10 @@ class Variables { } else if (variableString.match(this.ssmRefSyntax)) { varType = 'SSM parameter'; } - logWarning( - `A valid ${varType} to satisfy the declaration '${variableString}' could not be found.` - ); + logWarning(`A valid ${varType} to satisfy the declaration '${ + variableString}' could not be found.`); } + return valueToPopulate; } } diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index e59b552aa..04c2f814b 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -2,36 +2,43 @@ /* eslint-disable no-unused-expressions */ +const BbPromise = require('bluebird'); +const chai = require('chai'); const jc = require('json-cycle'); +const os = require('os'); const path = require('path'); const proxyquire = require('proxyquire'); +const sinon = require('sinon'); const YAML = require('js-yaml'); -const chai = require('chai'); -const Variables = require('../../lib/classes/Variables'); -const Utils = require('../../lib/classes/Utils'); + +const AwsProvider = require('../plugins/aws/provider/awsProvider'); const fse = require('../utils/fs/fse'); const Serverless = require('../../lib/Serverless'); -const sinon = require('sinon'); -const testUtils = require('../../tests/utils'); const slsError = require('./Error'); -const AwsProvider = require('../plugins/aws/provider/awsProvider'); -const BbPromise = require('bluebird'); -const os = require('os'); +const testUtils = require('../../tests/utils'); +const Utils = require('../../lib/classes/Utils'); +const Variables = require('../../lib/classes/Variables'); + +BbPromise.longStackTraces(true); + +chai.should(); chai.use(require('chai-as-promised')); chai.use(require('sinon-chai')); const expect = chai.expect; -describe('Variables', () => { - describe('#constructor()', () => { - const serverless = new Serverless(); +describe.only('Variables', () => { + let serverless; + beforeEach(() => { + serverless = new Serverless(); + }); + describe('#constructor()', () => { it('should attach serverless instance', () => { const variablesInstance = new Variables(serverless); - expect(typeof variablesInstance.serverless.version).to.be.equal('string'); + expect(variablesInstance.serverless).to.equal(serverless); }); - it('should not set variableSyntax in constructor', () => { const variablesInstance = new Variables(serverless); expect(variablesInstance.variableSyntax).to.be.undefined; @@ -40,110 +47,115 @@ describe('Variables', () => { describe('#loadVariableSyntax()', () => { it('should set variableSyntax', () => { - const serverless = new Serverless(); - + // eslint-disable-next-line no-template-curly-in-string serverless.service.provider.variableSyntax = '\\${{([ ~:a-zA-Z0-9._\'",\\-\\/\\(\\)]+?)}}'; - serverless.variables.loadVariableSyntax(); expect(serverless.variables.variableSyntax).to.be.a('RegExp'); }); }); describe('#populateService()', () => { - it('should call populateProperty method', () => { - const serverless = new Serverless(); - - const populatePropertyStub = sinon - .stub(serverless.variables, 'populateObject').resolves(); - + it('should call loadVariableSyntax and then populateProperty', () => { + const loadVariableSyntaxStub = sinon.stub(serverless.variables, 'loadVariableSyntax') + .returns(); + const populateObjectStub = sinon.stub(serverless.variables, 'populateObject').resolves(); return expect(serverless.variables.populateService()).to.be.fulfilled - .then(() => { - expect(populatePropertyStub.called).to.be.true; - }) - .finally(() => serverless.variables.populateObject.restore()); + .then(() => { + expect(loadVariableSyntaxStub).to.be.calledOnce; + expect(populateObjectStub).to.be.calledOnce; + expect(loadVariableSyntaxStub).to.be.calledBefore(populateObjectStub); + }) + .finally(() => { + populateObjectStub.restore(); + loadVariableSyntaxStub.restore(); + }); }); - - it('should use variableSyntax', () => { - const serverless = new Serverless(); - - const variableSyntax = '\\${{([ ~:a-zA-Z0-9._\'",\\-\\/\\(\\)]+?)}}'; - const fooValue = '${clientId()}'; - const barValue = 'test'; - - serverless.service.provider.variableSyntax = variableSyntax; - - serverless.service.custom = { - var: barValue, - }; - - serverless.service.resources = { - foo: fooValue, - bar: '${{self:custom.var}}', - }; - - return serverless.variables.populateService().then(() => { - expect(serverless.service.provider.variableSyntax).to.equal(variableSyntax); - expect(serverless.service.resources.foo).to.equal(fooValue); - expect(serverless.service.resources.bar).to.equal(barValue); + it('should remove problematic attributes bofore calling populateObject with the service', + () => { + const populateObjectStub = sinon.stub(serverless.variables, 'populateObject', (val) => { + expect(val).to.equal(serverless.service); + expect(val.provider.variableSyntax).to.be.undefined; + expect(val.serverless).to.be.undefined; + return BbPromise.resolve(); + }); + return expect(serverless.variables.populateService()).to.be.fulfilled + .then().finally(() => populateObjectStub.restore()); }); + }); + + describe('#getProperties', () => { + it('extracts all terminal properties of an object', () => { + const date = new Date(); + const regex = /^.*$/g; + const func = () => {}; + const obj = { + foo: { + bar: 'baz', + biz: 'buz', + }, + b: [ + { c: 'd' }, + { e: 'f' }, + ], + g: date, + h: regex, + i: func, + }; + const expected = [ + { path: ['foo', 'bar'], value: 'baz' }, + { path: ['foo', 'biz'], value: 'buz' }, + { path: ['b', 0, 'c'], value: 'd' }, + { path: ['b', 1, 'e'], value: 'f' }, + { path: ['g'], value: date }, + { path: ['h'], value: regex }, + { path: ['i'], value: func }, + ]; + const result = serverless.variables.getProperties(obj, true, obj); + expect(result).to.eql(expected); + }); + it('ignores self references', () => { + const obj = {}; + obj.self = obj; + const expected = []; + const result = serverless.variables.getProperties(obj, true, obj); + expect(result).to.eql(expected); }); }); describe('#populateObject()', () => { - it('should call populateProperty method', () => { - const serverless = new Serverless(); - const object = { - stage: '${opt:stage}', - }; - - const populatePropertyStub = sinon - .stub(serverless.variables, 'populateProperty').resolves('prod'); - - return serverless.variables.populateObject(object).then(() => { - expect(populatePropertyStub.called).to.be.true; - }) - .finally(() => serverless.variables.populateProperty.restore()); - }); - it('should populate object and return it', () => { - const serverless = new Serverless(); const object = { - stage: '${opt:stage}', + stage: '${opt:stage}', // eslint-disable-line no-template-curly-in-string }; const expectedPopulatedObject = { stage: 'prod', }; - sinon.stub(serverless.variables, 'populateProperty').resolves('prod'); + sinon.stub(serverless.variables, 'populateValue').resolves('prod'); - return serverless.variables.populateObject(object).then(populatedObject => { + return serverless.variables.populateObject(object).then((populatedObject) => { expect(populatedObject).to.deep.equal(expectedPopulatedObject); }) - .finally(() => serverless.variables.populateProperty.restore()); + .finally(() => serverless.variables.populateValue.restore()); }); it('should persist keys with dot notation', () => { - const serverless = new Serverless(); const object = { - stage: '${opt:stage}', + stage: '${opt:stage}', // eslint-disable-line no-template-curly-in-string }; object['some.nested.key'] = 'hello'; const expectedPopulatedObject = { stage: 'prod', }; expectedPopulatedObject['some.nested.key'] = 'hello'; - - const populatePropertyStub = sinon.stub(serverless.variables, 'populateProperty'); - populatePropertyStub.onCall(0).resolves('prod'); - populatePropertyStub.onCall(1).resolves('hello'); - - return serverless.variables.populateObject(object).then(populatedObject => { - expect(populatedObject).to.deep.equal(expectedPopulatedObject); - }) - .finally(() => serverless.variables.populateProperty.restore()); + const populateValueStub = sinon.stub(serverless.variables, 'populateValue', + // eslint-disable-next-line no-template-curly-in-string + val => (val === '${opt:stage}' ? BbPromise.resolve('prod') : BbPromise.resolve(val))); + return serverless.variables.populateObject(object) + .should.become(expectedPopulatedObject) + .then().finally(() => populateValueStub.restore()); }); describe('significant variable usage corner cases', () => { - let serverless; let service; const makeDefault = () => ({ service: 'my-service', @@ -152,8 +164,8 @@ describe('Variables', () => { }, }); beforeEach(() => { - serverless = new Serverless(); service = makeDefault(); + // eslint-disable-next-line no-template-curly-in-string service.provider.variableSyntax = '\\${([ ~:a-zA-Z0-9._\'",\\-\\/\\(\\)]+?)}'; // default serverless.variables.service = service; serverless.variables.loadVariableSyntax(); @@ -161,7 +173,7 @@ describe('Variables', () => { }); it('should properly replace self-references', () => { service.custom = { - me: '${self:}', + me: '${self:}', // eslint-disable-line no-template-curly-in-string }; const expected = makeDefault(); expected.custom = { @@ -174,7 +186,7 @@ describe('Variables', () => { it('should properly populate embedded variables', () => { service.custom = { val0: 'my value 0', - val1: '0', + val1: '0', // eslint-disable-next-line no-template-curly-in-string val2: '${self:custom.val${self:custom.val1}}', }; const expected = { @@ -188,7 +200,7 @@ describe('Variables', () => { }); it('should properly populate an overwrite with a default value that is a string', () => { service.custom = { - val0: 'my value', + val0: 'my value', // eslint-disable-next-line no-template-curly-in-string val1: '${self:custom.NOT_A_VAL1, self:custom.NOT_A_VAL2, "string"}', }; const expected = { @@ -201,7 +213,7 @@ describe('Variables', () => { }); it('should properly populate overwrites where the first value is valid', () => { service.custom = { - val0: 'my value', + val0: 'my value', // eslint-disable-next-line no-template-curly-in-string val1: '${self:custom.val0, self:custom.NOT_A_VAL1, self:custom.NOT_A_VAL2}', }; const expected = { @@ -214,7 +226,7 @@ describe('Variables', () => { }); it('should properly populate overwrites where the middle value is valid', () => { service.custom = { - val0: 'my value', + val0: 'my value', // eslint-disable-next-line no-template-curly-in-string val1: '${self:custom.NOT_A_VAL1, self:custom.val0, self:custom.NOT_A_VAL2}', }; const expected = { @@ -227,7 +239,7 @@ describe('Variables', () => { }); it('should properly populate overwrites where the last value is valid', () => { service.custom = { - val0: 'my value', + val0: 'my value', // eslint-disable-next-line no-template-curly-in-string val1: '${self:custom.NOT_A_VAL1, self:custom.NOT_A_VAL2, self:custom.val0}', }; const expected = { @@ -241,7 +253,7 @@ describe('Variables', () => { it('should properly populate overwrites with nested variables in the first value', () => { service.custom = { val0: 'my value', - val1: 0, + val1: 0, // eslint-disable-next-line no-template-curly-in-string val2: '${self:custom.val${self:custom.val1}, self:custom.NO_1, self:custom.NO_2}', }; const expected = { @@ -256,7 +268,7 @@ describe('Variables', () => { it('should properly populate overwrites with nested variables in the middle value', () => { service.custom = { val0: 'my value', - val1: 0, + val1: 0, // eslint-disable-next-line no-template-curly-in-string val2: '${self:custom.NO_1, self:custom.val${self:custom.val1}, self:custom.NO_2}', }; const expected = { @@ -271,7 +283,7 @@ describe('Variables', () => { it('should properly populate overwrites with nested variables in the last value', () => { service.custom = { val0: 'my value', - val1: 0, + val1: 0, // eslint-disable-next-line no-template-curly-in-string val2: '${self:custom.NO_1, self:custom.NO_2, self:custom.val${self:custom.val1}}', }; const expected = { @@ -286,8 +298,8 @@ describe('Variables', () => { it('should properly replace duplicate variable declarations', () => { service.custom = { val0: 'my value', - val1: '${self:custom.val0}', - val2: '${self:custom.val0}', + val1: '${self:custom.val0}', // eslint-disable-line no-template-curly-in-string + val2: '${self:custom.val0}', // eslint-disable-line no-template-curly-in-string }; const expected = { val0: 'my value', @@ -300,10 +312,10 @@ describe('Variables', () => { }); it('should recursively populate, regardless of order and duplication', () => { service.custom = { - val1: '${self:custom.depVal}', - depVal: '${self:custom.val0}', + val1: '${self:custom.depVal}', // eslint-disable-line no-template-curly-in-string + depVal: '${self:custom.val0}', // eslint-disable-line no-template-curly-in-string val0: 'my value', - val2: '${self:custom.depVal}', + val2: '${self:custom.depVal}', // eslint-disable-line no-template-curly-in-string }; const expected = { val1: 'my value', @@ -315,11 +327,21 @@ describe('Variables', () => { expect(result).to.eql(expected); })).to.be.fulfilled; }); - const pathAsyncLoadJs = 'async.load.js'; - const makeAsyncLoadJs = () => { - const SUtils = new Utils(); - const tmpDirPath = testUtils.getTmpDirPath(); - const fileContent = `'use strict'; + describe('file reading cases', () => { + let tmpDirPath; + beforeEach(() => { + tmpDirPath = testUtils.getTmpDirPath(); + fse.mkdirsSync(tmpDirPath); + serverless.config.update({ servicePath: tmpDirPath }); + }); + afterEach(() => { + fse.removeSync(tmpDirPath); + }); + const makeTempFile = (fileName, fileContent) => { + fse.writeFileSync(path.join(tmpDirPath, fileName), fileContent); + }; + const asyncFileName = 'async.load.js'; + const asyncContent = `'use strict'; let i = 0 const str = () => new Promise((resolve) => { setTimeout(() => { @@ -341,506 +363,380 @@ module.exports = { obj, }; `; - SUtils.writeFileSync(path.join(tmpDirPath, pathAsyncLoadJs), fileContent); - serverless.config.update({ servicePath: tmpDirPath }); - }; - it('should populate any given variable only once', () => { - makeAsyncLoadJs(); - service.custom = { - val1: '${self:custom.val0}', - val2: '${self:custom.val1}', - val0: `\${file(${pathAsyncLoadJs}):str}`, - }; - const expected = { - val1: 'my-async-value-1', - val2: 'my-async-value-1', - val0: 'my-async-value-1', - }; - return expect(serverless.variables.populateObject(service.custom).then((result) => { - expect(result).to.eql(expected); - })).to.be.fulfilled; - }); - it('should populate any given variable only once regardless of ordering or reference count', - () => { - makeAsyncLoadJs(); + it('should populate any given variable only once', () => { + makeTempFile(asyncFileName, asyncContent); service.custom = { - val9: '${self:custom.val7}', - val7: '${self:custom.val5}', - val5: '${self:custom.val3}', - val3: '${self:custom.val1}', - val1: '${self:custom.val0}', - val2: '${self:custom.val1}', - val4: '${self:custom.val3}', - val6: '${self:custom.val5}', - val8: '${self:custom.val7}', - val0: `\${file(${pathAsyncLoadJs}):str}`, + val1: '${self:custom.val0}', // eslint-disable-line no-template-curly-in-string + val2: '${self:custom.val1}', // eslint-disable-line no-template-curly-in-string + val0: `\${file(${asyncFileName}):str}`, }; const expected = { - val9: 'my-async-value-1', - val7: 'my-async-value-1', - val5: 'my-async-value-1', - val3: 'my-async-value-1', val1: 'my-async-value-1', val2: 'my-async-value-1', - val4: 'my-async-value-1', - val6: 'my-async-value-1', - val8: 'my-async-value-1', val0: 'my-async-value-1', }; - return expect(serverless.variables.populateObject(service.custom).then((result) => { - expect(result).to.eql(expected); - })).to.be.fulfilled; - } - ); - it('should populate async objects with contained variables', - () => { - makeAsyncLoadJs(); - serverless.variables.options = { - stage: 'dev', - }; - service.custom = { - obj: `\${file(${pathAsyncLoadJs}):obj}`, - }; - const expected = { - obj: { + return serverless.variables.populateObject(service.custom) + .should.become(expected); + }); + it('should populate any given variable only once regardless of ordering or reference count', + () => { + makeTempFile(asyncFileName, asyncContent); + service.custom = { + val9: '${self:custom.val7}', // eslint-disable-line no-template-curly-in-string + val7: '${self:custom.val5}', // eslint-disable-line no-template-curly-in-string + val5: '${self:custom.val3}', // eslint-disable-line no-template-curly-in-string + val3: '${self:custom.val1}', // eslint-disable-line no-template-curly-in-string + val1: '${self:custom.val0}', // eslint-disable-line no-template-curly-in-string + val2: '${self:custom.val1}', // eslint-disable-line no-template-curly-in-string + val4: '${self:custom.val3}', // eslint-disable-line no-template-curly-in-string + val6: '${self:custom.val5}', // eslint-disable-line no-template-curly-in-string + val8: '${self:custom.val7}', // eslint-disable-line no-template-curly-in-string + val0: `\${file(${asyncFileName}):str}`, + }; + const expected = { + val9: 'my-async-value-1', + val7: 'my-async-value-1', + val5: 'my-async-value-1', + val3: 'my-async-value-1', + val1: 'my-async-value-1', + val2: 'my-async-value-1', + val4: 'my-async-value-1', + val6: 'my-async-value-1', + val8: 'my-async-value-1', val0: 'my-async-value-1', - val1: 'dev', - }, - }; - return expect(serverless.variables.populateObject(service.custom).then((result) => { - expect(result).to.eql(expected); - })).to.be.fulfilled; - } - ); - const pathEmptyJs = 'empty.js'; - const makeEmptyJs = () => { - const SUtils = new Utils(); - const tmpDirPath = testUtils.getTmpDirPath(); - const fileContent = `'use strict'; + }; + return serverless.variables.populateObject(service.custom) + .should.become(expected); + }); + it('should populate async objects with contained variables', + () => { + makeTempFile(asyncFileName, asyncContent); + serverless.variables.options = { + stage: 'dev', + }; + service.custom = { + obj: `\${file(${asyncFileName}):obj}`, + }; + const expected = { + obj: { + val0: 'my-async-value-1', + val1: 'dev', + }, + }; + return serverless.variables.populateObject(service.custom) + .should.become(expected); + }); + const selfFileName = 'self.yml'; + const selfContent = `foo: baz +bar: \${self:custom.self.foo} +`; + it('should populate a "cyclic" reference across an unresolved dependency (issue #4687)', + () => { + makeTempFile(selfFileName, selfContent); + service.custom = { + self: `\${file(${selfFileName})}`, + }; + const expected = { + self: { + foo: 'baz', + bar: 'baz', + }, + }; + return serverless.variables.populateObject(service.custom) + .should.become(expected); + }); + const emptyFileName = 'empty.js'; + const emptyContent = `'use strict'; module.exports = { func: () => ({ value: 'a value' }), } `; - SUtils.writeFileSync(path.join(tmpDirPath, pathEmptyJs), fileContent); - serverless.config.update({ servicePath: tmpDirPath }); - }; - it('should reject population of an attribute not exported from a file', - () => { - makeEmptyJs(); - service.custom = { - val: `\${file(${pathEmptyJs}):func.notAValue}`, - }; - return expect(serverless.variables.populateObject(service.custom)) - .to.eventually.be.rejected; - } - ); + it('should reject population of an attribute not exported from a file', + () => { + makeTempFile(emptyFileName, emptyContent); + service.custom = { + val: `\${file(${emptyFileName}):func.notAValue}`, + }; + return serverless.variables.populateObject(service.custom) + .should.be.rejectedWith(serverless.classes.Error); + }); + }); }); }); describe('#populateProperty()', () => { - let serverless; - let overwriteStub; - let populateObjectStub; - let getValueFromSourceStub; - let populateVariableStub; - beforeEach(() => { - serverless = new Serverless(); - overwriteStub = sinon.stub(serverless.variables, 'overwrite'); - populateObjectStub = sinon.stub(serverless.variables, 'populateObject'); - getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); - populateVariableStub = sinon.stub(serverless.variables, 'populateVariable'); - }); - - afterEach(() => { - serverless.variables.overwrite.restore(); - serverless.variables.populateObject.restore(); - serverless.variables.getValueFromSource.restore(); - serverless.variables.populateVariable.restore(); + serverless.variables.loadVariableSyntax(); }); it('should call overwrite if overwrite syntax provided', () => { + // eslint-disable-next-line no-template-curly-in-string const property = 'my stage is ${opt:stage, self:provider.stage}'; - - serverless.variables.loadVariableSyntax(); - - overwriteStub.resolves('dev'); - populateVariableStub.resolves('my stage is dev'); - - return serverless.variables.populateProperty(property).then(newProperty => { - expect(overwriteStub.called).to.equal(true); - expect(populateVariableStub.called).to.equal(true); - expect(newProperty).to.equal('my stage is dev'); - - return BbPromise.resolve(); - }); + serverless.variables.options = { stage: 'dev' }; + serverless.service.provider.stage = 'prod'; + return serverless.variables.populateProperty(property) + .should.eventually.eql('my stage is dev'); }); it('should allow a single-quoted string if overwrite syntax provided', () => { + // eslint-disable-next-line no-template-curly-in-string const property = "my stage is ${opt:stage, 'prod'}"; - - serverless.variables.loadVariableSyntax(); - - overwriteStub.resolves('\'prod\''); - populateVariableStub.resolves('my stage is prod'); - - return expect(serverless.variables.populateProperty(property)).to.be.fulfilled - .then(newProperty => expect(newProperty).to.equal('my stage is prod')); + serverless.variables.options = {}; + return serverless.variables.populateProperty(property) + .should.eventually.eql('my stage is prod'); }); it('should allow a double-quoted string if overwrite syntax provided', () => { + // eslint-disable-next-line no-template-curly-in-string const property = 'my stage is ${opt:stage, "prod"}'; - - serverless.variables.loadVariableSyntax(); - - overwriteStub.resolves('\'prod\''); - populateVariableStub.resolves('my stage is prod'); - - return expect(serverless.variables.populateProperty(property)).to.be.fulfilled - .then(newProperty => expect(newProperty).to.equal('my stage is prod')); + serverless.variables.options = {}; + return serverless.variables.populateProperty(property) + .should.eventually.eql('my stage is prod'); }); it('should call getValueFromSource if no overwrite syntax provided', () => { + // eslint-disable-next-line no-template-curly-in-string const property = 'my stage is ${opt:stage}'; - - serverless.variables.loadVariableSyntax(); - - getValueFromSourceStub.resolves('prod'); - populateVariableStub.resolves('my stage is prod'); - - return serverless.variables.populateProperty(property).then(newProperty => { - expect(getValueFromSourceStub.called).to.be.true; - expect(populateVariableStub.called).to.be.true; - expect(newProperty).to.equal('my stage is prod'); - - return BbPromise.resolve(); - }); - }); - - it('should NOT call populateObject if variable value is a circular object', () => { - serverless.variables.options = { - stage: 'prod', - }; - const property = '${opt:stage}'; - const variableValue = { - stage: '${opt:stage}', - }; - const variableValuePopulated = { - stage: 'prod', - }; - - serverless.variables.cache['opt:stage'] = variableValuePopulated; - - serverless.variables.loadVariableSyntax(); - - populateObjectStub.resolves(variableValuePopulated); - getValueFromSourceStub.resolves(variableValue); - populateVariableStub.resolves(variableValuePopulated); - - return serverless.variables.populateProperty(property).then(newProperty => { - expect(populateObjectStub.called).to.equal(false); - expect(getValueFromSourceStub.called).to.equal(true); - expect(populateVariableStub.called).to.equal(true); - expect(newProperty).to.deep.equal(variableValuePopulated); - - return BbPromise.resolve(); - }); + serverless.variables.options = { stage: 'prod' }; + return serverless.variables.populateProperty(property) + .should.eventually.eql('my stage is prod'); }); it('should warn if an SSM parameter does not exist', () => { - const awsProvider = new AwsProvider(serverless, { stage: 'prod', region: 'us-west-2' }); - const param = '/some/path/to/invalidparam'; - const property = `\${ssm:${param}}`; - const error = new Error(`Parameter ${param} not found.`); - - serverless.variables.options = { + const options = { stage: 'prod', region: 'us-east-1', }; - serverless.variables.loadVariableSyntax(); - - serverless.variables.getValueFromSource.restore(); - serverless.variables.populateVariable.restore(); + serverless.variables.options = options; + const awsProvider = new AwsProvider(serverless, options); + const param = '/some/path/to/invalidparam'; + const property = `\${ssm:${param}}`; + const error = new Error(`Parameter ${param} not found.`, 123); const requestStub = sinon.stub(awsProvider, 'request', () => BbPromise.reject(error)); const warnIfNotFoundSpy = sinon.spy(serverless.variables, 'warnIfNotFound'); - - return expect(serverless.variables.populateProperty(property) - .then(newProperty => { + return serverless.variables.populateProperty(property) + .should.become(undefined) + .then(() => { expect(requestStub.callCount).to.equal(1); expect(warnIfNotFoundSpy.callCount).to.equal(1); - expect(newProperty).to.be.undefined; }) .finally(() => { - getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); - populateVariableStub = sinon.stub(serverless.variables, 'populateVariable'); - })).to.be.fulfilled; + requestStub.restore(); + warnIfNotFoundSpy.restore(); + }); }); it('should throw an Error if the SSM request fails', () => { - const awsProvider = new AwsProvider(serverless, { stage: 'prod', region: 'us-west-2' }); - const param = '/some/path/to/invalidparam'; - const property = `\${ssm:${param}}`; - const error = new Error('Some random failure.'); - - serverless.variables.options = { + const options = { stage: 'prod', region: 'us-east-1', }; - serverless.variables.loadVariableSyntax(); - - serverless.variables.getValueFromSource.restore(); + serverless.variables.options = options; + const awsProvider = new AwsProvider(serverless, options); + const param = '/some/path/to/invalidparam'; + const property = `\${ssm:${param}}`; + const error = new serverless.classes.Error('Some random failure.', 123); const requestStub = sinon.stub(awsProvider, 'request', () => BbPromise.reject(error)); - - return expect(serverless.variables.populateProperty(property) - .finally(() => { - getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); - expect(requestStub.callCount).to.equal(1); - })).to.be.rejectedWith(serverless.classes.Error); + return serverless.variables.populateProperty(property) + .should.be.rejectedWith(serverless.classes.Error) + .then(() => expect(requestStub.callCount).to.equal(1)) + .finally(() => requestStub.restore()); }); it('should run recursively if nested variables provided', () => { - const property = 'my stage is ${env:${opt.name}}'; - - serverless.variables.loadVariableSyntax(); - - getValueFromSourceStub.onCall(0).resolves('stage'); - getValueFromSourceStub.onCall(1).resolves('dev'); - populateVariableStub.onCall(0).resolves('my stage is ${env:stage}'); - populateVariableStub.onCall(1).resolves('my stage is dev'); - - return serverless.variables.populateProperty(property).then(newProperty => { - expect(getValueFromSourceStub.callCount).to.equal(2); - expect(populateVariableStub.callCount).to.equal(2); - expect(newProperty).to.equal('my stage is dev'); - }); + // eslint-disable-next-line no-template-curly-in-string + const property = 'my stage is ${env:${opt:name}}'; + process.env.TEST_VAR = 'dev'; + serverless.variables.options = { name: 'TEST_VAR' }; + return serverless.variables.populateProperty(property) + .should.eventually.eql('my stage is dev') + .then().finally(() => { delete process.env.TEST_VAR; }); }); }); describe('#populateVariable()', () => { it('should populate string variables as sub string', () => { - const serverless = new Serverless(); const valueToPopulate = 'dev'; - const matchedString = '${opt:stage}'; + const matchedString = '${opt:stage}'; // eslint-disable-line no-template-curly-in-string + // eslint-disable-next-line no-template-curly-in-string const property = 'my stage is ${opt:stage}'; - - return serverless.variables.populateVariable(property, matchedString, valueToPopulate) - .then(newProperty => { - expect(newProperty).to.equal('my stage is dev'); - }); + serverless.variables.populateVariable(property, matchedString, valueToPopulate) + .should.eql('my stage is dev'); }); it('should populate number variables as sub string', () => { - const serverless = new Serverless(); const valueToPopulate = 5; - const matchedString = '${opt:number}'; + const matchedString = '${opt:number}'; // eslint-disable-line no-template-curly-in-string + // eslint-disable-next-line no-template-curly-in-string const property = 'your account number is ${opt:number}'; - - return serverless.variables.populateVariable(property, matchedString, valueToPopulate) - .then(newProperty => { - expect(newProperty).to.equal('your account number is 5'); - }); + serverless.variables.populateVariable(property, matchedString, valueToPopulate) + .should.eql('your account number is 5'); }); it('should populate non string variables', () => { - const serverless = new Serverless(); const valueToPopulate = 5; - const matchedString = '${opt:number}'; - const property = '${opt:number}'; - + const matchedString = '${opt:number}'; // eslint-disable-line no-template-curly-in-string + const property = '${opt:number}'; // eslint-disable-line no-template-curly-in-string return serverless.variables.populateVariable(property, matchedString, valueToPopulate) - .then(newProperty => { - expect(newProperty).to.equal(5); - }); + .should.equal(5); }); it('should throw error if populating non string or non number variable as sub string', () => { - const serverless = new Serverless(); const valueToPopulate = {}; - const matchedString = '${opt:object}'; + const matchedString = '${opt:object}'; // eslint-disable-line no-template-curly-in-string + // eslint-disable-next-line no-template-curly-in-string const property = 'your account number is ${opt:object}'; - expect(() => serverless.variables - .populateVariable(property, matchedString, valueToPopulate)) - .to.throw(Error); + return expect(() => + serverless.variables.populateVariable(property, matchedString, valueToPopulate)) + .to.throw(serverless.classes.Error); }); }); describe('#overwrite()', () => { it('should overwrite undefined and null values', () => { - const serverless = new Serverless(); - const getValueFromSourceStub = sinon - .stub(serverless.variables, 'getValueFromSource'); - + const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves(undefined); getValueFromSourceStub.onCall(1).resolves(null); getValueFromSourceStub.onCall(2).resolves('variableValue'); - return serverless.variables.overwrite('opt:stage,env:stage,self:provider.stage') - .then(valueToPopulate => { + .should.be.fulfilled + .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); expect(getValueFromSourceStub).to.have.been.calledThrice; }) - .finally(() => serverless.variables.getValueFromSource.restore()); + .finally(() => getValueFromSourceStub.restore()); }); it('should overwrite empty object values', () => { - const serverless = new Serverless(); - const getValueFromSourceStub = sinon - .stub(serverless.variables, 'getValueFromSource'); - + const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves({}); getValueFromSourceStub.onCall(1).resolves('variableValue'); - - return serverless.variables.overwrite('opt:stage,env:stage').then(valueToPopulate => { - expect(valueToPopulate).to.equal('variableValue'); - expect(getValueFromSourceStub).to.have.been.calledTwice; - }) - .finally(() => serverless.variables.getValueFromSource.restore()); + return serverless.variables.overwrite('opt:stage,env:stage').should.be.fulfilled + .then((valueToPopulate) => { + expect(valueToPopulate).to.equal('variableValue'); + expect(getValueFromSourceStub).to.have.been.calledTwice; + }) + .finally(() => getValueFromSourceStub.restore()); }); it('should not overwrite 0 values', () => { - const serverless = new Serverless(); - const getValueFromSourceStub = sinon - .stub(serverless.variables, 'getValueFromSource'); - + const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves(0); getValueFromSourceStub.onCall(1).resolves('variableValue'); - getValueFromSourceStub.onCall(2).resolves('variableValue2'); - return serverless.variables.overwrite('opt:stage,env:stage,self:provider.stage') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal(0); - }) - .finally(() => serverless.variables.getValueFromSource.restore()); + return serverless.variables.overwrite('opt:stage,env:stage').should.become(0) + .then().finally(() => getValueFromSourceStub.restore()); }); it('should not overwrite false values', () => { - const serverless = new Serverless(); - const getValueFromSourceStub = sinon - .stub(serverless.variables, 'getValueFromSource'); - + const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves(false); getValueFromSourceStub.onCall(1).resolves('variableValue'); - getValueFromSourceStub.onCall(2).resolves('variableValue2'); - - return serverless.variables.overwrite('opt:stage,env:stage,self:provider.stage') - .then(valueToPopulate => { - expect(valueToPopulate).to.be.false; - }) - .finally(() => serverless.variables.getValueFromSource.restore()); + return serverless.variables.overwrite('opt:stage,env:stage').should.become(false) + .then().finally(() => getValueFromSourceStub.restore()); }); it('should skip getting values once a value has been found', () => { - const serverless = new Serverless(); - const getValueFromSourceStub = sinon - .stub(serverless.variables, 'getValueFromSource'); - + const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves(undefined); getValueFromSourceStub.onCall(1).resolves('variableValue'); getValueFromSourceStub.onCall(2).resolves('variableValue2'); - return serverless.variables.overwrite('opt:stage,env:stage,self:provider.stage') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal('variableValue'); - }) - .finally(() => serverless.variables.getValueFromSource.restore()); + .should.be.fulfilled + .then(valueToPopulate => expect(valueToPopulate).to.equal('variableValue')) + .finally(() => getValueFromSourceStub.restore()); }); }); describe('#getValueFromSource()', () => { it('should call getValueFromEnv if referencing env var', () => { - const serverless = new Serverless(); - const getValueFromEnvStub = sinon - .stub(serverless.variables, 'getValueFromEnv').resolves('variableValue'); - return serverless.variables.getValueFromSource('env:TEST_VAR') - .then(valueToPopulate => { + const getValueFromEnvStub = sinon.stub(serverless.variables, 'getValueFromEnv') + .resolves('variableValue'); + return serverless.variables.getValueFromSource('env:TEST_VAR').should.be.fulfilled + .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); expect(getValueFromEnvStub).to.have.been.called; - expect(getValueFromEnvStub.calledWith('env:TEST_VAR')).to.equal(true); + expect(getValueFromEnvStub).to.have.been.calledWith('env:TEST_VAR'); }) - .finally(() => serverless.variables.getValueFromEnv.restore()); + .finally(() => getValueFromEnvStub.restore()); }); it('should call getValueFromOptions if referencing an option', () => { - const serverless = new Serverless(); const getValueFromOptionsStub = sinon .stub(serverless.variables, 'getValueFromOptions') .resolves('variableValue'); - - return serverless.variables.getValueFromSource('opt:stage') - .then(valueToPopulate => { + return serverless.variables.getValueFromSource('opt:stage').should.be.fulfilled + .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); expect(getValueFromOptionsStub).to.have.been.called; - expect(getValueFromOptionsStub.calledWith('opt:stage')).to.equal(true); + expect(getValueFromOptionsStub).to.have.been.calledWith('opt:stage'); }) - .finally(() => serverless.variables.getValueFromOptions.restore()); + .finally(() => getValueFromOptionsStub.restore()); }); it('should call getValueFromSelf if referencing from self', () => { - const serverless = new Serverless(); - const getValueFromSelfStub = sinon - .stub(serverless.variables, 'getValueFromSelf').resolves('variableValue'); - - return serverless.variables.getValueFromSource('self:provider') - .then(valueToPopulate => { + const getValueFromSelfStub = sinon.stub(serverless.variables, 'getValueFromSelf') + .resolves('variableValue'); + return serverless.variables.getValueFromSource('self:provider').should.be.fulfilled + .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); expect(getValueFromSelfStub).to.have.been.called; - expect(getValueFromSelfStub.calledWith('self:provider')).to.equal(true); + expect(getValueFromSelfStub).to.have.been.calledWith('self:provider'); }) - .finally(() => serverless.variables.getValueFromSelf.restore()); + .finally(() => getValueFromSelfStub.restore()); }); it('should call getValueFromFile if referencing from another file', () => { - const serverless = new Serverless(); - const getValueFromFileStub = sinon - .stub(serverless.variables, 'getValueFromFile').resolves('variableValue'); - - return serverless.variables.getValueFromSource('file(./config.yml)') - .then(valueToPopulate => { + const getValueFromFileStub = sinon.stub(serverless.variables, 'getValueFromFile') + .resolves('variableValue'); + return serverless.variables.getValueFromSource('file(./config.yml)').should.be.fulfilled + .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); expect(getValueFromFileStub).to.have.been.called; expect(getValueFromFileStub).to.have.been.calledWith('file(./config.yml)'); }) - .finally(() => serverless.variables.getValueFromFile.restore()); + .finally(() => getValueFromFileStub.restore()); }); it('should call getValueFromCf if referencing CloudFormation Outputs', () => { - const serverless = new Serverless(); - const getValueFromCfStub = sinon - .stub(serverless.variables, 'getValueFromCf').resolves('variableValue'); - return serverless.variables.getValueFromSource('cf:test-stack.testOutput') - .then(valueToPopulate => { + const getValueFromCfStub = sinon.stub(serverless.variables, 'getValueFromCf') + .resolves('variableValue'); + return serverless.variables.getValueFromSource('cf:test-stack.testOutput').should.be.fulfilled + .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); expect(getValueFromCfStub).to.have.been.called; expect(getValueFromCfStub).to.have.been.calledWith('cf:test-stack.testOutput'); }) - .finally(() => serverless.variables.getValueFromCf.restore()); + .finally(() => getValueFromCfStub.restore()); }); it('should call getValueFromS3 if referencing variable in S3', () => { - const serverless = new Serverless(); - const getValueFromS3Stub = sinon - .stub(serverless.variables, 'getValueFromS3').resolves('variableValue'); + const getValueFromS3Stub = sinon.stub(serverless.variables, 'getValueFromS3') + .resolves('variableValue'); return serverless.variables.getValueFromSource('s3:test-bucket/path/to/key') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal('variableValue'); - expect(getValueFromS3Stub).to.have.been.called; - expect(getValueFromS3Stub).to.have.been.calledWith('s3:test-bucket/path/to/key'); - }) - .finally(() => serverless.variables.getValueFromS3.restore()); + .should.be.fulfilled + .then((valueToPopulate) => { + expect(valueToPopulate).to.equal('variableValue'); + expect(getValueFromS3Stub).to.have.been.called; + expect(getValueFromS3Stub).to.have.been.calledWith('s3:test-bucket/path/to/key'); + }) + .finally(() => getValueFromS3Stub.restore()); }); it('should call getValueFromSsm if referencing variable in SSM', () => { - const serverless = new Serverless(); - const getValueFromSsmStub = sinon - .stub(serverless.variables, 'getValueFromSsm').resolves('variableValue'); + const getValueFromSsmStub = sinon.stub(serverless.variables, 'getValueFromSsm') + .resolves('variableValue'); return serverless.variables.getValueFromSource('ssm:/test/path/to/param') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal('variableValue'); - expect(getValueFromSsmStub).to.have.been.called; - expect(getValueFromSsmStub).to.have.been.calledWith('ssm:/test/path/to/param'); - }) - .finally(() => serverless.variables.getValueFromSsm.restore()); + .should.be.fulfilled + .then((valueToPopulate) => { + expect(valueToPopulate).to.equal('variableValue'); + expect(getValueFromSsmStub).to.have.been.called; + expect(getValueFromSsmStub).to.have.been.calledWith('ssm:/test/path/to/param'); + }) + .finally(() => getValueFromSsmStub.restore()); }); - + it('should reject invalid sources', () => + serverless.variables.getValueFromSource('weird:source') + .should.be.rejectedWith(serverless.classes.Error)); describe('caching', () => { const sources = [ { function: 'getValueFromEnv', variableString: 'env:NODE_ENV' }, @@ -853,182 +749,77 @@ module.exports = { ]; sources.forEach((source) => { it(`should only call ${source.function} once, returning the cached value otherwise`, () => { - const serverless = new Serverless(); - const getValueFunctionStub = sinon - .stub(serverless.variables, source.function).resolves('variableValue'); - const firstCall = serverless.variables.getValueFromSource(source.variableString); - const secondCall = BbPromise.delay(100) - .then(() => serverless.variables.getValueFromSource(source.variableString)); - return BbPromise.all([firstCall, secondCall]) - .then(valueToPopulate => { - expect(valueToPopulate).to.deep.equal(['variableValue', 'variableValue']); + const value = 'variableValue'; + const getValueFunctionStub = sinon.stub(serverless.variables, source.function) + .resolves(value); + return BbPromise.all([ + serverless.variables.getValueFromSource(source.variableString).should.become(value), + BbPromise.delay(100).then(() => + serverless.variables.getValueFromSource(source.variableString).should.become(value)), + ]).then(() => { expect(getValueFunctionStub).to.have.been.calledOnce; expect(getValueFunctionStub).to.have.been.calledWith(source.variableString); - }) - .finally(() => serverless.variables[source.function].restore()); + }).finally(() => + getValueFunctionStub.restore()); }); }); }); - - it('should call populateObject if variable value is an object', () => { - const serverless = new Serverless(); - serverless.variables.options = { - stage: 'prod', - }; - const property = 'self:stage'; - const variableValue = { - stage: '${opt:stage}', - }; - const variableValuePopulated = { - stage: 'prod', - }; - - serverless.variables.loadVariableSyntax(); - - const populateObjectStub = sinon - .stub(serverless.variables, 'populateObject') - .resolves(variableValuePopulated); - const getValueFromSelfStub = sinon - .stub(serverless.variables, 'getValueFromSelf') - .resolves(variableValue); - - return serverless.variables.getValueFromSource(property) - .then(newProperty => { - expect(populateObjectStub.called).to.equal(true); - expect(getValueFromSelfStub.called).to.equal(true); - expect(newProperty).to.deep.equal(variableValuePopulated); - - return BbPromise.resolve(); - }) - .finally(() => { - serverless.variables.populateObject.restore(); - serverless.variables.getValueFromSelf.restore(); - }); - }); - - it('should NOT call populateObject if variable value is already cached', () => { - const serverless = new Serverless(); - serverless.variables.options = { - stage: 'prod', - }; - const property = 'opt:stage'; - const variableValue = { - stage: '${opt:stage}', - }; - const variableValuePopulated = { - stage: 'prod', - }; - - serverless.variables.cache['opt:stage'] = BbPromise.resolve(variableValuePopulated); - - serverless.variables.loadVariableSyntax(); - - const populateObjectStub = sinon - .stub(serverless.variables, 'populateObject') - .resolves(variableValuePopulated); - const getValueFromOptionsStub = sinon - .stub(serverless.variables, 'getValueFromOptions') - .resolves(variableValue); - - return serverless.variables.getValueFromSource(property) - .then(newProperty => { - expect(populateObjectStub.called).to.equal(false); - expect(getValueFromOptionsStub.called).to.equal(false); - expect(newProperty).to.deep.equal(variableValuePopulated); - - return BbPromise.resolve(); - }) - .finally(() => { - serverless.variables.populateObject.restore(); - serverless.variables.getValueFromOptions.restore(); - }); - }); - - it('should throw error if referencing an invalid source', () => { - const serverless = new Serverless(); - expect(() => serverless.variables.getValueFromSource('weird:source')) - .to.throw(Error); - }); }); describe('#getValueFromEnv()', () => { it('should get variable from environment variables', () => { - const serverless = new Serverless(); process.env.TEST_VAR = 'someValue'; - return serverless.variables.getValueFromEnv('env:TEST_VAR').then(valueToPopulate => { - expect(valueToPopulate).to.be.equal('someValue'); - }) - .finally(() => { - delete process.env.TEST_VAR; - }); + return serverless.variables.getValueFromEnv('env:TEST_VAR') + .finally(() => { delete process.env.TEST_VAR; }) + .should.become('someValue'); }); it('should allow top-level references to the environment variables hive', () => { - const serverless = new Serverless(); process.env.TEST_VAR = 'someValue'; - return serverless.variables.getValueFromEnv('env:').then(valueToPopulate => { + return serverless.variables.getValueFromEnv('env:').then((valueToPopulate) => { expect(valueToPopulate.TEST_VAR).to.be.equal('someValue'); }) - .finally(() => { - delete process.env.TEST_VAR; - }); + .finally(() => { delete process.env.TEST_VAR; }); }); }); describe('#getValueFromOptions()', () => { it('should get variable from options', () => { - const serverless = new Serverless(); - serverless.variables.options = { - stage: 'prod', - }; - return serverless.variables.getValueFromOptions('opt:stage').then(valueToPopulate => { - expect(valueToPopulate).to.be.equal('prod'); - }); + serverless.variables.options = { stage: 'prod' }; + return serverless.variables.getValueFromOptions('opt:stage').should.become('prod'); }); it('should allow top-level references to the options hive', () => { - const serverless = new Serverless(); - serverless.variables.options = { - stage: 'prod', - }; - return serverless.variables.getValueFromOptions('opt:').then(valueToPopulate => { - expect(valueToPopulate.stage).to.be.equal('prod'); - }); + serverless.variables.options = { stage: 'prod' }; + return serverless.variables.getValueFromOptions('opt:') + .should.become(serverless.variables.options); }); }); describe('#getValueFromSelf()', () => { it('should get variable from self serverless.yml file', () => { - const serverless = new Serverless(); serverless.variables.service = { service: 'testService', provider: serverless.service.provider, }; serverless.variables.loadVariableSyntax(); - return serverless.variables.getValueFromSelf('self:service').then(valueToPopulate => { - expect(valueToPopulate).to.be.equal('testService'); - }); + return serverless.variables.getValueFromSelf('self:service').should.become('testService'); }); it('should handle self-references to the root of the serverless.yml file', () => { - const serverless = new Serverless(); serverless.variables.service = { service: 'testService', provider: 'testProvider', defaults: serverless.service.defaults, }; - serverless.variables.loadVariableSyntax(); - - return serverless.variables.getValueFromSelf('self:').then(valueToPopulate => { - expect(valueToPopulate.provider).to.be.equal('testProvider'); - }); + return serverless.variables.getValueFromSelf('self:') + .should.eventually.equal(serverless.variables.service); }); }); describe('#getValueFromFile()', () => { it('should work for absolute paths with ~ ', () => { - const serverless = new Serverless(); const expectedFileName = `${os.homedir()}/somedir/config.yml`; const configYml = { test: 1, @@ -1038,17 +829,11 @@ module.exports = { prob: 'prob', }, }; - const fileExistsStub = sinon - .stub(serverless.utils, 'fileExistsSync').returns(true); - - const realpathSync = sinon - .stub(fse, 'realpathSync').returns(expectedFileName); - - const readFileSyncStub = sinon - .stub(serverless.utils, 'readFileSync').returns(configYml); - - return serverless.variables.getValueFromFile('file(~/somedir/config.yml)') - .then(valueToPopulate => { + const fileExistsStub = sinon.stub(serverless.utils, 'fileExistsSync').returns(true); + const realpathSync = sinon.stub(fse, 'realpathSync').returns(expectedFileName); + const readFileSyncStub = sinon.stub(serverless.utils, 'readFileSync').returns(configYml); + return serverless.variables.getValueFromFile('file(~/somedir/config.yml)').should.be.fulfilled + .then((valueToPopulate) => { expect(realpathSync).to.not.have.been.called; expect(fileExistsStub).to.have.been.calledWithMatch(expectedFileName); expect(readFileSyncStub).to.have.been.calledWithMatch(expectedFileName); @@ -1062,7 +847,6 @@ module.exports = { }); it('should populate an entire variable file', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const configYml = { @@ -1073,28 +857,19 @@ module.exports = { prob: 'prob', }, }; - - SUtils.writeFileSync(path.join(tmpDirPath, 'config.yml'), - YAML.dump(configYml)); - + SUtils.writeFileSync(path.join(tmpDirPath, 'config.yml'), YAML.dump(configYml)); serverless.config.update({ servicePath: tmpDirPath }); - - return serverless.variables.getValueFromFile('file(./config.yml)').then(valueToPopulate => { - expect(valueToPopulate).to.deep.equal(configYml); - }); + return serverless.variables.getValueFromFile('file(./config.yml)') + .should.eventually.eql(configYml); }); it('should get undefined if non existing file and the second argument is true', () => { - const serverless = new Serverless(); const tmpDirPath = testUtils.getTmpDirPath(); - serverless.config.update({ servicePath: tmpDirPath }); - const realpathSync = sinon.spy(fse, 'realpathSync'); const existsSync = sinon.spy(fse, 'existsSync'); - - return serverless.variables.getValueFromFile('file(./non-existing.yml)') - .then(valueToPopulate => { + return serverless.variables.getValueFromFile('file(./non-existing.yml)').should.be.fulfilled + .then((valueToPopulate) => { expect(realpathSync).to.not.have.been.called; expect(existsSync).to.have.been.calledOnce; expect(valueToPopulate).to.be.undefined; @@ -1106,166 +881,113 @@ module.exports = { }); it('should populate non json/yml files', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); - - SUtils.writeFileSync(path.join(tmpDirPath, 'someFile'), - 'hello world'); - + SUtils.writeFileSync(path.join(tmpDirPath, 'someFile'), 'hello world'); serverless.config.update({ servicePath: tmpDirPath }); - - return serverless.variables.getValueFromFile('file(./someFile)').then(valueToPopulate => { - expect(valueToPopulate).to.equal('hello world'); - }); + return serverless.variables.getValueFromFile('file(./someFile)') + .should.become('hello world'); }); it('should populate symlinks', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const realFilePath = path.join(tmpDirPath, 'someFile'); const symlinkPath = path.join(tmpDirPath, 'refSomeFile'); SUtils.writeFileSync(realFilePath, 'hello world'); fse.ensureSymlinkSync(realFilePath, symlinkPath); - serverless.config.update({ servicePath: tmpDirPath }); - - return expect(serverless.variables.getValueFromFile('file(./refSomeFile)')).to.be.fulfilled - .then(valueToPopulate => { - expect(valueToPopulate).to.equal('hello world'); - }) - .finally(() => { - fse.removeSync(realFilePath); - fse.removeSync(symlinkPath); - }); + return serverless.variables.getValueFromFile('file(./refSomeFile)') + .should.become('hello world') + .then().finally(() => { + fse.removeSync(realFilePath); + fse.removeSync(symlinkPath); + }); }); it('should trim trailing whitespace and new line character', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); - - SUtils.writeFileSync(path.join(tmpDirPath, 'someFile'), - 'hello world \n'); - + SUtils.writeFileSync(path.join(tmpDirPath, 'someFile'), 'hello world \n'); serverless.config.update({ servicePath: tmpDirPath }); - - return serverless.variables.getValueFromFile('file(./someFile)').then(valueToPopulate => { - expect(valueToPopulate).to.equal('hello world'); - }); + return serverless.variables.getValueFromFile('file(./someFile)') + .should.become('hello world'); }); it('should populate from another file when variable is of any type', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const configYml = { - test: 1, - test2: 'test2', - testObj: { + test0: 0, + test1: 'test1', + test2: { sub: 2, prob: 'prob', }, }; - - SUtils.writeFileSync(path.join(tmpDirPath, 'config.yml'), - YAML.dump(configYml)); - + SUtils.writeFileSync(path.join(tmpDirPath, 'config.yml'), YAML.dump(configYml)); serverless.config.update({ servicePath: tmpDirPath }); - - return serverless.variables.getValueFromFile('file(./config.yml):testObj.sub') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal(2); - }); + return serverless.variables.getValueFromFile('file(./config.yml):test2.sub') + .should.become(configYml.test2.sub); }); it('should populate from a javascript file', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const jsData = 'module.exports.hello=function(){return "hello world";};'; - SUtils.writeFileSync(path.join(tmpDirPath, 'hello.js'), jsData); - serverless.config.update({ servicePath: tmpDirPath }); - return serverless.variables.getValueFromFile('file(./hello.js):hello') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal('hello world'); - }); + .should.become('hello world'); }); it('should populate an entire variable exported by a javascript file', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const jsData = 'module.exports=function(){return { hello: "hello world" };};'; - SUtils.writeFileSync(path.join(tmpDirPath, 'hello.js'), jsData); - serverless.config.update({ servicePath: tmpDirPath }); - return serverless.variables.getValueFromFile('file(./hello.js)') - .then(valueToPopulate => { - expect(valueToPopulate.hello).to.equal('hello world'); - }); + .should.become({ hello: 'hello world' }); }); it('should throw if property exported by a javascript file is not a function', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const jsData = 'module.exports={ hello: "hello world" };'; - SUtils.writeFileSync(path.join(tmpDirPath, 'hello.js'), jsData); - serverless.config.update({ servicePath: tmpDirPath }); - - expect(() => serverless.variables - .getValueFromFile('file(./hello.js)')).to.throw(Error); + return serverless.variables.getValueFromFile('file(./hello.js)') + .should.be.rejectedWith(serverless.classes.Error); }); it('should populate deep object from a javascript file', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const jsData = `module.exports.hello=function(){ return {one:{two:{three: 'hello world'}}} };`; - SUtils.writeFileSync(path.join(tmpDirPath, 'hello.js'), jsData); - serverless.config.update({ servicePath: tmpDirPath }); serverless.variables.loadVariableSyntax(); - return serverless.variables.getValueFromFile('file(./hello.js):hello.one.two.three') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal('hello world'); - }); + .should.become('hello world'); }); it('should preserve the exported function context when executing', () => { - const serverless = new Serverless(); const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const jsData = ` module.exports.one = {two: {three: 'hello world'}} module.exports.hello=function(){ return this; };`; - SUtils.writeFileSync(path.join(tmpDirPath, 'hello.js'), jsData); - serverless.config.update({ servicePath: tmpDirPath }); serverless.variables.loadVariableSyntax(); - return serverless.variables.getValueFromFile('file(./hello.js):hello.one.two.three') - .then(valueToPopulate => { - expect(valueToPopulate).to.equal('hello world'); - }); + .should.become('hello world'); }); - it('should throw error if not using ":" syntax', () => { - const serverless = new Serverless(); + it('should file variable not using ":" syntax', () => { const SUtils = new Utils(); const tmpDirPath = testUtils.getTmpDirPath(); const configYml = { @@ -1276,20 +998,15 @@ module.exports = { prob: 'prob', }, }; - - SUtils.writeFileSync(path.join(tmpDirPath, 'config.yml'), - YAML.dump(configYml)); - + SUtils.writeFileSync(path.join(tmpDirPath, 'config.yml'), YAML.dump(configYml)); serverless.config.update({ servicePath: tmpDirPath }); - - expect(() => serverless.variables - .getValueFromFile('file(./config.yml).testObj.sub')).to.throw(Error); + return serverless.variables.getValueFromFile('file(./config.yml).testObj.sub') + .should.be.rejectedWith(serverless.classes.Error); }); }); describe('#getValueFromCf()', () => { it('should get variable from CloudFormation', () => { - const serverless = new Serverless(); const options = { stage: 'prod', region: 'us-west-2', @@ -1305,27 +1022,22 @@ module.exports = { }], }], }; - - const cfStub = sinon.stub(serverless.getProvider('aws'), 'request') - .resolves(awsResponseMock); + const cfStub = sinon.stub(serverless.getProvider('aws'), 'request', + () => BbPromise.resolve(awsResponseMock)); return serverless.variables.getValueFromCf('cf:some-stack.MockExport') - .then(valueToPopulate => { - expect(valueToPopulate).to.be.equal('MockValue'); + .should.become('MockValue') + .then(() => { expect(cfStub).to.have.been.calledOnce; expect(cfStub).to.have.been.calledWithExactly( 'CloudFormation', 'describeStacks', - { - StackName: 'some-stack', - }, - { useCache: true } - ); + { StackName: 'some-stack' }, + { useCache: true }); }) - .finally(() => serverless.getProvider('aws').request.restore()); + .finally(() => cfStub.restore()); }); - it('should throw an error when variable from CloudFormation does not exist', () => { - const serverless = new Serverless(); + it('should reject CloudFormation variables that do not exist', () => { const options = { stage: 'prod', region: 'us-west-2', @@ -1341,35 +1053,26 @@ module.exports = { }], }], }; - - const cfStub = sinon.stub(serverless.getProvider('aws'), 'request') - .resolves(awsResponseMock); - + const cfStub = sinon.stub(serverless.getProvider('aws'), 'request', + () => BbPromise.resolve(awsResponseMock)); return serverless.variables.getValueFromCf('cf:some-stack.DoestNotExist') - .then() - .catch(error => { + .should.be.rejectedWith(serverless.classes.Error, + /to request a non exported variable from CloudFormation/) + .then(() => { expect(cfStub).to.have.been.calledOnce; expect(cfStub).to.have.been.calledWithExactly( 'CloudFormation', 'describeStacks', - { - StackName: 'some-stack', - }, - { useCache: true } - ); - expect(error).to.be.an.instanceof(Error); - expect(error.message).to.match(/to request a non exported variable from CloudFormation/); + { StackName: 'some-stack' }, + { useCache: true }); }) - .finally(() => serverless.getProvider('aws').request.restore()); + .finally(() => cfStub.restore()); }); }); describe('#getValueFromS3()', () => { - let serverless; let awsProvider; - beforeEach(() => { - serverless = new Serverless(); const options = { stage: 'prod', region: 'us-west-2', @@ -1378,45 +1081,48 @@ module.exports = { serverless.setProvider('aws', awsProvider); serverless.variables.options = options; }); - it('should get variable from S3', () => { const awsResponseMock = { Body: 'MockValue', }; - const s3Stub = sinon.stub(awsProvider, 'request').resolves(awsResponseMock); - - return serverless.variables.getValueFromS3('s3:some.bucket/path/to/key').then(value => { - expect(value).to.be.equal('MockValue'); - expect(s3Stub).to.have.been.calledOnce; - expect(s3Stub).to.have.been.calledWithExactly( - 'S3', - 'getObject', - { - Bucket: 'some.bucket', - Key: 'path/to/key', - }, - { useCache: true } - ); - }) - .finally(() => serverless.getProvider('aws').request.restore()); + const s3Stub = sinon.stub(awsProvider, 'request', () => BbPromise.resolve(awsResponseMock)); + return serverless.variables.getValueFromS3('s3:some.bucket/path/to/key') + .should.become('MockValue') + .then(() => { + expect(s3Stub).to.have.been.calledOnce; + expect(s3Stub).to.have.been.calledWithExactly( + 'S3', + 'getObject', + { + Bucket: 'some.bucket', + Key: 'path/to/key', + }, + { useCache: true }); + }) + .finally(() => s3Stub.restore()); }); it('should throw error if error getting value from S3', () => { const error = new Error('The specified bucket is not valid'); - sinon.stub(awsProvider, 'request').rejects(error); - + const requestStub = sinon.stub(awsProvider, 'request', () => BbPromise.reject(error)); return expect(serverless.variables.getValueFromS3('s3:some.bucket/path/to/key')) - .to.be.rejectedWith('Error getting value for s3:some.bucket/path/to/key. ' + - 'The specified bucket is not valid'); + .to.be.rejectedWith( + serverless.classes.Error, + 'Error getting value for s3:some.bucket/path/to/key. The specified bucket is not valid') + .then().finally(() => requestStub.restore()); }); }); describe('#getValueFromSsm()', () => { - let serverless; + const param = 'Param-01_valid.chars'; + const value = 'MockValue'; + const awsResponseMock = { + Parameter: { + Value: value, + }, + }; let awsProvider; - beforeEach(() => { - serverless = new Serverless(); const options = { stage: 'prod', region: 'us-west-2', @@ -1425,154 +1131,112 @@ module.exports = { serverless.setProvider('aws', awsProvider); serverless.variables.options = options; }); - it('should get variable from Ssm using regular-style param', () => { - const param = 'Param-01_valid.chars'; - const value = 'MockValue'; - const awsResponseMock = { - Parameter: { - Value: value, - }, - }; - const ssmStub = sinon.stub(awsProvider, 'request').resolves(awsResponseMock); - - return serverless.variables.getValueFromSsm(`ssm:${param}`).then(resolved => { - expect(resolved).to.be.equal(value); - expect(ssmStub).to.have.been.calledOnce; - expect(ssmStub).to.have.been.calledWithExactly( - 'SSM', - 'getParameter', - { - Name: param, - WithDecryption: false, - }, - { useCache: true } - ); - }); + const ssmStub = sinon.stub(awsProvider, 'request', () => BbPromise.resolve(awsResponseMock)); + return serverless.variables.getValueFromSsm(`ssm:${param}`) + .should.become(value) + .then(() => { + expect(ssmStub).to.have.been.calledOnce; + expect(ssmStub).to.have.been.calledWithExactly( + 'SSM', + 'getParameter', + { + Name: param, + WithDecryption: false, + }, + { useCache: true }); + }) + .finally(() => ssmStub.restore()); }); - it('should get variable from Ssm using path-style param', () => { - const param = '/path/to/Param-01_valid.chars'; - const value = 'MockValue'; - const awsResponseMock = { - Parameter: { - Value: value, - }, - }; - const ssmStub = sinon.stub(awsProvider, 'request').resolves(awsResponseMock); - - return serverless.variables.getValueFromSsm(`ssm:${param}`).then(resolved => { - expect(resolved).to.be.equal(value); - expect(ssmStub).to.have.been.calledOnce; - expect(ssmStub).to.have.been.calledWithExactly( - 'SSM', - 'getParameter', - { - Name: param, - WithDecryption: false, - }, - { useCache: true } - ); - }); + const ssmStub = sinon.stub(awsProvider, 'request', () => BbPromise.resolve(awsResponseMock)); + return serverless.variables.getValueFromSsm(`ssm:${param}`) + .should.become(value) + .then(() => { + expect(ssmStub).to.have.been.calledOnce; + expect(ssmStub).to.have.been.calledWithExactly( + 'SSM', + 'getParameter', + { + Name: param, + WithDecryption: false, + }, + { useCache: true }); + }) + .finally(() => ssmStub.restore()); }); - it('should get encrypted variable from Ssm using extended syntax', () => { - const param = '/path/to/Param-01_valid.chars'; - const value = 'MockValue'; - const awsResponseMock = { - Parameter: { - Value: value, - }, - }; - const ssmStub = sinon.stub(awsProvider, 'request').resolves(awsResponseMock); - - return serverless.variables.getValueFromSsm(`ssm:${param}~true`).then(resolved => { - expect(resolved).to.be.equal(value); - expect(ssmStub).to.have.been.calledOnce; - expect(ssmStub).to.have.been.calledWithExactly( - 'SSM', - 'getParameter', - { - Name: param, - WithDecryption: true, - }, - { useCache: true } - ); - }); + const ssmStub = sinon.stub(awsProvider, 'request', () => BbPromise.resolve(awsResponseMock)); + return serverless.variables.getValueFromSsm(`ssm:${param}~true`) + .should.become(value) + .then(() => { + expect(ssmStub).to.have.been.calledOnce; + expect(ssmStub).to.have.been.calledWithExactly( + 'SSM', + 'getParameter', + { + Name: param, + WithDecryption: true, + }, + { useCache: true }); + }) + .finally(() => ssmStub.restore()); }); - it('should get unencrypted variable from Ssm using extended syntax', () => { - const param = '/path/to/Param-01_valid.chars'; - const value = 'MockValue'; - const awsResponseMock = { - Parameter: { - Value: value, - }, - }; - const ssmStub = sinon.stub(awsProvider, 'request').resolves(awsResponseMock); - - return serverless.variables.getValueFromSsm(`ssm:${param}~false`).then(resolved => { - expect(resolved).to.be.equal(value); - expect(ssmStub).to.have.been.calledOnce; - expect(ssmStub).to.have.been.calledWithExactly( - 'SSM', - 'getParameter', - { - Name: param, - WithDecryption: false, - }, - { useCache: true } - ); - }); + const ssmStub = sinon.stub(awsProvider, 'request', () => BbPromise.resolve(awsResponseMock)); + return serverless.variables.getValueFromSsm(`ssm:${param}~false`) + .should.become(value) + .then(() => { + expect(ssmStub).to.have.been.calledOnce; + expect(ssmStub).to.have.been.calledWithExactly( + 'SSM', + 'getParameter', + { + Name: param, + WithDecryption: false, + }, + { useCache: true }); + }) + .finally(() => ssmStub.restore()); }); - it('should ignore bad values for extended syntax', () => { - const param = '/path/to/Param-01_valid.chars'; - const value = 'MockValue'; - const awsResponseMock = { - Parameter: { - Value: value, - }, - }; - const ssmStub = sinon.stub(awsProvider, 'request').resolves(awsResponseMock); - - return serverless.variables.getValueFromSsm(`ssm:${param}~badvalue`).then(resolved => { - expect(resolved).to.be.equal(value); - expect(ssmStub).to.have.been.calledOnce; - expect(ssmStub).to.have.been.calledWithExactly( - 'SSM', - 'getParameter', - { - Name: param, - WithDecryption: false, - }, - { useCache: true } - ); - }); + const ssmStub = sinon.stub(awsProvider, 'request', () => BbPromise.resolve(awsResponseMock)); + return serverless.variables.getValueFromSsm(`ssm:${param}~badvalue`) + .should.become(value) + .then(() => { + expect(ssmStub).to.have.been.calledOnce; + expect(ssmStub).to.have.been.calledWithExactly( + 'SSM', + 'getParameter', + { + Name: param, + WithDecryption: false, + }, + { useCache: true }); + }) + .finally(() => ssmStub.restore()); }); it('should return undefined if SSM parameter does not exist', () => { - const param = 'ssm:/some/path/to/invalidparam'; const error = new Error(`Parameter ${param} not found.`); - sinon.stub(awsProvider, 'request').rejects(error); - - return expect(() => serverless.variables.getValueFromSsm(param).to.be(undefined)); + const requestStub = sinon.stub(awsProvider, 'request', () => BbPromise.reject(error)); + return serverless.variables.getValueFromSsm(`ssm:${param}`) + .should.become(undefined) + .then().finally(() => requestStub.restore()); }); - it('should throw exception if SSM request returns unexpected error', () => { - const param = 'ssm:/some/path/to/invalidparam'; + it('should reject if SSM request returns unexpected error', () => { const error = new Error( 'User: is not authorized to perform: ssm:GetParameter on resource: '); - sinon.stub(awsProvider, 'request').rejects(error); - - return expect(() => serverless.variables.getValueFromSsm(param).to.throw(error)); + const requestStub = sinon.stub(awsProvider, 'request', () => BbPromise.reject(error)); + return serverless.variables.getValueFromSsm(`ssm:${param}`) + .should.be.rejected + .then().finally(() => requestStub.restore()); }); }); describe('#getDeepValue()', () => { it('should get deep values', () => { - const serverless = new Serverless(); - const valueToPopulateMock = { service: 'testService', custom: { @@ -1581,41 +1245,29 @@ module.exports = { }, }, }; - serverless.variables.loadVariableSyntax(); - return serverless.variables.getDeepValue(['custom', 'subProperty', 'deep'], - valueToPopulateMock).then(valueToPopulate => { - expect(valueToPopulate).to.be.equal('deepValue'); - }); + valueToPopulateMock).should.become('deepValue'); }); - it('should not throw error if referencing invalid properties', () => { - const serverless = new Serverless(); - const valueToPopulateMock = { service: 'testService', custom: { subProperty: 'hello', }, }; - serverless.variables.loadVariableSyntax(); - return serverless.variables.getDeepValue(['custom', 'subProperty', 'deep', 'deeper'], - valueToPopulateMock).then(valueToPopulate => { - expect(valueToPopulate).to.deep.equal({}); - }); + valueToPopulateMock).should.eventually.deep.equal({}); }); it('should get deep values with variable references', () => { - const serverless = new Serverless(); - serverless.variables.service = { service: 'testService', custom: { - anotherVar: '${self:custom.var}', + anotherVar: '${self:custom.var}', // eslint-disable-line no-template-curly-in-string subProperty: { + // eslint-disable-next-line no-template-curly-in-string deep: '${self:custom.anotherVar.veryDeep}', }, var: { @@ -1624,73 +1276,56 @@ module.exports = { }, provider: serverless.service.provider, }; - serverless.variables.loadVariableSyntax(); - return serverless.variables.getDeepValue(['custom', 'subProperty', 'deep'], - serverless.variables.service).then(valueToPopulate => { - expect(valueToPopulate).to.be.equal('someValue'); - }); + serverless.variables.service).should.become('someValue'); }); }); - describe('#warnIfNotFound()', () => { let logWarningSpy; let consoleLogStub; let varProxy; - beforeEach(() => { logWarningSpy = sinon.spy(slsError, 'logWarning'); consoleLogStub = sinon.stub(console, 'log').returns(); - const ProxyQuiredVariables = proxyquire('./Variables.js', { - './Error': logWarningSpy, - }); - varProxy = new ProxyQuiredVariables(new Serverless()); + const ProxyQuiredVariables = proxyquire('./Variables.js', { './Error': logWarningSpy }); + varProxy = new ProxyQuiredVariables(serverless); }); - afterEach(() => { logWarningSpy.restore(); consoleLogStub.restore(); }); - it('should do nothing if variable has valid value.', () => { varProxy.warnIfNotFound('self:service', 'a-valid-value'); expect(logWarningSpy).to.not.have.been.calledOnce; }); - it('should log if variable has null value.', () => { varProxy.warnIfNotFound('self:service', null); expect(logWarningSpy).to.have.been.calledOnce; }); - it('should log if variable has undefined value.', () => { varProxy.warnIfNotFound('self:service', undefined); expect(logWarningSpy).to.have.been.calledOnce; }); - it('should log if variable has empty object value.', () => { varProxy.warnIfNotFound('self:service', {}); expect(logWarningSpy).to.have.been.calledOnce; }); - it('should detect the "environment variable" variable type', () => { varProxy.warnIfNotFound('env:service', null); expect(logWarningSpy).to.have.been.calledOnce; expect(logWarningSpy.args[0][0]).to.contain('environment variable'); }); - it('should detect the "option" variable type', () => { varProxy.warnIfNotFound('opt:service', null); expect(logWarningSpy).to.have.been.calledOnce; expect(logWarningSpy.args[0][0]).to.contain('option'); }); - it('should detect the "service attribute" variable type', () => { varProxy.warnIfNotFound('self:service', null); expect(logWarningSpy).to.have.been.calledOnce; expect(logWarningSpy.args[0][0]).to.contain('service attribute'); }); - it('should detect the "file" variable type', () => { varProxy.warnIfNotFound('file(service)', null); expect(logWarningSpy).to.have.been.calledOnce; From 8f69678a88a0b735e2a52ab19650aae203037c65 Mon Sep 17 00:00:00 2001 From: Erik Erikson Date: Sat, 3 Feb 2018 13:14:11 -0800 Subject: [PATCH 2/6] remove `.only` from describe for Variables tests add expected rejection message --- lib/classes/Variables.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index 04c2f814b..0beaf01ee 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -29,7 +29,7 @@ chai.use(require('sinon-chai')); const expect = chai.expect; -describe.only('Variables', () => { +describe('Variables', () => { let serverless; beforeEach(() => { serverless = new Serverless(); @@ -458,7 +458,8 @@ module.exports = { val: `\${file(${emptyFileName}):func.notAValue}`, }; return serverless.variables.populateObject(service.custom) - .should.be.rejectedWith(serverless.classes.Error); + .should.be.rejectedWith(serverless.classes.Error, + 'Invalid variable syntax when referencing file'); }); }); }); From 3606589ff2ce460038104f350add384ab88e0859 Mon Sep 17 00:00:00 2001 From: Erik Erikson Date: Tue, 6 Feb 2018 16:35:16 -0800 Subject: [PATCH 3/6] Do not double-reject errors Calling .then here creates a new promise. One that is not returned and generating a rejection within that promise can lead to unhandledRejection events being inappropriately raised. As such, don't declare unneeded variables. --- lib/classes/Variables.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js index a6842c4b3..ffbe65f3a 100644 --- a/lib/classes/Variables.js +++ b/lib/classes/Variables.js @@ -36,9 +36,9 @@ class PromiseTracker { const promise = prms; promise.waitList = `${variable} waited on by: ${specifier}`; promise.state = 'pending'; - promise.then( - (value) => { promise.state = 'resolved'; return Promise.resolve(value); }, - (error) => { promise.state = 'rejected'; return Promise.reject(error); }); + promise.then( // creates a promise with the following effects but that we otherwise ignore + () => { promise.state = 'resolved'; }, + () => { promise.state = 'rejected'; }); this.promiseList.push(promise); this.promiseMap[variable] = promise; return promise; @@ -393,7 +393,7 @@ class Variables { ].join(''); ret = BbPromise.reject(new this.serverless.classes.Error(errorMessage)); } - this.tracker.add(variableString, ret, propertyString); + ret = this.tracker.add(variableString, ret, propertyString); } return ret; } From 4400ffc9e4c4c04802e32ee7dd25b9cbc3463c37 Mon Sep 17 00:00:00 2001 From: Erik Erikson Date: Fri, 16 Feb 2018 17:13:03 -0800 Subject: [PATCH 4/6] fix #4311 & #4734, separate PromiseTracker, minutiae #4311 see prepopulateService - attempts to pre-populate the region and stage settings necessary for making a request to AWS, rejecting and dependencies thereon it runs into during that process see the `deep` variable work. this was a knock-on effect of providing pre-population. it actually represents an obscure class of bugs where the recursive population previously in getDeepValue caused the caller not to be in charge of population choices (thus fixing for pre-population) but also caused potential deadlocks resulting from getDeepValue creating circular dependencies. #4734 see splitByComma - a regex to do the splitting but ignore quoted commas was getting very deep and involved. Instead, identify quoted string boundaries, then identify commas (including white space around them) and take those commas not contained by quotes as the locations for splitting the given string. Trim the string as well (removing leading and trailing white space). add sophistication to the overwrite syntax, allowing for whitespace and repetitions (for robustness) add sophistication to the string ref syntax, allowing for use in identifying multiple quoted strings in a variable (i.e. for overwrites) #NotCreated fix a bug I created earlier in the branch that caused reporting to be less informative (see renderMatches) separate PromiseTracker move this class into its own file for the purpose of increasing testability and offering reuse minutiae filter the properties given to populateVariables so as to avoid attempting resolution on non-variables. Efficiency and noise reduction change. Also cleans up the code (e.g. see populateObject and its use) cleaning of overwrite as a side effect offer variable cleaning as a idiom reorder the Variables.js `require`s to be in alphabetical order --- lib/classes/PromiseTracker.js | 52 ++++ lib/classes/PromiseTracker.test.js | 62 ++++ lib/classes/Variables.js | 284 +++++++++++++------ lib/classes/Variables.test.js | 156 +++++++++- lib/plugins/aws/provider/awsProvider.js | 40 ++- lib/plugins/aws/provider/awsProvider.test.js | 247 ++++++++++------ 6 files changed, 635 insertions(+), 206 deletions(-) create mode 100644 lib/classes/PromiseTracker.js create mode 100644 lib/classes/PromiseTracker.test.js diff --git a/lib/classes/PromiseTracker.js b/lib/classes/PromiseTracker.js new file mode 100644 index 000000000..f19424788 --- /dev/null +++ b/lib/classes/PromiseTracker.js @@ -0,0 +1,52 @@ + +const logWarning = require('./Error').logWarning; + +class PromiseTracker { + constructor() { + this.promiseList = []; + this.promiseMap = {}; + this.startTime = Date.now(); + } + start() { + this.interval = setInterval(this.report.bind(this), 2500); + } + report() { + const delta = Date.now() - this.startTime; + logWarning('################################################################################'); + logWarning(`# ${delta}: ${this.getSettled().length} of ${ + this.getAll().length} promises have settled`); + const pending = this.getPending(); + logWarning(`# ${delta}: ${pending.length} unsettled promises:`); + pending.forEach((promise) => { + logWarning(`# ${delta}: ${promise.waitList}`); + }); + logWarning('################################################################################'); + } + stop() { + clearInterval(this.interval); + } + add(variable, prms, specifier) { + const promise = prms; + promise.waitList = `${variable} waited on by: ${specifier}`; + promise.state = 'pending'; + promise.then( // creates a promise with the following effects but that we otherwise ignore + () => { promise.state = 'resolved'; }, + () => { promise.state = 'rejected'; }); + this.promiseList.push(promise); + this.promiseMap[variable] = promise; + return promise; + } + contains(variable) { + return variable in this.promiseMap; + } + get(variable, specifier) { + const promise = this.promiseMap[variable]; + promise.waitList += ` ${specifier}`; + return promise; + } + getPending() { return this.promiseList.filter(p => (p.state === 'pending')); } + getSettled() { return this.promiseList.filter(p => (p.state !== 'pending')); } + getAll() { return this.promiseList; } +} + +module.exports = PromiseTracker; diff --git a/lib/classes/PromiseTracker.test.js b/lib/classes/PromiseTracker.test.js new file mode 100644 index 000000000..94d9a3c76 --- /dev/null +++ b/lib/classes/PromiseTracker.test.js @@ -0,0 +1,62 @@ +'use strict'; + +/* eslint-disable no-unused-expressions */ + +const BbPromise = require('bluebird'); +const chai = require('chai'); + +const PromiseTracker = require('../../lib/classes/PromiseTracker'); + +chai.use(require('chai-as-promised')); + +const expect = chai.expect; + +/** + * Mostly this class is tested by its use in peer ~/lib/classes/Variables.js + * + * Mostly, I'm creating coverage but if errors are discovered, coverage for the specific cases + * can be created here. + */ +describe('PromiseTracker', () => { + let promiseTracker; + beforeEach(() => { + promiseTracker = new PromiseTracker(); + }); + it('logs a warning without throwing', () => { + promiseTracker.add('foo', BbPromise.resolve(), '${foo:}'); + promiseTracker.add('foo', BbPromise.delay(10), '${foo:}'); + promiseTracker.report(); // shouldn't throw + }); + it('reports no pending promises when none have been added', () => { + const promises = promiseTracker.getPending(); + expect(promises).to.be.an.instanceof(Array); + expect(promises.length).to.equal(0); + }); + it('reports one pending promise when one has been added', () => { + const promise = BbPromise.delay(10); + promiseTracker.add('foo', promise, '${foo:}'); + const promises = promiseTracker.getPending(); + expect(promises).to.be.an.instanceof(Array); + expect(promises.length).to.equal(1); + expect(promises[0]).to.equal(promise); + }); + it('reports no settled promises when none have been added', () => { + const promises = promiseTracker.getSettled(); + expect(promises).to.be.an.instanceof(Array); + expect(promises.length).to.equal(0); + }); + it('reports one settled promise when one has been added', () => { + const promise = BbPromise.resolve(); + promiseTracker.add('foo', promise, '${foo:}'); + return BbPromise.delay(1).then(() => { + const promises = promiseTracker.getSettled(); + expect(promises).to.be.an.instanceof(Array); + expect(promises.length).to.equal(1); + expect(promises[0]).to.equal(promise); + }); + }); + it('reports no promises when none have been added', () => { + const promises = promiseTracker.getAll(); + expect(promises).to.be.an('array').that.is.empty; + }); +}); diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js index ffbe65f3a..4c19f43c7 100644 --- a/lib/classes/Variables.js +++ b/lib/classes/Variables.js @@ -1,60 +1,14 @@ 'use strict'; const _ = require('lodash'); -const path = require('path'); -const replaceall = require('replaceall'); -const logWarning = require('./Error').logWarning; const BbPromise = require('bluebird'); const os = require('os'); -const fse = require('../utils/fs/fse'); +const path = require('path'); +const replaceall = require('replaceall'); -class PromiseTracker { - constructor() { - this.promiseList = []; - this.promiseMap = {}; - this.startTime = Date.now(); - } - start() { - this.interval = setInterval(this.report.bind(this), 2500); - } - report() { - const delta = Date.now() - this.startTime; - logWarning('################################################################################'); - logWarning(`# ${delta}: ${this.getSettled().length} of ${ - this.getAll().length} promises have settled`); - const pending = this.getPending(); - logWarning(`# ${delta}: ${pending.length} unsettled promises:`); - pending.forEach((promise) => { - logWarning(`# ${delta}: ${promise.waitList}`); - }); - logWarning('################################################################################'); - } - stop() { - clearInterval(this.interval); - } - add(variable, prms, specifier) { - const promise = prms; - promise.waitList = `${variable} waited on by: ${specifier}`; - promise.state = 'pending'; - promise.then( // creates a promise with the following effects but that we otherwise ignore - () => { promise.state = 'resolved'; }, - () => { promise.state = 'rejected'; }); - this.promiseList.push(promise); - this.promiseMap[variable] = promise; - return promise; - } - contains(variable) { - return variable in this.promiseMap; - } - get(variable, specifier) { - const promise = this.promiseMap[variable]; - promise.waitList += ` ${specifier}`; - return promise; - } - getPending() { return this.promiseList.filter(p => (p.state === 'pending')); } - getSettled() { return this.promiseList.filter(p => (p.state !== 'pending')); } - getAll() { return this.promiseList; } -} +const fse = require('../utils/fs/fse'); +const logWarning = require('./Error').logWarning; +const PromiseTracker = require('./PromiseTracker'); class Variables { constructor(serverless) { @@ -62,14 +16,16 @@ class Variables { this.service = this.serverless.service; this.tracker = new PromiseTracker(); - this.overwriteSyntax = RegExp(/,/g); + this.deep = []; + this.deepRefSyntax = RegExp(/^(\${)?deep:\d+(\.[^}]+)*()}?$/); + this.overwriteSyntax = RegExp(/\s*(?:,\s*)+/g); this.fileRefSyntax = RegExp(/^file\((~?[a-zA-Z0-9._\-/]+?)\)/g); this.envRefSyntax = RegExp(/^env:/g); this.optRefSyntax = RegExp(/^opt:/g); this.selfRefSyntax = RegExp(/^self:/g); this.cfRefSyntax = RegExp(/^cf:/g); this.s3RefSyntax = RegExp(/^s3:(.+?)\/(.+)$/); - this.stringRefSyntax = RegExp(/('.*')|(".*")/g); + this.stringRefSyntax = RegExp(/(?:('|").*?\1)/g); this.ssmRefSyntax = RegExp(/^ssm:([a-zA-Z0-9_.\-/]+)[~]?(true|false)?/); } @@ -79,6 +35,68 @@ class Variables { // ############# // ## SERVICE ## // ############# + prepopulateService() { + const dependentServices = [ + { name: 'CloudFormation', regex: this.cfRefSyntax }, + { name: 'S3', regex: this.s3RefSyntax }, + { name: 'SSM', regex: this.ssmRefSyntax }, + ]; + const dependencyMessage = (configName, configValue, serviceName) => + `Variable Failure: value ${ + configName + } set to '${ + configValue + }' references ${ + serviceName + } which requires a ${ + configName + } value for use.`; + const getVariableParts = (variableString) => { + const matches = this.getMatches(variableString); + return matches.reduce( + (accumulation, current) => + accumulation.concat(this.splitByComma(current.variable)), + []); + }; + const serviceMatch = variablePart => + dependentServices.find((service) => { + let variable = variablePart; + if (variable.match(this.deepRefSyntax)) { + variable = this.getVariableFromDeep(variablePart); + } + return variable.match(service.regex); + }); + const getUntilValid = (config) => { + const parts = getVariableParts(config.value); + const service = parts.reduce( + (accumulation, part) => accumulation || serviceMatch(part), + undefined); + if (service) { + const msg = dependencyMessage(config.name, config.value, service.name); + return BbPromise.reject(new this.serverless.classes.Error(msg)); + } + return this.populateValue(config.value, false) + .then(populated => ( + populated.match(this.variableSyntax) ? + getUntilValid(_.assign(config, { value: populated })) : + _.assign({}, config, { populated }) + )); + }; + + const provider = this.serverless.getProvider('aws'); + if (provider) { + const requiredConfig = [ + _.assign({ name: 'region' }, provider.getRegionSourceValue()), + _.assign({ name: 'stage' }, provider.getStageSourceValue()), + ]; + const configToPopulate = requiredConfig.filter(config => + !_.isUndefined(config.value) && + (_.isString(config.value) && config.value.match(this.variableSyntax))); + const configPromises = configToPopulate.map(getUntilValid); + return this.assignProperties(provider, configPromises); + } + return BbPromise.resolve(); + } /** * Populate all variables in the service, conviently remove and restore the service attributes * that confuse the population methods. @@ -95,7 +113,8 @@ class Variables { this.service.provider.variableSyntax = undefined; // otherwise matches itself this.service.serverless = undefined; this.tracker.start(); - return this.populateObject(this.service) + return this.prepopulateService() + .then(() => this.populateObject(this.service)) .finally(() => { // restore this.tracker.stop(); @@ -179,10 +198,13 @@ class Variables { * @returns {Promise[]} The promises that will resolve to the * populated values of the given terminal properties */ - populateProperties(properties) { - return _.map(properties, property => - this.populateValue(property.value, false) - .then(populated => _.assign({}, property, { populated }))); + populateVariables(properties) { + const variables = properties.filter(property => + _.isString(property.value) && + property.value.match(this.variableSyntax)); + return _.map(variables, + variable => this.populateValue(variable.value, false) + .then(populated => _.assign({}, variable, { populated }))); } /** * Assign the populated values back to the target object @@ -193,16 +215,11 @@ class Variables { */ assignProperties(target, populations) { // eslint-disable-line class-methods-use-this return BbPromise.all(populations) - .then((results) => { - let changes = 0; - results.forEach((result) => { - if (result.value !== result.populated) { - changes += 1; - _.set(target, result.path, result.populated); - } - }); - return BbPromise.resolve(changes); - }); + .then((results) => results.forEach((result) => { + if (result.value !== result.populated) { + _.set(target, result.path, result.populated); + } + })); } /** * Populate the variables in the given object. @@ -211,18 +228,27 @@ class Variables { */ populateObject(objectToPopulate) { const leaves = this.getProperties(objectToPopulate, true, objectToPopulate); - const populations = this.populateProperties(leaves); + const populations = this.populateVariables(leaves); return this.assignProperties(objectToPopulate, populations) - .then((changes) => { - if (changes) { - return this.populateObject(objectToPopulate); - } - return objectToPopulate; - }); + .then(() => (populations.length ? + this.populateObject(objectToPopulate) : + objectToPopulate)); } // ############## // ## PROPERTY ## // ############## + /** + * Standard logic for cleaning a variable + * Example: cleanVariable('${opt:foo}') => 'opt:foo' + * @param match The variable match instance variable part + * @returns {string} The cleaned variable match + */ + cleanVariable(match) { + return match.replace( + this.variableSyntax, + (context, contents) => contents.trim() + ).replace(/\s/g, ''); + } /** * @typedef {Object} MatchResult * @property {String} match The original property value that matched the variable syntax @@ -244,8 +270,7 @@ class Variables { } return _.map(matches, match => ({ match, - variable: match.replace(this.variableSyntax, (context, contents) => contents.trim()) - .replace(/\s/g, ''), + variable: this.cleanVariable(match), })); } /** @@ -256,10 +281,11 @@ class Variables { */ populateMatches(matches) { return _.map(matches, (match) => { - if (match.variable.match(this.overwriteSyntax)) { - return this.overwrite(match.variable); + const parts = this.splitByComma(match.variable); + if (parts.length > 1) { + return this.overwrite(parts, match.match); } - return this.getValueFromSource(match.variable, match.match); + return this.getValueFromSource(parts[0], match.match); }); } /** @@ -272,7 +298,7 @@ class Variables { renderMatches(value, matches, results) { let result = value; for (let i = 0; i < matches.length; i += 1) { - this.warnIfNotFound(matches[i].match, results[i]); + this.warnIfNotFound(matches[i].variable, results[i]); result = this.populateVariable(result, matches[i].match, results[i]); } return result; @@ -340,16 +366,57 @@ class Variables { // ## VARIABLES ## // ############### /** - * Overwrite the given variable string, resolve each variable and resolve to the first valid - * value. + * Split a given string by whitespace padded commas excluding those within single or double quoted + * strings. + * @param string The string to split by comma. + */ + splitByComma(string) { + const input = string.trim(); + const stringMatches = []; + let match = this.stringRefSyntax.exec(input); + while (match) { + stringMatches.push({ + start: match.index, + end: this.stringRefSyntax.lastIndex, + }); + match = this.stringRefSyntax.exec(input); + } + const commaReplacements = []; + const contained = commaMatch => // curry the current commaMatch + stringMatch => // check whether stringMatch containing the commaMatch + stringMatch.start < commaMatch.index && + this.overwriteSyntax.lastIndex < stringMatch.end; + match = this.overwriteSyntax.exec(input); + while (match) { + const matchContained = contained(match); + const containedBy = stringMatches.find(matchContained); + if (!containedBy) { // if uncontained, this comma respresents a splitting location + commaReplacements.push({ + start: match.index, + end: this.overwriteSyntax.lastIndex, + }); + } + match = this.overwriteSyntax.exec(input); + } + let prior = 0; + const results = []; + commaReplacements.forEach((replacement) => { + results.push(input.slice(prior, replacement.start)); + prior = replacement.end; + }); + results.push(input.slice(prior)); + return results; + } + /** + * Resolve the given variable string that expresses a series of fallback values in case the + * initial values are not valid, resolving each variable and resolving to the first valid value. * @param variableStringsString The overwrite string of variables to populate and choose from. * @returns {Promise.|*} A promise resolving to the first validly populating variable * in the given variable strings string. */ - overwrite(variableStringsString) { - const variableStrings = variableStringsString.split(','); + overwrite(variableStrings, propertyString) { const variableValues = variableStrings.map(variableString => - this.getValueFromSource(variableString, variableStringsString)); + this.getValueFromSource(variableString, propertyString)); const validValue = value => ( value !== null && typeof value !== 'undefined' && @@ -385,6 +452,8 @@ class Variables { ret = this.getValueFromString(variableString); } else if (variableString.match(this.ssmRefSyntax)) { ret = this.getValueFromSsm(variableString); + } else if (variableString.match(this.deepRefSyntax)) { + ret = this.getValueFromDeep(variableString); } else { const errorMessage = [ `Invalid variable reference syntax for variable ${variableString}.`, @@ -585,17 +654,52 @@ class Variables { }); } + getDeepIndex(variableString) { + const deepIndexReplace = RegExp(/^deep:|(\.[^}]+)*$/g); + return variableString.replace(deepIndexReplace, ''); + } + getVariableFromDeep(variableString) { + const index = this.getDeepIndex(variableString); + return this.deep[index]; + } + getValueFromDeep(variableString) { + const deepPrefixReplace = RegExp(/(?:^deep:)\d+\.?/g); + const variable = this.getVariableFromDeep(variableString); + const deepRef = variableString.replace(deepPrefixReplace, ''); + const sourceString = `\${deep:\${${variable}}${deepRef.length ? `.${deepRef}` : ''}}`; + return this.getValueFromSource(variable, sourceString); + } + getDeepValue(deepProperties, valueToPopulate) { return BbPromise.reduce(deepProperties, (computedValueToPopulateParam, subProperty) => { let computedValueToPopulate = computedValueToPopulateParam; - if (typeof computedValueToPopulate === 'undefined') { + if ( // in build deep variable mode + _.isString(computedValueToPopulate) && + computedValueToPopulate.match(this.deepRefSyntax) + ) { + if (subProperty !== '') { + computedValueToPopulate = `${ + computedValueToPopulate.slice(0, computedValueToPopulate.length - 1) + }.${ + subProperty + }}`; + } + return BbPromise.resolve(computedValueToPopulate); + } else if (typeof computedValueToPopulate === 'undefined') { // in get deep value mode computedValueToPopulate = {}; } else if (subProperty !== '' || '' in computedValueToPopulate) { computedValueToPopulate = computedValueToPopulate[subProperty]; } - if (typeof computedValueToPopulate === 'string' && - computedValueToPopulate.match(this.variableSyntax)) { - return this.populateValue(computedValueToPopulate, false); + if ( + typeof computedValueToPopulate === 'string' && + computedValueToPopulate.match(this.variableSyntax) + ) { + const computedVariable = this.cleanVariable(computedValueToPopulate); + let index = this.deep.findIndex((item) => computedVariable === item); + if (index < 0) { + index = this.deep.push(computedVariable) - 1; + } + return BbPromise.resolve(`\${deep:${index}}`); } return BbPromise.resolve(computedValueToPopulate); }, valueToPopulate); diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index 0beaf01ee..6520f7c2b 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -58,7 +58,8 @@ describe('Variables', () => { it('should call loadVariableSyntax and then populateProperty', () => { const loadVariableSyntaxStub = sinon.stub(serverless.variables, 'loadVariableSyntax') .returns(); - const populateObjectStub = sinon.stub(serverless.variables, 'populateObject').resolves(); + const populateObjectStub = sinon.stub(serverless.variables, 'populateObject') + .returns(BbPromise.resolve()); return expect(serverless.variables.populateService()).to.be.fulfilled .then(() => { expect(loadVariableSyntaxStub).to.be.calledOnce; @@ -82,6 +83,65 @@ describe('Variables', () => { .then().finally(() => populateObjectStub.restore()); }); }); + describe('#prepopulateService', () => { + // TL;DR: call populateService to test prepopulateService (note addition of 'pre') + // + // The prepopulateService method basically assumes invocation of of populateService (i.e. that + // variable syntax is loaded, and that the service object is cleaned up. Just use + // populateService to do that work. + let awsProvider; + let populateObjectStub; + let requestStub; // just in case... don't want to actually call... + beforeEach(() => { + awsProvider = new AwsProvider(serverless, {}); + populateObjectStub = sinon.stub(serverless.variables, 'populateObject', () => + BbPromise.resolve()); + requestStub = sinon.stub(awsProvider, 'request', () => + BbPromise.reject(new Error('unexpected'))); + }); + afterEach(() => { + populateObjectStub.restore(); + requestStub.restore(); + }); + const prepopulatedProperties = [ + { name: 'region', getter: (provider) => provider.getRegion() }, + { name: 'stage', getter: (provider) => provider.getStage() }, + ]; + describe('basic population tests', () => { + prepopulatedProperties.forEach((property) => { + it(`should populate variables in ${property.name} values`, () => { + awsProvider.options[property.name] = '${self:foobar, "default"}'; + return serverless.variables.populateService().should.be.fulfilled + .then(() => expect(property.getter(awsProvider)).to.be.eql('default')); + }); + }); + }); + // + describe('dependent service rejections', () => { + const dependentConfigs = [ + { value: '${cf:stack.value}', name: 'CloudFormation' }, + { value: '${s3:bucket/key}', name: 'S3' }, + { value: '${ssm:/path/param}', name: 'SSM' }, + ]; + prepopulatedProperties.forEach(property => { + dependentConfigs.forEach(config => { + it(`should reject ${config.name} variables in ${property.name} values`, () => { + awsProvider.options[property.name] = config.value; + return serverless.variables.populateService() + .should.be.rejectedWith('Variable Failure'); + }); + }); + }); + it('should reject recursively dependent service dependencies', () => { + serverless.variables.service.custom = { + settings: '${s3:bucket/key}', + }; + awsProvider.options.region = '${self:custom.settings.region}'; + return serverless.variables.populateService() + .should.be.rejectedWith('Variable Failure'); + }); + }); + }); describe('#getProperties', () => { it('extracts all terminal properties of an object', () => { @@ -123,6 +183,9 @@ describe('Variables', () => { }); describe('#populateObject()', () => { + beforeEach(() => { + serverless.variables.loadVariableSyntax(); + }); it('should populate object and return it', () => { const object = { stage: '${opt:stage}', // eslint-disable-line no-template-curly-in-string @@ -593,13 +656,58 @@ module.exports = { }); }); + describe('#splitByComma', () => { + it('should return a given empty string', () => { + const input = ''; + const expected = [input]; + expect(serverless.variables.splitByComma(input)).to.eql(expected); + }); + it('should return a undelimited string', () => { + const input = 'foo:bar'; + const expected = [input]; + expect(serverless.variables.splitByComma(input)).to.eql(expected); + }); + it('should split basic comma delimited strings', () => { + const input = 'my,values,to,split'; + const expected = ['my', 'values', 'to', 'split']; + expect(serverless.variables.splitByComma(input)).to.eql(expected); + }); + it('should remove leading and following white space', () => { + const input = ' \t\nfoobar\n\t '; + const expected = ['foobar']; + expect(serverless.variables.splitByComma(input)).to.eql(expected); + }); + it('should remove white space surrounding commas', () => { + const input = 'a,b ,c , d, e , f\t,g\n,h,\ti,\nj,\t\n , \n\tk'; + const expected = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k']; + expect(serverless.variables.splitByComma(input)).to.eql(expected); + }); + it('should ignore quoted commas', () => { + const input = '",", \',\', ",\', \',\'", "\',\', \',\'", \',", ","\', \'",", ","\''; + const expected = [ + '","', + '\',\'', + '",\', \',\'"', + '"\',\', \',\'"', + '\',", ","\'', + '\'",", ","\'', + ]; + expect(serverless.variables.splitByComma(input)).to.eql(expected); + }); + it('should deal with a combination of these cases', () => { + const input = ' \t\n\'a\'\t\n , \n\t"foo,bar", opt:foo, ",", \',\', "\',\', \',\'", foo\n\t '; + const expected = ['\'a\'', '"foo,bar"', 'opt:foo', '","', '\',\'', '"\',\', \',\'"', 'foo']; + expect(serverless.variables.splitByComma(input)).to.eql(expected); + }); + }); + describe('#overwrite()', () => { it('should overwrite undefined and null values', () => { const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves(undefined); getValueFromSourceStub.onCall(1).resolves(null); getValueFromSourceStub.onCall(2).resolves('variableValue'); - return serverless.variables.overwrite('opt:stage,env:stage,self:provider.stage') + return serverless.variables.overwrite(['opt:stage', 'env:stage', 'self:provider.stage']) .should.be.fulfilled .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); @@ -612,7 +720,7 @@ module.exports = { const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves({}); getValueFromSourceStub.onCall(1).resolves('variableValue'); - return serverless.variables.overwrite('opt:stage,env:stage').should.be.fulfilled + return serverless.variables.overwrite(['opt:stage', 'env:stage']).should.be.fulfilled .then((valueToPopulate) => { expect(valueToPopulate).to.equal('variableValue'); expect(getValueFromSourceStub).to.have.been.calledTwice; @@ -624,7 +732,7 @@ module.exports = { const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves(0); getValueFromSourceStub.onCall(1).resolves('variableValue'); - return serverless.variables.overwrite('opt:stage,env:stage').should.become(0) + return serverless.variables.overwrite(['opt:stage', 'env:stage']).should.become(0) .then().finally(() => getValueFromSourceStub.restore()); }); @@ -632,7 +740,7 @@ module.exports = { const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource'); getValueFromSourceStub.onCall(0).resolves(false); getValueFromSourceStub.onCall(1).resolves('variableValue'); - return serverless.variables.overwrite('opt:stage,env:stage').should.become(false) + return serverless.variables.overwrite(['opt:stage', 'env:stage']).should.become(false) .then().finally(() => getValueFromSourceStub.restore()); }); @@ -641,11 +749,20 @@ module.exports = { getValueFromSourceStub.onCall(0).resolves(undefined); getValueFromSourceStub.onCall(1).resolves('variableValue'); getValueFromSourceStub.onCall(2).resolves('variableValue2'); - return serverless.variables.overwrite('opt:stage,env:stage,self:provider.stage') + return serverless.variables.overwrite(['opt:stage', 'env:stage', 'self:provider.stage']) .should.be.fulfilled .then(valueToPopulate => expect(valueToPopulate).to.equal('variableValue')) .finally(() => getValueFromSourceStub.restore()); }); + it('should properly handle string values containing commas', () => { + const str = '"foo,bar"'; + const getValueFromSourceStub = sinon.stub(serverless.variables, 'getValueFromSource') + .resolves(undefined); + return serverless.variables.overwrite(['opt:stage', str]) + .should.be.fulfilled + .then(() => expect(getValueFromSourceStub.getCall(1).args[0]).to.eql(str)) + .finally(() => getValueFromSourceStub.restore()); + }); }); describe('#getValueFromSource()', () => { @@ -1261,25 +1378,36 @@ module.exports = { return serverless.variables.getDeepValue(['custom', 'subProperty', 'deep', 'deeper'], valueToPopulateMock).should.eventually.deep.equal({}); }); - - it('should get deep values with variable references', () => { + it('should return a simple deep variable when final deep value is variable', () => { serverless.variables.service = { service: 'testService', custom: { - anotherVar: '${self:custom.var}', // eslint-disable-line no-template-curly-in-string subProperty: { // eslint-disable-next-line no-template-curly-in-string deep: '${self:custom.anotherVar.veryDeep}', }, - var: { - veryDeep: 'someValue', - }, }, provider: serverless.service.provider, }; serverless.variables.loadVariableSyntax(); - return serverless.variables.getDeepValue(['custom', 'subProperty', 'deep'], - serverless.variables.service).should.become('someValue'); + return serverless.variables.getDeepValue( + ['custom', 'subProperty', 'deep'], + serverless.variables.service + ).should.become('${deep:0}'); + }); + it('should return a deep continuation when middle deep value is variable', () => { + serverless.variables.service = { + service: 'testService', + custom: { + anotherVar: '${self:custom.var}', // eslint-disable-line no-template-curly-in-string + }, + provider: serverless.service.provider, + }; + serverless.variables.loadVariableSyntax(); + return serverless.variables.getDeepValue( + ['custom', 'anotherVar', 'veryDeep'], + serverless.variables.service) + .should.become('${deep:0.veryDeep}'); }); }); describe('#warnIfNotFound()', () => { diff --git a/lib/plugins/aws/provider/awsProvider.js b/lib/plugins/aws/provider/awsProvider.js index 6eae516f6..4d03455f2 100644 --- a/lib/plugins/aws/provider/awsProvider.js +++ b/lib/plugins/aws/provider/awsProvider.js @@ -330,13 +330,28 @@ class AwsProvider { credentials.useAccelerateEndpoint = true; // eslint-disable-line no-param-reassign } + getValues(source, paths) { + return paths.map(path => ({ + path, + value: _.get(source, path.join('.')), + })); + } + firstValue(values) { + return values.reduce((result, current) => (result.value ? result : current), {}); + } + + getRegionSourceValue() { + const values = this.getValues(this, [ + ['options', 'region'], + ['serverless', 'config', 'region'], + ['serverless', 'service', 'provider', 'region'], + ]); + return this.firstValue(values); + } getRegion() { const defaultRegion = 'us-east-1'; - - return _.get(this, 'options.region') - || _.get(this, 'serverless.config.region') - || _.get(this, 'serverless.service.provider.region') - || defaultRegion; + const regionSourceValue = this.getRegionSourceValue(); + return regionSourceValue.value || defaultRegion; } getServerlessDeploymentBucketName() { @@ -352,13 +367,18 @@ class AwsProvider { ).then((result) => result.StackResourceDetail.PhysicalResourceId); } + getStageSourceValue() { + const values = this.getValues(this, [ + ['options', 'stage'], + ['serverless', 'config', 'stage'], + ['serverless', 'service', 'provider', 'stage'], + ]); + return this.firstValue(values); + } getStage() { const defaultStage = 'dev'; - - return _.get(this, 'options.stage') - || _.get(this, 'serverless.config.stage') - || _.get(this, 'serverless.service.provider.stage') - || defaultStage; + const stageSourceValue = this.getStageSourceValue(); + return stageSourceValue.value || defaultStage; } getAccountId() { diff --git a/lib/plugins/aws/provider/awsProvider.test.js b/lib/plugins/aws/provider/awsProvider.test.js index 297b298b6..f69e97774 100644 --- a/lib/plugins/aws/provider/awsProvider.test.js +++ b/lib/plugins/aws/provider/awsProvider.test.js @@ -72,118 +72,118 @@ describe('AwsProvider', () => { // clear env delete process.env.AWS_CLIENT_TIMEOUT; }); - }); - describe('#constructor() certificate authority - environment variable', () => { - afterEach('Environment Variable Cleanup', () => { - // clear env - delete process.env.ca; - }); - it('should set AWS ca single', () => { - process.env.ca = '-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----'; - const newAwsProvider = new AwsProvider(serverless, options); + describe('certificate authority - environment variable', () => { + afterEach('Environment Variable Cleanup', () => { + // clear env + delete process.env.ca; + }); + it('should set AWS ca single', () => { + process.env.ca = '-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----'; + const newAwsProvider = new AwsProvider(serverless, options); - expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); + expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); + }); + + it('should set AWS ca multiple', () => { + const certContents = '-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----'; + process.env.ca = `${certContents},${certContents}`; + const newAwsProvider = new AwsProvider(serverless, options); + + expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); + }); }); - it('should set AWS ca multiple', () => { + describe('certificate authority - file', () => { const certContents = '-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----'; - process.env.ca = `${certContents},${certContents}`; - const newAwsProvider = new AwsProvider(serverless, options); + const tmpdir = os.tmpdir(); + let file1 = null; + let file2 = null; + beforeEach('Create CA Files and env vars', () => { + file1 = path.join(tmpdir, 'ca1.txt'); + file2 = path.join(tmpdir, 'ca2.txt'); + fs.writeFileSync(file1, certContents); + fs.writeFileSync(file2, certContents); + }); - expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); - }); - }); + afterEach('CA File Cleanup', () => { + // delete files + fs.unlinkSync(file1); + fs.unlinkSync(file2); + // clear env + delete process.env.ca; + delete process.env.cafile; + }); - describe('#constructor() certificate authority - file', () => { - const certContents = '-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----'; - const tmpdir = os.tmpdir(); - let file1 = null; - let file2 = null; - beforeEach('Create CA Files and env vars', () => { - file1 = path.join(tmpdir, 'ca1.txt'); - file2 = path.join(tmpdir, 'ca2.txt'); - fs.writeFileSync(file1, certContents); - fs.writeFileSync(file2, certContents); + it('should set AWS cafile single', () => { + process.env.cafile = file1; + const newAwsProvider = new AwsProvider(serverless, options); + + expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); + }); + + it('should set AWS cafile multiple', () => { + process.env.cafile = `${file1},${file2}`; + const newAwsProvider = new AwsProvider(serverless, options); + + expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); + }); + + it('should set AWS ca and cafile', () => { + process.env.ca = certContents; + process.env.cafile = file1; + const newAwsProvider = new AwsProvider(serverless, options); + + expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); + }); }); - afterEach('CA File Cleanup', () => { - // delete files - fs.unlinkSync(file1); - fs.unlinkSync(file2); - // clear env - delete process.env.ca; - delete process.env.cafile; - }); + describe('deploymentBucket configuration', () => { + it('should do nothing if not defined', () => { + serverless.service.provider.deploymentBucket = undefined; - it('should set AWS cafile single', () => { - process.env.cafile = file1; - const newAwsProvider = new AwsProvider(serverless, options); + const newAwsProvider = new AwsProvider(serverless, options); - expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); - }); + expect(newAwsProvider.serverless.service.provider.deploymentBucket).to.equal(undefined); + }); - it('should set AWS cafile multiple', () => { - process.env.cafile = `${file1},${file2}`; - const newAwsProvider = new AwsProvider(serverless, options); + it('should do nothing if the value is a string', () => { + serverless.service.provider.deploymentBucket = 'my.deployment.bucket'; - expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); - }); + const newAwsProvider = new AwsProvider(serverless, options); - it('should set AWS ca and cafile', () => { - process.env.ca = certContents; - process.env.cafile = file1; - const newAwsProvider = new AwsProvider(serverless, options); + expect(newAwsProvider.serverless.service.provider.deploymentBucket) + .to.equal('my.deployment.bucket'); + }); - expect(typeof newAwsProvider.sdk.config.httpOptions.agent).to.not.equal('undefined'); - }); - }); + it('should save a given object and use name from it', () => { + const deploymentBucketObject = { + name: 'my.deployment.bucket', + serverSideEncryption: 'AES256', + }; + serverless.service.provider.deploymentBucket = deploymentBucketObject; - describe('when checking for the deploymentBucket config', () => { - it('should do nothing if the deploymentBucket config is not used', () => { - serverless.service.provider.deploymentBucket = undefined; + const newAwsProvider = new AwsProvider(serverless, options); - const newAwsProvider = new AwsProvider(serverless, options); + expect(newAwsProvider.serverless.service.provider.deploymentBucket) + .to.equal('my.deployment.bucket'); + expect(newAwsProvider.serverless.service.provider.deploymentBucketObject) + .to.deep.equal(deploymentBucketObject); + }); - expect(newAwsProvider.serverless.service.provider.deploymentBucket).to.equal(undefined); - }); + it('should save a given object and nullify the name if one is not provided', () => { + const deploymentBucketObject = { + serverSideEncryption: 'AES256', + }; + serverless.service.provider.deploymentBucket = deploymentBucketObject; - it('should do nothing if the deploymentBucket config is a string', () => { - serverless.service.provider.deploymentBucket = 'my.deployment.bucket'; + const newAwsProvider = new AwsProvider(serverless, options); - const newAwsProvider = new AwsProvider(serverless, options); - - expect(newAwsProvider.serverless.service.provider.deploymentBucket) - .to.equal('my.deployment.bucket'); - }); - - it('should save the object and use the name for the deploymentBucket if provided', () => { - const deploymentBucketObject = { - name: 'my.deployment.bucket', - serverSideEncryption: 'AES256', - }; - serverless.service.provider.deploymentBucket = deploymentBucketObject; - - const newAwsProvider = new AwsProvider(serverless, options); - - expect(newAwsProvider.serverless.service.provider.deploymentBucket) - .to.equal('my.deployment.bucket'); - expect(newAwsProvider.serverless.service.provider.deploymentBucketObject) - .to.deep.equal(deploymentBucketObject); - }); - - it('should save the object and nullify the name if it is not provided', () => { - const deploymentBucketObject = { - serverSideEncryption: 'AES256', - }; - serverless.service.provider.deploymentBucket = deploymentBucketObject; - - const newAwsProvider = new AwsProvider(serverless, options); - - expect(newAwsProvider.serverless.service.provider.deploymentBucket) - .to.equal(null); - expect(newAwsProvider.serverless.service.provider.deploymentBucketObject) - .to.deep.equal(deploymentBucketObject); + expect(newAwsProvider.serverless.service.provider.deploymentBucket) + .to.equal(null); + expect(newAwsProvider.serverless.service.provider.deploymentBucketObject) + .to.deep.equal(deploymentBucketObject); + }); }); }); @@ -693,6 +693,69 @@ describe('AwsProvider', () => { }); }); + describe('values', () => { + const obj = { + a: 'b', + c: { + d: 'e', + f: { + g: 'h', + }, + }, + }; + const paths = [ + ['a'], + ['c', 'd'], + ['c', 'f', 'g'], + ]; + const getExpected = [ + { path: paths[0], value: obj.a }, + { path: paths[1], value: obj.c.d }, + { path: paths[2], value: obj.c.f.g }, + ]; + describe('#getValues', () => { + it('should return an array of values given paths to them', () => { + expect(awsProvider.getValues(obj, paths)).to.eql(getExpected); + }); + }); + describe('#firstValue', () => { + it('should ignore entries without a \'value\' attribute', () => { + const input = _.cloneDeep(getExpected); + delete input[0].value; + delete input[2].value; + expect(awsProvider.firstValue(input)).to.eql(getExpected[1]); + }); + it('should ignore entries with an undefined \'value\' attribute', () => { + const input = _.cloneDeep(getExpected); + input[0].value = undefined; + input[2].value = undefined; + expect(awsProvider.firstValue(input)).to.eql(getExpected[1]); + }); + it('should return the first value', () => { + expect(awsProvider.firstValue(getExpected)).to.equal(getExpected[0]); + }); + it('should return the middle value', () => { + const input = _.cloneDeep(getExpected); + delete input[0].value; + delete input[2].value; + expect(awsProvider.firstValue(input)).to.equal(input[1]); + }); + it('should return the last value', () => { + const input = _.cloneDeep(getExpected); + delete input[0].value; + delete input[1].value; + expect(awsProvider.firstValue(input)).to.equal(input[2]); + }); + it('should return the last object if none have valid values', () => { + const input = _.cloneDeep(getExpected); + delete input[0].value; + delete input[1].value; + delete input[2].value; + expect(awsProvider.firstValue(input)).to.equal(input[2]); + }); + }); + }); + describe('#getRegion()', () => { let newAwsProvider; From 91f50f691897a644eac14648ae07bc3febb4d163 Mon Sep 17 00:00:00 2001 From: Erik Erikson Date: Fri, 16 Feb 2018 17:26:40 -0800 Subject: [PATCH 5/6] add 'use strict' for node 4.x and 5.x --- lib/classes/PromiseTracker.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/classes/PromiseTracker.js b/lib/classes/PromiseTracker.js index f19424788..f76385355 100644 --- a/lib/classes/PromiseTracker.js +++ b/lib/classes/PromiseTracker.js @@ -1,3 +1,4 @@ +'use strict'; const logWarning = require('./Error').logWarning; From ffc7686465261d29b2aef8616cc5222ca1dd7829 Mon Sep 17 00:00:00 2001 From: Erik Erikson Date: Fri, 16 Feb 2018 18:46:28 -0800 Subject: [PATCH 6/6] fix pending promise test bugs --- lib/classes/PromiseTracker.test.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/classes/PromiseTracker.test.js b/lib/classes/PromiseTracker.test.js index 94d9a3c76..0202770d8 100644 --- a/lib/classes/PromiseTracker.test.js +++ b/lib/classes/PromiseTracker.test.js @@ -33,12 +33,15 @@ describe('PromiseTracker', () => { expect(promises.length).to.equal(0); }); it('reports one pending promise when one has been added', () => { - const promise = BbPromise.delay(10); + let resolve; + const promise = new BbPromise((rslv) => { resolve = rslv; }); promiseTracker.add('foo', promise, '${foo:}'); - const promises = promiseTracker.getPending(); - expect(promises).to.be.an.instanceof(Array); - expect(promises.length).to.equal(1); - expect(promises[0]).to.equal(promise); + return BbPromise.delay(1).then(() => { + const promises = promiseTracker.getPending(); + expect(promises).to.be.an.instanceof(Array); + expect(promises.length).to.equal(1); + expect(promises[0]).to.equal(promise); + }).then(() => { resolve(); }); }); it('reports no settled promises when none have been added', () => { const promises = promiseTracker.getSettled(); @@ -48,12 +51,11 @@ describe('PromiseTracker', () => { it('reports one settled promise when one has been added', () => { const promise = BbPromise.resolve(); promiseTracker.add('foo', promise, '${foo:}'); - return BbPromise.delay(1).then(() => { - const promises = promiseTracker.getSettled(); - expect(promises).to.be.an.instanceof(Array); - expect(promises.length).to.equal(1); - expect(promises[0]).to.equal(promise); - }); + promise.state = 'resolved'; + const promises = promiseTracker.getSettled(); + expect(promises).to.be.an.instanceof(Array); + expect(promises.length).to.equal(1); + expect(promises[0]).to.equal(promise); }); it('reports no promises when none have been added', () => { const promises = promiseTracker.getAll();