From 08ef171b0c5b9238df9fe0b86200a8d8425dcea5 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 6 Oct 2015 12:38:11 +0200 Subject: Get rid of package class --- brep/package | 295 +++++++++++++++++++++++---------------- brep/package-search.cxx | 17 ++- brep/package-version-details.cxx | 45 ++++-- brep/package-version-search.cxx | 59 ++++---- brep/package.cxx | 90 +++++------- brep/page | 28 ++++ brep/page.cxx | 28 ++++ 7 files changed, 333 insertions(+), 229 deletions(-) (limited to 'brep') diff --git a/brep/package b/brep/package index 6cd87e9..252c22f 100644 --- a/brep/package +++ b/brep/package @@ -40,10 +40,10 @@ namespace brep #include -// We have to keep this mapping at the global scope instead of inside -// the brep namespace because it needs to be also effective in the +// We have to keep these mappings at the global scope instead of inside +// the brep namespace because they need to be also effective in the // bpkg namespace from which we "borrow" types (and some of them use -// version). +// version and comparison). // #pragma db map type(bpkg::version) as(brep::_version) \ to(brep::_version{(?).epoch, \ @@ -68,34 +68,9 @@ namespace brep // to TEXT as a comma-separated list. // - // Version comparison operators. - // - template - inline auto - operator< (const T1& x, const T2& y) -> decltype (x.epoch < y.epoch) - { - return x.epoch < y.epoch || - (x.epoch == y.epoch && - x.canonical_upstream < y.canonical_upstream) || - (x.epoch == y.epoch && - x.canonical_upstream == y.canonical_upstream && - x.revision < y.revision); - } - - template - inline auto - order_by_version_desc (const T& x) -> decltype (x.epoch == 0) - { - return "ORDER BY" - + x.epoch + "DESC," - + x.canonical_upstream + "DESC," - + x.revision + "DESC"; - } - // Forward declarations. // class repository; - class package; class package_version; using strings = std::vector; @@ -146,20 +121,8 @@ namespace brep // Database mapping. // - #pragma db member(package) points_to(package) //on_delete(cascade) }; - inline bool - operator< (const package_version_id& x, const package_version_id& y) - { - int r (x.package.compare (y.package)); - - if (r != 0) - return r < 0; - - return x.version < y.version; - } - using priority = bpkg::priority; #pragma db value(priority) definition #pragma db member(priority::value) column("") @@ -216,6 +179,17 @@ namespace brep // tricky. Can stick to just a package name until get some clarity on // "foreign" package resolution. // + // 4. As we get rid of the package class the dependency resolution come to + // finding the best matching package version object. The question is + // if to resolve dependencies on the loading phase or in the WEB interface + // when required. The arguments in favour of doing that during loading + // phase are: + // * WEB interface get offloaded from a possibly expensive queries + // which otherwise have to be executed multiple times for the same + // dependency no matter the result would be the same. + // * No need to complicate persisted object model with repository + // relations otherwise required just for dependency resolution. + // using dependency = bpkg::dependency; #pragma db value(dependency) definition #pragma db member(dependency::condition) column("") @@ -313,77 +287,68 @@ namespace brep repository () = default; }; - #pragma db object pointer(std::shared_ptr) session - class package - { - public: - using url_type = brep::url; - using email_type = brep::email; - - package (std::string name, - std::string summary, - strings tags, - optional description, - url_type, - optional package_url, - email_type, - optional package_email); - - // Manifest data. - // - std::string name; - std::string summary; - strings tags; - optional description; - url_type url; - optional package_url; - email_type email; - optional package_email; - std::vector> versions; - - // Additional data. - // - - // Database mapping. - // - #pragma db member(name) id - #pragma db member(tags) id_column("package") value_column("tag") - #pragma db member(versions) inverse(id.data.package) value_not_null - - private: - friend class odb::access; - package () = default; - }; - + // @@ While it's very tempting to rename package_version to package wouldn't + // it be confusing having package denoting both specific package version + // and package as a collection of versions. + // #pragma db object pointer(std::shared_ptr) session class package_version { public: using repository_type = brep::repository; - using package_type = brep::package; using version_type = brep::version; using priority_type = brep::priority; using license_alternatives_type = brep::license_alternatives; + using url_type = brep::url; + using email_type = brep::email; using dependencies_type = brep::dependencies; using requirements_type = brep::requirements; - package_version (odb::lazy_shared_ptr, + // Create internal package version object. + // + package_version (std::string name, version_type, priority_type, + std::string summary, license_alternatives_type, + strings tags, + optional description, std::string changes, + url_type, + optional package_url, + email_type, + optional package_email, dependencies_type, requirements_type, optional location, std::shared_ptr); + // Create external package version object. + // + // External repository packages can appear on the WEB interface only on + // the package version details page in dependency list in the form of a + // link to the corresponding WEB page. The only package information + // required to compose such a link is the package name, version, and + // repository location. + // + package_version (std::string name, + version_type, + std::shared_ptr); + // Manifest data. // - odb::lazy_shared_ptr package; + std::string name; version_type version; priority_type priority; + std::string summary; license_alternatives_type license_alternatives; + strings tags; + optional description; std::string changes; + url_type url; + optional package_url; + email_type email; + optional package_email; dependencies_type dependencies; requirements_type requirements; odb::lazy_shared_ptr internal_repository; @@ -405,6 +370,8 @@ namespace brep { #pragma db column("") package_version_id data; + + #pragma db column("version_upstream") std::string upstream; }; @@ -414,8 +381,8 @@ namespace brep void _id (_id_type&&, odb::database&); + #pragma db member(name) transient #pragma db member(version) transient - #pragma db member(package) transient #pragma db member(id) virtual(_id_type) before id(data) \ get(_id) set(_id (std::move (?), (!))) column("") @@ -437,6 +404,10 @@ namespace brep set(_set (this.license_alternatives, (?))) \ id_column("") key_column("") value_column("license") + // tags + // + #pragma db member(tags) id_column("") value_column("tag") + // dependencies // using _dependency_key = index_pair; @@ -481,65 +452,141 @@ namespace brep package_version () = default; }; - #pragma db view object(package_version) \ - query((?) + order_by_version_desc (package_version::id.data.version) + \ - "LIMIT 1") - struct max_package_version - { - using version_type = brep::version; - version_type version; - - void - _id (package_version::_id_type&&); - - // Database mapping. - // - #pragma db member(version) transient - - // Can't specify column expression using aggregate max function for a - // data member of a composite value type. - // - #pragma db member(id) virtual(package_version::_id_type) \ - get() set(_id (std::move (?))) - }; - // Find the latest version of an internal package. // - #pragma db view object(package_version = version) \ - object(package_version = v: \ - version::id.data.package == v::id.data.package && \ - version::id.data.version < v::id.data.version) \ - object(package inner: version::id.data.package == package::name && \ - version::internal_repository.is_not_null () && \ - v::id.data.package.is_null ()) + #pragma db view object(package_version) \ + object(package_version = v: \ + package_version::id.data.package == v::id.data.package && \ + package_version::id.data.version < v::id.data.version) \ + query((package_version::internal_repository.is_not_null () && \ + v::id.data.package.is_null ()) + "AND" + (?)) struct latest_internal_package_version { - using package_type = brep::package; - std::shared_ptr package; - std::shared_ptr version; + using package_version_type = brep::package_version; + std::shared_ptr package_version; + + operator const std::shared_ptr& () const + {return package_version;} + + explicit operator package_version_type& () const + {return *package_version;} }; - #pragma db view object(package) \ - object(package_version: package_version::id.data.package == \ - package::name) \ + #pragma db view object(package_version) \ query(package_version::internal_repository.is_not_null () && (?)) struct internal_package_count { - #pragma db column("count(DISTINCT" + package::name + ")") - std::size_t count; + #pragma db column("count(DISTINCT" + \ + package_version::id.data.package+ ")") + + std::size_t result; + + operator std::size_t () const {return result;} }; #pragma db view object(package_version) struct package_version_count { #pragma db column("count(*)") - std::size_t count; + std::size_t result; + + operator std::size_t () const {return result;} }; + + // Version comparison operators. + // + // They allow comparing objects that have epoch, canonical_upstream, + // and revision data members. + // + template + inline auto + operator== (const T1& x, const T2& y) -> decltype (x.epoch == y.epoch) + { + return x.epoch == y.epoch && + x.canonical_upstream == y.canonical_upstream && + x.revision == y.revision; + } + + template + inline auto + operator!= (const T1& x, const T2& y) -> decltype (x.epoch != y.epoch) + { + return x.epoch != y.epoch || + x.canonical_upstream != y.canonical_upstream || + x.revision != y.revision; + } + + template + inline auto + operator< (const T1& x, const T2& y) -> decltype (x.epoch < y.epoch) + { + return x.epoch < y.epoch || + (x.epoch == y.epoch && x.canonical_upstream < y.canonical_upstream) || + (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream && + x.revision < y.revision); + } + + template + inline auto + operator<= (const T1& x, const T2& y) -> decltype (x.epoch <= y.epoch) + { + return x.epoch < y.epoch || + (x.epoch == y.epoch && x.canonical_upstream < y.canonical_upstream) || + (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream && + x.revision <= y.revision); + } + + template + inline auto + operator> (const T1& x, const T2& y) -> decltype (x.epoch > y.epoch) + { + return x.epoch > y.epoch || + (x.epoch == y.epoch && x.canonical_upstream > y.canonical_upstream) || + (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream && + x.revision > y.revision); + } + + template + inline auto + operator>= (const T1& x, const T2& y) -> decltype (x.epoch >= y.epoch) + { + return x.epoch > y.epoch || + (x.epoch == y.epoch && x.canonical_upstream > y.canonical_upstream) || + (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream && + x.revision >= y.revision); + } + + template + inline auto + order_by_version_desc (const T& x) -> //decltype ("ORDER BY" + x.epoch) + decltype (x.epoch == 0) + { + return "ORDER BY" + + x.epoch + "DESC," + + x.canonical_upstream + "DESC," + + x.revision + "DESC"; + } + + inline bool + operator< (const package_version_id& x, const package_version_id& y) + { + int r (x.package.compare (y.package)); + + if (r != 0) + return r < 0; + + return x.version < y.version; + } } namespace odb { + using ::brep::operator==; + using ::brep::operator!=; using ::brep::operator<; + using ::brep::operator<=; + using ::brep::operator>; + using ::brep::operator>=; using ::brep::order_by_version_desc; } diff --git a/brep/package-search.cxx b/brep/package-search.cxx index 9b3fb2c..cc15e02 100644 --- a/brep/package-search.cxx +++ b/brep/package-search.cxx @@ -91,7 +91,7 @@ namespace brep // @@ Query will include search criteria if specified. // - size_t pc (db_->query_value ().count); + size_t pc (db_->query_value ()); s << DIV(ID="packages") << "Packages (" << pc << ")" << ~DIV; @@ -100,20 +100,19 @@ namespace brep using query = query; auto r ( - db_->query ( - "ORDER BY" + query::package::name + + db_->query (query (true) + + "ORDER BY" + query::package_version::id.data.package + "OFFSET" + to_string (pr.page () * rop) + "LIMIT" + to_string (rop))); for (const auto& ip: r) { - const package& p = *ip.package; - const package_version& v = *ip.version; + const package_version& v (ip); s << DIV(CLASS="package") << DIV(CLASS="name") << A - << HREF << "/go/" << mime_url_encode (p.name); + << HREF << "/go/" << mime_url_encode (v.name); // Propagate search criteria to the package version search url. // @@ -121,11 +120,11 @@ namespace brep s << "?" << q; s << ~HREF - << p.name + << v.name << ~A << ~DIV - << DIV(CLASS="summary") << p.summary << ~DIV - << DIV_TAGS (p.tags) + << DIV(CLASS="summary") << v.summary << ~DIV + << DIV_TAGS (v.tags) << DIV_LICENSES (v.license_alternatives) << DIV(CLASS="dependencies") << "Dependencies: " << v.dependencies.size () diff --git a/brep/package-version-details.cxx b/brep/package-version-details.cxx index 0d08a45..0ad86d7 100644 --- a/brep/package-version-details.cxx +++ b/brep/package-version-details.cxx @@ -90,13 +90,16 @@ namespace brep << CSS_STYLE << ident << A_STYLE () << ident << "#name {font-size: xx-large; font-weight: bold;}" << ident - << ".url {margin: 0.3em 0 0;}" << ident - << ".priority, #licenses, #dependencies, #requirements, " + << ".url, .email {font-size: medium;}" << ident + << ".comment {font-size: small;}" << ident + << "#summary {font-size: x-large; margin: 0.2em 0 0;}" << ident + << "#description {margin: 0.5em 0 0;}" << ident + << ".tags {margin: 0.3em 0 0;}" << ident + << "#package, .priority, #licenses, #dependencies, #requirements, " "#locations, #changes {" << ident << " font-size: x-large;" << ident << " margin: 0.5em 0 0;" << ident << "}" << ident - << ".comment {font-size: medium;}" << ident << "ul {margin: 0; padding: 0 0 0 1em;}" << ident << "li {font-size: large; margin: 0.1em 0 0;}" << ident << ".conditional {font-weight: bold;}" << ident @@ -112,7 +115,7 @@ namespace brep bool not_found (false); shared_ptr pv; - transaction t (db_->begin ()); //@@ Not committed, other places? + transaction t (db_->begin ()); try { @@ -133,12 +136,20 @@ namespace brep throw invalid_request (404, "Package '" + name + "' not found"); assert (pv->location); - const string url (pv->internal_repository.load ()->location.string () + - "/" + pv->location->string ()); + const string u (pv->internal_repository.load ()->location.string () + + "/" + pv->location->string ()); + + s << DIV(CLASS="url") << A << HREF << u << ~HREF << u << ~A << ~DIV + << DIV(ID="summary") << pv->summary << ~DIV + << DIV_URL (pv->url) + << DIV_EMAIL (pv->email); + + if (pv->description) + s << DIV(ID="description") << *pv->description << ~DIV; const priority& pt (pv->priority); - s << DIV(CLASS="url") << A << HREF << url << ~HREF << url << ~A << ~DIV + s << DIV_TAGS (pv->tags) << DIV_PRIORITY (pt); if (!pt.comment.empty ()) @@ -249,6 +260,22 @@ namespace brep << ~DIV; } + if (pv->package_url || pv->package_email) + { + s << DIV(ID="package") + << "Package:" + << UL; + + if (pv->package_url) + s << LI << DIV_URL (*pv->package_url) << ~LI; + + if (pv->package_email) + s << LI << DIV_EMAIL (*pv->package_email) << ~LI; + + s << ~UL + << ~DIV; + } + const auto& er (pv->external_repositories); if (!er.empty ()) @@ -267,7 +294,9 @@ namespace brep u += ":" + to_string (l.port ()); u += "/go/" + mime_url_encode (p) + "/" + vs; - s << LI << A << HREF << u << ~HREF << u << ~A << ~LI; + s << LI + << DIV(CLASS="url") << A << HREF << u << ~HREF << u << ~A << ~DIV + << ~LI; } s << ~UL diff --git a/brep/package-version-search.cxx b/brep/package-version-search.cxx index 2774a16..7b7ed63 100644 --- a/brep/package-version-search.cxx +++ b/brep/package-version-search.cxx @@ -75,7 +75,8 @@ namespace brep << DIV_PAGER_STYLE () << ident << "#name {font-size: xx-large; font-weight: bold;}" << ident << "#summary {font-size: x-large; margin: 0.2em 0 0;}" << ident - << ".url {margin: 0.3em 0 0;}" << ident + << ".url, .email {font-size: medium;}" << ident + << ".comment {font-size: small;}" << ident << "#description {margin: 0.5em 0 0;}" << ident << ".tags {margin: 0.3em 0 0;}" << ident << "#versions {font-size: x-large; margin: 0.5em 0 0;}" << ident @@ -87,32 +88,32 @@ namespace brep << BODY << DIV(ID="name") << name << ~DIV; - shared_ptr p; size_t rop (options_->results_on_page ()); transaction t (db_->begin ()); - try - { - p = db_->load (name); - } - catch (const object_not_persistent& ) + shared_ptr pv; { - throw invalid_request (404, "Package '" + name + "' not found"); + using query = query; + + latest_internal_package_version v; + if (!db_->query_one ( + query::package_version::id.data.package == name, v)) + { + throw invalid_request (404, "Package '" + name + "' not found"); + } + + pv = v; } - s << DIV(ID="summary") << p->summary << ~DIV - << DIV(CLASS="url") - << A << HREF << p->url << ~HREF << p->url << ~A - << ~DIV - << DIV(CLASS="email") - << A << HREF << "mailto:" << p->email << ~HREF << p->email << ~A - << ~DIV; + s << DIV(ID="summary") << pv->summary << ~DIV + << DIV_URL (pv->url) + << DIV_EMAIL (pv->email); - if (p->description) - s << DIV(ID="description") << *p->description << ~DIV; + if (pv->description) + s << DIV(ID="description") << *pv->description << ~DIV; - s << DIV_TAGS (p->tags); + s << DIV_TAGS (pv->tags); size_t pvc; { @@ -122,23 +123,21 @@ namespace brep // pvc = db_->query_value ( query::id.data.package == name && - query::internal_repository.is_not_null ()).count; + query::internal_repository.is_not_null ()); } s << DIV(ID="versions") << "Versions (" << pvc << ")" << ~DIV; - if (p->package_url) - s << DIV(CLASS="url") - << A << HREF << *p->package_url << ~HREF << *p->package_url << ~A - << ~DIV; + // @@ Need to find some better place for package url and email or drop them + // from this page totally. + // +/* + if (pv->package_url) + s << DIV_URL (*pv->package_url); - if (p->package_email) - s << DIV(CLASS="email") - << A - << HREF << "mailto:" << *p->package_email << ~HREF - << *p->package_email - << ~A - << ~DIV; + if (pv->package_email) + s << DIV_EMAIL (*pv->package_email); +*/ // @@ Use appropriate view when clarify which package version info to be // displayed and search index structure get implemented. Query will also diff --git a/brep/package.cxx b/brep/package.cxx index 46953f4..3cf4063 100644 --- a/brep/package.cxx +++ b/brep/package.cxx @@ -16,70 +16,54 @@ using namespace odb::core; namespace brep { - // Utility functions - // - static inline bool - alpha (char c) - { - return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); - } - - static inline bool - digit (char c) - { - return c >= '0' && c <= '9'; - } - - // package - // - package:: - package (string n, - string s, - strings t, - optional d, - url_type u, - optional pu, - email_type e, - optional pe) - : name (move (n)), - summary (move (s)), - tags (move (t)), - description (move (d)), - url (move (u)), - package_url (move (pu)), - email (move (e)), - package_email (move (pe)) - { - } - // package_version // package_version:: - package_version (lazy_shared_ptr pk, + package_version (string nm, version_type vr, priority_type pr, + string sm, license_alternatives_type la, + strings tg, + optional ds, string ch, + url_type ur, + optional pu, + email_type em, + optional pe, dependencies_type dp, requirements_type rq, optional lc, shared_ptr rp) - : package (move (pk)), + : name (move (nm)), version (move (vr)), priority (move (pr)), + summary (move (sm)), license_alternatives (move (la)), + tags (move (tg)), + description (move (ds)), changes (move (ch)), + url (move (ur)), + package_url (move (pu)), + email (move (em)), + package_email (move (pe)), dependencies (move (dp)), requirements (move (rq)), + internal_repository (move (rp)), location (move (lc)) { - //@@ Can't be sure we are in transaction. Instead, make caller - // pass shared_ptr. - // - if (rp->internal) - internal_repository = move (rp); - else - external_repositories.emplace_back (move (rp)); + assert (internal_repository->internal); + } + + package_version:: + package_version (string nm, + version_type vr, + shared_ptr rp) + : name (move (nm)), + version (move (vr)) + { + assert (!rp->internal); + external_repositories.emplace_back (move (rp)); } package_version::_id_type package_version:: @@ -87,7 +71,7 @@ namespace brep { return _id_type { { - package.object_id (), + name, version.epoch, version.canonical_upstream, version.revision @@ -96,20 +80,10 @@ namespace brep } void package_version:: - _id (_id_type&& v, database& db) - { - const auto& dv (v.data.version); - package = lazy_shared_ptr (db, v.data.package); - version = version_type (dv.epoch, move (v.upstream), dv.revision); - assert (version.canonical_upstream == dv.canonical_upstream); - } - - // max_package_version - // - void max_package_version:: - _id (package_version::_id_type&& v) + _id (_id_type&& v, database&) { const auto& dv (v.data.version); + name = move (v.data.package); version = version_type (dv.epoch, move (v.upstream), dv.revision); assert (version.canonical_upstream == dv.canonical_upstream); } diff --git a/brep/page b/brep/page index c7febd8..d14bb41 100644 --- a/brep/page +++ b/brep/page @@ -55,6 +55,34 @@ namespace brep operator() (xml::serializer& s) const; }; + // Generates url element. + // + class DIV_URL + { + public: + DIV_URL (const url& u): url_ (u) {} + + void + operator() (xml::serializer& s) const; + + private: + const url& url_; + }; + + // Generates email element. + // + class DIV_EMAIL + { + public: + DIV_EMAIL (const email& e): email_ (e) {} + + void + operator() (xml::serializer& s) const; + + private: + const email& email_; + }; + // Generates package tags element. // class DIV_TAGS diff --git a/brep/page.cxx b/brep/page.cxx index 552be3c..b59cd4b 100644 --- a/brep/page.cxx +++ b/brep/page.cxx @@ -147,6 +147,34 @@ namespace brep s << ~DIV; } + // DIV_URL + // + void DIV_URL:: + operator() (serializer& s) const + { + s << DIV(CLASS="url") + << A << HREF << url_ << ~HREF << url_ << ~A; + + if (!url_.comment.empty ()) + s << DIV(CLASS="comment") << url_.comment << ~DIV; + + s << ~DIV; + } + + // DIV_EMAIL + // + void DIV_EMAIL:: + operator() (serializer& s) const + { + s << DIV(CLASS="email") + << A << HREF << "mailto:" << email_ << ~HREF << email_ << ~A; + + if (!email_.comment.empty ()) + s << DIV(CLASS="comment") << email_.comment << ~DIV; + + s << ~DIV; + } + // DIV_TAGS // void DIV_TAGS:: -- cgit v1.1