From 3df05b04f124fbc640cb99f2811babda72b55e5e Mon Sep 17 00:00:00 2001 From: "Eslam A. Hefnawy" Date: Fri, 20 May 2016 19:39:17 +0200 Subject: [PATCH] moved getPopulated logic to load() --- lib/classes/Service.js | 186 ++++++++++++++++++++++++++------------- tests/classes/Service.js | 148 ++++++++++++++----------------- 2 files changed, 190 insertions(+), 144 deletions(-) diff --git a/lib/classes/Service.js b/lib/classes/Service.js index 284c5c95f..8b503d939 100644 --- a/lib/classes/Service.js +++ b/lib/classes/Service.js @@ -1,7 +1,6 @@ 'use strict'; const SError = require('./Error'); -const BbPromise = require('bluebird'); const path = require('path'); const _ = require('lodash'); const traverse = require('traverse'); @@ -24,9 +23,9 @@ class Service { if (data) this.update(data); } - load() { + load(opts) { const that = this; - + const options = opts || {}; const servicePath = that.S.instances.config.servicePath; if (!servicePath) { @@ -46,82 +45,147 @@ class Service { .then(() => { that.S.instances.yamlParser.parse(path.join(servicePath, 'serverless.env.yaml')); }) - .then((serverlessEnvYaml) => { - that.environment = serverlessEnvYaml; - return BbPromise.resolve(that); - }); - } + .then((serverlessEnvYaml) => (that.environment = serverlessEnvYaml)) + .then(() => { + // temporally remove environment obj. Doesn't make sense to + // populate environment (stages, regions, vars) + const environment = this.environment; + this.environment = null; - getPopulated(opts) { - const options = opts || {}; - const serviceClone = _.cloneDeep(this); + // Validate: Check stage exists + if (options.stage) this.getStage(options.stage); - // temporally remove environment obj. Doesn't make sense to - // populate environment (stages, regions, vars) - serviceClone.environment = null; + // Validate: Check region exists in stage + if (options.region) this.getRegionInStage(options.stage, options.region); - // Validate: Check stage exists - if (options.stage) this.getStage(options.stage); + let varTemplateSyntax = /\${([\s\S]+?)}/g; - // Validate: Check region exists in stage - if (options.region) this.getRegionInStage(options.stage, options.region); + if (this.variableSyntax) { + varTemplateSyntax = RegExp(this.variableSyntax, 'g'); - let varTemplateSyntax = /\${([\s\S]+?)}/g; + // temporally remove variable syntax from service otherwise it'll match + this.variableSyntax = true; + } - if (this.variableSyntax) { - varTemplateSyntax = RegExp(serviceClone.variableSyntax, 'g'); + const variablesObject = this.getVariables(options.stage, options.region); - // temporally remove variable syntax from service otherwise it'll match - serviceClone.variableSyntax = null; - } + /* + * we can't use an arrow function in this case cause that would + * change the lexical scoping required by the traverse module + */ + traverse(this).forEach(function (valParam) { + const t = this; + let val = valParam; - const variablesObject = this.getVariables(options.stage, options.region); + // check if the current string is a variable + if (typeof(val) === 'string' && val.match(varTemplateSyntax)) { + // get all ${variable} in the string - /* - * we can't use an arrow function in this case cause that would - * change the lexical scoping required by the traverse module - */ - traverse(serviceClone).forEach(function (valParam) { - const t = this; - let val = valParam; + val.match(varTemplateSyntax).forEach((variableSyntax) => { + const variableName = variableSyntax + .replace(varTemplateSyntax, (match, varName) => varName.trim()); + let value; - // check if the current string is a variable - if (typeof(val) === 'string' && val.match(varTemplateSyntax)) { - // get all ${variable} in the string + if (variableName in variablesObject) { + value = variablesObject[variableName]; + } + // Populate + if (!value && !value !== '') { + // SCLI.log('WARNING: This variable is not defined: ' + variableName); + } else if (typeof value === 'string') { + // for string variables, we use replaceall in case the user + // includes the variable as a substring (ie. "hello ${name}") + val = replaceall(variableSyntax, value, val); + } else { + val = value; + } + }); - val.match(varTemplateSyntax).forEach((variableSyntax) => { - const variableName = variableSyntax - .replace(varTemplateSyntax, (match, varName) => varName.trim()); - let value; - - if (variableName in variablesObject) { - value = variablesObject[variableName]; - } - // Populate - if (!value && !value !== '') { - // SCLI.log('WARNING: This variable is not defined: ' + variableName); - } else if (typeof value === 'string') { - // for string variables, we use replaceall in case the user - // includes the variable as a substring (ie. "hello ${name}") - val = replaceall(variableSyntax, value, val); - } else { - val = value; + // Replace + t.update(val); } }); - // Replace - t.update(val); - } - }); + // put back environment that we temporally removed earlier + this.environment = environment; - // put back environment that we temporally removed earlier - serviceClone.environment = this.environment; + // put back variable syntax if we removed it for processing + if (this.variableSyntax) this.variableSyntax = varTemplateSyntax; - // put back variable syntax if we removed it for processing - if (this.variableSyntax) serviceClone.variableSyntax = varTemplateSyntax; - return serviceClone; + return this; + }); } + // getPopulated(opts) { + // const options = opts || {}; + // const serviceClone = _.cloneDeep(this); + // + // // temporally remove environment obj. Doesn't make sense to + // // populate environment (stages, regions, vars) + // serviceClone.environment = null; + // + // // Validate: Check stage exists + // if (options.stage) this.getStage(options.stage); + // + // // Validate: Check region exists in stage + // if (options.region) this.getRegionInStage(options.stage, options.region); + // + // let varTemplateSyntax = /\${([\s\S]+?)}/g; + // + // if (this.variableSyntax) { + // varTemplateSyntax = RegExp(serviceClone.variableSyntax, 'g'); + // + // // temporally remove variable syntax from service otherwise it'll match + // serviceClone.variableSyntax = null; + // } + // + // const variablesObject = this.getVariables(options.stage, options.region); + // + // /* + // * we can't use an arrow function in this case cause that would + // * change the lexical scoping required by the traverse module + // */ + // traverse(serviceClone).forEach(function (valParam) { + // const t = this; + // let val = valParam; + // + // // check if the current string is a variable + // if (typeof(val) === 'string' && val.match(varTemplateSyntax)) { + // // get all ${variable} in the string + // + // val.match(varTemplateSyntax).forEach((variableSyntax) => { + // const variableName = variableSyntax + // .replace(varTemplateSyntax, (match, varName) => varName.trim()); + // let value; + // + // if (variableName in variablesObject) { + // value = variablesObject[variableName]; + // } + // // Populate + // if (!value && !value !== '') { + // // SCLI.log('WARNING: This variable is not defined: ' + variableName); + // } else if (typeof value === 'string') { + // // for string variables, we use replaceall in case the user + // // includes the variable as a substring (ie. "hello ${name}") + // val = replaceall(variableSyntax, value, val); + // } else { + // val = value; + // } + // }); + // + // // Replace + // t.update(val); + // } + // }); + // + // // put back environment that we temporally removed earlier + // serviceClone.environment = this.environment; + // + // // put back variable syntax if we removed it for processing + // if (this.variableSyntax) serviceClone.variableSyntax = varTemplateSyntax; + // return serviceClone; + // } + update(data) { return _.merge(this, data); } diff --git a/tests/classes/Service.js b/tests/classes/Service.js index 8798186c8..a0a0d4eed 100644 --- a/tests/classes/Service.js +++ b/tests/classes/Service.js @@ -66,9 +66,10 @@ describe('Service', () => { const SUtils = new Utils(); const tmpDirPath = path.join(os.tmpdir(), (new Date).getTime().toString()); const serverlessYaml = { - service: 'testService', + service: '${testVar}', custom: { - customProp: 'value', + digit: '${testDigit}', + substring: 'Hello ${testSubstring}', }, plugins: ['testPlugin'], functions: { @@ -83,71 +84,10 @@ describe('Service', () => { }, }; const serverlessEnvYaml = { - vars: { - varA: 'varA', - }, - stages: { - dev: { - vars: { - varB: 'varB', - }, - regions: { - aws_useast1: { - vars: { - varC: 'varC', - }, - }, - }, - }, - }, - }; - - SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.yaml'), - YAML.dump(serverlessYaml)); - SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.env.yaml'), - YAML.dump(serverlessEnvYaml)); - - const S = new Serverless({ servicePath: tmpDirPath }); - serviceInstance = new Service(S); - }); - - /* - * Even though I wanna split this into multiple test cases - * that would mean loading from the filesystem on each test case (slow!!) - * and because the load() method returns a promise - * I can't put it in a before() block - */ - it('should load from filesystem', () => { - serviceInstance.load().then((loadedService) => { - expect(loadedService.service).to.be.equal('testService'); - expect(loadedService.custom).to.deep.equal({ customProp: 'value' }); - expect(loadedService.plugins).to.deep.equal(['testPlugin']); - expect(loadedService.environment.vars).to.deep.equal({ varA: 'varA' }); - expect(serviceInstance.environment.stages.dev.regions.aws_useast1.vars.varC) - .to.be.equal('varC'); - expect(loadedService.resources.aws).to.deep.equal({ resourcesProp: 'value' }); - expect(loadedService.resources.azure).to.deep.equal({}); - expect(loadedService.resources.google).to.deep.equal({}); - }); - }); - - it('should throw error if servicePath not configured', () => { - const S = new Serverless(); - serviceInstance = new Service(S); - expect(() => { serviceInstance.load(); }).to.throw(Error); - }); - }); - - describe('#getPopulated()', () => { - let serviceInstance; - beforeEach(() => { - const S = new Serverless(); - serviceInstance = new Service(S); - - serviceInstance.service = '${testVar}'; - serviceInstance.environment = { vars: { testVar: 'commonVar', + testDigit: 10, + testSubstring: 'World', }, stages: { dev: { @@ -164,43 +104,85 @@ describe('Service', () => { }, }, }; + + SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.yaml'), + YAML.dump(serverlessYaml)); + SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.env.yaml'), + YAML.dump(serverlessEnvYaml)); + + const S = new Serverless({ servicePath: tmpDirPath }); + serviceInstance = new Service(S); }); - it('should populate common variables', () => { - const populatedService = serviceInstance.getPopulated(); - expect(populatedService.service).to.be.equal('commonVar'); + it('should throw error if servicePath not configured', () => { + const S = new Serverless(); + const invalidService = new Service(S); + expect(() => invalidService.load()).to.throw(Error); }); - it('should populate stage variables', () => { + /* + * Even though I wanna split this into multiple test cases + * that would mean loading from the filesystem on each test case (slow!!) + * and because the load() method returns a promise + * I can't put it in a before() block + */ + it('should load and populate from filesystem', () => { + serviceInstance.load().then((loadedService) => { + expect(loadedService.service).to.be.equal('commonVar'); + expect(loadedService.plugins).to.deep.equal(['testPlugin']); + const commonVars = { + testVar: 'commonVar', + testDigit: 10, + testSubstring: 'Sorld', + }; + expect(loadedService.environment.vars).to.deep.equal(commonVars); + expect(serviceInstance.environment.stages.dev.regions.aws_useast1.vars.testVar) + .to.be.equal('regionVar'); + expect(loadedService.resources.aws).to.deep.equal({ resourcesProp: 'value' }); + expect(loadedService.resources.azure).to.deep.equal({}); + expect(loadedService.resources.google).to.deep.equal({}); + }); + }); + + it('should load and populate stage vars', () => { const options = { stage: 'dev', }; - const populatedService = serviceInstance.getPopulated(options); - expect(populatedService.service).to.be.equal('stageVar'); + serviceInstance.load(options).then((loadedService) => { + expect(loadedService.service).to.be.equal('stageVar'); + }); }); - it('should populate region variables', () => { + it('should load and populate region vars', () => { const options = { stage: 'dev', region: 'aws_useast1', }; - const populatedService = serviceInstance.getPopulated(options); - expect(populatedService.service).to.be.equal('regionVar'); + serviceInstance.load(options).then((loadedService) => { + expect(loadedService.service).to.be.equal('regionVar'); + }); }); - it('should populate with custom variable syntax', () => { + it('should load and populate non string variables', () => { + serviceInstance.load().then((loadedService) => { + expect(loadedService.custom.digit).to.be.equal(10); + }); + }); + + it('should load and populate substring variables', () => { + serviceInstance.load().then((loadedService) => { + expect(loadedService.custom.substring).to.be.equal('Hello World'); + }); + }); + + it('should load and populate with custom variable syntax', () => { serviceInstance.service = '${{testVar}}'; serviceInstance.variableSyntax = '\\${{([\\s\\S]+?)}}'; - const populatedService = serviceInstance.getPopulated(); - expect(populatedService.service).to.be.equal('commonVar'); + serviceInstance.load().then((loadedService) => { + expect(loadedService.service).to.be.equal('commonVar'); + }); delete serviceInstance.variableSyntax; }); - - it('should populate non string variables', () => { - serviceInstance.environment.vars.testVar = 10; - const populatedService = serviceInstance.getPopulated(); - expect(populatedService.service).to.be.equal(10); - }); }); describe('#update()', () => {