File 0001-history-Fix-dnf-history-rollback-when-a-pa... of Package libdnf

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-history-Fix-dnf-history-rollback-when-a-package-was-removed-RhBug1683134.patch of Package libdnf

From a96d701f7f55ff475e11ac9cda63b81c31c54e7a Mon Sep 17 00:00:00 2001
From: Daniel Mach <dmach@redhat.com>
Date: Wed, 6 May 2020 08:34:46 +0200
Subject: [PATCH] history: Fix dnf history rollback when a package was removed
 (RhBug:1683134)

---
 libdnf/transaction/MergedTransaction.cpp      |  25 +++-
 .../transaction/MergedTransactionTest.cpp     | 120 ++++++++++++++++--
 .../transaction/MergedTransactionTest.hpp     |   6 +-
 3 files changed, 137 insertions(+), 14 deletions(-)

diff --git a/libdnf/transaction/MergedTransaction.cpp b/libdnf/transaction/MergedTransaction.cpp
index a7c06ffa9..a8d878cb5 100644
--- a/libdnf/transaction/MergedTransaction.cpp
+++ b/libdnf/transaction/MergedTransaction.cpp
@@ -19,6 +19,7 @@
  */
 
 #include "MergedTransaction.hpp"
+#include <algorithm>
 #include <vector>
 
 namespace libdnf {
@@ -171,6 +172,21 @@ MergedTransaction::getConsoleOutput()
     return output;
 }
 
+
+static bool transaction_item_sort_function(const std::shared_ptr<TransactionItemBase> lhs, const std::shared_ptr<TransactionItemBase> rhs) {
+    if (lhs->isForwardAction() && rhs->isForwardAction()) {
+        return false;
+    }
+    if (lhs->isBackwardAction() && rhs->isBackwardAction()) {
+        return false;
+    }
+    if (lhs->isBackwardAction()) {
+        return true;
+    }
+    return false;
+}
+
+
 /**
  * Get list of transaction items involved in the merged transaction
  * Actions are merged using following rules:
@@ -203,6 +219,9 @@ MergedTransaction::getItems()
     // iterate over transaction
     for (auto t : transactions) {
         auto transItems = t->getItems();
+        // sort transaction items by their action type - forward/backward
+        // this fixes behavior of the merging algorithm in several edge cases
+        std::sort(transItems.begin(), transItems.end(), transaction_item_sort_function);
         // iterate over transaction items
         for (auto transItem : transItems) {
             // get item and its type
@@ -383,10 +402,6 @@ MergedTransaction::mergeItem(ItemPairMap &itemPairMap, TransactionItemBasePtr mT
     auto firstState = previousItemPair.first->getAction();
     auto newState = mTransItem->getAction();
 
-    if (firstState == TransactionItemAction::INSTALL && mTransItem->isBackwardAction()) {
-        return;
-    }
-
     switch (firstState) {
         case TransactionItemAction::REMOVE:
         case TransactionItemAction::OBSOLETED:
@@ -399,6 +414,8 @@ MergedTransaction::mergeItem(ItemPairMap &itemPairMap, TransactionItemBasePtr mT
                 // Install -> Remove = (nothing)
                 itemPairMap.erase(name);
                 break;
+            } else if (mTransItem->isBackwardAction()) {
+                break;
             }
             // altered -> transfer install to the altered package
             mTransItem->setAction(TransactionItemAction::INSTALL);
diff --git a/tests/libdnf/transaction/MergedTransactionTest.cpp b/tests/libdnf/transaction/MergedTransactionTest.cpp
index 90ad182cf..52507700b 100644
--- a/tests/libdnf/transaction/MergedTransactionTest.cpp
+++ b/tests/libdnf/transaction/MergedTransactionTest.cpp
@@ -700,7 +700,7 @@ MergedTransactionTest::test_downgrade()
 }
 
 void
-MergedTransactionTest::test_install_downgrade()
+MergedTransactionTest::test_install_downgrade_upgrade_remove()
 {
     auto trans1 = std::make_shared< libdnf::swdb_private::Transaction >(conn);
     trans1->addItem(
@@ -724,19 +724,123 @@ MergedTransactionTest::test_install_downgrade()
         TransactionItemReason::USER
     );
 
+    auto trans3 = std::make_shared< libdnf::swdb_private::Transaction >(conn);
+    trans3->addItem(
+        nevraToRPMItem(conn, "tour-0:4.6-1.noarch"),
+        "repo2",
+        TransactionItemAction::UPGRADED,
+        TransactionItemReason::USER
+    );
+    trans3->addItem(
+        nevraToRPMItem(conn, "tour-0:4.8-1.noarch"),
+        "repo1",
+        TransactionItemAction::UPGRADE,
+        TransactionItemReason::USER
+    );
+
+    auto trans4 = std::make_shared< libdnf::swdb_private::Transaction >(conn);
+    trans4->addItem(
+        nevraToRPMItem(conn, "tour-0:4.8-1.noarch"),
+        "repo1",
+        TransactionItemAction::REMOVE,
+        TransactionItemReason::USER
+    );
+
     MergedTransaction merged(trans1);
+
+    // test merging trans1, trans2
     merged.merge(trans2);
+    auto items2 = merged.getItems();
+    CPPUNIT_ASSERT_EQUAL(1, (int)items2.size());
+    auto item2 = items2.at(0);
+    CPPUNIT_ASSERT_EQUAL(std::string("tour-4.6-1.noarch"), item2->getItem()->toStr());
+    CPPUNIT_ASSERT_EQUAL(std::string("repo2"), item2->getRepoid());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemAction::INSTALL, item2->getAction());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemReason::USER, item2->getReason());
 
-    auto items = merged.getItems();
-    CPPUNIT_ASSERT_EQUAL(1, (int)items.size());
+    // test merging trans1, trans2, trans3
+    merged.merge(trans3);
+    auto items3 = merged.getItems();
+    CPPUNIT_ASSERT_EQUAL(1, (int)items3.size());
+    auto item3 = items3.at(0);
+    CPPUNIT_ASSERT_EQUAL(std::string("tour-4.8-1.noarch"), item3->getItem()->toStr());
+    CPPUNIT_ASSERT_EQUAL(std::string("repo1"), item3->getRepoid());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemAction::INSTALL, item3->getAction());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemReason::USER, item3->getReason());
 
-    auto item = items.at(0);
-    CPPUNIT_ASSERT_EQUAL(std::string("tour-4.6-1.noarch"), item->getItem()->toStr());
-    CPPUNIT_ASSERT_EQUAL(std::string("repo2"), item->getRepoid());
-    CPPUNIT_ASSERT_EQUAL(TransactionItemAction::INSTALL, item->getAction());
-    CPPUNIT_ASSERT_EQUAL(TransactionItemReason::USER, item->getReason());
+    // test merging trans1, trans2, trans3, trans4
+    merged.merge(trans4);
+    auto items4 = merged.getItems();
+    CPPUNIT_ASSERT_EQUAL(0, (int)items4.size());
+    // trans4 removes the package, empty output is expected
 }
 
+
+void
+MergedTransactionTest::test_downgrade_upgrade_remove()
+{
+    auto trans1 = std::make_shared< libdnf::swdb_private::Transaction >(conn);
+    trans1->addItem(
+        nevraToRPMItem(conn, "tour-0:4.6-1.noarch"),
+        "repo2",
+        TransactionItemAction::DOWNGRADE,
+        TransactionItemReason::USER
+    );
+    trans1->addItem(
+        nevraToRPMItem(conn, "tour-0:4.8-1.noarch"),
+        "repo1",
+        TransactionItemAction::DOWNGRADED,
+        TransactionItemReason::USER
+    );
+
+    // items are in reversed order than in test_install_downgrade_upgrade_remove()
+    // fixing this required ordering transaction items by forward/backward action
+    auto trans2 = std::make_shared< libdnf::swdb_private::Transaction >(conn);
+    trans2->addItem(
+        nevraToRPMItem(conn, "tour-0:4.8-1.noarch"),
+        "repo1",
+        TransactionItemAction::UPGRADE,
+        TransactionItemReason::USER
+    );
+    trans2->addItem(
+        nevraToRPMItem(conn, "tour-0:4.6-1.noarch"),
+        "repo2",
+        TransactionItemAction::UPGRADED,
+        TransactionItemReason::USER
+    );
+
+    auto trans3 = std::make_shared< libdnf::swdb_private::Transaction >(conn);
+    trans3->addItem(
+        nevraToRPMItem(conn, "tour-0:4.8-1.noarch"),
+        "repo1",
+        TransactionItemAction::REMOVE,
+        TransactionItemReason::USER
+    );
+
+    MergedTransaction merged(trans1);
+
+    // test merging trans1, trans2
+    merged.merge(trans2);
+    auto items2 = merged.getItems();
+    CPPUNIT_ASSERT_EQUAL(1, (int)items2.size());
+    auto item2 = items2.at(0);
+    CPPUNIT_ASSERT_EQUAL(std::string("tour-4.8-1.noarch"), item2->getItem()->toStr());
+    CPPUNIT_ASSERT_EQUAL(std::string("repo1"), item2->getRepoid());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemAction::REINSTALL, item2->getAction());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemReason::USER, item2->getReason());
+
+    // test merging trans1, trans2, trans3
+    merged.merge(trans3);
+    auto items3 = merged.getItems();
+    CPPUNIT_ASSERT_EQUAL(1, (int)items3.size());
+    auto item3 = items3.at(0);
+    CPPUNIT_ASSERT_EQUAL(std::string("tour-4.8-1.noarch"), item3->getItem()->toStr());
+    CPPUNIT_ASSERT_EQUAL(std::string("repo1"), item3->getRepoid());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemAction::REMOVE, item3->getAction());
+    CPPUNIT_ASSERT_EQUAL(TransactionItemReason::USER, item3->getReason());
+}
+
+
 void
 MergedTransactionTest::test_multilib_identity()
 {
diff --git a/tests/libdnf/transaction/MergedTransactionTest.hpp b/tests/libdnf/transaction/MergedTransactionTest.hpp
index 9f1ed660a..77585e865 100644
--- a/tests/libdnf/transaction/MergedTransactionTest.hpp
+++ b/tests/libdnf/transaction/MergedTransactionTest.hpp
@@ -26,7 +26,8 @@ class MergedTransactionTest : public CppUnit::TestCase {
     CPPUNIT_TEST(test_add_obsoleted_obsoleted);
 
     CPPUNIT_TEST(test_downgrade);
-    CPPUNIT_TEST(test_install_downgrade);
+    CPPUNIT_TEST(test_install_downgrade_upgrade_remove);
+    CPPUNIT_TEST(test_downgrade_upgrade_remove);
 
     CPPUNIT_TEST(test_multilib_identity);
 
@@ -56,7 +57,8 @@ class MergedTransactionTest : public CppUnit::TestCase {
     // END: tests ported from DNF unit tests
 
     void test_downgrade();
-    void test_install_downgrade();
+    void test_install_downgrade_upgrade_remove();
+    void test_downgrade_upgrade_remove();
 
     void test_multilib_identity();
 private:

Places

File 0001-history-Fix-dnf-history-rollback-when-a-package-was-removed-RhBug1683134.patch of Package libdnf

Places