Don't delete persistent entries in DeleteAllEntries. (#71)

This is a behavior change but without it DeleteAllEntries is dangerous and
not very useful, so I consider the current behavior to be a bug.

src/Storage.cpp

@@ -319,17 +319,7 @@ void Storage::ProcessIncoming(std::shared_ptr<Message> msg,
     }
     case Message::kClearEntries: {
       // update local
-      EntriesMap map;
-      m_entries.swap(map);
-      m_idmap.resize(0);
-
-      // set persistent dirty flag
-      m_persistent_dirty = true;
-
-      // notify
-      for (auto& entry : map)
-        m_notifier.NotifyEntry(entry.getKey(), entry.getValue()->value,
-                               NT_NOTIFY_DELETE);
+      DeleteAllEntriesImpl();
 
       // broadcast to all other connections (note for client there won't
       // be any other connections, so don't bother)
@@ -638,22 +628,35 @@ void Storage::DeleteEntry(StringRef name) {
   }
 }
 
+void Storage::DeleteAllEntriesImpl() {
+  if (m_entries.empty()) return;
+
+  // only delete non-persistent values
+  // can't erase without invalidating iterators, so build a new map
+  EntriesMap entries;
+  for (auto& i : m_entries) {
+    Entry* entry = i.getValue().get();
+    if (!entry->IsPersistent()) {
+      // notify it's being deleted
+      if (m_notifier.local_notifiers()) {
+        m_notifier.NotifyEntry(i.getKey(), i.getValue()->value,
+                               NT_NOTIFY_DELETE | NT_NOTIFY_LOCAL);
+      }
+      // remove it from idmap
+      if (entry->id != 0xffff) m_idmap[entry->id] = nullptr;
+    } else {
+      // add it to new entries
+      entries.insert(std::make_pair(i.getKey(), std::move(i.getValue())));
+    }
+  }
+  m_entries.swap(entries);
+}
+
 void Storage::DeleteAllEntries() {
   std::unique_lock<std::mutex> lock(m_mutex);
   if (m_entries.empty()) return;
-  EntriesMap map;
-  m_entries.swap(map);
-  m_idmap.resize(0);
 
-  // set persistent dirty flag
-  m_persistent_dirty = true;
-
-  // notify
-  if (m_notifier.local_notifiers()) {
-    for (auto& entry : map)
-      m_notifier.NotifyEntry(entry.getKey(), entry.getValue()->value,
-                             NT_NOTIFY_DELETE | NT_NOTIFY_LOCAL);
-  }
+  DeleteAllEntriesImpl();
 
   // generate message
   if (!m_queue_outgoing) return;

src/Storage.h

@@ -164,6 +164,7 @@ class Storage {
       bool periodic,
       std::vector<std::pair<std::string, std::shared_ptr<Value>>>* entries)
       const;
+  void DeleteAllEntriesImpl();
 
   ATOMIC_STATIC_DECL(Storage)
 };

test/unit/StorageTest.cpp

@@ -387,6 +387,19 @@ TEST_P(StorageTestPopulated, DeleteAllEntries) {
   EXPECT_EQ(Message::kClearEntries, msg->type());
 }
 
+TEST_P(StorageTestPopulated, DeleteAllEntriesPersistent) {
+  GetEntry("foo2")->flags = NT_PERSISTENT;
+  storage.DeleteAllEntries();
+  ASSERT_EQ(1u, entries().size());
+  EXPECT_EQ(1u, entries().count("foo2"));
+
+  ASSERT_EQ(1u, outgoing.size());
+  EXPECT_FALSE(outgoing[0].only);
+  EXPECT_FALSE(outgoing[0].except);
+  auto msg = outgoing[0].msg;
+  EXPECT_EQ(Message::kClearEntries, msg->type());
+}
+
 TEST_P(StorageTestPopulated, GetEntryInfoAll) {
   auto info = storage.GetEntryInfo("", 0u);
   ASSERT_EQ(4u, info.size());