Skip to content

Commit

Permalink
fix: write id to file upon successful submission (#2511)
Browse files Browse the repository at this point in the history 
* write id to file upon successful submission

* update comment to say saveIdToFile
  • Loading branch information
@eviljeff
eviljeff authored Oct 14, 2022
1 parent 00fd37c commit df6078c
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 82 deletions.
48 changes: 20 additions & 28 deletions src/cmd/sign.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import {prepareArtifactsDir} from '../util/artifacts.js';
import {createLogger} from '../util/logger.js';
import getValidatedManifest, {getManifestId} from '../util/manifest.js';
import type {ExtensionManifest} from '../util/manifest.js';
import { signAddon as defaultSubmitAddonSigner } from '../util/submit-addon.js';
import {
signAddon as defaultSubmitAddonSigner,
saveIdToFile,
} from '../util/submit-addon.js';
import type { SignResult } from '../util/submit-addon.js';
import {withTempDir} from '../util/temp-dir.js';

Expand Down Expand Up @@ -81,6 +84,7 @@ export default function sign(
await prepareArtifactsDir(artifactsDir);

let manifestData;
const savedIdPath = path.join(sourceDir, extensionIdFile);

if (preValidatedManifest) {
manifestData = preValidatedManifest;
Expand All @@ -91,7 +95,7 @@ export default function sign(
const [buildResult, idFromSourceDir] = await Promise.all([
build({sourceDir, ignoreFiles, artifactsDir: tmpDir.path()},
{manifestData, showReadyMessage: false}),
getIdFromSourceDir(sourceDir),
getIdFromFile(savedIdPath),
]);

const manifestId = getManifestId(manifestData);
Expand Down Expand Up @@ -169,10 +173,14 @@ export default function sign(
let result;
try {
if (useSubmissionApi) {
result = await submitAddon(
result = await submitAddon({
...signSubmitArgs,
amoBaseUrl,
// $FlowIgnore: we verify 'channel' is set above
{...signSubmitArgs, amoBaseUrl, channel, metaDataJson}
);
channel,
savedIdPath,
metaDataJson,
});
} else {
const { success, id: newId, downloadedFiles } = await signAddon({
...signSubmitArgs,
Expand All @@ -185,28 +193,26 @@ export default function sign(
throw new Error('The extension could not be signed');
}
result = { id: newId, downloadedFiles };
// All information about the downloaded files would have already been
// logged by signAddon(). submitAddon() calls saveIdToFile itself.
await saveIdToFile(savedIdPath, newId);
log.info(`Extension ID: ${newId}`);
log.info('SUCCESS');
}
} catch (clientError) {
log.info('FAIL');
throw new WebExtError(clientError.message);
}

// All information about the downloaded files would have
// already been logged by signAddon() or submitAddon().
await saveIdToSourceDir(sourceDir, result.id);
log.info(`Extension ID: ${result.id}`);
log.info('SUCCESS');
return result;
});
}


export async function getIdFromSourceDir(
sourceDir: string,
export async function getIdFromFile(
filePath: string,
asyncFsReadFile: typeof defaultAsyncFsReadFile = defaultAsyncFsReadFile,
): Promise<string | void> {
const filePath = path.join(sourceDir, extensionIdFile);

let content;

try {
Expand Down Expand Up @@ -236,17 +242,3 @@ export async function getIdFromSourceDir(

return id;
}


export async function saveIdToSourceDir(
sourceDir: string, id: string
): Promise<void> {
const filePath = path.join(sourceDir, extensionIdFile);
await fs.writeFile(filePath, [
'# This file was created by https://github.com/mozilla/web-ext',
'# Your auto-generated extension ID for addons.mozilla.org is:',
id.toString(),
].join('\n'));

log.debug(`Saved auto-generated ID ${id} to ${filePath}`);
}
25 changes: 23 additions & 2 deletions src/util/submit-addon.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,9 @@ export default class Client {
async postNewAddon(
xpiPath: string,
channel: string,
metaDataJson: Object
savedIdPath: string,
metaDataJson: Object,
saveIdToFileFunc: (string, string) => Promise<void> = saveIdToFile,
): Promise<SignResult> {
const uploadUuid = await this.doUploadSubmit(xpiPath, channel);
Expand All @@ -297,6 +299,11 @@ export default class Client {
[versionObject]: {id: newVersionId},
} = await this.doNewAddonSubmit(uploadUuid, metaDataJson);
await saveIdToFileFunc(savedIdPath, addonId);
log.info(`Generated extension ID: ${addonId}.`);
log.info('You must add the following to your manifest:');
log.info(`"browser_specific_settings": {"gecko": "${addonId}"}`);
const fileUrl = new URL(await this.waitForApproval(addonId, newVersionId));
return this.downloadSignedFile(fileUrl, addonId);
Expand Down Expand Up @@ -332,6 +339,7 @@ type signAddonParams = {|
xpiPath: string,
downloadDir: string,
channel: string,
savedIdPath: string,
metaDataJson?: Object,
SubmitClient?: typeof Client,
ApiAuthClass?: typeof JwtApiAuth,
Expand All @@ -346,6 +354,7 @@ export async function signAddon({
xpiPath,
downloadDir,
channel,
savedIdPath,
metaDataJson = {},
SubmitClient = Client,
ApiAuthClass = JwtApiAuth,
Expand Down Expand Up @@ -378,8 +387,20 @@ export async function signAddon({
// We specifically need to know if `id` has not been passed as a parameter because
// it's the indication that a new add-on should be created, rather than a new version.
if (id === undefined) {
return client.postNewAddon(xpiPath, channel, metaDataJson);
return client.postNewAddon(xpiPath, channel, savedIdPath, metaDataJson);
}
return client.putVersion(xpiPath, channel, id, metaDataJson);
}
export async function saveIdToFile(
filePath: string, id: string
): Promise<void> {
await fsPromises.writeFile(filePath, [
'# This file was created by https://github.com/mozilla/web-ext',
'# Your auto-generated extension ID for addons.mozilla.org is:',
id.toString(),
].join('\n'));
log.debug(`Saved auto-generated ID ${id} to ${filePath}`);
}
56 changes: 16 additions & 40 deletions tests/unit/test-cmd/test.sign.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import * as sinon from 'sinon';

import {UsageError, WebExtError} from '../../../src/errors.js';
import {getManifestId} from '../../../src/util/manifest.js';
import {saveIdToFile} from '../../../src/util/submit-addon.js';
import {withTempDir} from '../../../src/util/temp-dir.js';
import completeSignCommand, {
extensionIdFile, getIdFromSourceDir, saveIdToSourceDir,
extensionIdFile, getIdFromFile,
} from '../../../src/cmd/sign.js';
import {
basicManifest,
Expand Down Expand Up @@ -208,9 +209,10 @@ describe('sign', () => {
() => withTempDir(
async (tmpDir) => {
const sourceDir = path.join(tmpDir.path(), 'source-dir');
const idFile = path.join(sourceDir, extensionIdFile);
const stubs = getStubs();
await fs.mkdir(sourceDir);
await saveIdToSourceDir(sourceDir, 'some-other-id');
await saveIdToFile(idFile, 'some-other-id');
// Now, make a signing call with a custom ID.
const promiseSigned = sign(tmpDir, stubs, {
extraArgs: {
Expand Down Expand Up @@ -242,11 +244,12 @@ describe('sign', () => {
it('prefers a custom ID over an ID file', () => withTempDir(
(tmpDir) => {
const sourceDir = path.join(tmpDir.path(), 'source-dir');
const idFile = path.join(sourceDir, extensionIdFile);
const customId = 'some-custom-id';
const stubs = getStubs();
// First, save an extension ID like a previous signing call.
return fs.mkdir(sourceDir)
.then(() => saveIdToSourceDir(sourceDir, 'some-other-id'))
.then(() => saveIdToFile(idFile, 'some-other-id'))
// Now, make a signing call with a custom ID.
.then(() => sign(tmpDir, stubs, {
extraArgs: {
Expand Down Expand Up @@ -532,40 +535,13 @@ describe('sign', () => {
}
));

describe('saveIdToSourceDir', () => {

it('saves an extension ID to file', () => withTempDir(
(tmpDir) => {
const sourceDir = tmpDir.path();
return saveIdToSourceDir(sourceDir, 'some-id')
.then(() => fs.readFile(path.join(sourceDir, extensionIdFile)))
.then((content) => {
assert.include(content.toString(), 'some-id');
});
}
));

it('will overwrite an existing file', () => withTempDir(
(tmpDir) => {
const sourceDir = tmpDir.path();
return saveIdToSourceDir(sourceDir, 'first-id')
.then(() => saveIdToSourceDir(sourceDir, 'second-id'))
.then(() => getIdFromSourceDir(sourceDir))
.then((savedId) => {
assert.equal(savedId, 'second-id');
});
}
));

});

describe('getIdFromSourceDir', () => {
describe('getIdFromFile', () => {

it('gets a saved extension ID', () => withTempDir(
(tmpDir) => {
const sourceDir = tmpDir.path();
return saveIdToSourceDir(sourceDir, 'some-id')
.then(() => getIdFromSourceDir(sourceDir))
const idFile = path.join(tmpDir.path(), extensionIdFile);
return saveIdToFile(idFile, 'some-id')
.then(() => getIdFromFile(idFile))
.then((extensionId) => {
assert.equal(extensionId, 'some-id');
});
Expand All @@ -574,9 +550,9 @@ describe('sign', () => {

it('throws an error for empty files', () => withTempDir(
async (tmpDir) => {
const sourceDir = tmpDir.path();
await fs.writeFile(path.join(sourceDir, extensionIdFile), '');
const getIdPromise = getIdFromSourceDir(sourceDir);
const idFile = path.join(tmpDir.path(), extensionIdFile);
await fs.writeFile(idFile, '');
const getIdPromise = getIdFromFile(idFile);
await assert.isRejected(getIdPromise, UsageError);
await assert.isRejected(
getIdPromise, /No ID found in extension ID file/
Expand All @@ -586,8 +562,8 @@ describe('sign', () => {

it('returns empty ID when extension file does not exist', () => withTempDir(
(tmpDir) => {
const sourceDir = tmpDir.path();
return getIdFromSourceDir(sourceDir)
const idFile = path.join(tmpDir.path(), extensionIdFile);
return getIdFromFile(idFile)
.then((savedId) => {
assert.strictEqual(savedId, undefined);
});
Expand All @@ -599,7 +575,7 @@ describe('sign', () => {
throw new Error('Unexpected fs.readFile error');
});
await assert.isRejected(
getIdFromSourceDir('fakeSourceDir', fakeAsyncFsReadFile),
getIdFromFile('fakeIdFile', fakeAsyncFsReadFile),
/Unexpected fs.readFile error/);

sinon.assert.calledOnce(fakeAsyncFsReadFile);
Expand Down
64 changes: 52 additions & 12 deletions tests/unit/test-util/test.submit-addon.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { File, FormData, Response } from 'node-fetch';

import Client, {
JwtApiAuth,
saveIdToFile,
signAddon,
} from '../../../src/util/submit-addon.js';
import { withTempDir } from '../../../src/util/temp-dir.js';
Expand Down Expand Up @@ -75,6 +76,7 @@ describe('util.submit-addon', () => {
downloadDir: '/some-dir/',
xpiPath: '/some.xpi',
channel: 'some-channel',
savedIdPath: '.id-file',
};

it('creates Client with parameters', async () => {
Expand Down Expand Up @@ -118,13 +120,17 @@ describe('util.submit-addon', () => {
it('calls postNewAddon if `id` is undefined', async () => {
const xpiPath = 'this/path/xpi.xpi';
const channel = 'thisChannel';
const savedIdPath = '.some.id.file';
await signAddon({
...signAddonDefaults,
xpiPath,
channel,
savedIdPath,
});
sinon.assert.notCalled(putVersionStub);
sinon.assert.calledWith(postNewAddonStub, xpiPath, channel, {});
sinon.assert.calledWith(
postNewAddonStub, xpiPath, channel, savedIdPath, {}
);
});

it('calls putVersion if `id` is defined', async () => {
Expand Down Expand Up @@ -171,6 +177,7 @@ describe('util.submit-addon', () => {
postNewAddonStub,
signAddonDefaults.xpiPath,
signAddonDefaults.channel,
signAddonDefaults.savedIdPath,
metaDataJson
);
});
Expand Down Expand Up @@ -620,17 +627,23 @@ describe('util.submit-addon', () => {
{channel: 'listed', versionId: sampleVersionDetail.id},
{channel: 'unlisted', versionId: sampleVersionDetail2.id},
].forEach(({channel, versionId}) =>
it('uploads new listed add-on; downloads the signed xpi', async () => {
addUploadMocks();
mockNodeFetch(
nodeFetchStub,
new URL('/addons/addon/', baseUrl),
'POST',
[{ body: sampleAddonDetail, status: 200 }]
);
addApprovalMocks(versionId);
await client.postNewAddon(xpiPath, channel, {});
}));
it(
`uploads new ${channel} add-on; downloads the signed xpi`,
async () => {
addUploadMocks();
const saveIdStub = sinon.stub();
saveIdStub.resolves();
const idFile = 'id.file';
mockNodeFetch(
nodeFetchStub,
new URL('/addons/addon/', baseUrl),
'POST',
[{ body: sampleAddonDetail, status: 200 }]
);
addApprovalMocks(versionId);
await client.postNewAddon(xpiPath, channel, idFile, {}, saveIdStub);
sinon.assert.calledWith(saveIdStub, idFile, sampleAddonDetail.guid);
}));

it('uploads a new version; then downloads the signed xpi', async () => {
const channel = 'listed';
Expand Down Expand Up @@ -766,4 +779,31 @@ describe('util.submit-addon', () => {
});
});
});

describe('saveIdToFile', () => {

it('saves an extension ID to file', () => withTempDir(
(tmpDir) => {
const idFile = path.join(tmpDir.path(), 'extensionId.File');
return saveIdToFile(idFile, 'some-id')
.then(() => fs.readFile(idFile))
.then((content) => {
assert.include(content.toString(), 'some-id');
});
}
));

it('will overwrite an existing file', () => withTempDir(
(tmpDir) => {
const idFile = path.join(tmpDir.path(), 'extensionId.File');
return saveIdToFile(idFile, 'first-id')
.then(() => saveIdToFile(idFile, 'second-id'))
.then(() => fs.readFile(idFile))
.then((content) => {
assert.include(content.toString(), 'second-id');
});
}
));
});

});

0 comments on commit df6078c

Please sign in to comment.