diff --git a/src/main/native/cpp/Storage.cpp b/src/main/native/cpp/Storage.cpp index 433e00e..78fbfe2 100644 --- a/src/main/native/cpp/Storage.cpp +++ b/src/main/native/cpp/Storage.cpp @@ -282,7 +282,7 @@ void Storage::ProcessIncomingEntryDelete(std::shared_ptr msg, } // update local - DeleteEntryImpl(m_idmap[id], m_entries.end(), lock, false); + DeleteEntryImpl(m_idmap[id], lock, false); // broadcast to all other connections (note for client there won't // be any other connections, so don't bother) @@ -669,23 +669,20 @@ void Storage::DeleteEntry(StringRef name) { std::unique_lock lock(m_mutex); auto i = m_entries.find(name); if (i == m_entries.end()) return; - DeleteEntryImpl(i->getValue(), i, lock, true); + DeleteEntryImpl(i->getValue(), lock, true); } void Storage::DeleteEntry(unsigned int local_id) { std::unique_lock lock(m_mutex); if (local_id >= m_localmap.size()) return; - DeleteEntryImpl(m_localmap[local_id].get(), m_entries.end(), lock, true); + DeleteEntryImpl(m_localmap[local_id].get(), lock, true); } -void Storage::DeleteEntryImpl(Entry* entry, EntriesMap::iterator it, - std::unique_lock& lock, bool local) { +void Storage::DeleteEntryImpl(Entry* entry, std::unique_lock& lock, + bool local) { unsigned int id = entry->id; - // Erase entry from name and id mappings. - // Get iterator if it wasn't provided. - if (it == m_entries.end()) it = m_entries.find(entry->name); - if (it != m_entries.end()) m_entries.erase(it); + // Erase entry from id mapping. if (id < m_idmap.size()) m_idmap[id] = nullptr; // empty the value and reset id and local_write flag @@ -721,10 +718,6 @@ void Storage::DeleteEntryImpl(Entry* entry, EntriesMap::iterator it, template void Storage::DeleteAllEntriesImpl(bool local, F should_delete) { - if (m_entries.empty()) return; - - // can't erase without invalidating iterators, so build a new map - EntriesMap entries; for (auto& i : m_entries) { Entry* entry = i.getValue(); if (should_delete(entry)) { @@ -738,11 +731,7 @@ void Storage::DeleteAllEntriesImpl(bool local, F should_delete) { entry->value.reset(); continue; } - - // add it to new entries - entries.insert(std::make_pair(i.getKey(), std::move(i.getValue()))); } - m_entries.swap(entries); } void Storage::DeleteAllEntriesImpl(bool local) { diff --git a/src/main/native/cpp/Storage.h b/src/main/native/cpp/Storage.h index ea2240f..165c2ef 100644 --- a/src/main/native/cpp/Storage.h +++ b/src/main/native/cpp/Storage.h @@ -221,8 +221,8 @@ class Storage : public IStorage { std::unique_lock& lock, bool local); void SetEntryFlagsImpl(Entry* entry, unsigned int flags, std::unique_lock& lock, bool local); - void DeleteEntryImpl(Entry* entry, EntriesMap::iterator it, - std::unique_lock& lock, bool local); + void DeleteEntryImpl(Entry* entry, std::unique_lock& lock, + bool local); // Must be called with m_mutex held template diff --git a/src/test/native/cpp/StorageTest.cpp b/src/test/native/cpp/StorageTest.cpp index 44bad81..8c7c382 100644 --- a/src/test/native/cpp/StorageTest.cpp +++ b/src/test/native/cpp/StorageTest.cpp @@ -462,7 +462,10 @@ TEST_P(StorageTestPopulated, DeleteEntryExist) { NT_NOTIFY_DELETE | NT_NOTIFY_LOCAL, UINT_MAX)); storage.DeleteEntry("foo2"); - EXPECT_TRUE(entries().count("foo2") == 0); + ASSERT_EQ(1u, entries().count("foo2")); + EXPECT_EQ(nullptr, entries()["foo2"]->value); + EXPECT_EQ(0xffffu, entries()["foo2"]->id); + EXPECT_FALSE(entries()["foo2"]->local_write); if (GetParam()) { ASSERT_TRUE(idmap().size() >= 2); EXPECT_FALSE(idmap()[1]); @@ -482,7 +485,8 @@ TEST_P(StorageTestPopulated, DeleteAllEntries) { .Times(4); storage.DeleteAllEntries(); - ASSERT_TRUE(entries().empty()); + ASSERT_EQ(1u, entries().count("foo2")); + EXPECT_EQ(nullptr, entries()["foo2"]->value); } TEST_P(StorageTestPopulated, DeleteAllEntriesPersistent) { @@ -495,8 +499,8 @@ TEST_P(StorageTestPopulated, DeleteAllEntriesPersistent) { .Times(3); storage.DeleteAllEntries(); - ASSERT_EQ(1u, entries().size()); - EXPECT_EQ(1u, entries().count("foo2")); + ASSERT_EQ(1u, entries().count("foo2")); + EXPECT_NE(nullptr, entries()["foo2"]->value); } TEST_P(StorageTestPopulated, GetEntryInfoAll) { @@ -941,6 +945,20 @@ TEST_P(StorageTestPopulateOne, ProcessIncomingEntryAssignWithFlags) { conn.get(), conn); } +TEST_P(StorageTestPopulateOne, DeleteCheckHandle) { + EXPECT_CALL(dispatcher, QueueOutgoing(_, _, _)).Times(AnyNumber()); + EXPECT_CALL(notifier, NotifyEntry(_, _, _, _, _)).Times(AnyNumber()); + EXPECT_CALL(notifier, local_notifiers()) + .Times(AnyNumber()) + .WillRepeatedly(Return(false)); + + auto handle = storage.GetEntry("foo"); + storage.DeleteEntry("foo"); + storage.SetEntryTypeValue("foo", Value::MakeBoolean(true)); + auto handle2 = storage.GetEntry("foo"); + ASSERT_EQ(handle, handle2); +} + INSTANTIATE_TEST_CASE_P(StorageTestsEmpty, StorageTestEmpty, ::testing::Bool(), ); INSTANTIATE_TEST_CASE_P(StorageTestsPopulateOne, StorageTestPopulateOne,