From 4e32b5880f589f923d216faab569c1e87da5a81f Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 20 Jun 2020 18:02:31 +0200 Subject: [PATCH] [#1247] Checkpoint before rebase and resume --- src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h | 3 +- src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h | 3 +- src/hooks/dhcp/mysql_cb/mysql_cb_impl.h | 3 +- .../mysql_cb/tests/mysql_cb_dhcp4_unittest.cc | 2 +- .../mysql_cb/tests/mysql_cb_dhcp6_unittest.cc | 2 +- src/lib/database/audit_entry.h | 20 +++++- .../database/tests/audit_entry_unittest.cc | 64 ++++++++++++++++++- src/lib/dhcpsrv/config_backend_dhcp4.h | 3 +- src/lib/dhcpsrv/config_backend_dhcp6.h | 3 +- src/lib/dhcpsrv/config_backend_pool_dhcp4.h | 3 +- src/lib/dhcpsrv/config_backend_pool_dhcp6.h | 3 +- .../testutils/test_config_backend_dhcp4.h | 3 +- .../testutils/test_config_backend_dhcp6.h | 3 +- src/lib/process/cb_ctl_base.h | 2 +- 14 files changed, 100 insertions(+), 17 deletions(-) diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h index f5fd30b40b..4f729822fe 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h @@ -231,7 +231,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::ServerSelector& server_selector, diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h index 3f35e838c7..babeb76d78 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h @@ -231,7 +231,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::ServerSelector& server_selector, diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h index 9c3639d77c..d6b444b575 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h @@ -240,7 +240,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @param [out] audit_entries Reference to the container where fetched audit /// entries will be inserted. void getRecentAuditEntries(const int index, diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc index 0a09237b9a..16d9577155 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc @@ -442,7 +442,7 @@ public: << audit_entry->getObjectId() << ", " << static_cast(audit_entry->getModificationType()) << ", " << audit_entry->getModificationTime() << ", " - << audit_entry->getEntryId() << ", " + << audit_entry->getModificationId() << ", " << audit_entry->getLogMessage() << std::endl; } diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc index 9a7a0e8ae3..bf5948011c 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc @@ -490,7 +490,7 @@ public: << audit_entry->getObjectId() << ", " << static_cast(audit_entry->getModificationType()) << ", " << audit_entry->getModificationTime() << ", " - << audit_entry->getEntryId() << ", " + << audit_entry->getModificationId() << ", " << audit_entry->getLogMessage() << std::endl; } diff --git a/src/lib/database/audit_entry.h b/src/lib/database/audit_entry.h index f115fb1982..3dc46b982d 100644 --- a/src/lib/database/audit_entry.h +++ b/src/lib/database/audit_entry.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -40,7 +41,7 @@ typedef boost::shared_ptr AuditEntryPtr; /// entries with later modification time than stored. That way the /// server queries only for the audit entries it hasn't fetched yet. /// In the case two (or more) successive audit entries have the same -/// modification time the strictly increasing id is used. +/// modification time the strictly increasing modification id is used. /// /// When the modification type of the entry is set to /// @c AuditEntry::ModificationType::DELETE, the corresponding @@ -180,7 +181,7 @@ public: /// @brief Returns entry id. /// /// @return Identifier of the entry. - uint64_t getEntryId() const { + uint64_t getModificationId() const { return (id_); } @@ -224,6 +225,9 @@ struct AuditEntryObjectTypeTag { }; /// @brief Tag used to access index by modification time. struct AuditEntryModificationTimeIdTag { }; +/// @brief Tag used to access index by object id. +struct AuditEntryObjectIdTag { }; + /// @brief Multi index container holding @c AuditEntry instances. /// /// This container provides indexes to access the audit entries @@ -263,9 +267,19 @@ typedef boost::multi_index_container< boost::multi_index::const_mem_fun< AuditEntry, uint64_t, - &AuditEntry::getEntryId + &AuditEntry::getModificationId > > + >, + + // Third index allows for accessing by the object id. + boost::multi_index::hashed_non_unique< + boost::multi_index::tag, + boost::multi_index::const_mem_fun< + AuditEntry, + uint64_t, + &AuditEntry::getObjectId + > > > > AuditEntryCollection; diff --git a/src/lib/database/tests/audit_entry_unittest.cc b/src/lib/database/tests/audit_entry_unittest.cc index fbfdc1d2ae..8a61a2dd76 100644 --- a/src/lib/database/tests/audit_entry_unittest.cc +++ b/src/lib/database/tests/audit_entry_unittest.cc @@ -96,7 +96,7 @@ TEST_F(AuditEntryTest, create) { EXPECT_EQ(10, audit_entry->getObjectId()); EXPECT_EQ(AuditEntry::ModificationType::DELETE, audit_entry->getModificationType()); EXPECT_EQ(fixedTime(), audit_entry->getModificationTime()); - EXPECT_EQ(123, audit_entry->getEntryId()); + EXPECT_EQ(123, audit_entry->getModificationId()); EXPECT_EQ("deleted subnet 10", audit_entry->getLogMessage()); } @@ -110,7 +110,7 @@ TEST_F(AuditEntryTest, create) { EXPECT_EQ(123, audit_entry->getObjectId()); EXPECT_EQ(AuditEntry::ModificationType::CREATE, audit_entry->getModificationType()); EXPECT_TRUE(almostEqualTime(audit_entry->getModificationTime())); - EXPECT_EQ(234, audit_entry->getEntryId()); + EXPECT_EQ(234, audit_entry->getModificationId()); EXPECT_TRUE(audit_entry->getLogMessage().empty()); } } @@ -210,6 +210,32 @@ public: return (false); } + /// @brief Checks if the returned results range contains an @c AuditEntry + /// with a given object and modification types, and object identifier. + /// + /// @param object_type expected object type. + /// @param object_id expected object id. + /// @param modification_type expected modification type. + /// @param begin beginning of the results range to be examined. + /// @param end end of the results range to be examined. + template + bool includes(const std::string& object_type, const uint64_t object_id, + const AuditEntry::ModificationType& modification_type, + Iterator begin, Iterator end) { + // Iterate over the results range and look for the entry. + for (auto it = begin; it != end; ++it) { + if (((*it)->getObjectType() == object_type) && + ((*it)->getObjectId() == object_id) && + ((*it)->getModificationType() == modification_type)) { + // Entry found. + return (true); + } + } + + // Entry not found. + return (false); + } + /// @brief Audit entries used in the tests. AuditEntryCollection audit_entries_; @@ -297,6 +323,40 @@ TEST_F(AuditEntryCollectionTest, getByModificationTimeAndId) { EXPECT_TRUE(includes("dhcp4_subnet", 1000, lb, mod_time_idx.end())); EXPECT_TRUE(includes("dhcp4_shared_network", 1, lb, mod_time_idx.end())); EXPECT_TRUE(includes("dhcp4_option", 15, lb, mod_time_idx.end())); + + // Check the order is time first, id after. + create("dhcp4_subnet", 1000, AuditEntry::ModificationType::UPDATE, + diffTime(-8), 200, "updated subnet 1000"); + lb = mod_time_idx.lower_bound(mod); + ASSERT_EQ(4, std::distance(lb, mod_time_idx.end())); + EXPECT_TRUE(includes("dhcp4_subnet", 120, lb, mod_time_idx.end())); + EXPECT_TRUE(includes("dhcp4_subnet", 1000, lb, mod_time_idx.end())); + EXPECT_TRUE(includes("dhcp4_shared_network", 1, lb, mod_time_idx.end())); + EXPECT_TRUE(includes("dhcp4_option", 15, lb, mod_time_idx.end())); +} + +// Checks that entries can be found by object id. +TEST_F(AuditEntryCollectionTest, getByObjectId) { + const auto& object_id_idx = audit_entries_.get(); + + // Search for object id 10. + auto range = object_id_idx.equal_range(10); + ASSERT_EQ(1, std::distance(range.first, range.second)); + EXPECT_TRUE(includes("dhcp4_subnet", 10, range.first, range.second)); + + // Add another entry. + create("dhcp4_subnet", 10, AuditEntry::ModificationType::UPDATE, + diffTime(0), 111, "updated subnet 10"); + + // Now search should return two entries. + range = object_id_idx.equal_range(10); + ASSERT_EQ(2, std::distance(range.first, range.second)); + EXPECT_TRUE(includes("dhcp4_subnet", 10, + AuditEntry::ModificationType::CREATE, + range.first, range.second)); + EXPECT_TRUE(includes("dhcp4_subnet", 10, + AuditEntry::ModificationType::UPDATE, + range.first, range.second)); } } diff --git a/src/lib/dhcpsrv/config_backend_dhcp4.h b/src/lib/dhcpsrv/config_backend_dhcp4.h index 4f8ee608e9..f7aaaf8c8e 100644 --- a/src/lib/dhcpsrv/config_backend_dhcp4.h +++ b/src/lib/dhcpsrv/config_backend_dhcp4.h @@ -302,7 +302,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::ServerSelector& server_selector, diff --git a/src/lib/dhcpsrv/config_backend_dhcp6.h b/src/lib/dhcpsrv/config_backend_dhcp6.h index b11056e55e..dc4f6833f7 100644 --- a/src/lib/dhcpsrv/config_backend_dhcp6.h +++ b/src/lib/dhcpsrv/config_backend_dhcp6.h @@ -303,7 +303,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::ServerSelector& server_selector, diff --git a/src/lib/dhcpsrv/config_backend_pool_dhcp4.h b/src/lib/dhcpsrv/config_backend_pool_dhcp4.h index 8e6ba8b725..c86d740a34 100644 --- a/src/lib/dhcpsrv/config_backend_pool_dhcp4.h +++ b/src/lib/dhcpsrv/config_backend_pool_dhcp4.h @@ -228,7 +228,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::BackendSelector& backend_selector, diff --git a/src/lib/dhcpsrv/config_backend_pool_dhcp6.h b/src/lib/dhcpsrv/config_backend_pool_dhcp6.h index 2765acb9a2..1805b3d328 100644 --- a/src/lib/dhcpsrv/config_backend_pool_dhcp6.h +++ b/src/lib/dhcpsrv/config_backend_pool_dhcp6.h @@ -227,7 +227,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::BackendSelector& backend_selector, diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.h b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.h index fcda52b148..02a1803d0f 100644 --- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.h +++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.h @@ -221,7 +221,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::ServerSelector& server_selector, diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h index 9a1d25a106..2e5d92cf46 100644 --- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h +++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h @@ -221,7 +221,8 @@ public: /// @param modification_time Timestamp being a lower limit for the returned /// result set, i.e. entries later than specified time are returned. /// @param modification_id Identifier being a lower limit for the returned - /// result set, used when two (or more) entries have modification_time. + /// result set, used when two (or more) entries have the same + /// modification_time. /// @return Collection of audit entries. virtual db::AuditEntryCollection getRecentAuditEntries(const db::ServerSelector& server_selector, diff --git a/src/lib/process/cb_ctl_base.h b/src/lib/process/cb_ctl_base.h index 37daaaea9a..c74140185f 100644 --- a/src/lib/process/cb_ctl_base.h +++ b/src/lib/process/cb_ctl_base.h @@ -344,7 +344,7 @@ protected: // latest entry. const auto& index = audit_entries.get(); last_audit_entry_time_ = (*index.rbegin())->getModificationTime(); - last_audit_entry_id_ = (*index.rbegin())->getEntryId(); + last_audit_entry_id_ = (*index.rbegin())->getModificationId(); } /// @brief Stores the most recent audit entry timestamp. -- 2.47.2