Check new extension submissions for a name conflict

.github/workflows/auto-pr.yml

@@ -70,6 +70,14 @@ jobs:
                 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 🙏`
               })
 
+            if(error === "already-exists")
+              github.rest.issues.createComment({
+                issue_number: context.issue.number,
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                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 🙏`
+              })
+
             if(error) {
               core.setFailed("Extraction of the extension failed!");
               return;

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

@@ -1,4 +1,5 @@
 const { mkdir, rm } = require('fs/promises');
+const { cp } = require('fs').promises;
 const { verifyExtension } = require('../../scripts/check-single-extension');
 const { extractExtension } = require('../../scripts/extract-extension');
 
@@ -27,34 +28,65 @@ const wrappedVerifyExtension = async (extensionName) =>
   ).code;
 
 describe('Auto-pr pipeline', () => {
-  beforeAll(async () =>
-    mkdir(TEMPORARY_MOCK_EXTENSIONS_FOLDER + '/community', { recursive: true })
+  beforeEach(
+    async () =>
+      await cp(TEST_EXTENSIONS_FOLDER, TEMPORARY_MOCK_EXTENSIONS_FOLDER, {
+        recursive: true,
+      })
   );
-  beforeAll(async () =>
-    mkdir(TEMPORARY_MOCK_EXTENSIONS_FOLDER + '/reviewed', { recursive: true })
-  );
-  afterAll(async () =>
-    rm(TEMPORARY_MOCK_EXTENSIONS_FOLDER, { recursive: true })
+  afterEach(
+    async () => await rm(TEMPORARY_MOCK_EXTENSIONS_FOLDER, { recursive: true })
   );
 
-  test('extractExtension()', async () => {
-    expect(await wrappedExtractExtension(`empty`)).toBe('no-json-found');
-    expect(await wrappedExtractExtension(`invalid-zip`)).toBe('zip-error');
-    expect(await wrappedExtractExtension(`not-a-json`)).toBe('no-json-found');
-    expect(await wrappedExtractExtension(`path-shenanigans`)).toBe(
-      'invalid-file-name'
-    );
-    expect(await wrappedExtractExtension(`too-many-extensions`)).toBe(
-      'too-many-files'
-    );
+  describe('extractExtension()', () => {
+    test('Can detect extension file errors', async () => {
+      expect(await wrappedExtractExtension(`empty-file`)).toBe('zip-error');
+      expect(await wrappedExtractExtension(`empty-archive`)).toBe(
+        'no-json-found'
+      );
+      expect(await wrappedExtractExtension(`invalid-zip`)).toBe('zip-error');
+      expect(await wrappedExtractExtension(`not-a-json`)).toBe('no-json-found');
+      expect(await wrappedExtractExtension(`path-shenanigans`)).toBe(
+        'invalid-file-name'
+      );
+      expect(await wrappedExtractExtension(`too-many-extensions`)).toBe(
+        'too-many-files'
+      );
+    });
+
+    test('Can submit a valid new extension', async () => {
+      expect(await wrappedExtractExtension(`new-extension`)).toBeUndefined();
+    });
 
-    expect(await wrappedExtractExtension(`new-extension`)).toBeUndefined();
-    expect(await wrappedExtractExtension(`experimental-update`)).toBeUndefined();
-    expect(await wrappedExtractExtension(`reviewed-update`)).toBeUndefined();
+    test('Can detect that the new experimental extension already exists', async () => {
+      expect(await wrappedExtractExtension(`experimental-update`)).toBe(
+        'already-exists'
+      );
+    });
 
-    expect(await wrappedExtractExtension(`new-extension`, true)).toBe('nothing-to-update');
-    expect(await wrappedExtractExtension(`experimental-update`, true)).toBeUndefined();
-    expect(await wrappedExtractExtension(`reviewed-update`, true)).toBeUndefined();
+    test('Can detect that the new reviewed extension already exists', async () => {
+      expect(await wrappedExtractExtension(`reviewed-update`)).toBe(
+        'already-exists'
+      );
+    });
+
+    test("Can detect that the extension to update doesn't exist", async () => {
+      expect(await wrappedExtractExtension(`new-extension`, true)).toBe(
+        'nothing-to-update'
+      );
+    });
+
+    test('Can submit a valid experimental extension update', async () => {
+      expect(
+        await wrappedExtractExtension(`experimental-update`, true)
+      ).toBeUndefined();
+    });
+
+    test('Can submit a valid reviewed extension update', async () => {
+      expect(
+        await wrappedExtractExtension(`reviewed-update`, true)
+      ).toBeUndefined();
+    });
   });
 
   test(`verifyExtension()`, async () => {
@@ -67,7 +99,9 @@ describe('Auto-pr pipeline', () => {
     expect(await wrappedVerifyExtension(`RealExtension`)).toBe('invalid-json');
     expect(await wrappedVerifyExtension(`Share`)).toBe('rule-break');
     expect(await wrappedVerifyExtension(`Fake`)).toBe('unknown-json-contents');
-    expect(await wrappedVerifyExtension(`ArrayTools`)).toBe('gdevelop-project-file');
+    expect(await wrappedVerifyExtension(`ArrayTools`)).toBe(
+      'gdevelop-project-file'
+    );
 
     expect(await wrappedVerifyExtension(`UUID`)).toBe('success');
     expect(await wrappedVerifyExtension(`Clipboard`)).toBe('success');

scripts/extract-extension.js

@@ -9,7 +9,7 @@ const pipeline = require('util').promisify(require('stream').pipeline);
  * Extracts exactly one extension into the community extensions folder from a zip file.
  * @param {string} zipPath The path to the zip file to extract.
  * @param {{extensionsFolder?: string, preliminaryCheck?: boolean, isUpdate?: boolean}} [options]
- * @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.
+ * @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.
  */
 exports.extractExtension = async function (zipPath, options) {
   const {
@@ -18,11 +18,13 @@ exports.extractExtension = async function (zipPath, options) {
     isUpdate = false,
   } = options || {};
   // Load in the archive with JSZip
-  const zip = await JSZip.loadAsync(await readFile(zipPath)).catch((e) => {
-    console.warn(`JSZip loading error caught: `, e);
-    return null;
-  });
-  if (zip === null) return { error: 'zip-error' };
+  let zip;
+  try {
+    zip = await JSZip.loadAsync(await readFile(zipPath));
+  } catch (error) {
+    console.warn(`JSZip loading error caught: `, error);
+    return { error: 'zip-error' };
+  }
 
   // Find JSON files in the zip top level folder
   const jsonFiles = zip.file(/.*\.json$/);
@@ -42,21 +44,23 @@ exports.extractExtension = async function (zipPath, options) {
     return { error: 'invalid-file-name' };
 
   let tier = 'community';
+  const [community, reviewed] = await Promise.all([
+    readFile(`${extensionsFolder}/community/${extensionName}.json`).catch(
+      () => null
+    ),
+    readFile(`${extensionsFolder}/reviewed/${extensionName}.json`).catch(
+      () => null
+    ),
+  ]);
   if (isUpdate) {
-    const [community, reviewed] = await Promise.all([
-      readFile(`${extensionsFolder}/community/${extensionName}.json`).catch(
-        () => null
-      ),
-      readFile(`${extensionsFolder}/reviewed/${extensionName}.json`).catch(
-        () => null
-      ),
-    ]);
     if (!community && !reviewed) {
       return { error: 'nothing-to-update' };
     }
     if (reviewed) {
       tier = 'reviewed';
     }
+  } else if (community || reviewed) {
+    return { error: 'already-exists' };
   }
 
   try {