Parcourir la source

[2905] updated ClientList so the initial loading accepts load errors

...and will treat it as a broken (rather than nonexistent) zone for later
lookups.
JINMEI Tatuya il y a 12 ans
Parent
commit
f4130b0566

+ 3 - 1
src/lib/datasrc/client_list.cc

@@ -159,8 +159,10 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                         cache_conf->getLoadAction(rrclass_, zname);
                     // in this loop this should be always true
                     assert(load_action);
+                    // For the initial load, we'll let the writer handle
+                    // loading error and install an empty zone in the table.
                     memory::ZoneWriter writer(zt_segment, load_action, zname,
-                                              rrclass_, false);
+                                              rrclass_, true);
                     writer.load();
                     writer.install();
                     writer.cleanup();

+ 12 - 3
src/lib/datasrc/client_list.h

@@ -231,20 +231,29 @@ public:
     /// of the searched name is needed. Therefore, the call would look like:
     ///
     /// \code FindResult result(list->find(queried_name));
-    ///   FindResult result(list->find(queried_name));
     ///   if (result.datasrc_) {
-    ///       createTheAnswer(result.finder_);
+    ///       if (result.finder_) {
+    ///           createTheAnswer(result.finder_);
+    ///       } else {
+    ///           // broken zone, handle it accordingly.
+    ///       }
     ///   } else {
     ///       createNotAuthAnswer();
     /// } \endcode
     ///
+    /// Note that it is possible that \c finder_ is NULL while \c datasrc_
+    /// is not.  This happens if the zone is configured to be served from
+    /// the data source but it is broken in some sense and doesn't hold any
+    /// zone data, e.g., when the zone file has an error or the secondary
+    /// zone hasn't been transferred yet.  The caller needs to expect the case
+    /// and handle it accordingly.
+    ///
     /// The other scenario is manipulating zone data (XfrOut, XfrIn, DDNS,
     /// ...). In this case, the finder itself is not so important. However,
     /// we need an exact match (if we want to manipulate zone data, we must
     /// know exactly, which zone we are about to manipulate). Then the call
     ///
     /// \code FindResult result(list->find(zone_name, true, false));
-    ///   FindResult result(list->find(zone_name, true, false));
     ///   if (result.datasrc_) {
     ///       ZoneUpdaterPtr updater(result.datasrc_->getUpdater(zone_name);
     ///       ...

+ 26 - 7
src/lib/datasrc/tests/client_list_unittest.cc

@@ -208,6 +208,18 @@ public:
             EXPECT_EQ(dsrc.get(), result.dsrc_client_);
         }
     }
+
+    // check the result with empty (broken) zones.  Right now this can only
+    // happen for in-memory caches.
+    void emptyResult(const ClientList::FindResult& result, bool exact,
+                     const char* trace_txt)
+    {
+        SCOPED_TRACE(trace_txt);
+        ASSERT_FALSE(result.finder_);
+        EXPECT_EQ(exact, result.exact_match_);
+        EXPECT_TRUE(dynamic_cast<InMemoryClient*>(result.dsrc_client_));
+    }
+
     // Configure the list with multiple data sources, according to
     // some configuration. It uses the index as parameter, to be able to
     // loop through the configurations.
@@ -762,7 +774,7 @@ TEST_F(ListTest, masterFiles) {
               list_->getDataSources()[0].data_src_client_);
 
     // And it can search
-    positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "com",
+    positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "root",
                    true);
 
     // If cache is not enabled, nothing is loaded
@@ -820,7 +832,8 @@ TEST_F(ListTest, names) {
 
 TEST_F(ListTest, BadMasterFile) {
     // Configuration should succeed, and the good zones in the list
-    // below should be loaded. No bad zones should be loaded.
+    // below should be loaded.  Bad zones won't be "loaded" in its usual sense,
+    // but are still recognized with conceptual "empty" data.
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"MasterFiles\","
@@ -856,13 +869,19 @@ TEST_F(ListTest, BadMasterFile) {
 
     positiveResult(list_->find(Name("example.com."), true), ds_[0],
                    Name("example.com."), true, "example.com", true);
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.org."), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("foo.bar"), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.net."), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.edu."), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.info."), true));
+    // Bad cases: should result in "empty zone", whether the match is exact
+    // or partial.
+    emptyResult(list_->find(Name("foo.bar"), true), true, "foo.bar");
+    emptyResult(list_->find(Name("example.net."), true), true, "example.net");
+    emptyResult(list_->find(Name("example.edu."), true), true, "example.edu");
+    emptyResult(list_->find(Name("example.info."), true), true,
+                "example.info");
+    emptyResult(list_->find(Name("www.example.edu."), false), false,
+                "example.edu, partial");
     positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "root",
                    true);
+    // This one simply doesn't exist.
+    EXPECT_TRUE(list_->find(Name("example.org."), true) == negative_result_);
 }
 
 ConfigurableClientList::CacheStatus