[2833] pass allow_cache to CacheConfig

src/lib/datasrc/Makefile.am

@@ -39,7 +39,7 @@ libb10_datasrc_la_SOURCES += master_loader_callbacks.h
 libb10_datasrc_la_SOURCES += master_loader_callbacks.cc
 libb10_datasrc_la_SOURCES += rrset_collection_base.h rrset_collection_base.cc
 libb10_datasrc_la_SOURCES += zone_loader.h zone_loader.cc
-libb10_datasrc_la_SOURCES += zone_table_config.h zone_table_config.cc
+libb10_datasrc_la_SOURCES += cache_config.h cache_config.cc
 nodist_libb10_datasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 libb10_datasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
 

src/lib/datasrc/zone_table_config.cc

@@ -12,7 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <datasrc/zone_table_config.h>
+#include <datasrc/cache_config.h>
 #include <datasrc/client.h>
 #include <datasrc/memory/load_action.h>
 #include <dns/name.h>
@@ -48,8 +48,9 @@ getSegmentTypeFromConf(const Element& conf) {
 
 CacheConfig::CacheConfig(const std::string& datasrc_type,
                          const DataSourceClient* datasrc_client,
-                         const Element& datasrc_conf) :
-    enabled_(getEnabledFromConf(datasrc_conf)),
+                         const Element& datasrc_conf,
+                         bool allowed) :
+    enabled_(allowed && getEnabledFromConf(datasrc_conf)),
     segment_type_(getSegmentTypeFromConf(datasrc_conf)),
     datasrc_client_(datasrc_client)
 {
@@ -62,6 +63,7 @@ CacheConfig::CacheConfig(const std::string& datasrc_type,
             isc_throw(isc::InvalidParameter,
                       "data source client is given for MasterFiles");
         }
+
         if (!enabled_) {
             isc_throw(CacheConfigError,
                       "The cache must be enabled for the MasterFiles type");

src/lib/datasrc/zone_table_config.h

@@ -58,7 +58,8 @@ class CacheConfig {
 public:
     CacheConfig(const std::string& datasrc_type,
                 const DataSourceClient* datasrc_client,
-                const data::Element& datasrc_conf);
+                const data::Element& datasrc_conf,
+                bool allowed);
 
     /// \brief Return if the cache is enabled.
     ///

src/lib/datasrc/client_list.cc

@@ -17,7 +17,7 @@
 #include <datasrc/exceptions.h>
 #include <datasrc/client.h>
 #include <datasrc/factory.h>
-#include <datasrc/zone_table_config.h>
+#include <datasrc/cache_config.h>
 #include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_writer.h>
@@ -120,20 +120,21 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                           << name);
             }
 
-            if (type == "MasterFiles" && !allow_cache) { // XXX type specific
-                // We're not going to load these zones. Issue warnings about
-                // it.
-                LOG_WARN(logger, DATASRC_LIST_NOT_CACHED).
-                            arg(name).arg(rrclass_);
-                continue;
-            }
             // Create a client for the underling data source via factory.
             // (If it's our internal type of data source, this is essentially
             // no-op).
             const DataSourcePair dsrc_pair = getDataSourceClient(type,
                                                                  paramConf);
+            if (!allow_cache && !dsrc_pair.first) {
+                // We're not going to load these zones. Issue warnings about
+                // it.
+                LOG_WARN(logger, DATASRC_LIST_NOT_CACHED).
+                    arg(name).arg(rrclass_);
+                continue;
+            }
 
-            internal::CacheConfig cache_conf(type, dsrc_pair.first, *dconf);
+            internal::CacheConfig cache_conf(type, dsrc_pair.first, *dconf,
+                                             allow_cache);
             shared_ptr<ZoneTableSegment> ztable_segment;
             if (cache_conf.isEnabled()) {
                 ztable_segment.reset(ZoneTableSegment::create(
@@ -142,7 +143,6 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
             }
             new_data_sources.push_back(DataSourceInfo(dsrc_pair.first,
                                                       dsrc_pair.second,
-                                                      allow_cache &&
                                                       cache_conf.isEnabled(),
                                                       rrclass_, ztable_segment,
                                                       name));

src/lib/datasrc/tests/Makefile.am

@@ -59,7 +59,7 @@ run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += client_list_unittest.cc
 run_unittests_SOURCES += master_loader_callbacks_test.cc
 run_unittests_SOURCES += zone_loader_unittest.cc
-run_unittests_SOURCES += zone_table_config_unittest.cc
+run_unittests_SOURCES += cache_config_unittest.cc
 
 # We need the actual module implementation in the tests (they are not part
 # of libdatasrc)

src/lib/datasrc/tests/zone_table_config_unittest.cc

@@ -12,7 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <datasrc/zone_table_config.h>
+#include <datasrc/cache_config.h>
 #include <datasrc/tests/mock_client.h>
 
 #include <cc/data.h>
@@ -55,7 +55,7 @@ protected:
 
 TEST_F(CacheConfigTest, constructMasterFiles) {
     // A simple case: configuring a MasterFiles table with a single zone
-    const CacheConfig cache_conf("MasterFiles", 0, *master_config_);
+    const CacheConfig cache_conf("MasterFiles", 0, *master_config_, true);
     // getZoneConfig() returns a map containing exactly one entry
     // corresponding to the root zone information in the configuration.
     EXPECT_EQ(1, cache_conf.getZoneConfig().size());
@@ -73,41 +73,42 @@ TEST_F(CacheConfigTest, constructMasterFiles) {
                           " \"example.org\": \"file2\","
                           " \"example.info\": \"file3\"}"
                           "}"));
-    EXPECT_EQ(3, CacheConfig("MasterFiles", 0, *config_elem_multi).
+    EXPECT_EQ(3, CacheConfig("MasterFiles", 0, *config_elem_multi, true).
               getZoneConfig().size());
 
     // A bit unusual, but acceptable case: empty parameters, so no zones.
     EXPECT_TRUE(CacheConfig("MasterFiles", 0,
                             *Element::fromJSON("{\"cache-enable\": true,"
-                                               " \"params\": {}}")).
+                                               " \"params\": {}}"), true).
                 getZoneConfig().empty());
 }
 
 TEST_F(CacheConfigTest, badConstructMasterFiles) {
     // no "params"
     EXPECT_THROW(CacheConfig("MasterFiles", 0,
-                             *Element::fromJSON("{\"cache-enable\": true}")),
+                             *Element::fromJSON("{\"cache-enable\": true}"),
+                             true),
                  isc::data::TypeError);
 
     // no "cache-enable"
     EXPECT_THROW(CacheConfig("MasterFiles", 0,
-                             *Element::fromJSON("{\"params\": {}}")),
+                             *Element::fromJSON("{\"params\": {}}"), true),
                  CacheConfigError);
     // cache disabled for MasterFiles
     EXPECT_THROW(CacheConfig("MasterFiles", 0,
                              *Element::fromJSON("{\"cache-enable\": false,"
-                                                " \"params\": {}}")),
+                                                " \"params\": {}}"), true),
                  CacheConfigError);
     // type error for cache-enable
     EXPECT_THROW(CacheConfig("MasterFiles", 0,
                              *Element::fromJSON("{\"cache-enable\": 1,"
-                                                " \"params\": {}}")),
+                                                " \"params\": {}}"), true),
                  isc::data::TypeError);
 
     // "params" is not a map
     EXPECT_THROW(CacheConfig("MasterFiles", 0,
                              *Element::fromJSON("{\"cache-enable\": true,"
-                                                " \"params\": []}")),
+                                                " \"params\": []}"), true),
                  isc::data::TypeError);
 
     // bogus zone name
@@ -115,20 +116,20 @@ TEST_F(CacheConfigTest, badConstructMasterFiles) {
                                          "{\"cache-enable\": true,"
                                          " \"params\": "
                                          "{\"bad..name\": \"file1\"}}"));
-    EXPECT_THROW(CacheConfig("MasterFiles", 0, *bad_config),
+    EXPECT_THROW(CacheConfig("MasterFiles", 0, *bad_config, true),
                  isc::dns::EmptyLabel);
 
     // file name is not a string
     const ConstElementPtr bad_config2(Element::fromJSON(
                                           "{\"cache-enable\": true,"
                                           " \"params\": {\".\": 1}}"));
-    EXPECT_THROW(CacheConfig("MasterFiles", 0, *bad_config2),
+    EXPECT_THROW(CacheConfig("MasterFiles", 0, *bad_config2, true),
                  isc::data::TypeError);
 
     // Specify data source client (must be null for MasterFiles)
     EXPECT_THROW(CacheConfig("MasterFiles", &mock_client_,
                              *Element::fromJSON("{\"cache-enable\": true,"
-                                                " \"params\": {}}")),
+                                                " \"params\": {}}"), true),
                  isc::InvalidParameter);
 }
 
@@ -136,7 +137,7 @@ TEST_F(CacheConfigTest, constructWithMock) {
     // Performing equivalent set of tests as constructMasterFiles
 
     // Configure with a single zone.
-    const CacheConfig cache_conf("mock", &mock_client_, *mock_config_);
+    const CacheConfig cache_conf("mock", &mock_client_, *mock_config_, true);
     EXPECT_EQ(1, cache_conf.getZoneConfig().size());
     EXPECT_EQ(Name::ROOT_NAME(), cache_conf.getZoneConfig().begin()->first);
     EXPECT_EQ("", cache_conf.getZoneConfig().begin()->second);
@@ -148,46 +149,49 @@ TEST_F(CacheConfigTest, constructWithMock) {
                           " \"cache-zones\": "
                           "[\"example.com\", \"example.org\",\"example.info\"]"
                           "}"));
-    EXPECT_EQ(3, CacheConfig("mock", &mock_client_, *config_elem_multi).
+    EXPECT_EQ(3, CacheConfig("mock", &mock_client_, *config_elem_multi, true).
               getZoneConfig().size());
 
     // Empty
     EXPECT_TRUE(CacheConfig("mock", &mock_client_,
                             *Element::fromJSON("{\"cache-enable\": true,"
-                                               " \"cache-zones\": []}")).
+                                               " \"cache-zones\": []}"), true).
                 getZoneConfig().empty());
 
     // disabled.  value of cache-zones are ignored.
     const ConstElementPtr config_elem_disabled(
         Element::fromJSON("{\"cache-enable\": false,"
                           " \"cache-zones\": [\"example.com\"]}"));
-    EXPECT_TRUE(CacheConfig("mock", &mock_client_, *config_elem_disabled).
+    EXPECT_TRUE(CacheConfig("mock", &mock_client_, *config_elem_disabled, true).
                 getZoneConfig().empty());
 }
 
 TEST_F(CacheConfigTest, badConstructWithMock) {
     // no "cache-zones" (may become valid in future, but for now "notimp")
     EXPECT_THROW(CacheConfig("mock", &mock_client_,
-                             *Element::fromJSON("{\"cache-enable\": true}")),
+                             *Element::fromJSON("{\"cache-enable\": true}"),
+                             true),
                  isc::NotImplemented);
 
     // "cache-zones" is not a list
     EXPECT_THROW(CacheConfig("mock", &mock_client_,
                              *Element::fromJSON("{\"cache-enable\": true,"
-                                                " \"cache-zones\": {}}")),
+                                                " \"cache-zones\": {}}"),
+                             true),
                  isc::data::TypeError);
 
     // "cache-zone" entry is not a string
     EXPECT_THROW(CacheConfig("mock", &mock_client_,
                              *Element::fromJSON("{\"cache-enable\": true,"
-                                                " \"cache-zones\": [1]}")),
+                                                " \"cache-zones\": [1]}"),
+                             true),
                  isc::data::TypeError);
 
     // bogus zone name
     const ConstElementPtr bad_config(Element::fromJSON(
                                          "{\"cache-enable\": true,"
                                          " \"cache-zones\": [\"bad..\"]}"));
-    EXPECT_THROW(CacheConfig("mock", &mock_client_, *bad_config),
+    EXPECT_THROW(CacheConfig("mock", &mock_client_, *bad_config, true),
                  isc::dns::EmptyLabel);
 
     // duplicate zone name
@@ -195,11 +199,11 @@ TEST_F(CacheConfigTest, badConstructWithMock) {
                                          "{\"cache-enable\": true,"
                                          " \"cache-zones\": "
                                          " [\"example\", \"example\"]}"));
-    EXPECT_THROW(CacheConfig("mock", &mock_client_, *dup_config),
+    EXPECT_THROW(CacheConfig("mock", &mock_client_, *dup_config, true),
                  isc::InvalidParameter);
 
     // datasrc is null
-    EXPECT_THROW(CacheConfig("mock", 0, *mock_config_),
+    EXPECT_THROW(CacheConfig("mock", 0, *mock_config_, true),
                  isc::InvalidParameter);
 }
 
@@ -207,20 +211,20 @@ TEST_F(CacheConfigTest, getSegmentType) {
     // Default type
     EXPECT_EQ("local",
               CacheConfig("MasterFiles", 0,
-                          *master_config_).getSegmentType());
+                          *master_config_, true).getSegmentType());
 
     // If we explicitly configure it, that value should be used.
     ConstElementPtr config(Element::fromJSON("{\"cache-enable\": true,"
                                              " \"cache-type\": \"mapped\","
                                              " \"params\": {}}" ));
     EXPECT_EQ("mapped",
-              CacheConfig("MasterFiles", 0, *config).getSegmentType());
+              CacheConfig("MasterFiles", 0, *config, true).getSegmentType());
 
     // Wrong types: should be rejected at construction time
     ConstElementPtr badconfig(Element::fromJSON("{\"cache-enable\": true,"
                                                 " \"cache-type\": 1,"
                                                 " \"params\": {}}"));
-    EXPECT_THROW(CacheConfig("MasterFiles", 0, *badconfig),
+    EXPECT_THROW(CacheConfig("MasterFiles", 0, *badconfig, true),
                  isc::data::TypeError);
 }