split up find package logic to have both pkg lookup and ver lookup

previously package and version weren't done heirarchically, but now that they are there's different logic depending on whether you need a package or a version
ccrma · Apr 18, 2024 · 602ae78 · 602ae78
1 parent 47c3447
commit 602ae78
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 22 deletions.
diff --git a/src/manager.cpp b/src/manager.cpp
@@ -20,7 +20,7 @@ optional<Package> Manager::getPackage(string name) {
 
 bool Manager::install(std::string packageName) {
   // lookup package name (default to latest version)
-  auto pkg = package_list->lookup(packageName);
+  auto pkg = package_list->find_package(packageName);
 
   if (!pkg) {
     std::cerr << "Package " << packageName << " not found." << std::endl;

diff --git a/src/package.cpp b/src/package.cpp
@@ -119,13 +119,24 @@ void from_json(const json& j, PackageVersion& p) {
     j.at("files").get_to(p.files);
 }
 
+// find the latest compatible version
+// TODO expand filtering criteria beyond OS
 optional<PackageVersion> Package::latest_version(string os) {
-  optional<PackageVersion> latest;
+  optional<PackageVersion> latest_package;
+  optional<Version> latest_version;
+
+  for (PackageVersion pkg_ver : versions) {
+    if (pkg_ver.os != os) continue;
+
+    Version version = parseVersionString(pkg_ver.version);
+
+    if (version > latest_version) {
+      latest_package = pkg_ver;
+      latest_version = version;
+    }
 
-  for (PackageVersion ver : versions) {
-    if (ver.os != os) continue;
   }
-  return {};
+  return latest_package;
 }
 
 // converts a string of the format "1.2.3" into a Version struct

diff --git a/src/package_list.cpp b/src/package_list.cpp
@@ -6,7 +6,9 @@ PackageList::PackageList() {
   // throw std::runtime_error("not implemented");
 }
 
-PackageList::PackageList(string filepath) {
+PackageList::PackageList(string filepath, string operating_system) {
+  os = operating_system;
+
   std::ifstream f(filepath);
 
   // TODO better error checks
@@ -27,20 +29,35 @@ PackageList::PackageList(string filepath) {
   }
 }
 
-optional<Package> PackageList::lookup(string name) {
-  // TODO get highest version (how do I do that?)
+optional<Package> PackageList::find_package(string name) {
   for (auto package: packages) {
     if (package.name == name) {
       return package;
     }
   }
+
   return {};
 }
 
-optional<Package> PackageList::lookup(string name, string version) {
+optional<PackageVersion> PackageList::find_latest_package_version(string name) {
+  // TODO get highest version (how do I do that?)
   for (auto package: packages) {
     if (package.name == name) {
-      return package;
+      string os = whichOS();
+      return package.latest_version(os);
+    }
+  }
+  return {};
+}
+
+optional<PackageVersion> PackageList::find_package_version(string name, string version) {
+  for (auto package: packages) {
+    if (package.name == name) {
+      for (auto package_version: package.versions) {
+        if (package_version.version == version) {
+          return package_version;
+        }
+      }
     }
   }
   return {};

diff --git a/src/package_list.h b/src/package_list.h
@@ -6,6 +6,7 @@
 #include <nlohmann/json.hpp>
 
 #include "package.h"
+#include "util.h"
 
 using json = nlohmann::json;
 using std::string;
@@ -17,21 +18,27 @@ using std::optional;
 class PackageList {
  public:
   PackageList();
-  PackageList(string filepath);
+  PackageList(string filepath) : PackageList(filepath, whichOS()) {};
+  // set OS explicitly for unit tests
+  PackageList(string filepath, string operating_system);
 
  public:
+  optional<Package> find_package(string name);
   // return latest (maybe latest && most compatible?) version of package
-  optional<Package> lookup(string name);
+  optional<PackageVersion> find_latest_package_version(string name);
 
   // return specific version of package
-  optional<Package> lookup(string name, string version);
+  optional<PackageVersion> find_package_version(string name, string version);
 
 
   /* static void from_json(const json& j, PackageList& pkg_list); */
 
  private:
   // TODO could be a more efficient lookup implementation
   std::vector<Package> packages;
+
+  // set OS at initialization time
+  string os;
 };
 
 #endif
diff --git a/src/test/data/test-package-list.json b/src/test/data/test-package-list.json
@@ -10,11 +10,18 @@
             "keywords": ["KeywordA", "KeywordB"],
             "versions": [
                 {
-                    "version": "1.0",
+                    "version": "1.0.0",
                     "api_version": "9.0",
                     "language_version": "1.5.2.1",
                     "os": "linux",
                     "files": ["https://ccrma.stanford.edu/~nshaheed/chugins/Hydra/linux/butt.chug"]
+                },
+                {
+                    "version": "0.9.1",
+                    "api_version": "8.8",
+                    "language_version": "1.5.0.1",
+                    "os": "linux",
+                    "files": ["https://ccrma.stanford.edu/~nshaheed/chugins/Hydra/linux/butt.chug"]
                 }
             ]
         }

diff --git a/src/test/package_list_test.cpp b/src/test/package_list_test.cpp
@@ -5,7 +5,7 @@ using std::optional;
 
 TEST_CASE("Load db file", "[PackageList]") {
   std::string path = "./data/test-package-list.json";
-  PackageList pkglist = PackageList(path);
+  PackageList pkglist = PackageList(path, "linux");
 
   Package want;
   PackageVersion version;
@@ -29,15 +29,50 @@ TEST_CASE("Load db file", "[PackageList]") {
 
   // Package want = {"John", "Butt", "1.0", "9.0", {{"linux", {"https://ccrma.stanford.edu/~nshaheed/chugins/Hydra/linux/butt.chug"}}}, "http://example.com", "http://repo.com", "specA", {"AuthorA", "AuthorB"}, "MIT", "DescriptionA", {"KeywordA", "KeywordB"}};
 
-  optional<Package> got = pkglist.lookup("Butt");
+  optional<Package> got = pkglist.find_package("Butt");
 
+  REQUIRE(got.has_value());
 
-  REQUIRE(got == want);
+  REQUIRE(got.value().name == want.name);
+  REQUIRE(got.value().versions.size() > 0);
+}
+
+TEST_CASE("Find Package") {
+  std::string path = "./data/test-package-list.json";
+  PackageList pkglist = PackageList(path, "linux");
+
+  SECTION("Successfully find package") {
+    optional<Package> pkg = pkglist.find_package("Butt");
+    REQUIRE(pkg.value().name == "Butt");
+  }
+
+  SECTION("Can't find package") {
+    optional<Package> pkg = pkglist.find_package("Fork");
+    REQUIRE_FALSE(pkg);
+  }
+}
+
+TEST_CASE("Find Package Version") {
+  std::string path = "./data/test-package-list.json";
+  PackageList pkglist = PackageList(path, "linux");
+
+  SECTION("Successfully find package") {
+    optional<PackageVersion> version = pkglist.find_latest_package_version("Butt");
+    REQUIRE(version.value().version == "1.0.0");
+  }
 
-  got = pkglist.lookup("Butt", "1.0");
-  REQUIRE(got == want);
+  SECTION("Can't find package") {
+    optional<PackageVersion> version = pkglist.find_latest_package_version("Fork");
+    REQUIRE_FALSE(version);
+  }
 
+  SECTION("Successfully find version") {
+    optional<PackageVersion> version = pkglist.find_package_version("Butt", "1.0.0");
+    REQUIRE(version.value().version == "1.0.0");
+  }
 
-  got = pkglist.lookup("NotReal");
-  REQUIRE_FALSE(got);
+  SECTION("Unsuccessfully find version") {
+    optional<PackageVersion> version = pkglist.find_package_version("Butt", "1.2.0");
+    REQUIRE_FALSE(version.has_value());
+  }
 }
diff --git a/src/uninstaller.cpp b/src/uninstaller.cpp
@@ -7,7 +7,7 @@ Uninstaller::Uninstaller(PackageList* pkg_list) {
 
 bool Uninstaller::uninstall(std::string package_name) {
   // lookup package name (default to latest version)
-  auto pkg = package_list->lookup(package_name);
+  auto pkg = package_list->find_package(package_name);
 
   if (!pkg) {
     std::cerr << "BADBADBAD" << std::endl;