[2905] extend ZoneWriter so it can catch load error and install empty zone.

src/lib/datasrc/client.h

@@ -132,8 +132,8 @@ public:
     /// \brief A helper structure to represent the search result of
     /// \c find().
     ///
-    /// This is a straightforward pair of the result code and a share pointer
-    /// to the found zone to represent the result of \c find().
+    /// This is a straightforward tuple of the result code/flags and a shared
+    /// pointer to the found zone to represent the result of \c find().
     /// We use this in order to avoid overloading the return value for both
     /// the result code ("success" or "not found") and the found object,
     /// i.e., avoid using \c NULL to mean "not found", etc.
@@ -146,10 +146,13 @@ public:
     /// variables.
     struct FindResult {
         FindResult(result::Result param_code,
-                   const ZoneFinderPtr param_zone_finder) :
-            code(param_code), zone_finder(param_zone_finder)
+                   const ZoneFinderPtr param_zone_finder,
+                   result::ResultFlags param_flags = result::FLAGS_DEFAULT) :
+            code(param_code), flags(param_flags),
+            zone_finder(param_zone_finder)
         {}
         const result::Result code;
+        const result::ResultFlags flags;
         const ZoneFinderPtr zone_finder;
     };
 

src/lib/datasrc/client_list.cc

@@ -159,8 +159,8 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                         cache_conf->getLoadAction(rrclass_, zname);
                     // in this loop this should be always true
                     assert(load_action);
-                    memory::ZoneWriter writer(zt_segment,
-                        load_action, zname, rrclass_);
+                    memory::ZoneWriter writer(zt_segment, load_action, zname,
+                                              rrclass_, false);
                     writer.load();
                     writer.install();
                     writer.cleanup();
@@ -357,7 +357,7 @@ ConfigurableClientList::getCachedZoneWriter(const Name& name,
                                ZoneWriterPtr(
                                    new memory::ZoneWriter(
                                        *info.ztable_segment_,
-                                       load_action, name, rrclass_))));
+                                       load_action, name, rrclass_, false))));
     }
 
     // We can't find the specified zone.  If a specific data source was

src/lib/datasrc/memory/zone_writer.cc

@@ -16,6 +16,8 @@
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table_segment.h>
 
+#include <datasrc/exceptions.h>
+
 #include <memory>
 
 using std::auto_ptr;
@@ -27,13 +29,15 @@ namespace memory {
 ZoneWriter::ZoneWriter(ZoneTableSegment& segment,
                        const LoadAction& load_action,
                        const dns::Name& origin,
-                       const dns::RRClass& rrclass) :
+                       const dns::RRClass& rrclass,
+                       bool throw_on_load_error) :
     segment_(segment),
     load_action_(load_action),
     origin_(origin),
     rrclass_(rrclass),
     zone_data_(NULL),
-    state_(ZW_UNUSED)
+    state_(ZW_UNUSED),
+    catch_load_error_(throw_on_load_error)
 {
     if (!segment.isWritable()) {
         isc_throw(isc::InvalidOperation,
@@ -53,12 +57,20 @@ ZoneWriter::load() {
         isc_throw(isc::InvalidOperation, "Trying to load twice");
     }
 
-    zone_data_ = load_action_(segment_.getMemorySegment());
-    segment_.resetHeader();
+    try {
+        zone_data_ = load_action_(segment_.getMemorySegment());
+        segment_.resetHeader();
 
-    if (!zone_data_) {
-        // Bug inside load_action_.
-        isc_throw(isc::InvalidOperation, "No data returned from load action");
+        if (!zone_data_) {
+            // Bug inside load_action_.
+            isc_throw(isc::InvalidOperation,
+                      "No data returned from load action");
+        }
+    } catch (const ZoneLoaderException& ex) {
+        if (!catch_load_error_) {
+            throw;
+        }
+        segment_.resetHeader();
     }
 
     state_ = ZW_LOADED;
@@ -70,6 +82,10 @@ ZoneWriter::install() {
         isc_throw(isc::InvalidOperation, "No data to install");
     }
 
+    // Check the internal integrity assumption: we should have non NULL
+    // zone data or we've allowed load error to create an empty zone.
+    assert(zone_data_ || catch_load_error_);
+
     // FIXME: This retry is currently untested, as there seems to be no
     // reasonable way to create a zone table segment with non-local memory
     // segment. Once there is, we should provide the test.
@@ -79,10 +95,10 @@ ZoneWriter::install() {
             if (table == NULL) {
                 isc_throw(isc::Unexpected, "No zone table present");
             }
-            const ZoneTable::AddResult result(table->addZone(
-                                                  segment_.getMemorySegment(),
-                                                  rrclass_, origin_,
-                                                  zone_data_));
+            const ZoneTable::AddResult result(
+                zone_data_ ? table->addZone(segment_.getMemorySegment(),
+                                            rrclass_, origin_, zone_data_) :
+                table->addEmptyZone(segment_.getMemorySegment(), origin_));
             state_ = ZW_INSTALLED;
             zone_data_ = result.zone_data;
         } catch (const isc::util::MemorySegmentGrown&) {}

src/lib/datasrc/memory/zone_writer.h

@@ -42,15 +42,27 @@ class ZoneWriter {
 public:
     /// \brief Constructor
     ///
+    /// If \c catch_load_error is set to true, the \c load() method will
+    /// internally catch load related errors reported as a DataSourceError
+    /// exception, and subsequent \c install() method will add a special
+    /// empty zone to the zone table segment.  If it's set to false, \c load()
+    /// will simply propagate the exception.  This parameter would normally
+    /// be set to false as it's not desirable to install a broken zone;
+    /// however, it would be better to be set to true at the initial loading
+    /// so the zone table recognizes the existence of the zone (and being
+    /// aware that it's broken).
+    ///
     /// \throw isc::InvalidOperation if \c segment is read-only.
     ///
     /// \param segment The zone table segment to store the zone into.
     /// \param load_action The callback used to load data.
     /// \param name The name of the zone.
     /// \param rrclass The class of the zone.
+    /// \param catch_load_error true if loading errors are to be caught
+    /// internally; false otherwise.
     ZoneWriter(ZoneTableSegment& segment,
                const LoadAction& load_action, const dns::Name& name,
-               const dns::RRClass& rrclass);
+               const dns::RRClass& rrclass, bool catch_load_error);
 
     /// \brief Destructor.
     ~ZoneWriter();
@@ -71,6 +83,8 @@ public:
     /// \note After successful load(), you have to call cleanup() some time
     ///     later.
     /// \throw isc::InvalidOperation if called second time.
+    /// \throw DataSourceError load related error (not thrown if constructed
+    /// with catch_load_error being false).
     void load();
 
     /// \brief Put the changes to effect.
@@ -78,7 +92,11 @@ public:
     /// This replaces the old version of zone with the one previously prepared
     /// by load(). It takes ownership of the old zone data, if any.
     ///
-    /// You may call it only after successful load() and at most once.
+    /// You may call it only after successful load() and at most once.  It
+    /// includes the case the writer is constructed with catch_load_error
+    /// being true and load() encountered and caught a DataSourceError
+    /// exception.  In this case this method installs a special empty zone
+    /// to the table.
     ///
     /// The operation is expected to be fast and is meant to be used inside
     /// a critical section.
@@ -112,10 +130,15 @@ private:
         ZW_CLEANED
     };
     State state_;
+    const bool catch_load_error_;
 };
 
 }
 }
 }
 
-#endif
+#endif  // MEM_ZONE_WRITER_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/tests/client_list_unittest.cc

@@ -171,7 +171,7 @@ public:
                 new memory::ZoneWriter(
                     *dsrc_info.ztable_segment_,
                     cache_conf->getLoadAction(rrclass_, zone),
-                    zone, rrclass_));
+                    zone, rrclass_, false));
             writer->load();
             writer->install();
             writer->cleanup(); // not absolutely necessary, but just in case

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

@@ -41,7 +41,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);
+                              zname, zclass, false);
     writer.load();
     writer.install();
     writer.cleanup();
@@ -72,7 +72,7 @@ loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
                   const dns::RRClass& zclass, ZoneIterator& iterator)
 {
     memory::ZoneWriter writer(zt_sgmt, IteratorLoader(zclass, zname, iterator),
-                              zname, zclass);
+                              zname, zclass, false);
     writer.load();
     writer.install();
     writer.cleanup();

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

@@ -15,6 +15,7 @@
 #include <datasrc/memory/zone_writer.h>
 #include <datasrc/memory/zone_table_segment_local.h>
 #include <datasrc/memory/zone_data.h>
+#include <datasrc/exceptions.h>
 
 #include <dns/rrclass.h>
 #include <dns/name.h>
@@ -31,6 +32,7 @@ using boost::scoped_ptr;
 using boost::bind;
 using isc::dns::RRClass;
 using isc::dns::Name;
+using isc::datasrc::ZoneLoaderException;
 using namespace isc::datasrc::memory;
 using namespace isc::datasrc::memory::test;
 
@@ -60,9 +62,10 @@ protected:
         writer_(new
             ZoneWriter(*segment_,
                        bind(&ZoneWriterTest::loadAction, this, _1),
-                       Name("example.org"), RRClass::IN())),
+                       Name("example.org"), RRClass::IN(), false)),
         load_called_(false),
         load_throw_(false),
+        load_loader_throw_(false),
         load_null_(false),
         load_data_(false)
     {}
@@ -75,6 +78,7 @@ protected:
     scoped_ptr<ZoneWriter> writer_;
     bool load_called_;
     bool load_throw_;
+    bool load_loader_throw_;
     bool load_null_;
     bool load_data_;
 public:
@@ -87,6 +91,9 @@ public:
         if (load_throw_) {
             throw TestException();
         }
+        if (load_loader_throw_) {
+            isc_throw(ZoneLoaderException, "faked loader exception");
+        }
 
         if (load_null_) {
             // Be nasty to the caller and return NULL, which is forbidden
@@ -130,7 +137,7 @@ TEST_F(ZoneWriterTest, constructForReadOnlySegment) {
     ReadOnlySegment ztable_segment(RRClass::IN(), mem_sgmt);
     EXPECT_THROW(ZoneWriter(ztable_segment,
                             bind(&ZoneWriterTest::loadAction, this, _1),
-                            Name("example.org"), RRClass::IN()),
+                            Name("example.org"), RRClass::IN(), false),
                  isc::InvalidOperation);
 }
 
@@ -243,6 +250,30 @@ TEST_F(ZoneWriterTest, loadThrows) {
     EXPECT_NO_THROW(writer_->cleanup());
 }
 
+// Emulate the situation where load() throws loader error.
+TEST_F(ZoneWriterTest, loadLoaderException) {
+    // By default, the exception is propagated.
+    load_loader_throw_ = true;
+    EXPECT_THROW(writer_->load(), ZoneLoaderException);
+
+    // If we specify allowing load error, load() will succeed and install()
+    // adds an empty zone.
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), true));
+    writer_->load();
+    writer_->install();
+    writer_->cleanup();
+
+    // Check an empty zone has been really installed.
+    using namespace isc::datasrc::result;
+    const ZoneTable* ztable = segment_->getHeader().getTable();
+    ASSERT_TRUE(ztable);
+    const ZoneTable::FindResult result = ztable->findZone(Name("example.org"));
+    EXPECT_EQ(SUCCESS, result.code);
+    EXPECT_EQ(ZONE_EMPTY, result.flags);
+}
+
 // Check the strong exception guarantee - if it throws, nothing happened
 // to the content.
 TEST_F(ZoneWriterTest, retry) {

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -79,7 +79,7 @@ createInMemoryClient(RRClass zclass, const Name& zname) {
         ZoneTableSegment::create(zclass, cache_conf.getSegmentType()));
     memory::ZoneWriter writer(*ztable_segment,
                               cache_conf.getLoadAction(zclass, zname),
-                              zname, zclass);
+                              zname, zclass, false);
     writer.load();
     writer.install();
     writer.cleanup();

src/lib/datasrc/tests/zone_loader_unittest.cc

@@ -320,8 +320,9 @@ protected:
         if (filename) {
             boost::scoped_ptr<memory::ZoneWriter> writer(
                 new memory::ZoneWriter(*ztable_segment_,
-                                       cache_conf.getLoadAction(rrclass_, zone),
-                                       zone, rrclass_));
+                                       cache_conf.getLoadAction(rrclass_,
+                                                                zone),
+                                       zone, rrclass_, false));
             writer->load();
             writer->install();
             writer->cleanup();