Merge pull request #1245 from NoxHarmonium/issue-1242-duplicate-artif…

…act-log
serverless-heaven · Sep 26, 2022 · 48e7aea · 48e7aea
2 parents 606c560 + 3442b98
commit 48e7aea
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 24 deletions.
diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js
@@ -6,6 +6,7 @@ const path = require('path');
 const fse = require('fs-extra');
 
 const Packagers = require('./packagers');
+const { isProviderGoogle } = require('./utils');
 
 function rebaseFileReferences(pathToPackageRoot, moduleVersion) {
   if (/^(?:file:[^/]{2}|\.\/|\.\.\/)/.test(moduleVersion)) {
@@ -444,7 +445,7 @@ module.exports = {
 
               // GOOGLE: Copy modules only if not google-cloud-functions
               //         GCF Auto installs the package json
-              if (_.get(this.serverless, 'service.provider.name') === 'google') {
+              if (isProviderGoogle(this.serverless)) {
                 return BbPromise.resolve();
               }
 

diff --git a/lib/packageModules.js b/lib/packageModules.js
@@ -7,7 +7,7 @@ const glob = require('glob');
 const archiver = require('archiver');
 const semver = require('semver');
 const fs = require('fs');
-const { getAllNodeFunctions } = require('./utils');
+const { getAllNodeFunctions, isProviderGoogle } = require('./utils');
 
 function setArtifactPath(funcName, func, artifactPath) {
   const version = this.serverless.getVersion();
@@ -223,43 +223,40 @@ module.exports = {
     // When invoked as a part of `deploy function`,
     // only function passed with `-f` flag should be processed.
     const functionNames = this.options.function ? [this.options.function] : getAllNodeFunctions.call(this);
+    const serviceName = this.serverless.service.getServiceObject().name;
+    const individualPackagingEnabled = isIndividualPackaging.call(this);
+    const providerIsGoogle = isProviderGoogle(this.serverless);
 
     // Copy artifacts to package location
-    if (isIndividualPackaging.call(this)) {
+    if (individualPackagingEnabled) {
       _.forEach(functionNames, funcName => copyArtifactByName.call(this, funcName));
     } else {
       // Copy service packaged artifact
-      const serviceName = this.serverless.service.getServiceObject().name;
       copyArtifactByName.call(this, serviceName);
     }
 
+    // Loop through every function and make sure that the correct artifact is assigned
+    // (the one built by webpack)
     _.forEach(functionNames, funcName => {
       const func = this.serverless.service.getFunction(funcName);
 
-      const archiveName = isIndividualPackaging.call(this) ? funcName : this.serverless.service.getServiceObject().name;
+      // When individual packaging is enabled, each functions gets it's own
+      // artifact, otherwise every function gets set to the same artifact
+      const archiveName = individualPackagingEnabled ? funcName : serviceName;
 
       const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
       setArtifactPath.call(this, funcName, func, serverlessArtifact);
     });
 
-    // Set artifact locations
-    if (isIndividualPackaging.call(this)) {
-      _.forEach(functionNames, funcName => {
-        const func = this.serverless.service.getFunction(funcName);
-
-        const archiveName = funcName;
-
-        const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
-        setArtifactPath.call(this, funcName, func, serverlessArtifact);
-      });
-    } else {
-      const archiveName = this.serverless.service.getServiceObject().name;
+    // If we are deploying to 'google' we need to set an artifact for the whole service,
+    // rather than for each function, so there is special case here
+    if (!individualPackagingEnabled && providerIsGoogle) {
+      const archiveName = serviceName;
 
+      // This may look similar to the loop above, but note that this calls
+      // setServiceArtifactPath rather than setArtifactPath
       const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
-
-      if (_.get(this.serverless, 'service.provider.name') === 'google') {
-        setServiceArtifactPath.call(this, serverlessArtifact);
-      }
+      setServiceArtifactPath.call(this, serverlessArtifact);
     }
 
     return BbPromise.resolve();

diff --git a/lib/utils.js b/lib/utils.js
@@ -128,6 +128,16 @@ function getAllNodeFunctions() {
   });
 }
 
+/**
+ * Given a serverless instance, will return whether the provider is google (as opposed to another
+ * provider like 'aws')
+ * @param {*} serverless the serverless instance that holds the configuration
+ * @returns true if the provider is google, otherwise false
+ */
+function isProviderGoogle(serverless) {
+  return _.get(serverless, 'service.provider.name') === 'google';
+}
+
 module.exports = {
   guid,
   purgeCache,
@@ -137,5 +147,6 @@ module.exports = {
   safeJsonParse,
   splitLines,
   getAllNodeFunctions,
-  isNodeRuntime
+  isNodeRuntime,
+  isProviderGoogle
 };
diff --git a/lib/validate.js b/lib/validate.js
@@ -7,7 +7,7 @@ const glob = require('glob');
 const lib = require('./index');
 const _ = require('lodash');
 const Configuration = require('./Configuration');
-const { getAllNodeFunctions, isNodeRuntime } = require('./utils');
+const { getAllNodeFunctions, isNodeRuntime, isProviderGoogle } = require('./utils');
 
 /**
  * For automatic entry detection we sort the found files to solve ambiguities.
@@ -91,7 +91,7 @@ module.exports = {
 
       const handlerFile = getHandlerFile(handler);
       if (!handlerFile) {
-        if (_.get(this.serverless, 'service.provider.name') !== 'google') {
+        if (!isProviderGoogle(this.serverless)) {
           if (this.log) {
             this.log.warning();
             this.log.warning(

diff --git a/tests/utils.test.js b/tests/utils.test.js
@@ -125,4 +125,34 @@ describe('Utils', () => {
       expect(Utils.splitLines('a\r\nb\nc')).toEqual(['a', 'b', 'c']);
     });
   });
+
+  describe('isProviderGoogle', () => {
+    describe('when the provider is set to "google"', () => {
+      const mockServerless = {
+        service: {
+          provider: {
+            name: 'google'
+          }
+        }
+      };
+
+      it('should return true', () => {
+        expect(Utils.isProviderGoogle(mockServerless)).toBe(true);
+      });
+    });
+
+    describe('when the provider is set to "aws"', () => {
+      const mockServerless = {
+        service: {
+          provider: {
+            name: 'aws'
+          }
+        }
+      };
+
+      it('should return false', () => {
+        expect(Utils.isProviderGoogle(mockServerless)).toBe(false);
+      });
+    });
+  });
 });