Merge pull request #220 from pulsar-edit/existing-packages-check-with…

…-real-name

Check name availability via name within `package.json`

src/controllers/postPackages.js

@@ -101,30 +101,6 @@ module.exports = {
                         .addMessage("This package name is banned.");
     }
 
-    // Check that the package does NOT exists
-    // We will utilize our database.packageNameAvailability to see if the name is available
-    const nameAvailable = await context.database.packageNameAvailability(repo);
-
-    if (!nameAvailable.ok) {
-      // We need to ensure the error is not found or otherwise
-      if (nameAvailable.short !== "not_found") {
-        // the server failed for some other bubbled reason
-        const sso = new context.sso();
-
-        return sso.notOk().addContent(nameAvailable)
-                          .addCalls("auth.verifyAuth", user)
-                          .addCalls("db.packageNameAvailability", nameAvailable);
-      }
-
-      // But if the short is in fact "not_found" we can report the package as not being
-      // available at this name
-      const sso = new context.sso();
-
-      return sso.notOk().addShort("package_exists")
-                        .addCalls("auth.verifyAuth", user)
-                        .addCalls("db.packageNameAvailability", nameAvailable);
-    }
-
     // Now we know the package doesn't exist. And we want to check that the user
     // has permissions to this package
     const gitowner = await context.vcs.ownership(user.content, params.repository);
@@ -134,7 +110,6 @@ module.exports = {
 
       return sso.notOk().addContent(gitowner)
                         .addCalls("auth.verifyAuth", user)
-                        .addCalls("db.packageNameAvailability", nameAvailable)
                         .addCalls("vcs.ownership", gitowner);
     }
 
@@ -151,11 +126,37 @@ module.exports = {
 
       return sso.notOk().addContent(newPack)
                         .addCalls("auth.verifyAuth", user)
-                        .addCalls("db.packageNameAvailability", nameAvailable)
                         .addCalls("vcs.ownership", gitowner)
                         .addCalls("vcs.newPackageData", newPack);
     }
 
+    // Now that we have the name the package will actually take, we want
+    // to make sure this doesn't exist
+    const nameAvailable = await context.database.packageNameAvailability(newPack.content.name);
+
+    if (!nameAvailable.ok) {
+      // We need to ensure the error is not found or otherwise
+      if (nameAvailable.short !== "not_found") {
+        // the server failed for some other bubbled reason
+        const sso = new context.sso();
+
+        return sso.notOk().addContent(nameAvailable)
+                          .addCalls("auth.verifyAuth", user)
+                          .addCalls("vcs.ownership", gitowner)
+                          .addCalls("vcs.newPackageData", newPack)
+                          .addCalls("db.packageNameAvailability", nameAvailable);
+      }
+      // But if the short is in fact "not_found" we can report the package as
+      // not being available at this name
+      const sso = new context.sso();
+
+      return sso.notOk().addShort("package_exists")
+                       .addCalls("auth.verifyAuth", user)
+                       .addCalls("vcs.ownership", gitowner)
+                       .addCalls("vcs.newPackageData", newPack)
+                       .addCalls("db.packageNameAvailability", nameAvailable);
+    }
+
     // Now with valid package data, we can insert them into the DB
     const insertedNewPack = await context.database.insertNewPackage(newPack.content);
 
@@ -164,26 +165,26 @@ module.exports = {
 
       return sso.notOk().addContent(insertedNewPack)
                         .addCalls("auth.verifyAuth", user)
-                        .addCalls("db.packageNameAvailability", nameAvailable)
                         .addCalls("vcs.ownership", gitowner)
                         .addCalls("vcs.newPackageData", newPack)
+                        .addCalls("db.packageNameAvailability", nameAvailable)
                         .addCalls("db.insertNewPackage", insertedNewPack);
     }
 
     // Finally we can return what was actually put into the databse.
     // Retreive the data from database.getPackageByName() and
     // convert it inot a package object full format
-    const newDbPack = await context.database.getPackageByName(repo, true);
+    const newDbPack = await context.database.getPackageByName(newPack.content.name, true);
 
     if (!newDbPack.ok) {
       const sso = new context.sso();
 
       return sso.notOk().addContent(newDbPack)
                         .addCalls("auth.verifyAuth", user)
-                        .addCalls("db.packageNameAvailability", nameAvailable)
                         .addCalls("vcs.ownership", gitowner)
                         .addCalls("vcs.newPackageData", newPack)
                         .addCalls("db.insertNewPackage", insertedNewPack)
+                        .addCalls("db.packageNameAvailability", nameAvailable)
                         .addCalls("db.getPackageByName", newDbPack);
     }
 

tests/http/postPackages.test.js

@@ -82,38 +82,74 @@ describe("POST /api/packages Behaves as expected", () => {
         }
       };
     };
+    localContext.vcs.ownership = () => {
+      return {
+        ok: true,
+        content: "admin"
+      };
+    };
+    localContext.vcs.newPackageData = () => {
+      return {
+        ok: true,
+        content: {
+          name: "post-pkg-test-pkg-exists",
+          repository: {
+            url: "https://github.com/confused-Techie/post-pkg-test-pkg-exists",
+            type: "git"
+          },
+          owner: 'confused-Techie',
+          downloads: 0,
+          stargazers_count: 0,
+          creation_method: "Test Package",
+          releases: {
+            latest: "1.0.0"
+          },
+          readme: "This is a readme!",
+          metadata: { name: "post-pkg-test-pkg-exists" },
+          versions: {
+            "1.0.0": {
+              dist: {
+                tarball: "download-url",
+                sha: "1234"
+              },
+              name: "post-pkg-test-pkg-exists"
+            }
+          }
+        }
+      };
+    };
 
     await database.insertNewPackage({
-      name: "post-packages-test-package",
+      name: "post-pkg-test-pkg-exists",
       repository: {
-        url: "https://github.com/confused-Techie/package-backend",
+        url: "https://github.com/confused-Techie/post-pkg-test-pkg-exists",
         type: "git"
       },
       creation_method: "Test Package",
       owner: 'confused-Techie',
       releases: { latest: "1.1.0" },
       readme: "This is a readme!",
-      metadata: { name: "post-packages-test-package" },
+      metadata: { name: "post-pkg-test-pkg-exists" },
       versions: {
         "1.1.0": {
           dist: {
             tarball: "download-url",
             sha: "1234"
           },
-          name: "post-packages-test-package"
+          name: "post-pkg-test-pkg-exists"
         }
       }
     });
 
     const sso = await endpoint.logic({
-      repository: "confused-Techie/post-packages-test-package",
+      repository: "confused-Techie/post-pkg-test-pkg-exists",
       auth: "valid-token"
     }, localContext);
 
     expect(sso.ok).toBe(false);
     expect(sso.short).toBe("package_exists");
 
-    await database.removePackageByName("post-packages-test-package", true);
+    await database.removePackageByName("post-pkg-test-pkg-exists", true);
   });
 
   test("Successfully publishes a new package", async () => {
@@ -180,7 +216,7 @@ describe("POST /api/packages Behaves as expected", () => {
       repository: "confused-Techie/post-pkg-test-pkg-name",
       auth: "valid-token"
     }, localContext);
-
+    
     expect(sso.ok).toBe(true);
     expect(sso.content.name).toBe("post-pkg-test-pkg-name");
     expect(sso.content.releases.latest).toBe("1.0.0");