Browse Source

[2907] remove getZoneTableAccessor cache-enable test

Also redo the associated unit tests, and fix some white-space issue.
Paul Selkirk 12 years ago
parent
commit
a292c65c3c
2 changed files with 42 additions and 37 deletions
  1. 2 6
      src/lib/datasrc/client_list.cc
  2. 40 31
      src/lib/datasrc/tests/client_list_unittest.cc

+ 2 - 6
src/lib/datasrc/client_list.cc

@@ -416,12 +416,8 @@ ConfigurableClientList::getZoneTableAccessor(const std::string& datasrc_name,
         }
 
         const internal::CacheConfig* config(info.getCacheConfig());
-        if (!config->isEnabled() && datasrc_name.empty()) {
-            continue;
-	}
-
-	// If caching is disabled for the named data source, this will
-	// return an accessor to an effectivley empty table.
+        // If caching is disabled for the named data source, this will
+        // return an accessor to an effectivley empty table.
         return (boost::shared_ptr<const ZoneTableAccessor>
                 (new internal::ZoneTableAccessorCache(*config)));
     }

+ 40 - 31
src/lib/datasrc/tests/client_list_unittest.cc

@@ -254,7 +254,7 @@ public:
     }
     ConfigurableClientList::CacheStatus doReload(
         const Name& origin, const string& datasrc_name = "");
-    void accessorIterate(boost::shared_ptr<const ZoneTableAccessor>accessor,
+    void accessorIterate(boost::shared_ptr<const ZoneTableAccessor> accessor,
         int numZones, const string& zoneName);
 
     const RRClass rrclass_;
@@ -1130,11 +1130,15 @@ TEST_F(ListTest, reloadByDataSourceName) {
               doReload(Name("example.org"), "test_type4"));
 }
 
+// This takes the accessor provided by getZoneTableAccessor(), iterates
+// through the table, and verifies that the expected number of zones are
+// present, as well as the named zone.
 void
-ListTest::accessorIterate(boost::shared_ptr<const ZoneTableAccessor>accessor,
+ListTest::accessorIterate(boost::shared_ptr<const ZoneTableAccessor> accessor,
                           int numZones, const string& zoneName="")
 {
     // Confirm basic iterator behavior.
+    ASSERT_TRUE(accessor);
     ZoneTableAccessor::IteratorPtr it = accessor->getIterator();
     ASSERT_TRUE(it);
     if (numZones > 0) {
@@ -1143,15 +1147,19 @@ ListTest::accessorIterate(boost::shared_ptr<const ZoneTableAccessor>accessor,
         // Iterator does not guarantee ordering, so we look for the target
         // name anywhere in the table.
         bool found = false;
-        for (int i = 0; i < numZones; ++i) {
+        int i = 0;
+        while (!it->isLast()) {
+            ++i;
             if (Name(zoneName) == it->getCurrent().origin) {
                 found = true;
             }
             it->next();
         }
         EXPECT_TRUE(found);
+        EXPECT_EQ(i, numZones);
+    } else {
+        EXPECT_TRUE(it->isLast());
     }
-    EXPECT_TRUE(it->isLast());
 }
 
 TEST_F(ListTest, zoneTableAccessor) {
@@ -1161,55 +1169,56 @@ TEST_F(ListTest, zoneTableAccessor) {
     // null pointer treated as false
     EXPECT_FALSE(list_->getZoneTableAccessor("", true));
 
-    // empty list
+    // empty list; expect it to return an empty list
     list_->configure(config_elem_, true);
-    EXPECT_FALSE(list_->getZoneTableAccessor("", true));
-
-    // allow_cache = false
-    list_->configure(config_elem_zones_, false);
-    EXPECT_FALSE(list_->getZoneTableAccessor("", true));
-    EXPECT_FALSE(list_->getZoneTableAccessor("type1", true));
-
-    // allow_cache = true, use_cache = false
-    list_->configure(config_elem_zones_, true);
-    EXPECT_THROW(list_->getZoneTableAccessor("", false), isc::NotImplemented);
-    EXPECT_THROW(list_->getZoneTableAccessor("type1", false),
-                 isc::NotImplemented);
+    boost::shared_ptr<const ZoneTableAccessor>
+        z(list_->getZoneTableAccessor("", true));
+    accessorIterate(z, 0);
 
     const ConstElementPtr elem2(Element::fromJSON("["
         "{"
         "   \"type\": \"type1\","
-        "   \"cache-enable\": false,"
-        "   \"cache-zones\": [\"example.org\"],"
-        "   \"params\": [\"example.org\"]"
-        "},"
-        "{"
-        "   \"type\": \"type2\","
         "   \"cache-enable\": true,"
         "   \"cache-zones\": [\"example.com\"],"
         "   \"params\": [\"example.com\"]"
         "},"
         "{"
+        "   \"type\": \"type2\","
+        "   \"cache-enable\": false,"
+        "   \"params\": [\"example.org\"]"
+        "},"
+        "{"
         "   \"type\": \"type3\","
         "   \"cache-enable\": true,"
         "   \"cache-zones\": [\"example.net\", \"example.info\"],"
         "   \"params\": [\"example.net\", \"example.info\"]"
         "}]"));
+
+    // allow_cache = false
+    // ask for a non-existent zone table, expect null
+    list_->configure(elem2, false);
+    EXPECT_FALSE(list_->getZoneTableAccessor("bogus", true));
+    // ask for any zone table, expect an empty list
+    z = list_->getZoneTableAccessor("", true);
+    accessorIterate(z, 0);
+
+    // allow_cache = true, use_cache = false
     list_->configure(elem2, true);
+    EXPECT_THROW(list_->getZoneTableAccessor("", false), isc::NotImplemented);
+    EXPECT_THROW(list_->getZoneTableAccessor("type1", false),
+                 isc::NotImplemented);
 
     // datasrc not found, returns NULL pointer
-    boost::shared_ptr<const ZoneTableAccessor>
-            z(list_->getZoneTableAccessor("bogus", true));
-    EXPECT_FALSE(z);
-
-    // datasrc has cache disabled, returns accessor to empty list
-    z = list_->getZoneTableAccessor("type1", true);
-    accessorIterate(z, 0);
+    EXPECT_FALSE(list_->getZoneTableAccessor("bogus", true));
 
-    // return first enabled datasrc
+    // return first datasrc
     z = list_->getZoneTableAccessor("", true);
     accessorIterate(z, 1, "example.com");
 
+    // datasrc has cache disabled, returns accessor to empty list
+    z = list_->getZoneTableAccessor("type2", true);
+    accessorIterate(z, 0);
+
     // search by name
     z = list_->getZoneTableAccessor("type3", true);
     accessorIterate(z, 2, "example.net");