[2905] handle the empty-zone case in InMemoryClient.

src/lib/datasrc/client.h

@@ -187,8 +187,12 @@ public:
     ///   - \c result::PARTIALMATCH: A zone whose origin is a
     ///   super domain of \c name is found (but there is no exact match)
     ///   - \c result::NOTFOUND: For all other cases.
+    /// - \c flags: usually FLAGS_DEFAULT, but if the zone data are not
+    ///   available (possibly because an error was detected at load time)
+    ///   the ZONE_EMPTY flag is set.
     /// - \c zone_finder: Pointer to a \c ZoneFinder object for the found zone
-    /// if one is found; otherwise \c NULL.
+    /// if one is found and is not empty (flags doesn't have ZONE_EMPTY);
+    /// otherwise \c NULL.
     ///
     /// A specific derived version of this method may throw an exception.
     /// This interface does not specify which exceptions can happen (at least
@@ -218,6 +222,9 @@ public:
     /// \throw Others Possibly implementation specific exceptions (it is
     /// not fixed if a concrete implementation of this method can throw
     /// anything else.)
+    /// \throw EmptyZone the zone is supposed to exist in the data source,
+    /// but its content is not available.  This generally means there's an
+    /// error in the content.
     ///
     /// \param name The name of zone apex to be traversed. It doesn't do
     ///     nearest match as findZone.

src/lib/datasrc/exceptions.h

@@ -56,6 +56,17 @@ public:
         DataSourceError(file, line, what) {}
 };
 
+/// \brief An error indicating a zone is recognized but its content is not
+/// available.
+///
+/// This generally indicates a condition that there's an error in the zone
+/// content and it's not successfully loaded.
+class EmptyZone : public DataSourceError {
+public:
+    EmptyZone(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what) {}
+};
+
 /// Base class for a number of exceptions that are thrown while working
 /// with zones.
 struct ZoneException : public Exception {

src/lib/datasrc/memory/memory_client.cc

@@ -69,11 +69,11 @@ InMemoryClient::findZone(const isc::dns::Name& zone_name) const {
     const ZoneTable::FindResult result(zone_table->findZone(zone_name));
 
     ZoneFinderPtr finder;
-    if (result.code != result::NOTFOUND) {
+    if (result.code != result::NOTFOUND && result.zone_data) {
         finder.reset(new InMemoryZoneFinder(*result.zone_data, getClass()));
     }
 
-    return (DataSourceClient::FindResult(result.code, finder));
+    return (DataSourceClient::FindResult(result.code, finder, result.flags));
 }
 
 const ZoneData*
@@ -242,7 +242,12 @@ InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
     const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
     const ZoneTable::FindResult result(zone_table->findZone(name));
     if (result.code != result::SUCCESS) {
-        isc_throw(NoSuchZone, "No such zone: " + name.toText());
+        isc_throw(NoSuchZone, "no such zone for in-memory iterator: "
+                  << name.toText());
+    }
+    if (!result.zone_data) {
+        isc_throw(EmptyZone, "empty zone for in-memory iterator: "
+                  << name.toText());
     }
 
     return (ZoneIteratorPtr(new MemoryIterator(

src/lib/datasrc/memory/zone_table.cc

@@ -162,9 +162,10 @@ ZoneTable::findZone(const Name& name) const {
     // Can Not Happen (remember, NOTFOUND is handled).  node should also have
     // data because the tree is constructed in the way empty nodes would
     // be "invisible" for find().
-    assert(node != NULL && node->getData());
+    assert(node != NULL);
 
     const ZoneData* zone_data = node->getData();
+    assert(zone_data);
     const result::ResultFlags flags =
         zone_data->isEmpty() ? result::ZONE_EMPTY : result::FLAGS_DEFAULT;
     return (FindResult(my_result, zone_data->isEmpty() ? NULL : zone_data,

src/lib/datasrc/tests/memory/memory_client_unittest.cc

@@ -694,6 +694,15 @@ TEST_F(MemoryClientTest, getIterator) {
     EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected);
 }
 
+TEST_F(MemoryClientTest, getIteratorForEmptyZone) {
+    // trying to load a broken zone (zone file not existent).  It's internally
+    // stored an empty zone.
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/no-such-file.zone", true);
+    // Then getIterator will result in an exception.
+    EXPECT_THROW(client_->getIterator(Name("example.org")), EmptyZone);
+}
+
 TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
     loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
                       TEST_DATA_DIR "/example.org-multiple.zone");
@@ -791,6 +800,35 @@ TEST_F(MemoryClientTest, addEmptyRRsetThrows) {
     // Teardown checks for memory segment leaks
 }
 
+TEST_F(MemoryClientTest, findEmptyZone) {
+    // trying to load a broken zone (zone file not existent).  It's internally
+    // stored an empty zone.
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/no-such-file.zone", true);
+
+    using namespace isc::datasrc::result;
+
+    // findZone() returns the match, with NULL zone finder and the result
+    // flag indicating it's empty.
+    const DataSourceClient::FindResult result =
+        client_->findZone(Name("example.org"));
+    EXPECT_EQ(SUCCESS, result.code);
+    EXPECT_EQ(ZONE_EMPTY, result.flags);
+    EXPECT_FALSE(result.zone_finder);
+
+    // Same for the case of subdomain match
+    const DataSourceClient::FindResult result_sub =
+        client_->findZone(Name("www.example.org"));
+    EXPECT_EQ(PARTIALMATCH, result_sub.code);
+    EXPECT_EQ(ZONE_EMPTY, result_sub.flags);
+    EXPECT_FALSE(result_sub.zone_finder);
+
+    // findZoneData() will simply NULL (this is for testing only anyway,
+    // so any result would be okay as long as it doesn't cause disruption).
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL),
+              client_->findZoneData(Name("example.org")));
+}
+
 TEST_F(MemoryClientTest, findZoneData) {
     loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
                       TEST_DATA_DIR "/example.org-rrsigs.zone");

src/lib/datasrc/tests/memory/zone_loader_util.cc

@@ -33,7 +33,8 @@ namespace test {
 
 void
 loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
-                  const dns::RRClass& zclass, const std::string& zone_file)
+                  const dns::RRClass& zclass, const std::string& zone_file,
+                  bool load_error_ok)
 {
     const isc::datasrc::internal::CacheConfig cache_conf(
         "MasterFiles", NULL, *data::Element::fromJSON(
@@ -41,7 +42,7 @@ loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
             " \"params\": {\"" + zname.toText() + "\": \"" + zone_file +
             "\"}}"), true);
     memory::ZoneWriter writer(zt_sgmt, cache_conf.getLoadAction(zclass, zname),
-                              zname, zclass, false);
+                              zname, zclass, load_error_ok);
     writer.load();
     writer.install();
     writer.cleanup();

src/lib/datasrc/tests/memory/zone_loader_util.h

@@ -33,9 +33,14 @@ namespace test {
 /// This function does nothing special, simply provides a shortcut for commonly
 /// used pattern that would be used in tests with a ZoneTableSegment loading
 /// a zone from file into it.
+///
+/// If the optional load_error_ok parameter is set to true, it will create
+/// an internal empty zone in the table when it encounters a loading error.
+/// Otherwise ZoneLoaderException will be thrown in such cases.
 void
 loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
-                  const dns::RRClass& zclass, const std::string& zone_file);
+                  const dns::RRClass& zclass, const std::string& zone_file,
+                  bool load_error_ok = false);
 
 /// \brief A shortcut utility to load a specified zone into ZoneTableSegment
 /// from a zone iterator.