Merge #2835

The interface to get cache properties from the client list.

src/bin/auth/tests/auth_srv_unittest.cc

@@ -1759,7 +1759,7 @@ public:
              data_sources_.push_back(
                  DataSourceInfo(client.get(),
                                 isc::datasrc::DataSourceClientContainerPtr(),
-                                false, RRClass::IN(), ztable_segment_));
+                                false, RRClass::IN(), ztable_segment_, ""));
         }
     }
 private:

src/bin/cfgmgr/plugins/datasrc.spec.pre.in

@@ -63,6 +63,11 @@
                                     "item_optional": false,
                                     "item_default": ""
                                 }
+                            },
+                            {
+                                "item_name": "name",
+                                "item_type": "string",
+                                "item_optional": true
                             }
                         ]
                     }

src/bin/cfgmgr/plugins/tests/datasrc_test.py

@@ -153,6 +153,101 @@ class DatasrcTest(unittest.TestCase):
             }
         }]})
 
+    def test_names_present(self):
+        """
+        Test we don't choke on configuration with the "name" being present on
+        some items.
+        """
+        self.accept({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "Whatever"
+        }]})
+
+    def test_names_default_classes(self):
+        """
+        Test we can have a client of the same type in different classes
+        without specified name. The defaults should be derived both from
+        the type and the class.
+        """
+        self.accept({
+        "IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }],
+        "CH": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }]})
+
+    def test_names_collision(self):
+        """
+        Reject when two names are the same.
+
+        Cases are:
+        - Explicit names.
+        - Two default names turn out to be the same (same type and class).
+        - One explicit is set to the same as the default one.
+        """
+        self.reject({"IN": [
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "Whatever"
+        },
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "Whatever"
+        }]})
+        # The same, but across different classes is allowed (we would
+        # identify the data source by class+name tuple)
+        self.accept({
+        "IN": [
+            {
+                "type": "MasterFiles",
+                "cache-enable": True,
+                "params": {},
+                "name": "Whatever"
+            }
+        ],
+        "CH": [
+            {
+                "type": "MasterFiles",
+                "cache-enable": True,
+                "params": {},
+                "name": "Whatever"
+            }
+        ]})
+        self.reject({"IN": [
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        },
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }]})
+        self.reject({"IN": [
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "MasterFiles"
+        },
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }]})
+
 if __name__ == '__main__':
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()

src/lib/cc/data.h

@@ -216,7 +216,7 @@ public:
     //@{
     /// Returns the ElementPtr at the given key
     /// \param name The key of the Element to return
-    /// \return The ElementPtr at the given key
+    /// \return The ElementPtr at the given key, or null if not present
     virtual ConstElementPtr get(const std::string& name) const;
 
     /// Sets the ElementPtr at the given key

src/lib/datasrc/client_list.cc

@@ -27,6 +27,7 @@
 #include <util/memory_segment_local.h>
 
 #include <memory>
+#include <set>
 #include <boost/foreach.hpp>
 #include <boost/bind.hpp>
 
@@ -47,9 +48,11 @@ namespace datasrc {
 ConfigurableClientList::DataSourceInfo::DataSourceInfo(
     DataSourceClient* data_src_client,
     const DataSourceClientContainerPtr& container, bool has_cache,
-    const RRClass& rrclass, const shared_ptr<ZoneTableSegment>& segment) :
+    const RRClass& rrclass, const shared_ptr<ZoneTableSegment>& segment,
+    const string& name) :
     data_src_client_(data_src_client),
-    container_(container)
+    container_(container),
+    name_(name)
 {
     if (has_cache) {
         cache_.reset(new InMemoryClient(segment, rrclass));
@@ -59,8 +62,9 @@ ConfigurableClientList::DataSourceInfo::DataSourceInfo(
 
 ConfigurableClientList::DataSourceInfo::DataSourceInfo(
     const RRClass& rrclass, const shared_ptr<ZoneTableSegment>& segment,
-    bool has_cache) :
-    data_src_client_(NULL)
+    bool has_cache, const string& name) :
+    data_src_client_(NULL),
+    name_(name)
 {
     if (has_cache) {
         cache_.reset(new InMemoryClient(segment, rrclass));
@@ -92,6 +96,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
         vector<DataSourceInfo> new_data_sources;
         shared_ptr<ZoneTableSegment> ztable_segment(
             ZoneTableSegment::create(*config, rrclass_));
+        set<string> used_names;
         for (; i < config->size(); ++i) {
             // Extract the parameters
             const ConstElementPtr dconf(config->get(i));
@@ -108,6 +113,13 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
             const bool want_cache(allow_cache &&
                                   dconf->contains("cache-enable") &&
                                   dconf->get("cache-enable")->boolValue());
+            // Get the name (either explicit, or guess)
+            const ConstElementPtr name_elem(dconf->get("name"));
+            const string name(name_elem ? name_elem->stringValue() : type);
+            if (!used_names.insert(name).second) {
+                isc_throw(ConfigurationError, "Duplicit name in client list: "
+                          << name);
+            }
 
             if (type == "MasterFiles") {
                 // In case the cache is not allowed, we just skip the master
@@ -130,7 +142,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                 }
                 new_data_sources.push_back(DataSourceInfo(rrclass_,
                                                           ztable_segment,
-                                                          true));
+                                                          true, name));
             } else {
                 // Ask the factory to create the data source for us
                 const DataSourcePair ds(this->getDataSourceClient(type,
@@ -138,7 +150,8 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                 // And put it into the vector
                 new_data_sources.push_back(DataSourceInfo(ds.first, ds.second,
                                                           want_cache, rrclass_,
-                                                          ztable_segment));
+                                                          ztable_segment,
+                                                          name));
             }
 
             if (want_cache) {
@@ -462,5 +475,18 @@ ConfigurableClientList::getDataSourceClient(const string& type,
     return (DataSourcePair(&container->getInstance(), container));
 }
 
+vector<DataSourceStatus>
+ConfigurableClientList::getStatus() const {
+    vector<DataSourceStatus> result;
+    BOOST_FOREACH(const DataSourceInfo& info, data_sources_) {
+        // TODO: Once we support mapped cache, decide when we need the
+        // SEGMENT_WAITING.
+        result.push_back(DataSourceStatus(info.name_, info.cache_ ?
+                                          SEGMENT_INUSE : SEGMENT_UNUSED,
+                                          "local"));
+    }
+    return (result);
+}
+
 }
 }

src/lib/datasrc/client_list.h

@@ -46,6 +46,76 @@ class InMemoryClient;
 class ZoneWriter;
 }
 
+/// \brief Segment status of the cache
+///
+/// Describes the status in which the memory segment for the in-memory cache of
+// /given data source is.
+enum MemorySegmentState {
+    /// \brief No segment used for this data source.
+    ///
+    /// This is usually a result of the cache being disabled.
+    SEGMENT_UNUSED,
+
+    /// \brief It is a mapped segment and we wait for information how to map
+    ///     it.
+    SEGMENT_WAITING,
+
+    /// \brief The segment is ready to be used.
+    SEGMENT_INUSE
+};
+
+/// \brief Status of one data source.
+///
+/// This indicates the status a data soure is in. It is used with segment
+/// and cache management, to discover the data sources that need external
+/// mapping or local loading.
+///
+/// In future, it may be extended for other purposes, such as performing an
+/// operation on named data source.
+class DataSourceStatus {
+public:
+    /// \brief Constructor
+    ///
+    /// Sets initial values. It doesn't matter what is provided for the type
+    /// if state is SEGMENT_UNUSED, the value is effectively ignored.
+    DataSourceStatus(const std::string& name, MemorySegmentState state,
+                     const std::string& type) :
+        name_(name),
+        type_(type),
+        state_(state)
+    {}
+
+    /// \brief Get the segment state
+    MemorySegmentState getSegmentState() const {
+        return (state_);
+    }
+
+    /// \brief Get the segment type
+    ///
+    /// \note Specific values of the type are only meaningful for the
+    ///     corresponding memory segment implementation and modules that
+    ///     directly manage the segments. Other normal applications should
+    ///     treat them as opaque identifiers.
+    ///
+    /// \throw isc::InvalidOperation if called and state is SEGMENT_UNUSED.
+    const std::string& getSegmentType() const {
+        if (getSegmentState() == SEGMENT_UNUSED) {
+            isc_throw(isc::InvalidOperation,
+                      "No segment used, no type therefore.");
+        }
+        return (type_);
+    }
+
+    /// \brief Get the name.
+    const std::string& getName() const {
+        return (name_);
+    }
+private:
+    std::string name_;
+    std::string type_;
+    MemorySegmentState state_;
+};
+
 /// \brief The list of data source clients.
 ///
 /// The purpose of this class is to hold several data source clients and search
@@ -332,13 +402,14 @@ public:
                        const boost::shared_ptr
                            <isc::datasrc::memory::ZoneTableSegment>&
                                ztable_segment,
-                       bool has_cache = false);
+                       bool has_cache = false,
+                       const std::string& name = std::string());
         DataSourceInfo(DataSourceClient* data_src_client,
                        const DataSourceClientContainerPtr& container,
                        bool has_cache, const dns::RRClass& rrclass,
                        const boost::shared_ptr
                            <isc::datasrc::memory::ZoneTableSegment>&
-                               ztable_segment);
+                               ztable_segment, const std::string& name);
         DataSourceClient* data_src_client_;
         DataSourceClientContainerPtr container_;
 
@@ -350,6 +421,7 @@ public:
         const DataSourceClient* getCacheClient() const;
         boost::shared_ptr<memory::InMemoryClient> cache_;
         boost::shared_ptr<memory::ZoneTableSegment> ztable_segment_;
+        std::string name_;
     };
 
     /// \brief The collection of data sources.
@@ -379,6 +451,15 @@ public:
     virtual DataSourcePair getDataSourceClient(const std::string& type,
                                                const data::ConstElementPtr&
                                                configuration);
+
+    /// \brief Get status information of all internal data sources.
+    ///
+    /// Get a DataSourceStatus for current state of each data source client
+    /// in this list.
+    ///
+    /// This may throw standard exceptions, such as std::bad_alloc. Otherwise,
+    /// it is exception free.
+    std::vector<DataSourceStatus> getStatus() const;
 public:
     /// \brief Access to the data source clients.
     ///

src/lib/datasrc/tests/client_list_unittest.cc

@@ -278,7 +278,7 @@ public:
             ds_.push_back(ds);
             ds_info_.push_back(ConfigurableClientList::DataSourceInfo(
                                    ds.get(), DataSourceClientContainerPtr(),
-                                   false, rrclass_, ztable_segment_));
+                                   false, rrclass_, ztable_segment_, ""));
         }
     }
 
@@ -512,12 +512,12 @@ TEST_F(ListTest, configureMulti) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"type1\","
-        "   \"cache\": \"off\","
+        "   \"cache-enable\": false,"
         "   \"params\": {}"
         "},"
         "{"
         "   \"type\": \"type2\","
-        "   \"cache\": \"off\","
+        "   \"cache-enable\": false,"
         "   \"params\": {}"
         "}]"
     ));
@@ -546,7 +546,7 @@ TEST_F(ListTest, configureParams) {
         ConstElementPtr elem(Element::fromJSON(string("["
             "{"
             "   \"type\": \"t\","
-            "   \"cache\": \"off\","
+            "   \"cache-enable\": false,"
             "   \"params\": ") + *param +
             "}]"));
         list_->configure(elem, true);
@@ -555,6 +555,33 @@ TEST_F(ListTest, configureParams) {
     }
 }
 
+TEST_F(ListTest, status) {
+    EXPECT_TRUE(list_->getStatus().empty());
+    const ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"type1\","
+        "   \"cache-enable\": false,"
+        "   \"params\": {}"
+        "},"
+        "{"
+        "   \"type\": \"type2\","
+        "   \"cache-enable\": true,"
+        "   \"cache-zones\": [],"
+        "   \"name\": \"Test name\","
+        "   \"params\": {}"
+        "}]"
+    ));
+    list_->configure(elem, true);
+    const vector<DataSourceStatus> statuses(list_->getStatus());
+    ASSERT_EQ(2, statuses.size());
+    EXPECT_EQ("type1", statuses[0].getName());
+    EXPECT_EQ(SEGMENT_UNUSED, statuses[0].getSegmentState());
+    EXPECT_THROW(statuses[0].getSegmentType(), isc::InvalidOperation);
+    EXPECT_EQ("Test name", statuses[1].getName());
+    EXPECT_EQ(SEGMENT_INUSE, statuses[1].getSegmentState());
+    EXPECT_EQ("local", statuses[1].getSegmentType());
+}
+
 TEST_F(ListTest, wrongConfig) {
     const char* configs[] = {
         // A lot of stuff missing from there
@@ -834,6 +861,54 @@ TEST_F(ListTest, masterFiles) {
     EXPECT_EQ(0, list_->getDataSources().size());
 }
 
+// Test the names are set correctly and collission is detected.
+TEST_F(ListTest, names) {
+    // Explicit name
+    const ConstElementPtr elem1(Element::fromJSON("["
+        "{"
+        "   \"type\": \"MasterFiles\","
+        "   \"cache-enable\": true,"
+        "   \"params\": {"
+        "       \".\": \"" TEST_DATA_DIR "/root.zone\""
+        "   },"
+        "   \"name\": \"Whatever\""
+        "}]"));
+    list_->configure(elem1, true);
+    EXPECT_EQ("Whatever", list_->getDataSources()[0].name_);
+
+    // Default name
+    const ConstElementPtr elem2(Element::fromJSON("["
+        "{"
+        "   \"type\": \"MasterFiles\","
+        "   \"cache-enable\": true,"
+        "   \"params\": {"
+        "       \".\": \"" TEST_DATA_DIR "/root.zone\""
+        "   }"
+        "}]"));
+    list_->configure(elem2, true);
+    EXPECT_EQ("MasterFiles", list_->getDataSources()[0].name_);
+
+    // Collission
+    const ConstElementPtr elem3(Element::fromJSON("["
+        "{"
+        "   \"type\": \"MasterFiles\","
+        "   \"cache-enable\": true,"
+        "   \"params\": {"
+        "       \".\": \"" TEST_DATA_DIR "/root.zone\""
+        "   }"
+        "},"
+        "{"
+        "   \"type\": \"MasterFiles\","
+        "   \"cache-enable\": true,"
+        "   \"params\": {"
+        "       \".\": \"" TEST_DATA_DIR "/root.zone\""
+        "   },"
+        "   \"name\": \"MasterFiles\""
+        "}]"));
+    EXPECT_THROW(list_->configure(elem3, true),
+                 ConfigurableClientList::ConfigurationError);
+}
+
 TEST_F(ListTest, BadMasterFile) {
     // Configuration should succeed, and the good zones in the list
     // below should be loaded. No bad zones should be loaded.
@@ -1088,4 +1163,16 @@ TYPED_TEST(ReloadTest, reloadMasterFile) {
                                                          RRType::TXT())->code);
 }
 
+// Check the status holds data
+TEST(DataSourceStatus, status) {
+    const DataSourceStatus status("Test", SEGMENT_INUSE, "local");
+    EXPECT_EQ("Test", status.getName());
+    EXPECT_EQ(SEGMENT_INUSE, status.getSegmentState());
+    EXPECT_EQ("local", status.getSegmentType());
+    const DataSourceStatus status_unused("Unused", SEGMENT_UNUSED, "");
+    EXPECT_EQ("Unused", status_unused.getName());
+    EXPECT_EQ(SEGMENT_UNUSED, status_unused.getSegmentState());
+    EXPECT_THROW(status_unused.getSegmentType(), isc::InvalidOperation);
+}
+
 }