fix: deploy skill package when icon uri path relative to its asset files

lib/controllers/skill-infrastructure-controller/index.js

@@ -8,6 +8,7 @@ const MultiTasksView = require("../../view/multi-tasks-view");
 const Messenger = require("../../view/messenger");
 const hashUtils = require("../../utils/hash-utils");
 const stringUtils = require("../../utils/string-utils");
+const profileHelper = require("../../utils/profile-helper");
 const CONSTANTS = require("../../utils/constants");
 const {isAcSkill, syncManifest} = require("../../utils/ac-util");
 const acdl = require("@alexa/acdl");
@@ -314,46 +315,24 @@ module.exports = class SkillInfrastructureController {
    * @param {Function} callback
    */
   _ensureSkillManifestGotUpdated(callback) {
-    let skillMetaController;
+    let vendorId;
 
     try {
-      skillMetaController = new SkillMetadataController({profile: this.profile, doDebug: this.doDebug});
+      vendorId = profileHelper.resolveVendorId(this.profile);
     } catch (err) {
       return callback(err);
     }
 
-    skillMetaController.updateSkillManifest((err) => {
-      callback(err);
-    });
-  }
-
-  /**
-   * Poll skill's manifest status until the status is not IN_PROGRESS.
-   *
-   * @param {Object} smapiClient
-   * @param {String} skillId
-   * @param {Function} callback
-   */
-  _pollSkillStatus(smapiClient, skillId, callback) {
-    const retryConfig = {
-      base: 2000,
-      factor: 1.12,
-      maxRetry: 50,
-    };
-    const retryCall = (loopCallback) => {
-      smapiClient.skill.getSkillStatus(skillId, [CONSTANTS.SKILL.RESOURCES.MANIFEST], (statusErr, statusResponse) => {
-        if (statusErr) {
-          return loopCallback(statusErr);
-        }
-        if (statusResponse.statusCode >= 300) {
-          return loopCallback(jsonView.toString(statusResponse.body));
-        }
-        loopCallback(null, statusResponse);
+    // deploy skill package if the skill manifest has icon file uri, otherwise update the skill manifest
+    if (Manifest.getInstance().hasIconFileUri()) {
+      this.skillMetaController.deploySkillPackage(vendorId, this.ignoreHash, (err) => {
+        callback(err);
       });
-    };
-    const shouldRetryCondition = (retryResponse) =>
-      R.view(R.lensPath(["body", "manifest", "lastUpdateRequest", "status"]), retryResponse) === CONSTANTS.SKILL.SKILL_STATUS.IN_PROGRESS;
-    retryUtils.retry(retryConfig, retryCall, shouldRetryCondition, (err, res) => callback(err, err ? null : res));
+    } else {
+      this.skillMetaController.updateSkillManifest((err) => {
+        callback(err);
+      });
+    }
   }
 
   /**

lib/controllers/skill-metadata-controller/index.js

@@ -284,12 +284,11 @@ module.exports = class SkillMetadataController {
    * @param {error} callback
    */
   updateSkillManifest(callback) {
-    const smapiClient = new SmapiClient({profile: this.profile, doDebug: this.doDebug});
     const skillId = ResourcesConfig.getInstance().getSkillId(this.profile);
     const content = Manifest.getInstance().content;
     const stage = CONSTANTS.SKILL.STAGE.DEVELOPMENT;
 
-    smapiClient.skill.manifest.updateManifest(skillId, stage, content, null, (updateErr, updateResponse) => {
+    this.smapiClient.skill.manifest.updateManifest(skillId, stage, content, null, (updateErr, updateResponse) => {
       if (updateErr) {
         return callback(updateErr);
       }
@@ -298,7 +297,7 @@ module.exports = class SkillMetadataController {
       }
 
       // poll manifest status until finish
-      this._pollSkillManifestStatus(smapiClient, skillId, (pollErr, pollResponse) => {
+      this._pollSkillManifestStatus(skillId, (pollErr, pollResponse) => {
         if (pollErr) {
           return callback(pollErr);
         }
@@ -317,19 +316,18 @@ module.exports = class SkillMetadataController {
   /**
    * Poll skill's manifest status until the status is not IN_PROGRESS.
    *
-   * @param {Object} smapiClient
    * @param {String} skillId
    * @param {Function} callback
    */
-  _pollSkillManifestStatus(smapiClient, skillId, callback) {
+  _pollSkillManifestStatus(skillId, callback) {
     const retryConfig = {
       base: 2000,
       factor: 1.12,
       maxRetry: 50,
     };
 
     const retryCall = (loopCallback) => {
-      smapiClient.skill.getSkillStatus(skillId, [CONSTANTS.SKILL.RESOURCES.MANIFEST], (statusErr, statusResponse) => {
+      this.smapiClient.skill.getSkillStatus(skillId, [CONSTANTS.SKILL.RESOURCES.MANIFEST], (statusErr, statusResponse) => {
         if (statusErr) {
           return loopCallback(statusErr);
         }

lib/model/manifest.js

@@ -128,4 +128,14 @@ module.exports = class Manifest extends ConfigFile {
   setEventsSubscriptions(subscriptions) {
     this.setProperty(["manifest", Manifest.endpointTypes.EVENTS, "subscriptions"], subscriptions);
   }
+
+  /**
+   * Returns if skill manifest has icon file uri
+   * @return {Boolean}
+   */
+  hasIconFileUri() {
+    return Object.values(this.getPublishingLocales())
+      .flatMap((locale) => Object.entries(locale))
+      .some(([key, value]) => (key === "smallIconUri" || key === "largeIconUri") && value.startsWith("file://"));
+  }
 };

test/unit/commands/configure/aws-profile-setup-helper-test.js

@@ -213,6 +213,7 @@ describe("Command: Configure - AWS profile setup helper test", () => {
       sinon.stub(ui, "addNewCredentials").callsArgWith(0, null, TEST_CREDENTIALS);
       sinon.stub(awsProfileHandler, "addProfile");
       sinon.stub(profileHelper, "setupProfile");
+      sinon.useFakeTimers().tickAsync(CONSTANTS.CONFIGURATION.OPEN_BROWSER_DELAY);
 
       // call
       proxyHelper.setupAwsProfile({askProfile: TEST_PROFILE, needBrowser: true}, (err, awsProfile) => {

test/unit/controller/hosted-skill-controller/index-test.js

@@ -392,6 +392,7 @@ describe("Controller test - hosted skill controller test", () => {
       const stubTestFunc = sinon.stub(httpClient, "request"); // stub getSkillStatus smapi request
       stubTestFunc.onCall(0).callsArgWith(3, null, TEST_STATUS_RESPONSE_0);
       stubTestFunc.onCall(1).callsArgWith(3, null, TEST_STATUS_RESPONSE_1);
+      sinon.useFakeTimers().tickAsync(CONSTANTS.CONFIGURATION.RETRY.MAX_RETRY_INTERVAL);
       // call
       hostedSkillController.checkSkillStatus(TEST_SKILL_ID, (err, res) => {
         expect(err).equal(null);

test/unit/controller/skill-infrastructure-controller-test.js

@@ -763,40 +763,60 @@ describe("Controller test - skill infrastructure controller test", () => {
 
   describe("# test class method: _ensureSkillManifestGotUpdated", () => {
     const skillInfraController = new SkillInfrastructureController(TEST_CONFIGURATION);
+    let deploySkillPackageStub, updateSkillManifestStub;
 
     beforeEach(() => {
-      new ResourcesConfig(FIXTURE_RESOURCES_CONFIG_FILE_PATH);
       new Manifest(FIXTURE_MANIFEST_FILE_PATH);
-      sinon.stub(fs, "writeFileSync");
+      deploySkillPackageStub = sinon.stub(SkillMetadataController.prototype, "deploySkillPackage").yields();
+      updateSkillManifestStub = sinon.stub(SkillMetadataController.prototype, "updateSkillManifest").yields();
     });
 
     afterEach(() => {
-      ResourcesConfig.dispose();
       Manifest.dispose();
       sinon.restore();
     });
 
-    it("| update skill manifest fails, expect error called back", (done) => {
+    it("| resolve vendor id fails, expect error called back", (done) => {
+      // setup
+      const TEST_ERROR = new Error("error");
+      sinon.stub(profileHelper, "resolveVendorId").throws(TEST_ERROR);
+      // call
+      skillInfraController._ensureSkillManifestGotUpdated((err, res) => {
+        // verify
+        expect(res).equal(undefined);
+        expect(err).equal(TEST_ERROR);
+        expect(deploySkillPackageStub.calledOnce).equal(false);
+        expect(updateSkillManifestStub.calledOnce).equal(false);
+        done();
+      });
+    });
+
+    it("| skill manifest has no icon file uri, expect updateSkillManifest to be called with no error", (done) => {
       // setup
       sinon.stub(profileHelper, "resolveVendorId");
-      sinon.stub(SkillMetadataController.prototype, "updateSkillManifest").yields("error");
+      sinon.stub(Manifest.prototype, "hasIconFileUri").returns(false);
       // call
       skillInfraController._ensureSkillManifestGotUpdated((err, res) => {
         // verify
         expect(res).equal(undefined);
-        expect(err).equal("error");
+        expect(err).equal(undefined);
+        expect(deploySkillPackageStub.calledOnce).equal(false);
+        expect(updateSkillManifestStub.calledOnce).equal(true);
         done();
       });
     });
 
-    it("| update skill manifest succeeds, expect call back with no error", (done) => {
+    it("| skill manifest has icon file uri, expect deploySkillPackage to be called with no error", (done) => {
+      // setup
       sinon.stub(profileHelper, "resolveVendorId");
-      sinon.stub(SkillMetadataController.prototype, "updateSkillManifest").yields();
+      sinon.stub(Manifest.prototype, "hasIconFileUri").returns(true);
       // call
       skillInfraController._ensureSkillManifestGotUpdated((err, res) => {
         // verify
         expect(res).equal(undefined);
         expect(err).equal(undefined);
+        expect(deploySkillPackageStub.calledOnce).equal(true);
+        expect(updateSkillManifestStub.calledOnce).equal(false);
         done();
       });
     });

test/unit/controller/skill-metadata-controller-test.js

@@ -1104,6 +1104,7 @@ describe("Controller test - skill metadata controller test", () => {
 
     it("| poll status retries with getImportStatus warnings, expects warnings logged once", (done) => {
       // setup
+      sinon.useFakeTimers().tickAsync(CONSTANTS.CONFIGURATION.RETRY.MAX_RETRY_INTERVAL);
       requestStub
         .onCall(0)
         .callsArgWith(3, null, smapiImportStatusResponseWithWarningsInProgress)
@@ -1373,6 +1374,7 @@ describe("Controller test - skill metadata controller test", () => {
 
       it("| poll import status with multiple retries, expect callback with correct response", (done) => {
         // setup
+        sinon.useFakeTimers().tickAsync(CONSTANTS.CONFIGURATION.RETRY.MAX_RETRY_INTERVAL);
         requestStub
           .onCall(0)
           .callsArgWith(3, null, smapiImportStatusResponseInProgress)
@@ -1424,6 +1426,7 @@ describe("Controller test - skill metadata controller test", () => {
 
       it("| poll import status with getSkillStatus build failures", (done) => {
         // setup
+        sinon.useFakeTimers().tickAsync(CONSTANTS.CONFIGURATION.RETRY.MAX_RETRY_INTERVAL);
         requestStub
           .onCall(0)
           .callsArgWith(3, null, smapiImportStatusResponseInProgress)
@@ -1445,6 +1448,7 @@ describe("Controller test - skill metadata controller test", () => {
 
       it("| poll status smapi calls return success, expect GetSkillStatus calls", (done) => {
         // setup
+        sinon.useFakeTimers().tickAsync(CONSTANTS.CONFIGURATION.RETRY.MAX_RETRY_INTERVAL);
         requestStub
           .onCall(0)
           .callsArgWith(3, null, smapiImportStatusResponseInProgress)
@@ -1465,6 +1469,7 @@ describe("Controller test - skill metadata controller test", () => {
 
       it("| poll status smapi calls return empty SkillID, expect no GetSkillStatus calls", (done) => {
         // setup
+        sinon.useFakeTimers().tickAsync(CONSTANTS.CONFIGURATION.RETRY.MAX_RETRY_INTERVAL);
         requestStub
           .onCall(0)
           .callsArgWith(3, null, smapiImportStatusResponse200EmptySkillID)
@@ -1586,7 +1591,7 @@ describe("Controller test - skill metadata controller test", () => {
     it("| update manifest callback with error when poll skill status fails", (done) => {
       // setup
       sinon.stub(httpClient, "request").callsArgWith(3, null, {});
-      sinon.stub(SkillMetadataController.prototype, "_pollSkillManifestStatus").callsArgWith(2, "TEST_ERROR");
+      sinon.stub(SkillMetadataController.prototype, "_pollSkillManifestStatus").callsArgWith(1, "TEST_ERROR");
 
       // call
       skillMetaController.updateSkillManifest((err, res) => {
@@ -1608,7 +1613,7 @@ describe("Controller test - skill metadata controller test", () => {
         },
       };
 
-      sinon.stub(SkillMetadataController.prototype, "_pollSkillManifestStatus").callsArgWith(2, undefined, pollResponse);
+      sinon.stub(SkillMetadataController.prototype, "_pollSkillManifestStatus").callsArgWith(1, undefined, pollResponse);
       sinon.stub(httpClient, "request").callsArgWith(3, null, {});
 
       // call
@@ -1631,7 +1636,7 @@ describe("Controller test - skill metadata controller test", () => {
         },
       };
 
-      sinon.stub(SkillMetadataController.prototype, "_pollSkillManifestStatus").callsArgWith(2, undefined, pollResponse);
+      sinon.stub(SkillMetadataController.prototype, "_pollSkillManifestStatus").callsArgWith(1, undefined, pollResponse);
       sinon.stub(httpClient, "request").callsArgWith(3, null, {});
 
       // call

test/unit/model/manifest-test.js

@@ -211,5 +211,37 @@ describe("Model test - manifest file test", () => {
         Manifest.dispose();
       });
     });
+
+    describe("# verify function hasIconFileUri", () => {
+      it("| icon https uri, expect false", () => {
+        new Manifest(MANIFEST_FILE);
+        Manifest.getInstance().setPublishingLocales({
+          "en-US": {
+            name: "skillName",
+            summary: "one sentence description",
+            description: "skill description",
+            smallIconUri: "https://some.url.com/images/en-US_smallIcon.png",
+            largeIconUri: "https://some.url.com/images/en-US_largeIconUri.png",
+          },
+        });
+        expect(Manifest.getInstance().hasIconFileUri()).equal(false);
+        Manifest.dispose();
+      });
+
+      it("| icon file uri relative to skill package assets, expect true", () => {
+        new Manifest(MANIFEST_FILE);
+        Manifest.getInstance().setPublishingLocales({
+          "en-US": {
+            name: "skillName",
+            summary: "one sentence description",
+            description: "skill description",
+            smallIconUri: "file://assets/images/en-US_smallIcon.png",
+            largeIconUri: "file://assets/images/en-US_largeIconUri.png",
+          },
+        });
+        expect(Manifest.getInstance().hasIconFileUri()).equal(true);
+        Manifest.dispose();
+      });
+    });
   });
 });