Merge #2046

With integration into the rest.

Conflicts:
	src/lib/datasrc/client_list.h

src/bin/auth/datasrc_configurator.h

@@ -142,7 +142,7 @@ public:
                         push_back(RollbackConfiguration(rrclass,
                             list->getConfiguration()));
                 } else {
-                    list.reset(new List);
+                    list.reset(new List(rrclass));
                     need_set = true;
                     rollback_sets.push_back(RollbackPair(rrclass, ListPtr()));
                 }

src/bin/auth/tests/auth_srv_unittest.cc

@@ -1687,9 +1687,9 @@ TEST_F(AuthSrvTest, clientList) {
               server.getClientList(RRClass::IN()));
     // Put something in.
     const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list(new isc::datasrc::ConfigurableClientList());
+        list(new isc::datasrc::ConfigurableClientList(RRClass::IN()));
     const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list2(new isc::datasrc::ConfigurableClientList());
+        list2(new isc::datasrc::ConfigurableClientList(RRClass::CH()));
     server.setClientList(RRClass::IN(), list);
     server.setClientList(RRClass::CH(), list2);
     // There are two things in the list and they are IN and CH

src/bin/auth/tests/datasrc_configurator_unittest.cc

@@ -35,7 +35,7 @@ class DatasrcConfiguratorTest;
 
 class FakeList {
 public:
-    FakeList() :
+    FakeList(const RRClass&) :
         configuration_(new ListElement)
     {}
     void configure(const ConstElementPtr& configuration, bool allow_cache) {

src/lib/datasrc/client_list.cc

@@ -30,11 +30,19 @@ namespace datasrc {
 
 ConfigurableClientList::DataSourceInfo::DataSourceInfo(
     DataSourceClient* data_src_client,
-    const DataSourceClientContainerPtr& container, bool hasCache) :
+    const DataSourceClientContainerPtr& container, bool has_cache) :
     data_src_client_(data_src_client),
     container_(container)
 {
-    if (hasCache) {
+    if (has_cache) {
+        cache_.reset(new InMemoryClient);
+    }
+}
+
+ConfigurableClientList::DataSourceInfo::DataSourceInfo(bool has_cache) :
+    data_src_client_(NULL)
+{
+    if (has_cache) {
         cache_.reset(new InMemoryClient);
     }
 }
@@ -63,49 +71,83 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
             if (paramConf == ConstElementPtr()) {
                 paramConf.reset(new NullElement());
             }
-            // TODO: Special-case the master files type.
-            // Ask the factory to create the data source for us
-            const DataSourcePair ds(this->getDataSourceClient(type,
-                                                              paramConf));
             const bool want_cache(allow_cache &&
                                   dconf->contains("cache-enable") &&
                                   dconf->get("cache-enable")->boolValue());
-            // And put it into the vector
-            new_data_sources.push_back(DataSourceInfo(ds.first, ds.second,
-                                                      want_cache));
+
+            if (type == "MasterFiles") {
+                // In case the cache is not allowed, we just skip the master
+                // files (at least for now)
+                if (!allow_cache) {
+                    continue;
+                }
+                if (!want_cache) {
+                    isc_throw(ConfigurationError, "The cache must be enabled "
+                              "for the MasterFiles type");
+                }
+                new_data_sources.push_back(DataSourceInfo(true));
+            } else {
+                // Ask the factory to create the data source for us
+                const DataSourcePair ds(this->getDataSourceClient(type,
+                                                                  paramConf));
+                // And put it into the vector
+                new_data_sources.push_back(DataSourceInfo(ds.first, ds.second,
+                                                          want_cache));
+            }
+
             if (want_cache) {
-                if (!dconf->contains("cache-zones")) {
+                if (!dconf->contains("cache-zones") && type != "MasterFiles") {
                     isc_throw(isc::NotImplemented, "Auto-detection of zones "
                               "to cache is not yet implemented, supply "
                               "cache-zones parameter");
                     // TODO: Auto-detect list of all zones in the
                     // data source.
                 }
-                const ConstElementPtr zones(dconf->get("cache-zones"));
+
+                // List the zones we are loading
+                vector<string> zones_origins;
+                if (type == "MasterFiles") {
+                    const map<string, ConstElementPtr>
+                        zones_files(paramConf->mapValue());
+                    for (map<string, ConstElementPtr>::const_iterator
+                         it(zones_files.begin()); it != zones_files.end();
+                         ++it) {
+                        zones_origins.push_back(it->first);
+                    }
+                } else {
+                    const ConstElementPtr zones(dconf->get("cache-zones"));
+                    for (size_t i(0); i < zones->size(); ++i) {
+                        zones_origins.push_back(zones->get(i)->stringValue());
+                    }
+                }
+
                 const shared_ptr<InMemoryClient>
                     cache(new_data_sources.back().cache_);
                 const DataSourceClient* const
                     client(new_data_sources.back().data_src_client_);
-                for (size_t i(0); i < zones->size(); ++i) {
-                    const Name origin(zones->get(i)->stringValue());
-                    const DataSourceClient::FindResult
-                        zone(client->findZone(origin));
-                    if (zone.code != result::SUCCESS) {
-                        // The data source does not contain the zone, it can't
-                        // be cached.
-                        isc_throw(ConfigurationError, "Unable to cache "
-                                  "non-existent zone " << origin);
-                    }
+                for (vector<string>::const_iterator it(zones_origins.begin());
+                     it != zones_origins.end(); ++it) {
+                    const Name origin(*it);
                     shared_ptr<InMemoryZoneFinder>
                         finder(new
-                            InMemoryZoneFinder(zone.zone_finder->getClass(),
-                                               origin));
-                    ZoneIteratorPtr iterator(client->getIterator(origin));
-                    if (!iterator) {
-                        isc_throw(isc::Unexpected, "Got NULL iterator for "
-                                  "zone " << origin);
+                            InMemoryZoneFinder(rrclass_, origin));
+                    if (type == "MasterFiles") {
+                        finder->load(paramConf->get(*it)->stringValue());
+                    } else {
+                        ZoneIteratorPtr iterator;
+                        try {
+                            iterator = client->getIterator(origin);
+                        }
+                        catch (const DataSourceError&) {
+                            isc_throw(ConfigurationError, "Unable to cache "
+                                      "non-existent zone " << origin);
+                        }
+                        if (!iterator) {
+                            isc_throw(isc::Unexpected, "Got NULL iterator for "
+                                      "zone " << origin);
+                        }
+                        finder->load(*iterator);
                     }
-                    finder->load(*iterator);
                     cache->addZone(finder);
                 }
             }

src/lib/datasrc/client_list.h

@@ -16,6 +16,7 @@
 #define DATASRC_CONTAINER_H
 
 #include <dns/name.h>
+#include <dns/rrclass.h>
 #include <cc/data.h>
 #include <exceptions/exceptions.h>
 
@@ -186,7 +187,11 @@ typedef boost::shared_ptr<const ClientList> ConstClientListPtr;
 /// inherited except for tests.
 class ConfigurableClientList : public ClientList {
 public:
-    ConfigurableClientList() :
+    /// \brief Constructor
+    ///
+    /// \param rrclass For which class the list should work.
+    ConfigurableClientList(const isc::dns::RRClass &rrclass) :
+        rrclass_(rrclass),
         configuration_(new isc::data::ListElement)
     {}
     /// \brief Exception thrown when there's an error in configuration.
@@ -242,16 +247,11 @@ public:
     ///
     /// \todo The content yet to be defined.
     struct DataSourceInfo {
-        /// \brief Default constructor.
-        ///
-        /// Don't use directly. It is here so the structure can live in
-        /// a vector.
-        DataSourceInfo() :
-            data_src_client_(NULL)
-        {}
+        // Plays a role of default constructor too (for vector)
+        DataSourceInfo(bool has_cache = false);
         DataSourceInfo(DataSourceClient* data_src_client,
                        const DataSourceClientContainerPtr& container,
-                       bool hasCache);
+                       bool has_cache);
         DataSourceClient* data_src_client_;
         DataSourceClientContainerPtr container_;
         boost::shared_ptr<InMemoryClient> cache_;
@@ -291,9 +291,6 @@ protected:
     virtual DataSourcePair getDataSourceClient(const std::string& type,
                                                const data::ConstElementPtr&
                                                configuration);
-private:
-    /// \brief Currently active configuration.
-    isc::data::ConstElementPtr configuration_;
 public:
     /// \brief Access to the data source clients.
     ///
@@ -302,6 +299,10 @@ public:
     /// it might be, so it is just made public (there's no real reason to
     /// hide it).
     const DataSources& getDataSources() const { return (data_sources_); }
+private:
+    const isc::dns::RRClass rrclass_;
+    /// \brief Currently active configuration.
+    isc::data::ConstElementPtr configuration_;
 };
 
 } // namespace datasrc

src/lib/datasrc/tests/client_list_unittest.cc

@@ -45,7 +45,7 @@ public:
         Name getOrigin() const { return (origin_); }
         // The rest is not to be called, so just have them
         RRClass getClass() const {
-            return (RRClass::IN());
+            isc_throw(isc::NotImplemented, "Not implemented");
         }
         shared_ptr<Context> find(const Name&, const RRType&,
                                  const FindOptions)
@@ -106,6 +106,8 @@ public:
         type_(type),
         configuration_(configuration)
     {
+        EXPECT_NE("MasterFiles", type) << "MasterFiles is a special case "
+            "and it never should be created as a data source client";
         if (configuration_->getType() == Element::list) {
             for (size_t i(0); i < configuration_->size(); ++i) {
                 zones.insert(Name(configuration_->get(i)->stringValue()));
@@ -148,7 +150,12 @@ public:
         } else if (name == Name("null.org")) {
             return (ZoneIteratorPtr());
         } else {
-            return (ZoneIteratorPtr(new Iterator(name)));
+            FindResult result(findZone(name));
+            if (result.code == isc::datasrc::result::SUCCESS) {
+                return (ZoneIteratorPtr(new Iterator(name)));
+            } else {
+                isc_throw(DataSourceError, "No such zone");
+            }
         }
     }
     const string type_;
@@ -162,6 +169,9 @@ private:
 // some methods to dig directly in the internals, for the tests.
 class TestedList : public ConfigurableClientList {
 public:
+    TestedList(const RRClass& rrclass) :
+        ConfigurableClientList(rrclass)
+    {}
     DataSources& getDataSources() { return (data_sources_); }
     // Overwrite the list's method to get a data source with given type
     // and configuration. We mock the data source and don't create the
@@ -210,7 +220,7 @@ class ListTest : public ::testing::Test {
 public:
     ListTest() :
         // The empty list corresponds to a list with no elements inside
-        list_(new TestedList()),
+        list_(new TestedList(RRClass::IN())),
         config_elem_(Element::fromJSON("["
             "{"
             "   \"type\": \"test_type\","
@@ -502,6 +512,47 @@ TEST_F(ListTest, wrongConfig) {
          "{\"type\": \"x\", \"cache-enable\": true, \"cache-zones\": 13}]",
         "[{\"type\": \"test_type\", \"params\": 13}, "
          "{\"type\": \"x\", \"cache-enable\": true, \"cache-zones\": {}}]",
+        // Some bad inputs for MasterFiles special case
+
+        // It must have the cache enabled
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": {}}]",
+        // No cache-zones allowed here
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": true,"
+         "\"param\": {}, \"cache-zones\": []}]",
+        // Some bad types of params
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": []}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": 13}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": true}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": null}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": \"x\"}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": {\".\": 13}}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": {\".\": true}}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": {\".\": null}}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": {\".\": []}}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, "
+         "{\"type\": \"MasterFiles\", \"cache-enable\": false,"
+         "\"params\": {\".\": {}}}]",
         NULL
     };
     // Put something inside to see it survives the exception
@@ -615,6 +666,8 @@ TEST_F(ListTest, cacheZones) {
     EXPECT_EQ(result::SUCCESS, cache->findZone(Name("example.org")).code);
     EXPECT_EQ(result::SUCCESS, cache->findZone(Name("example.com")).code);
     EXPECT_EQ(result::NOTFOUND, cache->findZone(Name("example.cz")).code);
+    EXPECT_EQ(RRClass::IN(),
+              cache->findZone(Name("example.org")).zone_finder->getClass());
 
     // These are cached and answered from the cache
     positiveResult(list_->find(Name("example.com.")), ds_[0],
@@ -674,4 +727,27 @@ TEST_F(ListTest, badCache) {
     checkDS(0, "test_type", "{}", false);
 }
 
+TEST_F(ListTest, masterFiles) {
+    const ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"MasterFiles\","
+        "   \"cache-enable\": true,"
+        "   \"params\": {"
+        "       \".\": \"" TEST_DATA_DIR "/root.zone\""
+        "   }"
+        "}]"));
+    list_->configure(elem, true);
+
+    // It has only the cache
+    EXPECT_EQ(NULL, list_->getDataSources()[0].data_src_client_);
+
+    // And it can search
+    positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "com",
+                   true);
+
+    // If cache is not enabled, nothing is loaded
+    list_->configure(elem, false);
+    EXPECT_EQ(0, list_->getDataSources().size());
+}
+
 }