Merge #2376

Conflicts:
	src/lib/datasrc/datasrc_messages.mes

src/lib/datasrc/Makefile.am

@@ -36,6 +36,8 @@ libb10_datasrc_la_SOURCES += database.h database.cc
 libb10_datasrc_la_SOURCES += factory.h factory.cc
 libb10_datasrc_la_SOURCES += client_list.h client_list.cc
 libb10_datasrc_la_SOURCES += memory_datasrc.h memory_datasrc.cc
+libb10_datasrc_la_SOURCES += master_loader_callbacks.h
+libb10_datasrc_la_SOURCES += master_loader_callbacks.cc
 nodist_libb10_datasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 libb10_datasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
 

src/lib/datasrc/datasrc_messages.mes

@@ -315,6 +315,18 @@ An error was found in the zone data when it was being loaded from
 another data source. The zone was not loaded. The specific error is
 shown in the message, and should be addressed.
 
+% DATASRC_MASTER_LOAD_ERROR %1:%2: Zone '%3/%4' contains error: %5
+There's an error in the given master file. The zone won't be loaded for
+this reason. Parsing might follow, so you might get further errors and
+warnings to fix everything at once. But in case the error is serious enough,
+the parser might just give up or get confused and generate false errors
+afterwards.
+
+% DATASRC_MASTER_LOAD_WARN %1:%2: Zone '%3/%4' has a potential problem: %5
+There's something suspicious in the master file. This is a warning only.
+It may be a problem or it may be harmless, but it should be checked.
+This problem does not stop the zone from being loaded.
+
 % DATASRC_MEM_ADD_RRSET adding RRset '%1/%2' into zone '%3'
 Debug information. An RRset is being added to the in-memory data source.
 

src/lib/datasrc/master_loader_callbacks.cc

+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/master_loader_callbacks.h>
+#include <datasrc/zone.h>
+#include <datasrc/logger.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
+
+#include <string>
+#include <boost/bind.hpp>
+
+namespace isc {
+namespace datasrc {
+
+namespace {
+
+void
+logError(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
+         bool* ok, const std::string& source, size_t line,
+         const std::string& reason)
+{
+    LOG_ERROR(logger, DATASRC_MASTER_LOAD_ERROR).arg(source).arg(line).
+        arg(name).arg(rrclass).arg(reason);
+    if (ok != NULL) {
+        *ok = false;
+    }
+}
+
+void
+logWarning(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
+         const std::string& source, size_t line, const std::string& reason)
+{
+    LOG_WARN(logger, DATASRC_MASTER_LOAD_WARN).arg(source).arg(line).
+        arg(name).arg(rrclass).arg(reason);
+}
+
+}
+
+isc::dns::MasterLoaderCallbacks
+createMasterLoaderCallbacks(const isc::dns::Name& name,
+                            const isc::dns::RRClass& rrclass, bool* ok)
+{
+    return (isc::dns::MasterLoaderCallbacks(boost::bind(&logError, name,
+                                                        rrclass, ok, _1, _2,
+                                                        _3),
+                                            boost::bind(&logWarning, name,
+                                                        rrclass, _1, _2, _3)));
+}
+
+isc::dns::AddRRsetCallback
+createMasterLoaderAddCallback(ZoneUpdater& updater) {
+    return (boost::bind(&ZoneUpdater::addRRset, &updater,
+                        // The callback provides a shared pointer, we
+                        // need the object. This bind unpacks the object.
+                        boost::bind(&isc::dns::RRsetPtr::operator*, _1)));
+}
+
+}
+}

src/lib/datasrc/master_loader_callbacks.h

+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DATASRC_MASTER_LOADER_CALLBACKS_H
+#define DATASRC_MASTER_LOADER_CALLBACKS_H
+
+#include <dns/master_loader_callbacks.h>
+
+namespace isc {
+namespace dns {
+class Name;
+class RRClass;
+}
+namespace datasrc {
+
+class ZoneUpdater;
+
+/// \brief Create issue callbacks for MasterLoader
+///
+/// This will create set of callbacks for the MasterLoader that
+/// will be used to report any issues found in the zone data.
+///
+/// \param name Name of the zone. Used in logging.
+/// \param rrclass The class of the zone. Used in logging.
+/// \param ok If this is non-NULL and there are any errors during
+///     the loading, it is set to false. Otherwise, it is untouched.
+/// \return Set of callbacks to be passed to the master loader.
+/// \throw std::bad_alloc when allocation fails.
+isc::dns::MasterLoaderCallbacks
+createMasterLoaderCallbacks(const isc::dns::Name& name,
+                            const isc::dns::RRClass& rrclass, bool* ok);
+
+/// \brief Create a callback for MasterLoader to add RRsets.
+///
+/// This creates a callback that can be used by the MasterLoader to add
+/// loaded RRsets into a zone updater.
+///
+/// The zone updater should be opened in the replace mode no changes should
+/// have been done to it yet (but it is not checked). It is not commited
+/// automatically and it is up to the caller to commit the changes (or not).
+/// It must not be destroyed for the whole time of loading.
+///
+/// The function is mostly straightforward packing of the updater.addRRset
+/// into a boost::function, it is defined explicitly due to small technical
+/// annoyences around boost::bind application, so it can be reused.
+///
+/// \param updater The zone updater to use.
+/// \return The callback to be passed to MasterLoader.
+/// \throw std::bad_alloc when allocation fails.
+isc::dns::AddRRsetCallback
+createMasterLoaderAddCallback(ZoneUpdater& updater);
+
+}
+}
+
+#endif

src/lib/datasrc/tests/Makefile.am

@@ -59,6 +59,7 @@ run_unittests_SOURCES += zonetable_unittest.cc
 run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += client_list_unittest.cc
+run_unittests_SOURCES += master_loader_callbacks_test.cc
 
 # We need the actual module implementation in the tests (they are not part
 # of libdatasrc)

src/lib/datasrc/tests/master_loader_callbacks_test.cc

+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/master_loader_callbacks.h>
+#include <datasrc/zone.h>
+
+#include <dns/rrset.h>
+#include <dns/rrclass.h>
+#include <dns/rrttl.h>
+
+#include <exceptions/exceptions.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/lexical_cast.hpp>
+
+#include <list>
+#include <string>
+
+using namespace isc::datasrc;
+
+namespace {
+
+// An updater for the tests. Most of the virtual methods throw
+// NotImplemented, as they are not used in the tests.
+class MockUpdater : public ZoneUpdater {
+public:
+    // We do the adding in this test. We currently only check these are
+    // the correct ones, according to a predefined set in a list.
+    virtual void addRRset(const isc::dns::AbstractRRset& rrset) {
+        ASSERT_FALSE(expected_rrsets_.empty());
+        // In our tests, pointer equality is enough.
+        EXPECT_EQ(expected_rrsets_.front().get(), &rrset);
+        // And remove this RRset, as it has been used.
+        expected_rrsets_.pop_front();
+    }
+    // The unused but required methods
+    virtual ZoneFinder& getFinder() {
+        isc_throw(isc::NotImplemented, "Not to be called in this test");
+    }
+    virtual void deleteRRset(const isc::dns::AbstractRRset&) {
+        isc_throw(isc::NotImplemented, "Not to be called in this test");
+    }
+    virtual void commit() {
+        isc_throw(isc::NotImplemented, "Not to be called in this test");
+    }
+    // The RRsets that are expected to appear through addRRset.
+    std::list<isc::dns::RRsetPtr> expected_rrsets_;
+};
+
+class MasterLoaderCallbackTest : public ::testing::Test {
+protected:
+    MasterLoaderCallbackTest() :
+        ok_(true),
+        callbacks_(createMasterLoaderCallbacks(isc::dns::Name("example.org"),
+                                               isc::dns::RRClass::IN(), &ok_))
+    {}
+    // Generate a new RRset, put it to the updater and return it.
+    isc::dns::RRsetPtr generateRRset() {
+        const isc::dns::RRsetPtr
+            result(new isc::dns::RRset(isc::dns::Name("example.org"),
+                                       isc::dns::RRClass::IN(),
+                                       isc::dns::RRType::A(),
+                                       isc::dns::RRTTL(3600)));
+        updater_.expected_rrsets_.push_back(result);
+        return (result);
+    }
+    // An updater to be passed to the context
+    MockUpdater updater_;
+    // Is the loading OK?
+    bool ok_;
+    // The tested context
+    isc::dns::MasterLoaderCallbacks callbacks_;
+};
+
+// Check it doesn't crash if we don't provide the OK
+TEST_F(MasterLoaderCallbackTest, noOkProvided) {
+    createMasterLoaderCallbacks(isc::dns::Name("example.org"),
+                                isc::dns::RRClass::IN(), NULL).
+        error("No source", 1, "No reason");
+}
+
+// Check the callbacks can be called, don't crash and the error one switches
+// to non-OK mode. This, however, does not stop anybody from calling more
+// callbacks.
+TEST_F(MasterLoaderCallbackTest, callbacks) {
+    EXPECT_NO_THROW(callbacks_.warning("No source", 1, "Just for fun"));
+    // The warning does not hurt the OK mode.
+    EXPECT_TRUE(ok_);
+    // Now the error
+    EXPECT_NO_THROW(callbacks_.error("No source", 2, "Some error"));
+    // The OK is turned off once there's at least one error
+    EXPECT_FALSE(ok_);
+
+    // Not being OK does not hurt that much, we can still call the callbacks
+    EXPECT_NO_THROW(callbacks_.warning("No source", 3, "Just for fun"));
+    // The OK is not reset back to true
+    EXPECT_FALSE(ok_);
+    EXPECT_NO_THROW(callbacks_.error("No source", 4, "Some error"));
+}
+
+// Try adding some RRsets.
+TEST_F(MasterLoaderCallbackTest, addRRset) {
+    isc::dns::AddRRsetCallback
+        callback(createMasterLoaderAddCallback(updater_));
+    // Put some of them in.
+    EXPECT_NO_THROW(callback(generateRRset()));
+    EXPECT_NO_THROW(callback(generateRRset()));
+    // They all get pushed there right away, so there are none in the queue
+    EXPECT_TRUE(updater_.expected_rrsets_.empty());
+}
+
+}

src/lib/dns/Makefile.am

@@ -119,6 +119,7 @@ libb10_dns___la_SOURCES += tsigerror.h tsigerror.cc
 libb10_dns___la_SOURCES += tsigkey.h tsigkey.cc
 libb10_dns___la_SOURCES += tsigrecord.h tsigrecord.cc
 libb10_dns___la_SOURCES += character_string.h character_string.cc
+libb10_dns___la_SOURCES += master_loader_callbacks.h
 libb10_dns___la_SOURCES += rdata/generic/detail/nsec_bitmap.h
 libb10_dns___la_SOURCES += rdata/generic/detail/nsec_bitmap.cc
 libb10_dns___la_SOURCES += rdata/generic/detail/nsec3param_common.cc

src/lib/dns/master_loader_callbacks.h

+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef MASTER_LOADER_CALLBACKS_H
+#define MASTER_LOADER_CALLBACKS_H
+
+#include <exceptions/exceptions.h>
+
+#include <string>
+#include <boost/function.hpp>
+#include <boost/shared_ptr.hpp>
+
+namespace isc {
+namespace dns {
+
+class AbstractRRset;
+typedef boost::shared_ptr<AbstractRRset> RRsetPtr;
+
+/// \brief Type of callback to add a RRset.
+///
+/// This type of callback is used by the loader to report another loaded
+/// RRset. The RRset is no longer preserved by the loader and is fully
+/// owned by the callback.
+///
+/// \param RRset The rrset to add. It does not contain the accompanying
+///     RRSIG (if the zone is signed), they are reported with separate
+///     calls to the callback.
+typedef boost::function<void(const RRsetPtr& rrset)> AddRRsetCallback;
+
+/// \brief Set of issue callbacks for a loader.
+///
+/// This holds a set of callbacks by which a loader (such as MasterLoader)
+/// can report loaded RRsets, errors and other unusual conditions.
+///
+/// All the callbacks must be set.
+class MasterLoaderCallbacks {
+public:
+    /// \brief Type of one callback to report problems.
+    ///
+    /// This is the type of one callback used to report an unusual
+    /// condition or error.
+    ///
+    /// \param source_name The name of the source where the problem happened.
+    ///     This is usually a file name.
+    /// \param source_line Position of the problem, counted in lines from the
+    ///     beginning of the source.
+    /// \param reason Human readable description of what happened.
+    typedef boost::function<void(const std::string& source_name,
+                                 size_t source_line,
+                                 const std::string& reason)> IssueCallback;
+
+    /// \brief Constructor
+    ///
+    /// Initializes the callbacks.
+    ///
+    /// \param error The error callback to use.
+    /// \param warning The warning callback to use.
+    /// \throw isc::InvalidParameter if any of the callbacks is empty.
+    MasterLoaderCallbacks(const IssueCallback& error,
+                          const IssueCallback& warning) :
+        error_(error),
+        warning_(warning)
+    {
+        if (error_.empty() || warning_.empty()) {
+            isc_throw(isc::InvalidParameter,
+                      "Empty function passed as callback");
+        }
+    }
+
+    /// \brief Call callback for serious errors
+    ///
+    /// This is called whenever there's a serious problem which makes the data
+    /// being loaded unusable. Further processing may or may not happen after
+    /// this (for example to detect further errors), but the data should not
+    /// be used.
+    ///
+    /// It calls whatever was passed to the error parameter to the constructor.
+    ///
+    /// If the caller of the loader wants to abort, it is possible to throw
+    /// from the callback, which aborts the load.
+    void error(const std::string& source_name, size_t source_line,
+               const std::string& reason)
+    {
+        error_(source_name, source_line, reason);
+    }
+
+    /// \brief Call callback for potential problems
+    ///
+    /// This is called whenever a minor problem is discovered. This might mean
+    /// the data is completely OK, it just looks suspicious.
+    ///
+    /// It calls whatever was passed to the warn parameter to the constructor.
+    ///
+    /// The loading will continue after the callback. If the caller wants to
+    /// abort (which is probably not a very good idea, since warnings
+    /// may be false positives), it is possible to throw from inside the
+    /// callback.
+    void warning(const std::string& source_name, size_t source_line,
+                 const std::string& reason)
+    {
+        warning_(source_name, source_line, reason);
+    }
+
+private:
+    IssueCallback error_, warning_;
+};
+
+}
+}
+
+#endif // LOADER_CALLBACKS_H

src/lib/dns/tests/Makefile.am

@@ -71,6 +71,7 @@ run_unittests_SOURCES += tsigerror_unittest.cc
 run_unittests_SOURCES += tsigkey_unittest.cc
 run_unittests_SOURCES += tsigrecord_unittest.cc
 run_unittests_SOURCES += character_string_unittest.cc
+run_unittests_SOURCES += master_loader_callbacks_test.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 # We shouldn't need to include BOTAN_LIBS here, but there

src/lib/dns/tests/master_loader_callbacks_test.cc

+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/master_loader_callbacks.h>
+#include <dns/rrset.h>
+#include <dns/name.h>
+#include <dns/rrttl.h>
+#include <dns/rrclass.h>
+
+#include <exceptions/exceptions.h>
+
+#include <gtest/gtest.h>
+#include <boost/bind.hpp>
+
+namespace {
+
+using std::string;
+using namespace isc::dns;
+
+class MasterLoaderCallbacksTest : public ::testing::Test {
+protected:
+    MasterLoaderCallbacksTest() :
+        issue_called_(false),
+        rrset_(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
+                         RRTTL(3600))),
+        error_(boost::bind(&MasterLoaderCallbacksTest::checkCallback, this,
+                           true, _1, _2, _3)),
+        warning_(boost::bind(&MasterLoaderCallbacksTest::checkCallback, this,
+                             false, _1, _2, _3)),
+        callbacks_(error_, warning_)
+    {}
+
+    void checkCallback(bool error, const string& source, size_t line,
+                       const string& reason)
+    {
+        issue_called_ = true;
+        last_was_error_ = error;
+        EXPECT_EQ("source", source);
+        EXPECT_EQ(1, line);
+        EXPECT_EQ("reason", reason);
+    }
+    bool last_was_error_;
+    bool issue_called_;
+    const RRsetPtr rrset_;
+    const MasterLoaderCallbacks::IssueCallback error_, warning_;
+    MasterLoaderCallbacks callbacks_;
+};
+
+// Check the constructor rejects empty callbacks, but accepts non-empty ones
+TEST_F(MasterLoaderCallbacksTest, constructor) {
+    EXPECT_THROW(MasterLoaderCallbacks(MasterLoaderCallbacks::IssueCallback(),
+                                       warning_), isc::InvalidParameter);
+    EXPECT_THROW(MasterLoaderCallbacks(error_,
+                                       MasterLoaderCallbacks::IssueCallback()),
+                 isc::InvalidParameter);
+    EXPECT_NO_THROW(MasterLoaderCallbacks(error_, warning_));
+}
+
+// Call the issue callbacks
+TEST_F(MasterLoaderCallbacksTest, issueCall) {
+    callbacks_.error("source", 1, "reason");
+    EXPECT_TRUE(last_was_error_);
+    EXPECT_TRUE(issue_called_);
+
+    issue_called_ = false;
+
+    callbacks_.warning("source", 1, "reason");
+    EXPECT_FALSE(last_was_error_);
+    EXPECT_TRUE(issue_called_);
+}
+
+}