From: Michal 'vorner' Vaner <michal.vaner@nic.cz>
Date: Thu, 21 Jun 2012 11:34:36 +0000 (+0200)
Subject: [1976] Fix deletions
X-Git-Tag: trac2351_base~97^2~7^2~2^2~21^2~14
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1124a637dfa3b123519bbd0e5e28d3b370c0bf4e;p=thirdparty%2Fkea.git

[1976] Fix deletions

It looks like the configuration is provided as a whole, not a diff only.
At least to the callback.
---

diff --git a/src/bin/auth/datasrc_configurator.h b/src/bin/auth/datasrc_configurator.h
index bd80c783f8..681d2657fc 100644
--- a/src/bin/auth/datasrc_configurator.h
+++ b/src/bin/auth/datasrc_configurator.h
@@ -21,6 +21,8 @@
 #include <config/ccsession.h>
 #include <cc/data.h>
 
+#include <set>
+
 /// \brief A class to configure the authoritative server's data source lists
 ///
 /// This will hook into the data_sources module configuration and it will
@@ -115,25 +117,33 @@ public:
             isc_throw(isc::InvalidOperation,
                       "Can't reconfigure while not inited");
         }
+        // Get the configuration and current state.
         typedef std::map<std::string, isc::data::ConstElementPtr> Map;
         const Map& map(config->mapValue());
-        for (Map::const_iterator it(map.begin()); it != map.end(); ++ it) {
+        const std::vector<isc::dns::RRClass>
+            activeVector(server_->getClientListClasses());
+        std::set<isc::dns::RRClass> active(activeVector.begin(),
+                                           activeVector.end());
+        // Go through the configuration and change everything.
+        for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
             isc::dns::RRClass rrclass(it->first);
-            if (it->second->getType() == isc::data::Element::null) {
-                server_->setClientList(rrclass, ListPtr());
-            } else {
-                ListPtr list(server_->getClientList(rrclass));
-                bool need_set(false);
-                if (!list) {
-                    list.reset(new List);
-                    need_set = true;
-                }
-                list->configure(*it->second, true);
-                if (need_set) {
-                    server_->setClientList(rrclass, list);
-                }
+            active.erase(rrclass);
+            ListPtr list(server_->getClientList(rrclass));
+            bool need_set(false);
+            if (!list) {
+                list.reset(new List);
+                need_set = true;
+            }
+            list->configure(*it->second, true);
+            if (need_set) {
+                server_->setClientList(rrclass, list);
             }
         }
+        // Remove the ones that are not in the configuration.
+        for (std::set<isc::dns::RRClass>::iterator it(active.begin());
+             it != active.end(); ++it) {
+            server_->setClientList(*it, ListPtr());
+        }
     }
 };
 
diff --git a/src/bin/auth/tests/datasrc_configurator_unittest.cc b/src/bin/auth/tests/datasrc_configurator_unittest.cc
index 55fa1b61fb..85c413e5d7 100644
--- a/src/bin/auth/tests/datasrc_configurator_unittest.cc
+++ b/src/bin/auth/tests/datasrc_configurator_unittest.cc
@@ -65,6 +65,14 @@ public:
             (list ? list->getConf() : "") + "\n";
         lists_[rrclass] = list;
     }
+    vector<RRClass> getClientListClasses() const {
+        vector<RRClass> result;
+        for (map<RRClass, ListPtr>::const_iterator it(lists_.begin());
+             it != lists_.end(); ++it) {
+            result.push_back(it->first);
+        }
+        return (result);
+    }
 protected:
     DatasrcConfiguratorTest() :
         session(ElementPtr(new ListElement), ElementPtr(new ListElement),
@@ -176,39 +184,37 @@ TEST_F(DatasrcConfiguratorTest, multiple) {
 
 // Check we can add another one later and the old one does not get
 // overwritten.
+//
+// It's almost like above, but we initialize first with single-list
+// config.
 TEST_F(DatasrcConfiguratorTest, updateAdd) {
-    // TODO: Make sure the communication protocol really works on
-    // the semi-diff principle here, not by sending everything.
-    // (actually, sending everything would work, as per above, two
-    // tests, but this test would be wrong).
     doInInit();
     const ElementPtr
-        config(Element::fromJSON("{\"CH\": [{\"type\": \"yyy\"}]}"));
+        config(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
+                                 "\"CH\": [{\"type\": \"xxx\"}]}"));
     session.addMessage(createCommand("config_update", config), "data_sources",
                        "*");
     log_ = "";
     mccs->checkCommand();
     // This one does not set
-    EXPECT_EQ("get CH\nset CH yyy\n", log_);
+    EXPECT_EQ("get CH\nset CH xxx\nget IN\n", log_);
     // But this should contain the yyy configuration
-    EXPECT_EQ("yyy", lists_[RRClass::CH()]->getConf());
-    EXPECT_EQ("xxx", lists_[RRClass::IN()]->getConf());
+    EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
+    EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ(2, lists_.size());
 }
 
 // We delete a class list in this test.
 TEST_F(DatasrcConfiguratorTest, updateDelete) {
-    // TODO: Make sure the protocol sends the diff and a delete
-    // is done by a null element. Where is a documentation for this?
     doInInit();
     const ElementPtr
-        config(Element::fromJSON("{\"IN\": null}"));
+        config(Element::fromJSON("{}"));
     session.addMessage(createCommand("config_update", config), "data_sources",
                        "*");
     log_ = "";
     mccs->checkCommand();
-    // This one does not set
     EXPECT_EQ("set IN \n", log_);
+    EXPECT_FALSE(lists_[RRClass::IN()]);
 }