Skip to content

Commit

Permalink
fix: deploy skill package when icon uri path relative to its asset fi…
Browse files Browse the repository at this point in the history 
…les (#487)
  • Loading branch information
@jsetton
jsetton authored Aug 9, 2023
1 parent abbdc57 commit 4b6ac2f
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 50 deletions.
45 changes: 12 additions & 33 deletions lib/controllers/skill-infrastructure-controller/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
});
}
}

/**
Expand Down
10 changes: 4 additions & 6 deletions lib/controllers/skill-metadata-controller/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
10 changes: 10 additions & 0 deletions lib/model/manifest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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://"));
}
};
1 change: 1 addition & 0 deletions test/unit/commands/configure/aws-profile-setup-helper-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions test/unit/controller/hosted-skill-controller/index-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
36 changes: 28 additions & 8 deletions test/unit/controller/skill-infrastructure-controller-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down
11 changes: 8 additions & 3 deletions test/unit/controller/skill-metadata-controller-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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) => {
Expand All @@ -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
Expand All @@ -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
Expand Down
32 changes: 32 additions & 0 deletions test/unit/model/manifest-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
});

0 comments on commit 4b6ac2f

Please sign in to comment.