BIND 10 trac2376, updated. 4464e2f16486fbabb5c0a0ddf1e98f60b97d2c94 [2384] Hide the callbacks inside the class
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Nov 9 13:55:24 UTC 2012
The branch, trac2376 has been updated
via 4464e2f16486fbabb5c0a0ddf1e98f60b97d2c94 (commit)
via 7f2b26ced954b545c910cc812788155ed9d5bf91 (commit)
from 737fb65557c0473043a4e626e8b00b6fefbf2364 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 4464e2f16486fbabb5c0a0ddf1e98f60b97d2c94
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Nov 9 14:53:28 2012 +0100
[2384] Hide the callbacks inside the class
They are set in constructor and used through methods. This is a trick to
make them not modifiable after the construction.
Update existing tests accordingly. Also add new tests for the
constructor and the methods.
commit 7f2b26ced954b545c910cc812788155ed9d5bf91
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Nov 9 13:55:53 2012 +0100
[2384] Extract the callbacks to separate header file
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/loader_context.cc | 7 +-
src/lib/datasrc/tests/loader_context_test.cc | 6 +-
src/lib/dns/Makefile.am | 2 +-
.../dns/{master_loader.h => loader_callbacks.h} | 69 ++++++++++--------
src/lib/dns/master_loader.h | 51 +------------
src/lib/dns/tests/Makefile.am | 1 +
src/lib/dns/tests/loader_callbacks_test.cc | 76 ++++++++++++++++++++
7 files changed, 124 insertions(+), 88 deletions(-)
copy src/lib/dns/{master_loader.h => loader_callbacks.h} (66%)
create mode 100644 src/lib/dns/tests/loader_callbacks_test.cc
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/loader_context.cc b/src/lib/datasrc/loader_context.cc
index bce0039..21c4d48 100644
--- a/src/lib/datasrc/loader_context.cc
+++ b/src/lib/datasrc/loader_context.cc
@@ -22,13 +22,12 @@ namespace isc {
namespace datasrc {
LoaderContext::LoaderContext(ZoneUpdater& updater) :
+ callbacks_(boost::bind(&LoaderContext::handleError, this, _1, _2, _3, _4),
+ boost::bind(&LoaderContext::handleWarning, this, _1, _2, _3,
+ _4)),
updater_(updater),
ok_(true)
{
- callbacks_.error = boost::bind(&LoaderContext::handleError, this, _1, _2,
- _3, _4);
- callbacks_.warning = boost::bind(&LoaderContext::handleWarning, this, _1,
- _2, _3, _4);
}
void
diff --git a/src/lib/datasrc/tests/loader_context_test.cc b/src/lib/datasrc/tests/loader_context_test.cc
index 4ef9b24..7166dfb 100644
--- a/src/lib/datasrc/tests/loader_context_test.cc
+++ b/src/lib/datasrc/tests/loader_context_test.cc
@@ -86,17 +86,13 @@ protected:
TEST_F(LoaderContextTest, constructor) {
// The status is OK, nothing broke yet
EXPECT_TRUE(context_.ok());
- // Get the callbacks and check both are non-empty
- const isc::dns::LoaderCallbacks& callbacks(context_.getCallbacks());
- EXPECT_FALSE(callbacks.warning.empty());
- EXPECT_FALSE(callbacks.error.empty());
}
// 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(LoaderContextTest, callbacks) {
- const isc::dns::LoaderCallbacks& callbacks(context_.getCallbacks());
+ isc::dns::LoaderCallbacks& callbacks(context_.getCallbacks());
EXPECT_NO_THROW(callbacks.warning("No source", 1, 1, "Just for fun"));
// The warning does not hurt the OK mode.
EXPECT_TRUE(context_.ok());
diff --git a/src/lib/dns/Makefile.am b/src/lib/dns/Makefile.am
index 4349e8d..6cf292c 100644
--- a/src/lib/dns/Makefile.am
+++ b/src/lib/dns/Makefile.am
@@ -96,7 +96,7 @@ libb10_dns___la_SOURCES += exceptions.h exceptions.cc
libb10_dns___la_SOURCES += labelsequence.h labelsequence.cc
libb10_dns___la_SOURCES += masterload.h masterload.cc
libb10_dns___la_SOURCES += master_lexer.h master_lexer.cc
-libb10_dns___la_SOURCES += master_loader.h
+libb10_dns___la_SOURCES += master_loader.h loader_callbacks.h
libb10_dns___la_SOURCES += message.h message.cc
libb10_dns___la_SOURCES += messagerenderer.h messagerenderer.cc
libb10_dns___la_SOURCES += name.h name.cc
diff --git a/src/lib/dns/loader_callbacks.h b/src/lib/dns/loader_callbacks.h
new file mode 100644
index 0000000..2933e31
--- /dev/null
+++ b/src/lib/dns/loader_callbacks.h
@@ -0,0 +1,109 @@
+// 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 LOADER_CALLBACKS_H
+#define LOADER_CALLBACKS_H
+
+#include <exceptions/exceptions.h>
+
+#include <string>
+#include <boost/function.hpp>
+
+namespace isc {
+namespace dns {
+
+/// \brief Set of callbacks for a loader.
+///
+/// This holds a set of callbacks by which a loader (such as MasterLoader)
+/// can report errors and other unusual conditions.
+///
+/// All the callbacks must be set.
+class LoaderCallbacks {
+public:
+ /// \brief Type of one callback
+ ///
+ /// 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 source_byte Position of the problem, counted in number of bytes
+ /// 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,
+ size_t source_byte,
+ const std::string& reason)> Callback;
+
+ /// \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.
+ LoaderCallbacks(const Callback& error, const Callback& 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,
+ size_t source_byte, const std::string& reason)
+ {
+ error_(source_name, source_line, source_byte, 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 error 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,
+ size_t source_byte, const std::string& reason)
+ {
+ warning_(source_name, source_line, source_byte, reason);
+ }
+
+private:
+ Callback error_, warning_;
+};
+
+}
+}
+
+#endif // LOADER_CALLBACKS_H
diff --git a/src/lib/dns/master_loader.h b/src/lib/dns/master_loader.h
index 497bd80..946853b 100644
--- a/src/lib/dns/master_loader.h
+++ b/src/lib/dns/master_loader.h
@@ -15,61 +15,12 @@
#ifndef MASTER_LOADER_H
#define MASTER_LOADER_H
+#include <dns/loader_callbacks.h>
#include <dns/rrset.h>
-#include <string>
-#include <boost/function.hpp>
-
namespace isc {
namespace dns {
-/// \brief Set of callbacks for a loader.
-///
-/// This holds a set of callbacks by which a loader (such as MasterLoader)
-/// can report errors and other unusual conditions.
-///
-/// All the callbacks must be set.
-struct LoaderCallbacks {
- /// \brief Type of one callback
- ///
- /// 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 source_byte Position of the problem, counted in number of bytes
- /// 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,
- size_t source_byte,
- const std::string& reason)> Callback;
-
- /// \brief 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.
- ///
- /// If the caller of the loader wants to abort, it is possible to throw
- /// from the callback, which aborts the load.
- Callback error;
-
- /// \brief 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.
- ///
- /// 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.
- Callback warning;
-};
-
/// \brief Abstract base class used as destination for data loaded by
/// MasterLoader.
///
diff --git a/src/lib/dns/tests/Makefile.am b/src/lib/dns/tests/Makefile.am
index 6df0e62..214f55b 100644
--- a/src/lib/dns/tests/Makefile.am
+++ b/src/lib/dns/tests/Makefile.am
@@ -69,6 +69,7 @@ run_unittests_SOURCES += tsigkey_unittest.cc
run_unittests_SOURCES += tsigrecord_unittest.cc
run_unittests_SOURCES += character_string_unittest.cc
run_unittests_SOURCES += run_unittests.cc
+run_unittests_SOURCES += loader_callbacks_test.cc
run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
# We shouldn't need to include BOTAN_LIBS here, but there
# is one test system where the path for GTEST_LDFLAGS contains
diff --git a/src/lib/dns/tests/loader_callbacks_test.cc b/src/lib/dns/tests/loader_callbacks_test.cc
new file mode 100644
index 0000000..bfea869
--- /dev/null
+++ b/src/lib/dns/tests/loader_callbacks_test.cc
@@ -0,0 +1,76 @@
+// 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/loader_callbacks.h>
+
+#include <exceptions/exceptions.h>
+
+#include <gtest/gtest.h>
+#include <boost/bind.hpp>
+
+namespace {
+
+using std::string;
+using namespace isc::dns;
+
+class LoaderCallbacksTest : public ::testing::Test {
+protected:
+ LoaderCallbacksTest() :
+ called_(false),
+ error_(boost::bind(&LoaderCallbacksTest::checkCallback, this, true, _1,
+ _2, _3, _4)),
+ warning_(boost::bind(&LoaderCallbacksTest::checkCallback, this, false,
+ _1, _2, _3, _4)),
+ callbacks_(error_, warning_)
+ {}
+
+ void checkCallback(bool error, const string& source, size_t line,
+ size_t byte, const string& reason)
+ {
+ called_ = true;
+ last_was_error_ = error;
+ EXPECT_EQ("source", source);
+ EXPECT_EQ(1, line);
+ EXPECT_EQ(2, byte);
+ EXPECT_EQ("reason", reason);
+ }
+ bool last_was_error_;
+ bool called_;
+ const LoaderCallbacks::Callback error_, warning_;
+ LoaderCallbacks callbacks_;
+};
+
+// Check the constructor rejects empty callbacks, but accepts non-empty ones
+TEST_F(LoaderCallbacksTest, constructor) {
+ EXPECT_THROW(LoaderCallbacks(LoaderCallbacks::Callback(), warning_),
+ isc::InvalidParameter);
+ EXPECT_THROW(LoaderCallbacks(error_, LoaderCallbacks::Callback()),
+ isc::InvalidParameter);
+ EXPECT_NO_THROW(LoaderCallbacks(error_, warning_));
+}
+
+// Call the callbacks
+TEST_F(LoaderCallbacksTest, call) {
+ callbacks_.error("source", 1, 2, "reason");
+ EXPECT_TRUE(last_was_error_);
+ EXPECT_TRUE(called_);
+
+ called_ = false;
+
+ callbacks_.warning("source", 1, 2, "reason");
+ EXPECT_FALSE(last_was_error_);
+ EXPECT_TRUE(called_);
+}
+
+}
More information about the bind10-changes
mailing list