Browse Source

[2833] added begin/end to CacheConfig instead of getZoneConfig.

that would be better abstraction.  also remove getLoadAction for now as
we won't implement it in this task.
JINMEI Tatuya 12 years ago
parent
commit
517ca63bd5

+ 2 - 2
src/lib/datasrc/cache_config.cc

@@ -59,7 +59,7 @@ CacheConfig::CacheConfig(const std::string& datasrc_type,
         params.reset(new NullElement());
     }
     if (datasrc_type == "MasterFiles") {
-        if (datasrc_client) {
+        if (datasrc_client_) {
             isc_throw(isc::InvalidParameter,
                       "data source client is given for MasterFiles");
         }
@@ -79,7 +79,7 @@ CacheConfig::CacheConfig(const std::string& datasrc_type,
             zone_config_[dns::Name(it->first)] = it->second->stringValue();
         }
     } else {
-        if (!datasrc_client) {
+        if (!datasrc_client_) {
             isc_throw(isc::InvalidParameter,
                       "data source client is missing for data source type: "
                       << datasrc_type);

+ 8 - 8
src/lib/datasrc/cache_config.h

@@ -21,6 +21,8 @@
 #include <cc/data.h>
 #include <datasrc/memory/load_action.h>
 
+#include <boost/noncopyable.hpp>
+
 #include <map>
 #include <string>
 
@@ -54,7 +56,7 @@ public:
 /// defined in an "internal" namespace, and isn't expected to be used by
 /// other classes or user applications.  Likewise, this file is not expected
 /// to be installed with other publicly usable header files.
-class CacheConfig {
+class CacheConfig : boost::noncopyable {
 public:
     CacheConfig(const std::string& datasrc_type,
                 const DataSourceClient* datasrc_client,
@@ -71,25 +73,23 @@ public:
     /// \throw None
     const std::string& getSegmentType() const { return (segment_type_); }
 
-    /// Return corresponding \c LoadAction for the given name of zone.
-    /// It would return a different functor depending on the details of the
-    /// underlying data source.
-    memory::LoadAction getLoadAction(const dns::Name& zone_name) const;
-
     /// This allows ZoneTableSegment to iterate over all zones to be loaded
     /// in to memory.  In this initial implementation we directly give
     /// read-only access to the underlying map to minimize the diff, but
     /// it's not clean in terms of encapsulation and performance (eventually
     /// we may have to look up in the underlying data source to get the list
     /// of zones, in which case constructing a map can be very expensive).
-    typedef std::map<dns::Name, std::string> Zones;
-    const Zones& getZoneConfig() const { return (zone_config_); }
+    typedef std::map<dns::Name, std::string>::const_iterator ConstZoneIterator;
+    ConstZoneIterator begin() const { return (zone_config_.begin()); }
+    ConstZoneIterator end() const { return (zone_config_.end()); }
 
 private:
     const bool enabled_; // if the use of in-memory zone table is enabled
     const std::string segment_type_;
     // client of underlying data source, will be NULL for MasterFile datasrc
     const DataSourceClient* datasrc_client_;
+
+    typedef std::map<dns::Name, std::string> Zones;
     Zones zone_config_;
 };
 }

+ 22 - 21
src/lib/datasrc/tests/cache_config_unittest.cc

@@ -20,6 +20,8 @@
 
 #include <gtest/gtest.h>
 
+#include <iterator>             // for std::distance
+
 using namespace isc::datasrc;
 using namespace isc::data;
 using namespace isc::dns;
@@ -53,15 +55,15 @@ protected:
     const ConstElementPtr mock_config_; // valid config for MasterFiles
 };
 
+size_t
+countZones(const CacheConfig& cache_config) {
+    return (std::distance(cache_config.begin(), cache_config.end())); 
+}
+
 TEST_F(CacheConfigTest, constructMasterFiles) {
     // A simple case: configuring a MasterFiles table with a single zone
     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());
-    EXPECT_EQ(Name::ROOT_NAME(), cache_conf.getZoneConfig().begin()->first);
-    EXPECT_EQ(TEST_DATA_DIR "/root.zone",
-              cache_conf.getZoneConfig().begin()->second);
+    EXPECT_EQ(1, countZones(cache_conf));
 
     // With multiple zones.  There shouldn't be anything special, so we
     // only check the size of getZoneConfig.  Note that the constructor
@@ -73,14 +75,14 @@ TEST_F(CacheConfigTest, constructMasterFiles) {
                           " \"example.org\": \"file2\","
                           " \"example.info\": \"file3\"}"
                           "}"));
-    EXPECT_EQ(3, CacheConfig("MasterFiles", 0, *config_elem_multi, true).
-              getZoneConfig().size());
+    EXPECT_EQ(3, countZones(CacheConfig("MasterFiles", 0, *config_elem_multi,
+                                        true)));
 
     // A bit unusual, but acceptable case: empty parameters, so no zones.
-    EXPECT_TRUE(CacheConfig("MasterFiles", 0,
-                            *Element::fromJSON("{\"cache-enable\": true,"
-                                               " \"params\": {}}"), true).
-                getZoneConfig().empty());
+    EXPECT_EQ(0, countZones(
+                  CacheConfig("MasterFiles", 0,
+                              *Element::fromJSON("{\"cache-enable\": true,"
+                                                 " \"params\": {}}"), true)));
 }
 
 TEST_F(CacheConfigTest, badConstructMasterFiles) {
@@ -143,9 +145,7 @@ TEST_F(CacheConfigTest, constructWithMock) {
 
     // Configure with a single zone.
     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);
+    EXPECT_EQ(1, countZones(cache_conf));
     EXPECT_TRUE(cache_conf.isEnabled());
 
     // Configure with multiple zones.
@@ -154,14 +154,15 @@ TEST_F(CacheConfigTest, constructWithMock) {
                           " \"cache-zones\": "
                           "[\"example.com\", \"example.org\",\"example.info\"]"
                           "}"));
-    EXPECT_EQ(3, CacheConfig("mock", &mock_client_, *config_elem_multi, true).
-              getZoneConfig().size());
+    EXPECT_EQ(3, countZones(CacheConfig("mock", &mock_client_,
+                                        *config_elem_multi, true)));
 
     // Empty
-    EXPECT_TRUE(CacheConfig("mock", &mock_client_,
-                            *Element::fromJSON("{\"cache-enable\": true,"
-                                               " \"cache-zones\": []}"), true).
-                getZoneConfig().empty());
+    EXPECT_EQ(0, countZones(
+                  CacheConfig("mock", &mock_client_,
+                              *Element::fromJSON("{\"cache-enable\": true,"
+                                                 " \"cache-zones\": []}"),
+                              true)));
 
     // disabled.  value of cache-zones are ignored.
     const ConstElementPtr config_elem_disabled(