From 3e37999a5f9efd4caf44c40985b3e1254660a625 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 12 Oct 2015 14:07:16 +0200 Subject: Repository and package id mapping refactoring --- brep/package | 160 ++++++++++++++++++++------------------- brep/package-search.cxx | 6 +- brep/package-version-details.cxx | 3 +- brep/package-version-search.cxx | 10 +-- brep/package.cxx | 42 ++-------- loader/loader.cxx | 3 +- tests/loader/driver.cxx | 85 +++++++-------------- 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 using optional = butl::optional; + // 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; + #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; + #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; - // requirements // using requirement_alternatives = bpkg::requirement_alternatives; + using requirements = std::vector; + #pragma db value(requirement_alternatives) definition - using requirements = std::vector; + // 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); + package (std::string name, version_type, std::shared_ptr); // 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 (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 (id); + p = db_->load (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 ( - 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 ( - 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; auto r ( db_->query ( - (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 lc, shared_ptr 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 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 ( - 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& 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 fpv1 ( - db.load ( - package_id { - "libfoo", {fv1.epoch, fv1.canonical_upstream, fv1.revision}})); + db.load (package_id ("libfoo", version ("1.0")))); assert (check_location (fpv1)); - version fv2 ("1.2.2"); shared_ptr fpv2 ( - db.load ( - package_id { - "libfoo", {fv2.epoch, fv2.canonical_upstream, fv2.revision}})); + db.load (package_id ("libfoo", version ("1.2.2")))); assert (check_location (fpv2)); - version fv3 ("1.2.3-4"); shared_ptr fpv3 ( - db.load ( - package_id { - "libfoo", {fv3.epoch, fv3.canonical_upstream, fv3.revision}})); + db.load (package_id ("libfoo", version ("1.2.3-4")))); assert (check_location (fpv3)); - version fv4 ("1.2.4"); shared_ptr fpv4 ( - db.load ( - package_id { - "libfoo", {fv4.epoch, fv4.canonical_upstream, fv4.revision}})); + db.load (package_id ("libfoo", version ("1.2.4")))); assert (check_location (fpv4)); - version xv ("1.0.0-1"); shared_ptr xpv ( - db.load ( - package_id { - "libstudxml", {xv.epoch, xv.canonical_upstream, xv.revision}})); + db.load (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{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 epv ( - db.load ( - package_id { - "libexp", {ev.epoch, ev.canonical_upstream, ev.revision}})); + db.load (package_id ("libexp", version ("1+1.2")))); assert (check_location (epv)); - version fv5 ("1.2.4-1"); shared_ptr fpv5 ( - db.load ( - package_id { - "libfoo", {fv5.epoch, fv5.canonical_upstream, fv5.revision}})); + db.load (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 bpv ( - db.load ( - package_id { - "libbar", {bv.epoch, bv.canonical_upstream, bv.revision}})); + db.load (package_id ("libbar", version ("2.3.5")))); assert (check_location (bpv)); - version fv0 ("0.1"); shared_ptr fpv0 ( - db.load ( - package_id { - "libfoo", {fv0.epoch, fv0.canonical_upstream, fv0.revision}})); + db.load (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 bpv ( - db.load ( - package_id {"libbar", {bv.epoch, bv.canonical_upstream, bv.revision}})); + db.load (package_id ("libbar", version ("2.3.5")))); assert (bpv->summary == "test"); -- cgit v1.1