From dd6241b539692ec22387fec3c298b84bec1648c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 23 Jan 2024 12:25:24 +0100 Subject: [PATCH 1/5] Add pImpl to `libdnf5::transaction::TransactionItem` --- .../libdnf5/transaction/transaction_item.hpp | 51 +++++------- libdnf5/transaction/transaction_item.cpp | 79 +++++++++++++++++-- 2 files changed, 92 insertions(+), 38 deletions(-) diff --git a/include/libdnf5/transaction/transaction_item.hpp b/include/libdnf5/transaction/transaction_item.hpp index fc122a5c0..2b8f270cd 100644 --- a/include/libdnf5/transaction/transaction_item.hpp +++ b/include/libdnf5/transaction/transaction_item.hpp @@ -50,22 +50,28 @@ class TransactionItem { /// Get action associated with the transaction item in the transaction /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.getAction() - Action get_action() const noexcept { return action; } + Action get_action() const noexcept; /// Get reason of the action associated with the transaction item in the transaction /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.getReason() - Reason get_reason() const noexcept { return reason; } + Reason get_reason() const noexcept; /// Get transaction item repoid (text identifier of a repository) /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.getRepoid() - const std::string & get_repoid() const noexcept { return repoid; } + const std::string & get_repoid() const noexcept; /// Get transaction item state /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.getState() - State get_state() const noexcept { return state; } + State get_state() const noexcept; + + ~TransactionItem(); + TransactionItem(const TransactionItem & src); + TransactionItem & operator=(const TransactionItem & src); + TransactionItem(TransactionItem && src) noexcept; + TransactionItem & operator=(TransactionItem && src) noexcept; private: friend RpmDbUtils; @@ -82,15 +88,15 @@ class TransactionItem { explicit TransactionItem(const Transaction & trans); /// Get database id (primary key) of the transaction item (table 'trans_item') - int64_t get_id() const noexcept { return id; } + int64_t get_id() const noexcept; /// Set database id (primary key) of the transaction item (table 'trans_item') - void set_id(int64_t value) { id = value; } + void set_id(int64_t value); /// Set action associated with the transaction item in the transaction /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.setAction(libdnf::TransactionItemAction value) - void set_action(Action value) { action = value; } + void set_action(Action value); /// Get name of the action associated with the transaction item in the transaction /// @@ -105,17 +111,17 @@ class TransactionItem { /// Set reason of the action associated with the transaction item in the transaction /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.setReason(libdnf::TransactionItemReason value) - void set_reason(Reason value) { reason = value; } + void set_reason(Reason value); /// Set transaction item state /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.setState(libdnf::TransactionItemState value) - void set_state(State value) { state = value; } + void set_state(State value); /// Get transaction item repoid (text identifier of a repository) /// // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItemBase.setRepoid(const std::string & value) - void set_repoid(const std::string & value) { repoid = value; } + void set_repoid(const std::string & value); /// Has the item appeared on the system during the transaction? /// @@ -139,29 +145,14 @@ class TransactionItem { // TODO(dmach): Review and bring back if needed //void saveState(); - // TODO(dmach): move to sack, resolve for all packages; return the user who initially installed the package - // @replaces libdnf:transaction/TransactionItem.hpp:method:TransactionItem.getInstalledBy() - uint32_t getInstalledBy() const; - /// Get database id (primary key) of the item (table 'item'; other item tables such 'rpm' inherit from it via 1:1 relation) - int64_t get_item_id() const noexcept { return item_id; } + int64_t get_item_id() const noexcept; /// Set database id (primary key) of the item (table 'item'; other item tables such 'rpm' inherit from it via 1:1 relation) - void set_item_id(int64_t value) { item_id = value; } - int64_t id = 0; - Action action = Action::INSTALL; - Reason reason = Reason::NONE; - State state = State::STARTED; - std::string repoid; - - int64_t item_id = 0; - - // TODO(lukash) this won't be safe in bindings (or in general when a - // TransactionItem is kept around after a Transaction is destroyed), but we - // can't easily use a WeakPtr here, since the Transactions are expected to - // be at least movable, and the WeakPtrGuard would make the Transaction - // unmovable - const Transaction * trans = nullptr; + void set_item_id(int64_t value); + + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/transaction_item.cpp b/libdnf5/transaction/transaction_item.cpp index 137a2933a..39d33b4fc 100644 --- a/libdnf5/transaction/transaction_item.cpp +++ b/libdnf5/transaction/transaction_item.cpp @@ -20,7 +20,6 @@ along with libdnf. If not, see . #include "libdnf5/transaction/transaction_item.hpp" -#include "db/repo.hpp" #include "libdnf5/transaction/transaction.hpp" #include "libdnf5/transaction/transaction_item_action.hpp" @@ -28,35 +27,99 @@ along with libdnf. If not, see . namespace libdnf5::transaction { +class TransactionItem::Impl { +public: + Impl(const Transaction & trans); + +private: + friend TransactionItem; + + int64_t id = 0; + Action action = Action::INSTALL; + Reason reason = Reason::NONE; + State state = State::STARTED; + std::string repoid; + int64_t item_id = 0; + + // TODO(lukash) this won't be safe in bindings (or in general when a + // TransactionItem is kept around after a Transaction is destroyed), but we + // can't easily use a WeakPtr here, since the Transactions are expected to + // be at least movable, and the WeakPtrGuard would make the Transaction + // unmovable + const Transaction * trans = nullptr; +}; + +TransactionItem::~TransactionItem() = default; + +TransactionItem::TransactionItem(const TransactionItem & mpkg) : p_impl(new Impl(*mpkg.p_impl)) {} +TransactionItem::TransactionItem(TransactionItem && mpkg) noexcept = default; + +TransactionItem & TransactionItem::operator=(const TransactionItem & mpkg) { + if (this != &mpkg) { + if (p_impl) { + *p_impl = *mpkg.p_impl; + } else { + p_impl = std::make_unique(*mpkg.p_impl); + } + } + + return *this; +} +TransactionItem & TransactionItem::operator=(TransactionItem && mpkg) noexcept = default; + +TransactionItem::Impl::Impl(const Transaction & trans) + : trans(&trans) {} std::string TransactionItem::get_action_name() { - return transaction_item_action_to_string(action); + return transaction_item_action_to_string(p_impl->action); } std::string TransactionItem::get_action_short() { - return transaction_item_action_to_letter(action); + return transaction_item_action_to_letter(p_impl->action); } -TransactionItem::TransactionItem(const Transaction & trans) : trans{&trans} {} +TransactionItem::TransactionItem(const Transaction & trans) + : p_impl(std::make_unique(trans)) {} bool TransactionItem::is_inbound_action() const { - return transaction_item_action_is_inbound(action); + return transaction_item_action_is_inbound(p_impl->action); } bool TransactionItem::is_outbound_action() const { - return transaction_item_action_is_outbound(action); + return transaction_item_action_is_outbound(p_impl->action); } const Transaction & TransactionItem::get_transaction() const { - libdnf_assert(trans, "Transaction in TransactionItem was not set."); - return *trans; + libdnf_assert(p_impl->trans, "Transaction in TransactionItem was not set."); + return *p_impl->trans; } +TransactionItemAction TransactionItem::get_action() const noexcept { return p_impl->action; } +TransactionItemReason TransactionItem::get_reason() const noexcept { return p_impl->reason; } +const std::string & TransactionItem::get_repoid() const noexcept { return p_impl->repoid; } +TransactionItemState TransactionItem::get_state() const noexcept { return p_impl->state; } + +int64_t TransactionItem::get_id() const noexcept { return p_impl->id; } + +void TransactionItem::set_id(int64_t value) { p_impl->id = value; } + +void TransactionItem::set_action(Action value) { p_impl->action = value; } + +void TransactionItem::set_reason(Reason value) { p_impl->reason = value; } + +void TransactionItem::set_state(State value) { p_impl->state = value; } + +void TransactionItem::set_repoid(const std::string & value) { p_impl->repoid = value; } + +int64_t TransactionItem::get_item_id() const noexcept { return p_impl->item_id; } + +void TransactionItem::set_item_id(int64_t value) { p_impl->item_id = value; } + /* void TransactionItem::saveReplacedBy() From 2c0c739daba9f1aeb985b4414d86db42e9e1f4f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 23 Jan 2024 14:39:02 +0100 Subject: [PATCH 2/5] Add pImpl to `libdnf5::transaction::CompsEnvironment` Also add pImpl to `libdnf5::transaction::CompsEnvironmentGroup`. --- .../libdnf5/transaction/comps_environment.hpp | 61 +++++---- libdnf5/transaction/comps_environment.cpp | 125 +++++++++++++++++- libdnf5/transaction/transaction_item.cpp | 55 +++++--- 3 files changed, 191 insertions(+), 50 deletions(-) diff --git a/include/libdnf5/transaction/comps_environment.hpp b/include/libdnf5/transaction/comps_environment.hpp index dd7b09772..d7313f5d5 100644 --- a/include/libdnf5/transaction/comps_environment.hpp +++ b/include/libdnf5/transaction/comps_environment.hpp @@ -47,6 +47,12 @@ class CompsEnvironment : public TransactionItem { // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.toStr() std::string to_string() const { return get_environment_id(); } + CompsEnvironment(const CompsEnvironment & src); + CompsEnvironment & operator=(const CompsEnvironment & src); + CompsEnvironment(CompsEnvironment && src) noexcept; + CompsEnvironment & operator=(CompsEnvironment && src) noexcept; + ~CompsEnvironment(); + private: friend Transaction; friend CompsEnvironmentDbUtils; @@ -57,42 +63,42 @@ class CompsEnvironment : public TransactionItem { /// Get text id of the environment (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.getEnvironmentId() - const std::string & get_environment_id() const noexcept { return environment_id; } + const std::string & get_environment_id() const noexcept; /// Set text id of the environment (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.setEnvironmentId(const std::string & value) - void set_environment_id(const std::string & value) { environment_id = value; } + void set_environment_id(const std::string & value); /// Get name of the environment (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.getName() - const std::string & get_name() const noexcept { return name; } + const std::string & get_name() const noexcept; /// Set name of the environment (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.setName(const std::string & value) - void set_name(const std::string & value) { name = value; } + void set_name(const std::string & value); /// Get translated name of the environment in the current locale (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.getTranslatedName() - const std::string & get_translated_name() const noexcept { return translated_name; } + const std::string & get_translated_name() const noexcept; /// Set translated name of the environment in the current locale (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.setTranslatedName(const std::string & value) - void set_translated_name(const std::string & value) { translated_name = value; } + void set_translated_name(const std::string & value); /// Get types of the packages to be installed with the environment (related xml elements: ``) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.getPackageTypes() - libdnf5::comps::PackageType get_package_types() const noexcept { return package_types; } + libdnf5::comps::PackageType get_package_types() const noexcept; /// Set types of the packages to be installed with the environment (related xml elements: ``) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.setPackageTypes(libdnf::CompsPackageType value) - void set_package_types(libdnf5::comps::PackageType value) { package_types = value; } + void set_package_types(libdnf5::comps::PackageType value); /// Create a new CompsEnvironmentGroup object and return a reference to it. /// The object is owned by the CompsEnvironment. @@ -101,23 +107,28 @@ class CompsEnvironment : public TransactionItem { /// Get list of groups associated with the environment. /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentItem.getGroups() - std::vector & get_groups() { return groups; } + std::vector & get_groups(); // TODO(dmach): rewrite into TransactionSack.list_installed_environments(); how to deal with references to different transactions? We don't want all of them loaded into memory. //static std::vector< TransactionItemPtr > getTransactionItemsByPattern( // libdnf5::utils::SQLite3Ptr conn, // const std::string &pattern); - std::string environment_id; - std::string name; - std::string translated_name; - libdnf5::comps::PackageType package_types = libdnf5::comps::PackageType::DEFAULT; - std::vector groups; + class Impl; + std::unique_ptr p_impl; }; // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:class:CompsEnvironmentGroup class CompsEnvironmentGroup { +public: + ~CompsEnvironmentGroup(); + CompsEnvironmentGroup(const CompsEnvironmentGroup & src); + CompsEnvironmentGroup & operator=(const CompsEnvironmentGroup & src); + CompsEnvironmentGroup(CompsEnvironmentGroup && src) noexcept; + CompsEnvironmentGroup & operator=(CompsEnvironmentGroup && src) noexcept; + CompsEnvironmentGroup(); + private: friend Transaction; friend CompsEnvironmentGroupDbUtils; @@ -125,47 +136,45 @@ class CompsEnvironmentGroup { /// Get database id (primary key) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.getId() - int64_t get_id() const noexcept { return id; } + int64_t get_id() const noexcept; /// Set database id (primary key) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.setId(int64_t value) - void set_id(int64_t value) { id = value; } + void set_id(int64_t value); /// Get groupid of a group associated with a comps environment (xml element: `VALUE`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.getGroupId() - const std::string & get_group_id() const noexcept { return group_id; } + const std::string & get_group_id() const noexcept; /// Set groupid of a group associated with a comps environment (xml element: `VALUE`) /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.setGroupId(const std::string & value) - void set_group_id(const std::string & value) { group_id = value; } + void set_group_id(const std::string & value); /// Get a flag that determines if the group was present after the transaction it's associated with has finished. /// If the group was installed before running the transaction, it's still counted as installed. /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.getInstalled() - bool get_installed() const noexcept { return installed; } + bool get_installed() const noexcept; /// Set a flag that determines if the group was present after the transaction it's associated with has finished. /// If the group was installed before running the transaction, it's still counted as installed. /// // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.setInstalled(bool value) - void set_installed(bool value) { installed = value; } + void set_installed(bool value); // TODO(dmach): this is not entirely clear; investigate and document // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.getGroupType() - libdnf5::comps::PackageType get_group_type() const noexcept { return group_type; } + libdnf5::comps::PackageType get_group_type() const noexcept; // TODO(dmach): this is not entirely clear; investigate and document // @replaces libdnf:transaction/CompsEnvironmentItem.hpp:method:CompsEnvironmentGroup.setGroupType(libdnf::CompsPackageType value) - void set_group_type(libdnf5::comps::PackageType value) { group_type = value; } + void set_group_type(libdnf5::comps::PackageType value); - int64_t id = 0; - std::string group_id; - bool installed = false; - libdnf5::comps::PackageType group_type; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/comps_environment.cpp b/libdnf5/transaction/comps_environment.cpp index 6d70cdabe..556b5ef3d 100644 --- a/libdnf5/transaction/comps_environment.cpp +++ b/libdnf5/transaction/comps_environment.cpp @@ -21,7 +21,6 @@ along with libdnf. If not, see . #include "libdnf5/transaction/comps_environment.hpp" #include "db/comps_environment.hpp" -#include "db/comps_environment_group.hpp" #include "libdnf5/transaction/transaction.hpp" #include "libdnf5/transaction/transaction_item.hpp" @@ -30,7 +29,124 @@ along with libdnf. If not, see . namespace libdnf5::transaction { -CompsEnvironment::CompsEnvironment(const Transaction & trans) : TransactionItem::TransactionItem(trans) {} +class CompsEnvironment::Impl { +public: +private: + friend CompsEnvironment; + + std::string environment_id; + std::string name; + std::string translated_name; + libdnf5::comps::PackageType package_types = libdnf5::comps::PackageType::DEFAULT; + std::vector groups; +}; + +CompsEnvironment::CompsEnvironment(const Transaction & trans) + : TransactionItem::TransactionItem(trans), + p_impl(std::make_unique()) {} + +CompsEnvironment::CompsEnvironment(const CompsEnvironment & src) + : TransactionItem(src), + p_impl(new Impl(*src.p_impl)) {} +CompsEnvironment::CompsEnvironment(CompsEnvironment && src) noexcept = default; + +CompsEnvironment & CompsEnvironment::operator=(const CompsEnvironment & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +CompsEnvironment & CompsEnvironment::operator=(CompsEnvironment && src) noexcept = default; +CompsEnvironment::~CompsEnvironment() = default; + + +const std::string & CompsEnvironment::get_environment_id() const noexcept { + return p_impl->environment_id; +} +void CompsEnvironment::set_environment_id(const std::string & value) { + p_impl->environment_id = value; +} +const std::string & CompsEnvironment::get_name() const noexcept { + return p_impl->name; +} +void CompsEnvironment::set_name(const std::string & value) { + p_impl->name = value; +} +const std::string & CompsEnvironment::get_translated_name() const noexcept { + return p_impl->translated_name; +} +void CompsEnvironment::set_translated_name(const std::string & value) { + p_impl->translated_name = value; +} +libdnf5::comps::PackageType CompsEnvironment::get_package_types() const noexcept { + return p_impl->package_types; +} +void CompsEnvironment::set_package_types(libdnf5::comps::PackageType value) { + p_impl->package_types = value; +} +CompsEnvironmentGroup & CompsEnvironment::new_group() { + return p_impl->groups.emplace_back(); +} +std::vector & CompsEnvironment::get_groups() { + return p_impl->groups; +} + +class CompsEnvironmentGroup::Impl { +private: + friend CompsEnvironmentGroup; + int64_t id = 0; + std::string group_id; + bool installed = false; + libdnf5::comps::PackageType group_type; +}; + + +CompsEnvironmentGroup::CompsEnvironmentGroup(const CompsEnvironmentGroup & src) : p_impl(new Impl(*src.p_impl)) {} +CompsEnvironmentGroup::CompsEnvironmentGroup(CompsEnvironmentGroup && src) noexcept = default; + +CompsEnvironmentGroup & CompsEnvironmentGroup::operator=(const CompsEnvironmentGroup & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +CompsEnvironmentGroup & CompsEnvironmentGroup::operator=(CompsEnvironmentGroup && src) noexcept = default; +CompsEnvironmentGroup::CompsEnvironmentGroup() : p_impl(std::make_unique()) {} +CompsEnvironmentGroup::~CompsEnvironmentGroup() = default; +int64_t CompsEnvironmentGroup::get_id() const noexcept { + return p_impl->id; +} +void CompsEnvironmentGroup::set_id(int64_t value) { + p_impl->id = value; +} +const std::string & CompsEnvironmentGroup::get_group_id() const noexcept { + return p_impl->group_id; +} +void CompsEnvironmentGroup::set_group_id(const std::string & value) { + p_impl->group_id = value; +} +bool CompsEnvironmentGroup::get_installed() const noexcept { + return p_impl->installed; +} +void CompsEnvironmentGroup::set_installed(bool value) { + p_impl->installed = value; +} +libdnf5::comps::PackageType CompsEnvironmentGroup::get_group_type() const noexcept { + return p_impl->group_type; +} +void CompsEnvironmentGroup::set_group_type(libdnf5::comps::PackageType value) { + p_impl->group_type = value; +} /* @@ -70,9 +186,4 @@ CompsEnvironmentItem::getTransactionItemsByPattern(libdnf5::utils::SQLite3Ptr co */ -CompsEnvironmentGroup & CompsEnvironment::new_group() { - return groups.emplace_back(); -} - - } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/transaction_item.cpp b/libdnf5/transaction/transaction_item.cpp index 39d33b4fc..174670886 100644 --- a/libdnf5/transaction/transaction_item.cpp +++ b/libdnf5/transaction/transaction_item.cpp @@ -20,7 +20,6 @@ along with libdnf. If not, see . #include "libdnf5/transaction/transaction_item.hpp" - #include "libdnf5/transaction/transaction.hpp" #include "libdnf5/transaction/transaction_item_action.hpp" @@ -67,8 +66,7 @@ TransactionItem & TransactionItem::operator=(const TransactionItem & mpkg) { } TransactionItem & TransactionItem::operator=(TransactionItem && mpkg) noexcept = default; -TransactionItem::Impl::Impl(const Transaction & trans) - : trans(&trans) {} +TransactionItem::Impl::Impl(const Transaction & trans) : trans(&trans) {} std::string TransactionItem::get_action_name() { return transaction_item_action_to_string(p_impl->action); @@ -80,8 +78,7 @@ std::string TransactionItem::get_action_short() { } -TransactionItem::TransactionItem(const Transaction & trans) - : p_impl(std::make_unique(trans)) {} +TransactionItem::TransactionItem(const Transaction & trans) : p_impl(std::make_unique(trans)) {} bool TransactionItem::is_inbound_action() const { @@ -99,26 +96,50 @@ const Transaction & TransactionItem::get_transaction() const { return *p_impl->trans; } -TransactionItemAction TransactionItem::get_action() const noexcept { return p_impl->action; } -TransactionItemReason TransactionItem::get_reason() const noexcept { return p_impl->reason; } -const std::string & TransactionItem::get_repoid() const noexcept { return p_impl->repoid; } -TransactionItemState TransactionItem::get_state() const noexcept { return p_impl->state; } +TransactionItemAction TransactionItem::get_action() const noexcept { + return p_impl->action; +} +TransactionItemReason TransactionItem::get_reason() const noexcept { + return p_impl->reason; +} +const std::string & TransactionItem::get_repoid() const noexcept { + return p_impl->repoid; +} +TransactionItemState TransactionItem::get_state() const noexcept { + return p_impl->state; +} -int64_t TransactionItem::get_id() const noexcept { return p_impl->id; } +int64_t TransactionItem::get_id() const noexcept { + return p_impl->id; +} -void TransactionItem::set_id(int64_t value) { p_impl->id = value; } +void TransactionItem::set_id(int64_t value) { + p_impl->id = value; +} -void TransactionItem::set_action(Action value) { p_impl->action = value; } +void TransactionItem::set_action(Action value) { + p_impl->action = value; +} -void TransactionItem::set_reason(Reason value) { p_impl->reason = value; } +void TransactionItem::set_reason(Reason value) { + p_impl->reason = value; +} -void TransactionItem::set_state(State value) { p_impl->state = value; } +void TransactionItem::set_state(State value) { + p_impl->state = value; +} -void TransactionItem::set_repoid(const std::string & value) { p_impl->repoid = value; } +void TransactionItem::set_repoid(const std::string & value) { + p_impl->repoid = value; +} -int64_t TransactionItem::get_item_id() const noexcept { return p_impl->item_id; } +int64_t TransactionItem::get_item_id() const noexcept { + return p_impl->item_id; +} -void TransactionItem::set_item_id(int64_t value) { p_impl->item_id = value; } +void TransactionItem::set_item_id(int64_t value) { + p_impl->item_id = value; +} /* void From 44f80d7ea21aff2e6a9f4284f882968a72802585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 24 Jan 2024 09:30:48 +0100 Subject: [PATCH 3/5] Add pImpl to `libdnf5::transaction::CompsGroup` Also add pImpl to `libdnf5::transaction::CompsGroupPackage`. --- include/libdnf5/transaction/comps_group.hpp | 61 +++++----- libdnf5/transaction/comps_group.cpp | 121 ++++++++++++++++++-- 2 files changed, 148 insertions(+), 34 deletions(-) diff --git a/include/libdnf5/transaction/comps_group.hpp b/include/libdnf5/transaction/comps_group.hpp index cdeedeb36..5e35a582c 100644 --- a/include/libdnf5/transaction/comps_group.hpp +++ b/include/libdnf5/transaction/comps_group.hpp @@ -47,6 +47,12 @@ class CompsGroup : public TransactionItem { // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.toStr() std::string to_string() const { return get_group_id(); } + CompsGroup(const CompsGroup & src); + CompsGroup & operator=(const CompsGroup & src); + CompsGroup(CompsGroup && src) noexcept; + CompsGroup & operator=(CompsGroup && src) noexcept; + ~CompsGroup(); + private: friend Transaction; friend CompsGroupPackage; @@ -58,42 +64,42 @@ class CompsGroup : public TransactionItem { /// Get text id of the group (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.getGroupId() - const std::string & get_group_id() const noexcept { return group_id; } + const std::string & get_group_id() const noexcept; /// Get text id of the group (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.setGroupId(const std::string & value) - void set_group_id(const std::string & value) { group_id = value; } + void set_group_id(const std::string & value); /// Get name of the group (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.getName() - const std::string & get_name() const noexcept { return name; } + const std::string & get_name() const noexcept; /// Set name of the group (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.setName(const std::string & value) - void set_name(const std::string & value) { name = value; } + void set_name(const std::string & value); /// Get translated name of the group in the current locale (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.getTranslatedName() - const std::string & get_translated_name() const noexcept { return translated_name; } + const std::string & get_translated_name() const noexcept; /// Set translated name of the group in the current locale (xml element: `VALUE...`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.setTranslatedName(const std::string & value) - void set_translated_name(const std::string & value) { translated_name = value; } + void set_translated_name(const std::string & value); /// Get types of the packages to be installed with the group (related xml elements: ``) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.getPackageTypes() - libdnf5::comps::PackageType get_package_types() const noexcept { return package_types; } + libdnf5::comps::PackageType get_package_types() const noexcept; /// Set types of the packages to be installed with the group (related xml elements: ``) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.setPackageTypes(libdnf::CompsPackageType value) - void set_package_types(libdnf5::comps::PackageType value) { package_types = value; } + void set_package_types(libdnf5::comps::PackageType value); /// Create a new CompsGroupPackage object and return a reference to it. /// The object is owned by the CompsGroup. @@ -102,18 +108,15 @@ class CompsGroup : public TransactionItem { /// Get list of packages associated with the group. /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupItem.getPackages() - std::vector & get_packages() { return packages; } + std::vector & get_packages(); // TODO(dmach): rewrite into TransactionSack.list_installed_groups(); how to deal with references to different transactions? We don't want all of them loaded into memory. //static std::vector< TransactionItemPtr > getTransactionItemsByPattern( // libdnf5::utils::SQLite3Ptr conn, // const std::string &pattern); - std::string group_id; - std::string name; - std::string translated_name; - libdnf5::comps::PackageType package_types; - std::vector packages; + class Impl; + std::unique_ptr p_impl; }; @@ -121,6 +124,14 @@ class CompsGroup : public TransactionItem { /// // @replaces libdnf:transaction/CompsGroupItem.hpp:class:CompsGroupPackage class CompsGroupPackage { +public: + ~CompsGroupPackage(); + CompsGroupPackage(const CompsGroupPackage & src); + CompsGroupPackage & operator=(const CompsGroupPackage & src); + CompsGroupPackage(CompsGroupPackage && src) noexcept; + CompsGroupPackage & operator=(CompsGroupPackage && src) noexcept; + CompsGroupPackage(); + private: friend Transaction; friend CompsGroupPackageDbUtils; @@ -128,51 +139,49 @@ class CompsGroupPackage { /// Get database id (primary key) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.getId() - int64_t get_id() const noexcept { return id; } + int64_t get_id() const noexcept; /// Set database id (primary key) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.setId(int64_t value) - void set_id(int64_t value) { id = value; } + void set_id(int64_t value); /// Get name of a package associated with a comps group (xml element: `VALUE`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.getName() - const std::string & get_name() const noexcept { return name; } + const std::string & get_name() const noexcept; /// Set name of a package associated with a comps group (xml element: `VALUE`) /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.setName(const std::string & value) - void set_name(const std::string & value) { name = value; } + void set_name(const std::string & value); /// Get a flag that determines if the package was present after the transaction it's associated with has finished. /// If the package was installed before running the transaction, it's still counted as installed. /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.getInstalled() - bool get_installed() const noexcept { return installed; } + bool get_installed() const noexcept; /// Set a flag that determines if the package was present after the transaction it's associated with has finished. /// If the package was installed before running the transaction, it's still counted as installed. /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.setInstalled(bool value) - void set_installed(bool value) { installed = value; } + void set_installed(bool value); /// Get type of package associated with a comps group (xml element: ``) /// See `enum class comps::PackageType` documentation for more details. /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.getPackageType() - libdnf5::comps::PackageType get_package_type() const noexcept { return package_type; } + libdnf5::comps::PackageType get_package_type() const noexcept; /// Set type of package associated with a comps group (xml element: ``) /// See `enum class libdnf5::comps::PackageType` documentation for more details. /// // @replaces libdnf:transaction/CompsGroupItem.hpp:method:CompsGroupPackage.setPackageType(libdnf::PackageType value) - void set_package_type(libdnf5::comps::PackageType value) { package_type = value; } + void set_package_type(libdnf5::comps::PackageType value); - int64_t id = 0; - std::string name; - bool installed = false; - libdnf5::comps::PackageType package_type = libdnf5::comps::PackageType::DEFAULT; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/comps_group.cpp b/libdnf5/transaction/comps_group.cpp index 92a8e546d..c6b484322 100644 --- a/libdnf5/transaction/comps_group.cpp +++ b/libdnf5/transaction/comps_group.cpp @@ -21,19 +21,128 @@ along with libdnf. If not, see . #include "libdnf5/transaction/comps_group.hpp" #include "db/comps_group.hpp" -#include "db/comps_group_package.hpp" #include "libdnf5/transaction/transaction.hpp" #include "libdnf5/transaction/transaction_item.hpp" -#include - namespace libdnf5::transaction { +class CompsGroup::Impl { +private: + friend CompsGroup; + + std::string group_id; + std::string name; + std::string translated_name; + libdnf5::comps::PackageType package_types; + std::vector packages; +}; + + +CompsGroup::CompsGroup(const Transaction & trans) + : TransactionItem::TransactionItem(trans), + p_impl(std::make_unique()) {} + +CompsGroup::CompsGroup(const CompsGroup & src) : TransactionItem(src), p_impl(new Impl(*src.p_impl)) {} +CompsGroup::CompsGroup(CompsGroup && src) noexcept = default; -CompsGroup::CompsGroup(const Transaction & trans) : TransactionItem::TransactionItem(trans) {} +CompsGroup & CompsGroup::operator=(const CompsGroup & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + return *this; +} +CompsGroup & CompsGroup::operator=(CompsGroup && src) noexcept = default; +CompsGroup::~CompsGroup() = default; + +const std::string & CompsGroup::get_group_id() const noexcept { + return p_impl->group_id; +} +void CompsGroup::set_group_id(const std::string & value) { + p_impl->group_id = value; +} +const std::string & CompsGroup::get_name() const noexcept { + return p_impl->name; +} +void CompsGroup::set_name(const std::string & value) { + p_impl->name = value; +} +const std::string & CompsGroup::get_translated_name() const noexcept { + return p_impl->translated_name; +} +void CompsGroup::set_translated_name(const std::string & value) { + p_impl->translated_name = value; +} +libdnf5::comps::PackageType CompsGroup::get_package_types() const noexcept { + return p_impl->package_types; +} +void CompsGroup::set_package_types(libdnf5::comps::PackageType value) { + p_impl->package_types = value; +} +CompsGroupPackage & CompsGroup::new_package() { + return p_impl->packages.emplace_back(); +} +std::vector & CompsGroup::get_packages() { + return p_impl->packages; +} + +class CompsGroupPackage::Impl { +private: + friend CompsGroupPackage; + int64_t id = 0; + std::string name; + bool installed = false; + libdnf5::comps::PackageType package_type = libdnf5::comps::PackageType::DEFAULT; +}; + +CompsGroupPackage::CompsGroupPackage(const CompsGroupPackage & src) : p_impl(new Impl(*src.p_impl)) {} +CompsGroupPackage::CompsGroupPackage(CompsGroupPackage && src) noexcept = default; + +CompsGroupPackage & CompsGroupPackage::operator=(const CompsGroupPackage & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +CompsGroupPackage & CompsGroupPackage::operator=(CompsGroupPackage && src) noexcept = default; +CompsGroupPackage::CompsGroupPackage() : p_impl(std::make_unique()) {} +CompsGroupPackage::~CompsGroupPackage() = default; + +int64_t CompsGroupPackage::get_id() const noexcept { + return p_impl->id; +} +void CompsGroupPackage::set_id(int64_t value) { + p_impl->id = value; +} +const std::string & CompsGroupPackage::get_name() const noexcept { + return p_impl->name; +} +void CompsGroupPackage::set_name(const std::string & value) { + p_impl->name = value; +} +bool CompsGroupPackage::get_installed() const noexcept { + return p_impl->installed; +} +void CompsGroupPackage::set_installed(bool value) { + p_impl->installed = value; +} +libdnf5::comps::PackageType CompsGroupPackage::get_package_type() const noexcept { + return p_impl->package_type; +} +void CompsGroupPackage::set_package_type(libdnf5::comps::PackageType value) { + p_impl->package_type = value; +} /* std::vector< TransactionItemPtr > @@ -71,9 +180,5 @@ CompsGroup::getTransactionItemsByPattern(libdnf5::utils::SQLite3Ptr conn, const } */ -CompsGroupPackage & CompsGroup::new_package() { - return packages.emplace_back(); -} - } // namespace libdnf5::transaction From 53ce09ce1587afa67a2d42a57c44aa47be79bbf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 24 Jan 2024 09:35:31 +0100 Subject: [PATCH 4/5] Add pImpl to `libdnf5::transaction::Package` --- include/libdnf5/transaction/rpm_package.hpp | 35 ++++++------ libdnf5/transaction/rpm_package.cpp | 63 ++++++++++++++++++++- 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/include/libdnf5/transaction/rpm_package.hpp b/include/libdnf5/transaction/rpm_package.hpp index 235ff5bec..22a2cce88 100644 --- a/include/libdnf5/transaction/rpm_package.hpp +++ b/include/libdnf5/transaction/rpm_package.hpp @@ -21,10 +21,8 @@ along with libdnf. If not, see . #define LIBDNF5_TRANSACTION_RPM_PACKAGE_HPP #include "transaction_item.hpp" -#include "transaction_item_reason.hpp" #include -#include namespace libdnf5::transaction { @@ -42,33 +40,39 @@ class Package : public TransactionItem { /// Get package name /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.getName() - const std::string & get_name() const noexcept { return name; } + const std::string & get_name() const noexcept; /// Get package epoch /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.getEpoch() - const std::string & get_epoch() const noexcept { return epoch; } + const std::string & get_epoch() const noexcept; /// Get package release /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.getRelease() - const std::string & get_release() const noexcept { return release; } + const std::string & get_release() const noexcept; /// Get package arch /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.getArch() - const std::string & get_arch() const noexcept { return arch; } + const std::string & get_arch() const noexcept; /// Get package version /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.getVersion() - const std::string & get_version() const noexcept { return version; } + const std::string & get_version() const noexcept; /// Get string representation of the object, which equals to package NEVRA /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.toStr() std::string to_string() const; + ~Package(); + Package(const Package & src); + Package & operator=(const Package & src); + Package(Package && src) noexcept; + Package & operator=(Package && src) noexcept; + private: friend RpmDbUtils; friend Transaction; @@ -78,7 +82,7 @@ class Package : public TransactionItem { /// Set package name /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.setName(const std::string & value) - void set_name(const std::string & value) { name = value; } + void set_name(const std::string & value); /// Get package epoch as an integer uint32_t get_epoch_int() const; @@ -86,22 +90,22 @@ class Package : public TransactionItem { /// Set package epoch /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.setEpoch(int32_t value) - void set_epoch(const std::string & value) { epoch = value; } + void set_epoch(const std::string & value); /// Set package version /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.setVersion(const std::string & value) - void set_version(const std::string & value) { version = value; } + void set_version(const std::string & value); /// Set package release /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.setRelease(const std::string & value) - void set_release(const std::string & value) { release = value; } + void set_release(const std::string & value); /// Set package arch /// // @replaces libdnf:transaction/RPMItem.hpp:method:RPMItem.setArch(const std::string & value) - void set_arch(const std::string & value) { arch = value; } + void set_arch(const std::string & value); /* // TODO(dmach): Implement TransactionSack.new_filter().filter_package_pattern() @@ -115,11 +119,8 @@ class Package : public TransactionItem { bool operator<(const Package & other) const; - std::string name; - std::string epoch; - std::string version; - std::string release; - std::string arch; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/rpm_package.cpp b/libdnf5/transaction/rpm_package.cpp index 9207e4afd..fe9c4fdf2 100644 --- a/libdnf5/transaction/rpm_package.cpp +++ b/libdnf5/transaction/rpm_package.cpp @@ -25,16 +25,73 @@ along with libdnf. If not, see . #include "libdnf5/rpm/nevra.hpp" #include "libdnf5/transaction/transaction.hpp" -#include -#include #include namespace libdnf5::transaction { +class Package::Impl { +private: + friend Package; + std::string name; + std::string epoch; + std::string version; + std::string release; + std::string arch; +}; + +Package::Package(const Transaction & trans) + : TransactionItem::TransactionItem(trans), + p_impl(std::make_unique()) {} + +Package::Package(const Package & src) : TransactionItem(src), p_impl(new Impl(*src.p_impl)) {} +Package::Package(Package && src) noexcept = default; + +Package & Package::operator=(const Package & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +Package & Package::operator=(Package && src) noexcept = default; +Package::~Package() = default; -Package::Package(const Transaction & trans) : TransactionItem::TransactionItem(trans) {} +const std::string & Package::get_name() const noexcept { + return p_impl->name; +} +const std::string & Package::get_epoch() const noexcept { + return p_impl->epoch; +} +const std::string & Package::get_release() const noexcept { + return p_impl->release; +} +const std::string & Package::get_arch() const noexcept { + return p_impl->arch; +} +const std::string & Package::get_version() const noexcept { + return p_impl->version; +} +void Package::set_name(const std::string & value) { + p_impl->name = value; +} +void Package::set_epoch(const std::string & value) { + p_impl->epoch = value; +} +void Package::set_version(const std::string & value) { + p_impl->version = value; +} +void Package::set_release(const std::string & value) { + p_impl->release = value; +} +void Package::set_arch(const std::string & value) { + p_impl->arch = value; +} uint32_t Package::get_epoch_int() const { if (get_epoch().empty()) { From 29c2aa808a27bb15f661210242f04b3994c44baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 24 Jan 2024 09:37:36 +0100 Subject: [PATCH 5/5] Add pImpl to `libdnf5::transaction::TransactionHistory` It explicitely deletes copy and move constructors becuase TransactionHistory contains weak ptr guard and it cannot be easily moved or copied. --- .../transaction/transaction_history.hpp | 12 ++++--- libdnf5/transaction/transaction_history.cpp | 31 ++++++++++++++----- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/include/libdnf5/transaction/transaction_history.hpp b/include/libdnf5/transaction/transaction_history.hpp index 7074773d6..a7373f31e 100644 --- a/include/libdnf5/transaction/transaction_history.hpp +++ b/include/libdnf5/transaction/transaction_history.hpp @@ -36,8 +36,13 @@ class TransactionHistory { public: explicit TransactionHistory(const libdnf5::BaseWeakPtr & base); explicit TransactionHistory(libdnf5::Base & base); + ~TransactionHistory(); + TransactionHistory(const TransactionHistory & src) = delete; + TransactionHistory & operator=(const TransactionHistory & src) = delete; + TransactionHistory(TransactionHistory && src) noexcept = delete; + TransactionHistory & operator=(TransactionHistory && src) noexcept = delete; - TransactionHistoryWeakPtr get_weak_ptr() { return TransactionHistoryWeakPtr(this, &guard); } + TransactionHistoryWeakPtr get_weak_ptr(); /// Lists all transaction IDs from the transaction history database. The /// result is sorted in ascending order. @@ -72,9 +77,8 @@ class TransactionHistory { /// Create a new Transaction object. libdnf5::transaction::Transaction new_transaction(); - BaseWeakPtr base; - - WeakPtrGuard guard; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/transaction_history.cpp b/libdnf5/transaction/transaction_history.cpp index 351746b43..5b6afd967 100644 --- a/libdnf5/transaction/transaction_history.cpp +++ b/libdnf5/transaction/transaction_history.cpp @@ -27,35 +27,52 @@ along with libdnf. If not, see . namespace libdnf5::transaction { +class TransactionHistory::Impl { +public: + Impl(const libdnf5::BaseWeakPtr & base); -TransactionHistory::TransactionHistory(const libdnf5::BaseWeakPtr & base) : base{base} {} +private: + friend TransactionHistory; + BaseWeakPtr base; + + WeakPtrGuard guard; +}; + +TransactionHistory::Impl::Impl(const libdnf5::BaseWeakPtr & base) : base(base) {} + +TransactionHistory::TransactionHistory(const libdnf5::BaseWeakPtr & base) : p_impl(std::make_unique(base)) {} TransactionHistory::TransactionHistory(libdnf5::Base & base) : TransactionHistory(base.get_weak_ptr()) {} +TransactionHistory::~TransactionHistory() = default; Transaction TransactionHistory::new_transaction() { - return Transaction(base); + return Transaction(p_impl->base); } std::vector TransactionHistory::list_transaction_ids() { - return TransactionDbUtils::select_transaction_ids(base); + return TransactionDbUtils::select_transaction_ids(p_impl->base); } std::vector TransactionHistory::list_transactions(const std::vector & ids) { - return TransactionDbUtils::select_transactions_by_ids(base, ids); + return TransactionDbUtils::select_transactions_by_ids(p_impl->base, ids); } std::vector TransactionHistory::list_transactions(int64_t start, int64_t end) { - return TransactionDbUtils::select_transactions_by_range(base, start, end); + return TransactionDbUtils::select_transactions_by_range(p_impl->base, start, end); } std::vector TransactionHistory::list_all_transactions() { - return TransactionDbUtils::select_transactions_by_ids(base, {}); + return TransactionDbUtils::select_transactions_by_ids(p_impl->base, {}); } BaseWeakPtr TransactionHistory::get_base() const { - return base; + return p_impl->base; +} + +TransactionHistoryWeakPtr TransactionHistory::get_weak_ptr() { + return {this, &p_impl->guard}; } } // namespace libdnf5::transaction