[2834] fixed a regression: missing return; added a test that would cover it.

and also updated doc to clarify some result codes of getCachedZoneWriter()

src/lib/datasrc/client_list.cc

@@ -343,7 +343,7 @@ ConfigurableClientList::getCachedZoneWriter(const Name& name) {
     const memory::LoadAction load_action =
         result.info->getCacheConfig()->getLoadAction(rrclass_, name);
     if (!load_action) {
-        ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr());
+        return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr()));
     }
     return (ZoneWriterPair(ZONE_SUCCESS,
                            ZoneWriterPtr(

src/lib/datasrc/client_list.h

@@ -342,8 +342,10 @@ public:
     /// \brief Result of the reload() method.
     enum ReloadResult {
         CACHE_DISABLED,     ///< The cache is not enabled in this list.
-        ZONE_NOT_CACHED,    ///< Zone is served directly, not from cache.
-        ZONE_NOT_FOUND,     ///< Zone does not exist or not cached.
+        ZONE_NOT_CACHED,    ///< Zone is served directly, not from cache
+	                    ///  (including the case cache is disabled for
+	                    ///  the specific data source).
+        ZONE_NOT_FOUND,     ///< Zone does not exist in this list.
         ZONE_SUCCESS        ///< The zone was successfully reloaded or
                             ///  the writer provided.
     };

src/lib/datasrc/tests/client_list_unittest.cc

@@ -134,8 +134,9 @@ public:
     }
 
     // Install a "fake" cached zone using a temporary underlying data source
-    // client.
-    void prepareCache(size_t index, const Name& zone) {
+    // client.  If 'enabled' is set to false, emulate a disabled cache, in
+    // which case there will be no data in memory.
+    void prepareCache(size_t index, const Name& zone, bool enabled = true) {
         ConfigurableClientList::DataSourceInfo& dsrc_info =
                 list_->getDataSources()[index];
         MockDataSourceClient* mock_client =
@@ -149,7 +150,9 @@ public:
         // Build new cache config to load the specified zone, and replace
         // the data source info with the new config.
         ConstElementPtr cache_conf_elem =
-            Element::fromJSON("{\"type\": \"mock\", \"cache-enable\": true,"
+            Element::fromJSON("{\"type\": \"mock\","
+                              " \"cache-enable\": " +
+                              string(enabled ? "true," : "false,") +
                               " \"cache-zones\": "
                               "   [\"" + zone.toText() + "\"]}");
         boost::shared_ptr<internal::CacheConfig> cache_conf(
@@ -161,13 +164,15 @@ public:
             cache_conf, rrclass_, dsrc_info.name_);
 
         // Load the data into the zone table.
-        boost::scoped_ptr<memory::ZoneWriter> writer(
-            dsrc_info.ztable_segment_->getZoneWriter(
-                cache_conf->getLoadAction(rrclass_, zone),
-                zone, rrclass_));
-        writer->load();
-        writer->install();
-        writer->cleanup(); // not absolutely necessary, but just in case
+        if (enabled) {
+            boost::scoped_ptr<memory::ZoneWriter> writer(
+                dsrc_info.ztable_segment_->getZoneWriter(
+                    cache_conf->getLoadAction(rrclass_, zone),
+                    zone, rrclass_));
+            writer->load();
+            writer->install();
+            writer->cleanup(); // not absolutely necessary, but just in case
+        }
 
         // On completion of load revert to the previous state of underlying
         // data source.
@@ -891,7 +896,7 @@ TYPED_TEST(ReloadTest, reloadSuccess) {
 }
 
 // The cache is not enabled. The load should be rejected.
-TYPED_TEST(ReloadTest, reloadNotEnabled) {
+TYPED_TEST(ReloadTest, reloadNotAllowed) {
     this->list_->configure(this->config_elem_zones_, false);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the thing.
@@ -910,6 +915,17 @@ TYPED_TEST(ReloadTest, reloadNotEnabled) {
                        RRType::A())->code);
 }
 
+// Similar to the previous case, but the cache is disabled in config.
+TYPED_TEST(ReloadTest, reloadNotEnabled) {
+    this->list_->configure(this->config_elem_zones_, true);
+    const Name name("example.org");
+    // We put the cache, actually disabling it.
+    this->prepareCache(0, name, false);
+    // In this case we cannot really look up due to the limitation of
+    // the mock implementation.  We only check reload fails.
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_CACHED, this->doReload(name));
+}
+
 // Test several cases when the zone does not exist
 TYPED_TEST(ReloadTest, reloadNoSuchZone) {
     this->list_->configure(this->config_elem_zones_, true);