Merge pull request #232 from pulsar-edit/combine-filter-and-search

Make Searches Filterable

src/controllers/getPackagesSearch.js

@@ -40,6 +40,21 @@ module.exports = {
     filter: (context, req) => {
       return context.query.filter(req);
     },
+    fileExtension: (context, req) => {
+      return context.query.fileExtension(req);
+    },
+    serviceType: (context, req) => {
+      return context.query.serviceType(req);
+    },
+    service: (context, req) => {
+      return context.query.service(req);
+    },
+    serviceVersion: (context, req) => {
+      return context.query.serviceVersion(req);
+    },
+    owner: (context, req) => {
+      return context.query.owner(req);
+    },
   },
 
   /**
@@ -58,13 +73,7 @@ module.exports = {
     // This is only an effort to get this working quickly and should be changed later.
     // This also means for now, the default sorting method will be downloads, not relevance.
 
-    const packs = await context.database.simpleSearch(
-      params.query,
-      params.page,
-      params.direction,
-      params.sort,
-      params.filter === "theme"
-    );
+    const packs = await context.database.getSortedPackages(params);
 
     if (!packs.ok) {
       if (packs.short === "not_found") {
@@ -82,7 +91,7 @@ module.exports = {
 
       const sso = new context.sso();
 
-      return sso.notOk().addContent(packs).addCalls("db.simpleSearch", packs);
+      return sso.notOk().addContent(packs).addCalls("db.getSortedPackages", packs);
     }
 
     const newPacks = await context.utils.constructPackageObjectShort(

src/controllers/getThemesSearch.js

@@ -40,18 +40,16 @@ module.exports = {
   },
 
   async logic(params, context) {
-    const packs = await context.database.simpleSearch(
-      params.query,
-      params.page,
-      params.direction,
-      params.sort,
+
+    const packs = await context.database.getSortedPackages(
+      params,
       true
     );
 
     if (!packs.ok) {
       const sso = new context.sso();
 
-      return sso.notOk().addContent(packs).addCalls("db.simpleSearch", packs);
+      return sso.notOk().addContent(packs).addCalls("db.getSortedPackages", packs);
     }
 
     const newPacks = await context.utils.constructPackageObjectShort(

src/database.js

@@ -1440,96 +1440,6 @@ async function getStarringUsersByPointer(pointer) {
   }
 }
 
-/**
- * @async
- * @function simpleSearch
- * @description The current Fuzzy-Finder implementation of search. Ideally eventually
- * will use a more advanced search method.
- * @param {string} term - The search term.
- * @param {string} dir - String flag for asc/desc order.
- * @param {string} sort - The sort method.
- * @param {boolean} [themes=false] - Optional Parameter to specify if this should only return themes.
- * @returns {object} A server status object containing the results and the pagination object.
- */
-async function simpleSearch(term, page, dir, sort, themes = false) {
-  try {
-    sqlStorage ??= setupSQL();
-
-    // Parse the sort method
-    const orderType = getOrderField(sort, sqlStorage);
-
-    if (orderType === null) {
-      logger.generic(3, `Unrecognized Sorting Method Provided: ${sort}`);
-      return {
-        ok: false,
-        content: `Unrecognized Sorting Method Provided: ${sort}`,
-        short: "server_error",
-      };
-    }
-
-    // We obtain the lowercase version of term since names should be in
-    // lowercase format (see atom-backend issue #86).
-    const lcterm = term.toLowerCase();
-
-    const wordSeparators = /[-. ]/g; // Word Separators: - . SPACE
-
-    const searchTerm = lcterm.replace(wordSeparators, "_");
-    // Replaces all word separators with '_' which matches any single character
-
-    const limit = paginated_amount;
-    const offset = page > 1 ? (page - 1) * limit : 0;
-
-    const command = await sqlStorage`
-      WITH search_query AS (
-        SELECT DISTINCT ON (p.name) p.name, p.data, p.downloads, p.owner,
-          (p.stargazers_count + p.original_stargazers) AS stargazers_count,
-          v.semver, p.created, v.updated, p.creation_method
-        FROM packages AS p
-          INNER JOIN names AS n ON (p.pointer = n.pointer AND n.name LIKE ${
-            "%" + searchTerm + "%"
-          }
-          ${
-            themes === true
-              ? sqlStorage`AND p.package_type = 'theme'`
-              : sqlStorage``
-          })
-          INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE)
-        ORDER BY p.name, v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC
-      )
-      SELECT *, COUNT(*) OVER() AS query_result_count
-      FROM search_query
-      ORDER BY ${orderType} ${
-      dir === "desc" ? sqlStorage`DESC` : sqlStorage`ASC`
-    }
-      LIMIT ${limit}
-      OFFSET ${offset};
-    `;
-
-    const resultCount = command[0]?.query_result_count ?? 0;
-    const quotient = Math.trunc(resultCount / limit);
-    const remainder = resultCount % limit;
-    const totalPages = quotient + (remainder > 0 ? 1 : 0);
-
-    return {
-      ok: true,
-      content: command,
-      pagination: {
-        count: resultCount,
-        page: page < totalPages ? page : totalPages,
-        total: totalPages,
-        limit: limit,
-      },
-    };
-  } catch (err) {
-    return {
-      ok: false,
-      content: "Generic Error",
-      short: "server_error",
-      error: err,
-    };
-  }
-}
-
 /**
  * @async
  * @function getUserCollectionById
@@ -1568,6 +1478,42 @@ function getEmptyClause() {
   return emptyClause;
 }
 
+function queryClause(opts) {
+  if (typeof opts.query !== "string") {
+    return getEmptyClause();
+  }
+
+  // We obtain the lowercase version of the query since names should be in
+  // lowercase format (see atom-backend issue #86)
+  const lcterm = opts.query.toLowerCase();
+
+  const wordSeparators = /[-. ]/g; // Word Separators: - . SPACE
+
+  const searchTerm = lcterm.replace(wordSeparators, "_");
+  // Replaces all word separators with '_' which matches any single character
+
+  return sqlStorage`AND p.name LIKE ${"%" + searchTerm + "%" }`;
+}
+
+function filterClause(opts) {
+  if (typeof opts.filter !== "string") {
+    return getEmptyClause();
+  }
+
+  if (opts.filter === "theme") {
+    return sqlStorage`AND p.package_type = 'theme'`;
+  } else if (opts.filter === "package") {
+    // Since our fork from Atom, we have made the choice to return themes and packages
+    // on basic searches, meaning that `ppm`s filter of `package` has always returned
+    // packages and themes.
+    // If we decide to change this, uncomment the below line.
+    //return sqlStorage`AND p.package_type = 'package'`;
+    return getEmptyClause();
+  } else {
+    return getEmptyClause();
+  }
+}
+
 function ownerClause(opts) {
   if (typeof opts.owner !== "string") {
     return getEmptyClause();
@@ -1643,9 +1589,11 @@ async function getSortedPackages(opts, themes = false) {
           v.semver, p.created, v.updated, p.creation_method
         FROM packages AS p
           INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE
+          ${queryClause(opts)}
+          ${filterClause(opts)}
           ${
             themes === true
-              ? sqlStorage`AND p.package_type = 'theme'`
+              ? filterClause({ filter: "theme" })
               : sqlStorage``
           })
 
@@ -1834,7 +1782,6 @@ module.exports = {
   updatePackageIncrementDownloadByName,
   updatePackageDecrementDownloadByName,
   getFeaturedThemes,
-  simpleSearch,
   updateIncrementStar,
   updateDecrementStar,
   insertNewUser,

tests/database/database.test.js

@@ -87,23 +87,6 @@ describe("Get Sorted Packages", () => {
   });
 });
 
-describe("Get Package Search", () => {
-  test("Should return good pagination when there are no results", async () => {
-    const obj = await database.simpleSearch(
-      "will-never-match-a-search",
-      1,
-      "desc",
-      "relevance"
-    );
-    expect(obj.ok).toBeTruthy();
-    expect(Array.isArray(obj.content)).toBeTruthy();
-    expect(obj.content.length).toBe(0);
-    expect(obj.pagination.count).toBe(0);
-    expect(obj.pagination.page).toBe(0);
-    expect(obj.pagination.total).toBe(0);
-  });
-});
-
 describe("Package Lifecycle Tests", () => {
   // Below are what we will call lifecycle tests.
   // That is tests that will test multiple actions against the same package,

tests/http/getPackagesSearch.test.js

@@ -65,4 +65,50 @@ describe("Behaves as expected", () => {
     expect(sso.content[0].name).toBe("get-packages-search-theme-test");
     expect(sso).toMatchEndpointSuccessObject(endpoint);
   });
+
+  test("Successfully searches and filters by supported extensions", async () => {
+    await database.applyFeatures(
+      {
+        hasSnippets: false,
+        hasGrammar: true,
+        supportedLanguages: ["js", "ts"]
+      },
+      "get-packages-search-test",
+      "1.0.0"
+    );
+
+    let sso = await endpoint.logic(
+      {
+        sort: "downloads",
+        page: 1,
+        direction: "desc",
+        query: "get packages search",
+        filter: "package",
+        fileExtension: "js"
+      },
+      context
+    );
+
+    expect(sso.ok).toBe(true);
+    expect(sso.content.length).toBe(1);
+    expect(sso.content[0].name).toBe("get-packages-search-test");
+    expect(sso).toMatchEndpointSuccessObject(endpoint);
+
+    // Now if we exclude the package via the supported file extensions
+    let ssoEmpty = await endpoint.logic(
+      {
+        sort: "downloads",
+        page: 1,
+        direction: "desc",
+        query: "get packages search",
+        filter: "package",
+        fileExtension: "sql"
+      },
+      context
+    );
+
+    expect(ssoEmpty.ok).toBe(true);
+    expect(ssoEmpty.content.length).toBe(0);
+
+  });
 });

tests/http/getThemesSearch.test.js

@@ -7,7 +7,7 @@ const genPackage = require("../helpers/package.jest.js");
 describe("Behaves as expected", () => {
   test("Calls the correct function", async () => {
     const localContext = context;
-    const spy = jest.spyOn(localContext.database, "simpleSearch");
+    const spy = jest.spyOn(localContext.database, "getSortedPackages");
 
     await endpoint.logic({}, localContext);
 
@@ -63,7 +63,7 @@ describe("Behaves as expected", () => {
     // Moved to last position, since it modifies our shallow copied context
     const localContext = context;
     localContext.database = {
-      simpleSearch: () => {
+      getSortedPackages: () => {
         return { ok: false, content: "Test Error" };
       },
     };