Skip to content

Commit 9b6fa77

Browse files
D8HD8H
committed
Check new extension submissions for a name conflict
1 parent fb75e51 commit 9b6fa77

File tree

5 files changed

+84
-38
lines changed

5 files changed

+84
-38
lines changed

‎.github/workflows/auto-pr.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ jobs:
7070
body: `👋 Hey @${{ github.event.issue.user.login }}, thanks for your submission! We are sorry, but the name of the extension you submitted for an update doesn't match any existing extension. If you want to update one of your ingoing extension submission, please follow the instruction on your first submission 🙏`
7171
})
7272
73+
if(error === "already-exists")
74+
github.rest.issues.createComment({
75+
issue_number: context.issue.number,
76+
owner: context.repo.owner,
77+
repo: context.repo.repo,
78+
body: `👋 Hey @${{ github.event.issue.user.login }}, thanks for your submission! We are sorry, but the name of the extension you submitted is already taken. Please submit an update instead or choose another name 🙏`
79+
})
80+
7381
if(error) {
7482
core.setFailed("Extraction of the extension failed!");
7583
return;

‎__tests__/auto-pr/auto-pr.spec.js‎

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const { mkdir, rm } = require('fs/promises');
2+
const { cp } = require('fs').promises;
23
const { verifyExtension } = require('../../scripts/check-single-extension');
34
const { extractExtension } = require('../../scripts/extract-extension');
45

@@ -27,34 +28,65 @@ const wrappedVerifyExtension = async (extensionName) =>
2728
).code;
2829

2930
describe('Auto-pr pipeline', () => {
30-
beforeAll(async () =>
31-
mkdir(TEMPORARY_MOCK_EXTENSIONS_FOLDER + '/community', { recursive: true })
31+
beforeEach(
32+
async () =>
33+
await cp(TEST_EXTENSIONS_FOLDER, TEMPORARY_MOCK_EXTENSIONS_FOLDER, {
34+
recursive: true,
35+
})
3236
);
33-
beforeAll(async () =>
34-
mkdir(TEMPORARY_MOCK_EXTENSIONS_FOLDER + '/reviewed', { recursive: true })
35-
);
36-
afterAll(async () =>
37-
rm(TEMPORARY_MOCK_EXTENSIONS_FOLDER, { recursive: true })
37+
afterEach(
38+
async () => await rm(TEMPORARY_MOCK_EXTENSIONS_FOLDER, { recursive: true })
3839
);
3940

40-
test('extractExtension()', async () => {
41-
expect(await wrappedExtractExtension(`empty`)).toBe('no-json-found');
42-
expect(await wrappedExtractExtension(`invalid-zip`)).toBe('zip-error');
43-
expect(await wrappedExtractExtension(`not-a-json`)).toBe('no-json-found');
44-
expect(await wrappedExtractExtension(`path-shenanigans`)).toBe(
45-
'invalid-file-name'
46-
);
47-
expect(await wrappedExtractExtension(`too-many-extensions`)).toBe(
48-
'too-many-files'
49-
);
41+
describe('extractExtension()', () => {
42+
test('Can detect extension file errors', async () => {
43+
expect(await wrappedExtractExtension(`empty-file`)).toBe('zip-error');
44+
expect(await wrappedExtractExtension(`empty-archive`)).toBe(
45+
'no-json-found'
46+
);
47+
expect(await wrappedExtractExtension(`invalid-zip`)).toBe('zip-error');
48+
expect(await wrappedExtractExtension(`not-a-json`)).toBe('no-json-found');
49+
expect(await wrappedExtractExtension(`path-shenanigans`)).toBe(
50+
'invalid-file-name'
51+
);
52+
expect(await wrappedExtractExtension(`too-many-extensions`)).toBe(
53+
'too-many-files'
54+
);
55+
});
56+
57+
test('Can submit a valid new extension', async () => {
58+
expect(await wrappedExtractExtension(`new-extension`)).toBeUndefined();
59+
});
5060

51-
expect(await wrappedExtractExtension(`new-extension`)).toBeUndefined();
52-
expect(await wrappedExtractExtension(`experimental-update`)).toBeUndefined();
53-
expect(await wrappedExtractExtension(`reviewed-update`)).toBeUndefined();
61+
test('Can detect that the new experimental extension already exists', async () => {
62+
expect(await wrappedExtractExtension(`experimental-update`)).toBe(
63+
'already-exists'
64+
);
65+
});
5466

55-
expect(await wrappedExtractExtension(`new-extension`, true)).toBe('nothing-to-update');
56-
expect(await wrappedExtractExtension(`experimental-update`, true)).toBeUndefined();
57-
expect(await wrappedExtractExtension(`reviewed-update`, true)).toBeUndefined();
67+
test('Can detect that the new reviewed extension already exists', async () => {
68+
expect(await wrappedExtractExtension(`reviewed-update`)).toBe(
69+
'already-exists'
70+
);
71+
});
72+
73+
test("Can detect that the extension to update doesn't exist", async () => {
74+
expect(await wrappedExtractExtension(`new-extension`, true)).toBe(
75+
'nothing-to-update'
76+
);
77+
});
78+
79+
test('Can submit a valid experimental extension update', async () => {
80+
expect(
81+
await wrappedExtractExtension(`experimental-update`, true)
82+
).toBeUndefined();
83+
});
84+
85+
test('Can submit a valid reviewed extension update', async () => {
86+
expect(
87+
await wrappedExtractExtension(`reviewed-update`, true)
88+
).toBeUndefined();
89+
});
5890
});
5991

6092
test(`verifyExtension()`, async () => {
@@ -67,7 +99,9 @@ describe('Auto-pr pipeline', () => {
6799
expect(await wrappedVerifyExtension(`RealExtension`)).toBe('invalid-json');
68100
expect(await wrappedVerifyExtension(`Share`)).toBe('rule-break');
69101
expect(await wrappedVerifyExtension(`Fake`)).toBe('unknown-json-contents');
70-
expect(await wrappedVerifyExtension(`ArrayTools`)).toBe('gdevelop-project-file');
102+
expect(await wrappedVerifyExtension(`ArrayTools`)).toBe(
103+
'gdevelop-project-file'
104+
);
71105

72106
expect(await wrappedVerifyExtension(`UUID`)).toBe('success');
73107
expect(await wrappedVerifyExtension(`Clipboard`)).toBe('success');

__tests__/auto-pr/test-zips/empty.zip renamed to __tests__/auto-pr/test-zips/empty-archive.zip

File renamed without changes.

‎__tests__/auto-pr/test-zips/empty-file.zip‎

Whitespace-only changes.

‎scripts/extract-extension.js‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const pipeline = require('util').promisify(require('stream').pipeline);
99
* Extracts exactly one extension into the community extensions folder from a zip file.
1010
* @param {string} zipPath The path to the zip file to extract.
1111
* @param {{extensionsFolder?: string, preliminaryCheck?: boolean, isUpdate?: boolean}} [options]
12-
* @returns {Promise<{error: "too-many-files" | "no-json-found"| "invalid-file-name" | "zip-error" | "nothing-to-update", details?: any} | {error?: undefined,extensionName: string, tier: string}>} the name of the extracted extension if successful, else a generic error code.
12+
* @returns {Promise<{error: "too-many-files" | "no-json-found"| "invalid-file-name" | "zip-error" | "nothing-to-update" | "already-exists", details?: any} | {error?: undefined,extensionName: string, tier: string}>} the name of the extracted extension if successful, else a generic error code.
1313
*/
1414
exports.extractExtension = async function (zipPath, options) {
1515
const {
@@ -18,11 +18,13 @@ exports.extractExtension = async function (zipPath, options) {
1818
isUpdate = false,
1919
} = options || {};
2020
// Load in the archive with JSZip
21-
const zip = await JSZip.loadAsync(await readFile(zipPath)).catch((e) => {
22-
console.warn(`JSZip loading error caught: `, e);
23-
return null;
24-
});
25-
if (zip === null) return { error: 'zip-error' };
21+
let zip;
22+
try {
23+
zip = await JSZip.loadAsync(await readFile(zipPath));
24+
} catch (error) {
25+
console.warn(`JSZip loading error caught: `, error);
26+
return { error: 'zip-error' };
27+
}
2628

2729
// Find JSON files in the zip top level folder
2830
const jsonFiles = zip.file(/.*\.json$/);
@@ -42,21 +44,23 @@ exports.extractExtension = async function (zipPath, options) {
4244
return { error: 'invalid-file-name' };
4345

4446
let tier = 'community';
47+
const [community, reviewed] = await Promise.all([
48+
readFile(`${extensionsFolder}/community/${extensionName}.json`).catch(
49+
() => null
50+
),
51+
readFile(`${extensionsFolder}/reviewed/${extensionName}.json`).catch(
52+
() => null
53+
),
54+
]);
4555
if (isUpdate) {
46-
const [community, reviewed] = await Promise.all([
47-
readFile(`${extensionsFolder}/community/${extensionName}.json`).catch(
48-
() => null
49-
),
50-
readFile(`${extensionsFolder}/reviewed/${extensionName}.json`).catch(
51-
() => null
52-
),
53-
]);
5456
if (!community && !reviewed) {
5557
return { error: 'nothing-to-update' };
5658
}
5759
if (reviewed) {
5860
tier = 'reviewed';
5961
}
62+
} else if (community || reviewed) {
63+
return { error: 'already-exists' };
6064
}
6165

6266
try {

0 commit comments

Comments
 (0)