Repository and package id mapping refactoring

7 files changed, 128 insertions, 181 deletions

diff --git a/brep/package b/brep/package
index 800f716..aaba0ac 100644
--- a/brep/package
+++ b/brep/package
@@ -29,13 +29,14 @@ namespace brep
   // Use an image type to map bpkg::version to the database since there
   // is no way to modify individual components directly.
   //
+
   #pragma db value
   struct _version
   {
     std::uint16_t epoch;
-    std::string upstream;
-    std::uint16_t revision;
     std::string canonical_upstream;
+    std::uint16_t revision;
+    std::string upstream;
   };
 }
 
@@ -48,9 +49,9 @@ namespace brep
 //
 #pragma db map type(bpkg::version) as(brep::_version)                     \
   to(brep::_version{(?).epoch,                                            \
-                    (?).upstream,                                         \
+                    (?).canonical_upstream,                               \
                     (?).revision,                                         \
-                    (?).canonical_upstream})                              \
+                    (?).upstream})                                        \
   from(bpkg::version ((?).epoch, std::move ((?).upstream), (?).revision))
 
 #pragma db map type(bpkg::comparison) as(std::string) \
@@ -79,6 +80,8 @@ namespace brep
   template <typename T>
   using optional = butl::optional<T>;
 
+  // path
+  //
   using path = butl::path;
 
   #pragma db map type(path) as(std::string)  \
@@ -96,44 +99,65 @@ namespace brep
   #pragma db map type(dir_path) as(std::string)  \
     to((?).string ()) from(brep::dir_path (?))
 
+  // timestamp
+  //
   using timestamp = butl::timestamp;
 
   #pragma db map type(timestamp) as(std::uint64_t)   \
     to(std::chrono::system_clock::to_time_t (?))     \
     from(std::chrono::system_clock::from_time_t (?))
 
+  // version
+  //
   using version = bpkg::version;
-  using repository_location = bpkg::repository_location;
 
   #pragma db value
-  struct package_id
+  struct canonical_version
   {
-    std::string name;
+    std::uint16_t epoch;
+    std::string   canonical_upstream;
+    std::uint16_t revision;
+  };
 
-    #pragma db value
-    struct version_type
-    {
-      std::uint16_t epoch;
-      std::string canonical_upstream;
-      std::uint16_t revision;
-    };
+  #pragma db value transient
+  struct upstream_version: version
+  {
+    #pragma db member(upstream_) virtual(std::string) \
+      get(this.upstream)                              \
+      set(this = brep::version (0, std::move (?), 0))
 
-    version_type version;
+    upstream_version () = default;
+    upstream_version (version v): version (std::move (v)) {}
+    upstream_version&
+    operator= (version v) {version& b (*this); b = v; return *this;}
 
-    // Database mapping.
-    //
+    void
+    init (const canonical_version& cv, const upstream_version& uv)
+    {
+      *this = version (cv.epoch, uv.upstream, cv.revision);
+      assert (cv.canonical_upstream == canonical_upstream);
+    }
   };
 
+  // priority
+  //
   using priority = bpkg::priority;
+
   #pragma db value(priority) definition
   #pragma db member(priority::value) column("")
 
+  // url
+  //
   using url = bpkg::url;
+
   #pragma db value(url) definition
   #pragma db member(url::value) virtual(std::string) before access(this) \
           column("")
 
+  // email
+  //
   using email = bpkg::email;
+
   #pragma db value(email) definition
   #pragma db member(email::value) virtual(std::string) before access(this) \
           column("")
@@ -141,14 +165,15 @@ namespace brep
   // licenses
   //
   using licenses = bpkg::licenses;
-  #pragma db value(licenses) definition
-
   using license_alternatives = std::vector<licenses>;
 
+  #pragma db value(licenses) definition
+
   // dependencies
   //
   using comparison = bpkg::comparison;
   using dependency_constraint = bpkg::dependency_constraint;
+
   #pragma db value(dependency_constraint) definition
 
   // Notes:
@@ -192,20 +217,26 @@ namespace brep
   //      relations otherwise required just for dependency resolution.
   //
   using dependency = bpkg::dependency;
+  using dependency_alternatives = bpkg::dependency_alternatives;
+  using dependencies = std::vector<dependency_alternatives>;
+
   #pragma db value(dependency) definition
   #pragma db member(dependency::constraint) column("")
-
-  using dependency_alternatives = bpkg::dependency_alternatives;
   #pragma db value(dependency_alternatives) definition
 
-  using dependencies = std::vector<dependency_alternatives>;
-
   // requirements
   //
   using requirement_alternatives = bpkg::requirement_alternatives;
+  using requirements = std::vector<requirement_alternatives>;
+
   #pragma db value(requirement_alternatives) definition
 
-  using requirements = std::vector<requirement_alternatives>;
+  // repository_location
+  //
+  using repository_location = bpkg::repository_location;
+
+  #pragma db map type(repository_location) as(std::string) \
+    to((?).string ()) from(brep::repository_location (?))
 
   #pragma db object pointer(std::shared_ptr) session
   class repository
@@ -220,9 +251,9 @@ namespace brep
     // Create external repository.
     //
     explicit
-    repository (repository_location l)
-        : location (std::move (l)), internal (false) {}
+    repository (repository_location);
 
+    std::string name; // Object id (canonical name).
     repository_location location;
     std::string display_name;
 
@@ -244,29 +275,31 @@ namespace brep
 
     // Database mapping.
     //
-    #pragma db value
-    struct _id_type
-    {
-      std::string canonical_name;
-      std::string location;
-    };
-
-    _id_type
-    _id () const;
-
-    void
-    _id (_id_type&&);
-
-    #pragma db member(location) transient
+    #pragma db member(name) id
 
-    #pragma db member(id) virtual(_id_type) before id(canonical_name)   \
-      get(_id) set(_id (std::move (?))) column("")
+    #pragma db member(location)                                  \
+      set(this.location = std::move (?);                         \
+          assert (this.name == this.location.canonical_name ()))
 
   private:
     friend class odb::access;
     repository () = default;
   };
 
+  #pragma db value
+  struct package_id
+  {
+    std::string name;
+    canonical_version version;
+
+    package_id () = default;
+    package_id (std::string n, const brep::version& v)
+        : name (std::move (n)),
+          version {v.epoch, v.canonical_upstream, v.revision}
+    {
+    }
+  };
+
   #pragma db object pointer(std::shared_ptr) session
   class package
   {
@@ -307,14 +340,12 @@ namespace brep
     // required to compose such a link is the package name, version, and
     // repository location.
     //
-    package (std::string name,
-             version_type,
-             std::shared_ptr<repository_type>);
+    package (std::string name, version_type, std::shared_ptr<repository_type>);
 
     // Manifest data.
     //
-    std::string name;
-    version_type version;
+    package_id id;
+    upstream_version version;
     priority_type priority;
     std::string summary;
     license_alternatives_type license_alternatives;
@@ -338,30 +369,8 @@ namespace brep
 
     // Database mapping.
     //
-
-    // id
-    //
-    #pragma db value
-    struct _id_type
-    {
-      #pragma db column("")
-      package_id data;
-
-      #pragma db column("version_upstream")
-      std::string upstream;
-    };
-
-    _id_type
-    _id () const;
-
-    void
-    _id (_id_type&&, odb::database&);
-
-    #pragma db member(name) transient
-    #pragma db member(version) transient
-
-    #pragma db member(id) virtual(_id_type) before id(data) \
-      get(_id) set(_id (std::move (?), (!))) column("")
+    #pragma db member(id) id column("")
+    #pragma db member(version) set(this.version.init (this.id.version, (?)))
 
     // license
     //
@@ -431,11 +440,10 @@ namespace brep
   // Find an internal package of the latest version.
   //
   #pragma db view object(package)                         \
-    object(package = p:                                   \
-           package::id.data.name == p::id.data.name &&    \
-           package::id.data.version < p::id.data.version) \
+    object(package = p: package::id.name == p::id.name && \
+           package::id.version < p::id.version)           \
     query((package::internal_repository.is_not_null () && \
-           p::id.data.name.is_null ()) + "AND" + (?))
+           p::id.name.is_null ()) + "AND" + (?))
   struct latest_internal_package
   {
     using package_type = brep::package;
@@ -451,7 +459,7 @@ namespace brep
     query(package::internal_repository.is_not_null () && (?))
   struct internal_package_name_count
   {
-    #pragma db column("count(DISTINCT" + package::id.data.name+ ")")
+    #pragma db column("count(DISTINCT" + package::id.name + ")")
     std::size_t result;
 
     operator std::size_t () const {return result;}
diff --git a/brep/package-search.cxx b/brep/package-search.cxx
index 27709c8..ebc5055 100644
--- a/brep/package-search.cxx
+++ b/brep/package-search.cxx
@@ -99,7 +99,7 @@ namespace brep
 
     auto r (
       db_->query<latest_internal_package> (query (true) +
-        "ORDER BY" + query::package::id.data.name +
+        "ORDER BY" + query::package::id.name +
         "OFFSET" + to_string (pr.page () * rop) +
         "LIMIT" + to_string (rop)));
 
@@ -110,7 +110,7 @@ namespace brep
       s << DIV(CLASS="package")
         <<   DIV(CLASS="name")
         <<     A
-        <<     HREF << "/go/" << mime_url_encode (p.name);
+        <<     HREF << "/go/" << mime_url_encode (p.id.name);
 
       // Propagate search criteria to the package version search url.
       //
@@ -118,7 +118,7 @@ namespace brep
         s << "?" << q;
 
       s <<     ~HREF
-        <<       p.name
+        <<       p.id.name
         <<     ~A
         <<   ~DIV
         <<   DIV(CLASS="summary") << p.summary << ~DIV
diff --git a/brep/package-version-details.cxx b/brep/package-version-details.cxx
index 543f3b6..64945c4 100644
--- a/brep/package-version-details.cxx
+++ b/brep/package-version-details.cxx
@@ -119,8 +119,7 @@ namespace brep
 
     try
     {
-      package_id id {n, v.epoch, v.canonical_upstream, v.revision};
-      p = db_->load<package> (id);
+      p = db_->load<package> (package_id (n, v));
 
       // If the requested package turned up to be an "external" one just
       // respond that no "internal" package is present.
diff --git a/brep/package-version-search.cxx b/brep/package-version-search.cxx
index 8433b75..933aaa8 100644
--- a/brep/package-version-search.cxx
+++ b/brep/package-version-search.cxx
@@ -98,7 +98,7 @@ namespace brep
 
       latest_internal_package ip;
       if (!db_->query_one<latest_internal_package> (
-            query::package::id.data.name == name, ip))
+            query::package::id.name == name, ip))
       {
         throw invalid_request (404, "Package '" + name + "' not found");
       }
@@ -122,8 +122,7 @@ namespace brep
       // @@ Query will also include search criteria if specified.
       //
       pvc = db_->query_value<package_count> (
-        query::id.data.name == name &&
-        query::internal_repository.is_not_null ());
+        query::id.name == name && query::internal_repository.is_not_null ());
     }
 
     s << DIV(ID="versions") << "Versions (" << pvc << ")" << ~DIV;
@@ -146,9 +145,8 @@ namespace brep
     using query = query<package>;
     auto r (
       db_->query<package> (
-        (query::id.data.name == name &&
-         query::internal_repository.is_not_null ()) +
-        order_by_version_desc (query::id.data.version) +
+        (query::id.name == name && query::internal_repository.is_not_null ()) +
+        order_by_version_desc (query::id.version) +
         "OFFSET" + to_string (pr.page () * rop) +
         "LIMIT" + to_string (rop)));
 
diff --git a/brep/package.cxx b/brep/package.cxx
index be14034..fd40bc6 100644
--- a/brep/package.cxx
+++ b/brep/package.cxx
@@ -35,7 +35,7 @@ namespace brep
            requirements_type rq,
            optional<path> lc,
            shared_ptr<repository_type> rp)
-      : name (move (nm)),
+      : id (move (nm), vr),
         version (move (vr)),
         priority (move (pr)),
         summary (move (sm)),
@@ -59,35 +59,13 @@ namespace brep
   package (string nm,
            version_type vr,
            shared_ptr<repository_type> rp)
-      : name (move (nm)),
+      : id (move (nm), vr),
         version (move (vr))
   {
     assert (!rp->internal);
     external_repositories.emplace_back (move (rp));
   }
 
-  package::_id_type package::
-  _id () const
-  {
-    return _id_type {
-      {
-        name,
-        version.epoch,
-        version.canonical_upstream,
-        version.revision
-      },
-      version.upstream};
-  }
-
-  void package::
-  _id (_id_type&& v, database&)
-  {
-    const auto& dv (v.data.version);
-    name = move (v.data.name);
-    version = version_type (dv.epoch, move (v.upstream), dv.revision);
-    assert (version.canonical_upstream == dv.canonical_upstream);
-  }
-
   // repository
   //
   repository::
@@ -97,18 +75,14 @@ namespace brep
         local_path (move (p)),
         internal (true)
   {
+    name = location.canonical_name ();
   }
 
-  repository::_id_type repository::
-  _id () const
-  {
-    return _id_type {location.canonical_name (), location.string ()};
-  }
-
-  void repository::
-  _id (_id_type&& l)
+  repository::
+  repository (repository_location l)
+      : location (move (l)),
+        internal (false)
   {
-    location = repository_location (move (l.location));
-    assert (location.canonical_name () == l.canonical_name);
+    name = location.canonical_name ();
   }
 }
diff --git a/loader/loader.cxx b/loader/loader.cxx
index 5a71305..dd9cb5a 100644
--- a/loader/loader.cxx
+++ b/loader/loader.cxx
@@ -227,8 +227,7 @@ changed (const internal_repositories& repos, database& db)
   //
   auto rs (
     db.query<repository> (
-      query::internal &&
-      !query::id.canonical_name.in_range (names.begin (), names.end ())));
+      query::internal && !query::name.in_range (names.begin (), names.end ())));
 
   return !rs.empty ();
 }
diff --git a/tests/loader/driver.cxx b/tests/loader/driver.cxx
index ef0654f..77e39a2 100644
--- a/tests/loader/driver.cxx
+++ b/tests/loader/driver.cxx
@@ -42,7 +42,7 @@ check_location (shared_ptr<package>& p)
     return !p->location;
   else
     return p->location && *p->location ==
-      path (p->name + "-" + p->version.string () + ".tar.gz");
+      path (p->id.name + "-" + p->version.string () + ".tar.gz");
 }
 
 int
@@ -111,45 +111,28 @@ main (int argc, char* argv[])
               file_mtime (dir_path (sr->local_path) / path ("repositories")));
       assert (sr->internal);
 
-      version fv1 ("1.0");
       shared_ptr<package> fpv1 (
-        db.load<package> (
-          package_id {
-            "libfoo", {fv1.epoch, fv1.canonical_upstream, fv1.revision}}));
+        db.load<package> (package_id ("libfoo", version ("1.0"))));
       assert (check_location (fpv1));
 
-      version fv2 ("1.2.2");
       shared_ptr<package> fpv2 (
-        db.load<package> (
-          package_id {
-            "libfoo", {fv2.epoch, fv2.canonical_upstream, fv2.revision}}));
+        db.load<package> (package_id ("libfoo", version ("1.2.2"))));
       assert (check_location (fpv2));
 
-      version fv3 ("1.2.3-4");
       shared_ptr<package> fpv3 (
-        db.load<package> (
-          package_id {
-            "libfoo", {fv3.epoch, fv3.canonical_upstream, fv3.revision}}));
+        db.load<package> (package_id ("libfoo", version ("1.2.3-4"))));
       assert (check_location (fpv3));
 
-      version fv4 ("1.2.4");
       shared_ptr<package> fpv4 (
-        db.load<package> (
-          package_id {
-            "libfoo", {fv4.epoch, fv4.canonical_upstream, fv4.revision}}));
+        db.load<package> (package_id ("libfoo", version ("1.2.4"))));
       assert (check_location (fpv4));
 
-      version xv ("1.0.0-1");
       shared_ptr<package> xpv (
-        db.load<package> (
-          package_id {
-            "libstudxml", {xv.epoch, xv.canonical_upstream, xv.revision}}));
+        db.load<package> (package_id ("libstudxml", version ("1.0.0-1"))));
       assert (check_location (xpv));
 
       // Verify libstudxml package version.
       //
-      assert (xpv->name == "libstudxml");
-      assert (xpv->version == version ("1.0.0-1"));
       assert (xpv->summary == "Modern C++ XML API");
       assert (xpv->tags == strings ({"c++", "xml", "parser", "serializer",
               "pull", "streaming", "modern"}));
@@ -189,8 +172,8 @@ main (int argc, char* argv[])
 
       // Verify libfoo package versions.
       //
-      assert (fpv1->name == "libfoo");
-      assert (fpv1->version == version ("1.0"));
+      // libfoo-1.0
+      //
       assert (fpv1->summary == "The Foo Library");
       assert (fpv1->tags.empty ());
       assert (!fpv1->description);
@@ -212,8 +195,8 @@ main (int argc, char* argv[])
       assert (fpv1->dependencies.empty ());
       assert (fpv1->requirements.empty ());
 
-      assert (fpv2->name == "libfoo");
-      assert (fpv2->version == version ("1.2.2"));
+      // libfoo-1.2.2
+      //
       assert (fpv2->summary == "The Foo library");
       assert (fpv2->tags == strings ({"c++", "foo"}));
       assert (!fpv2->description);
@@ -249,8 +232,8 @@ main (int argc, char* argv[])
 
       assert (fpv2->requirements.empty ());
 
-      assert (fpv3->name == "libfoo");
-      assert (fpv3->version == version ("1.2.3-4"));
+      // libfoo-1.2.3-4
+      //
       assert (fpv3->summary == "The Foo library");
       assert (fpv3->tags == strings ({"c++", "foo"}));
       assert (!fpv3->description);
@@ -277,8 +260,8 @@ main (int argc, char* argv[])
                  brep::optional<dependency_constraint> (
                    dependency_constraint{comparison::ge, version ("2.0.0")})}));
 
-      assert (fpv4->name == "libfoo");
-      assert (fpv4->version == version ("1.2.4"));
+      // libfoo-1.2.4
+      //
       assert (fpv4->summary == "The Foo Library");
       assert (fpv4->tags == strings ({"c++", "foo"}));
       assert (*fpv4->description == "Very good foo library.");
@@ -322,24 +305,18 @@ main (int argc, char* argv[])
               file_mtime (dir_path (mr->local_path) / path ("repositories")));
       assert (mr->internal);
 
-      version ev ("1+1.2");
       shared_ptr<package> epv (
-        db.load<package> (
-          package_id {
-            "libexp", {ev.epoch, ev.canonical_upstream, ev.revision}}));
+        db.load<package> (package_id ("libexp", version ("1+1.2"))));
       assert (check_location (epv));
 
-      version fv5 ("1.2.4-1");
       shared_ptr<package> fpv5 (
-        db.load<package> (
-          package_id {
-            "libfoo", {fv5.epoch, fv5.canonical_upstream, fv5.revision}}));
+        db.load<package> (package_id ("libfoo", version ("1.2.4-1"))));
       assert (check_location (fpv5));
 
       // Verify libfoo package versions.
       //
-      assert (fpv5->name == "libfoo");
-      assert (fpv5->version == version ("1.2.4-1"));
+      // libfoo-1.2.4-1
+      //
       assert (fpv5->summary == "The Foo Math Library");
       assert (fpv5->tags == strings ({"c++", "foo", "math"}));
       assert (*fpv5->description ==
@@ -425,8 +402,8 @@ main (int argc, char* argv[])
 
       // Verify libexp package version.
       //
-      assert (epv->name == "libexp");
-      assert (epv->version == version ("1+1.2"));
+      // libexp-1+1.2
+      //
       assert (epv->summary == "The exponent");
       assert (epv->tags == strings ({"c++", "exponent"}));
       assert (!epv->description);
@@ -465,24 +442,18 @@ main (int argc, char* argv[])
       assert (cr->repositories_timestamp == timestamp_nonexistent);
       assert (!cr->internal);
 
-      version bv ("2.3.5");
       shared_ptr<package> bpv (
-        db.load<package> (
-          package_id {
-            "libbar", {bv.epoch, bv.canonical_upstream, bv.revision}}));
+        db.load<package> (package_id ("libbar", version ("2.3.5"))));
       assert (check_location (bpv));
 
-      version fv0 ("0.1");
       shared_ptr<package> fpv0 (
-        db.load<package> (
-          package_id {
-            "libfoo", {fv0.epoch, fv0.canonical_upstream, fv0.revision}}));
+        db.load<package> (package_id ("libfoo", version ("0.1"))));
       assert (check_location (fpv0));
 
       // Verify libbar package version.
       //
-      assert (bpv->name == "libbar");
-      assert (bpv->version == version ("2.3.5"));
+      // libbar-2.3.5
+      //
       assert (bpv->summary.empty ());
       assert (bpv->tags.empty ());
       assert (!bpv->description);
@@ -504,8 +475,8 @@ main (int argc, char* argv[])
 
       // Verify libfoo package versions.
       //
-      assert (fpv0->name == "libfoo");
-      assert (fpv0->version == version ("0.1"));
+      // libfoo-0.1
+      //
       assert (fpv0->summary.empty ());
       assert (fpv0->tags.empty ());
       assert (!fpv0->description);
@@ -537,10 +508,8 @@ main (int argc, char* argv[])
     assert (process (ld_args).wait ());
     transaction t (db.begin ());
 
-    version bv ("2.3.5");
     shared_ptr<package> bpv (
-      db.load<package> (
-        package_id {"libbar", {bv.epoch, bv.canonical_upstream, bv.revision}}));
+      db.load<package> (package_id ("libbar", version ("2.3.5"))));
 
     assert (bpv->summary == "test");