[2252] Merge branch 'master' of git://git.bind10.isc.org/bind10 into trac2252

Conflicts:
	tests/lettuce/features/xfrin_notify_handling.feature

ChangeLog

@@ -1,3 +1,107 @@
+606.	[bug]		jinmei
+	b10-xfrout now correctly stops sending notify requests once it
+	receives a valid response.  It previously handled it as if the
+	requests are timed out and resent it a few times in a short
+	period.
+	(Trac #2879, git 4c45f29f28ae766a9f7dc3142859f1d0000284e1)
+
+605.	[bug]		tmark
+	Modified perfdhcp to calculate the times displayed for packet sent 
+	and received as time elapsed since perfdhcp process start time.  
+	Previously these were times since the start of the epoch.
+	However the large numbers involved caused loss of precision
+	in the calculation of the test statistics.
+	(Trac #2785, git e9556924dcd1cf285dc358c47d65ed7c413e02cf)
+
+604.	[func]		marcin
+	libdhcp++: abstracted methods which open sockets and send/receive
+	DHCP4 packets to a separate class. Other classes will be derived
+	from it to implement OS-specific methods of DHCPv4 packets filtering.
+	The primary purpose for this change is to add support for Direct
+	DHCPv4 response to a client which doesn't have an address yet on
+	different OSes.
+	(Trac #991, git 33ffc9a750cd3fb34158ef676aab6b05df0302e2)
+
+603.	[func]		tmark
+	The directory in which the b10-dchp4 and b10-dhcp6 server id files has
+	been changed from the local state directory (set by the "configure"
+	--localstatedir switch) to the "bind10" subdirectory of it. After an
+	upgrade, server id files in the former location will be orphaned and
+	should be manually removed.
+	(Trac #2770, git a622140d411b3f07a68a1451e19df36118a80650)
+
+602.	[bug]		tmark
+	Perdhcp will now exit gracefully if the command line argument for
+	IP version (-4 or -6) does not match the command line argument
+	given for the server. Prior to this perfdhcp would core when given
+	an IP version of -6 but a valid IPv4 address for server.
+	(Trac #2784, git 96b66c0c79dccf9a0206a45916b9b23fe9b94f74)
+
+601.	[bug]*		jinmei, vorner
+	The "delete record" interface of the database based data source
+	was extended so that the parameter includes reversed name in
+	addition to the actual name.  This may help the underlying
+	accessor implementation if reversed names are more convenient
+	for the delete operation.  This was the case for the SQLite3
+	accessor implementation, and it now performs delete operations
+	much faster.  At a higher level, this means IXFR and DDNS Updates
+	to the sqlite3 database are no longer so slow on large zones as
+	they were before.
+	(Trac #2877, git 33bd949ac7288c61ed0a664b7329b50b36d180e5)
+
+600.	[bug]		tmark
+	Changed mysql_lease_mgr to set the SQL mode option to STRICT. This
+	causes mysql it to treat invalid input data as an error. Rather than
+	"successfully" inserting a too large value by truncating it, the
+	insert will fail, and the lease manager will throw an exception.
+	Also, attempts to create a HWAddr (hardware address) object with
+	too long an array of data now throw an exception.
+	(Trac #2387, git cac02e9290600407bd6f3071c6654c1216278616)
+
+599.	[func]		tomek
+	libdhcp++: Pkt6 class is now able to parse and build relayed DHCPv6
+	messages.
+	(Trac #2827, git 29c3f7f4e82d7e85f0f5fb692345fd55092796b4)
+
+bind10-1.0.0beta1 released on April 4, 2013
+
+598.	[func]*		jinmei
+	The separate "static" data source is now deprecated as it can be
+	served in the more generic "MasterFiles" type of data source.
+	This means existing configuration may not work after an update.
+	If "config show data_sources/classes/CH[0]" on bindctl contains a
+	"static" type of data source, you'll need to update it as follows:
+	> config set data_sources/classes/CH[0]/type MasterFiles
+	> config set data_sources/classes/CH[0]/params {"BIND": =>
+	  "<the value of current data_sources/classes/CH[0]/params>"}
+	> config set data_sources/classes/CH[0]/cache-enable true
+	> config commit
+	(Same for CH[1], CH[2], IN[0], etc, if applicable, although it
+	should be very unlikely in practice.  Also note: '=>' above
+	indicates the next line is actually part of the command.  Do
+	not type in this "arrow").
+	(Part of Trac #2833, git 0363b4187fe3c1a148ad424af39e12846610d2d7)
+
+597.	[func]		tmark
+	b10-dhcp6: Added unit tests for handling requests when no
+	IPv6 subnets are configured/defined. Testing these conditions
+	was overlooked during implementation of Trac #2719.
+	(Trac #2721, git ce7f53b2de60e2411483b4aa31c714763a36da64)
+
+596.	[bug]		jinmei
+	Added special handling for the case where b10-auth receives a
+	NOTIFY message, but zonemgr isn't running. Previously this was
+	logged as a communications problem at the ERROR level, resulting
+	in increasing noise when zonemgr is intentionally stopped. Other
+	than the log level there is no change in externally visible
+	behavior.
+	(Trac #2562, git 119eed9938b17cbad3a74c823aa9eddb7cd337c2)
+
+595.	[bug]		tomek
+	All DHCP components now gracefully refuse to handle too short
+	DUIDs and client-id.
+	(Trac #2723, git a043d8ecda6aff57922fe98a33c7c3f6155d5d64)
+
 594.	[func]		muks, pselkirk
 	libdns++: the NSEC, DS, DLV, and AFSDB Rdata classes now use the
 	generic lexer in constructors from text.  This means that the name
@@ -50,7 +154,7 @@
 	statements. Also, the sqlite3-specific log messages have been moved
 	from the general datasource library to the sqlite3 datasource
 	(which also explicitely loads its messages).
-	(Trac 2746, git 1c004d95a8b715500af448683e4a07e9b66ea926)
+	(Trac #2746, git 1c004d95a8b715500af448683e4a07e9b66ea926)
 
 586.	[func]		marcin
 	libdhcp++: Removed unnecesary calls to the function which

doc/guide/bind10-guide.xml

@@ -772,6 +772,16 @@ as a dependency earlier -->
             </listitem>
           </varlistentry>
 
+          <varlistentry>
+            <term>--without-werror</term>
+            <listitem>
+              <simpara>Disable the default use of the
+		<option>-Werror</option> compiler flag so that
+		compiler warnings aren't build failures.
+              </simpara>
+            </listitem>
+          </varlistentry>
+
           </variablelist>
           <note>
             <para>
@@ -2487,8 +2497,8 @@ can use various data source backends.
       <para>
         The configuration is located in data_sources/classes. Each item there
         represents one RR class and a list used to answer queries for that
-        class. The default contains two classes. The CH class contains a static
-        data source &mdash; one that serves things like
+        class. The default contains two classes. The CH class contains a
+        built-in data source &mdash; one that serves things like
         <quote>AUTHORS.BIND.</quote>. The IN class contains single SQLite3
         data source with database file located at
         <filename>/usr/local/var/bind10/zone.sqlite3</filename>.
@@ -2555,7 +2565,7 @@ can use various data source backends.
         </para>
 
         <para>
-          First, let's disable the static data source
+          First, let's disable the built-in data source
           (<quote>VERSION.BIND</quote> and friends). As it is the only
           data source in the CH class, we can remove the whole class.
 
@@ -4100,7 +4110,7 @@ Dhcp4/subnet4	[]	list	(default)
 &gt; <userinput>config commit</userinput>
 </screen>
     Even though the "container" option does not carry any data except
-    sub-options, the "data" field must be explictly set to an empty value.
+    sub-options, the "data" field must be explicitly set to an empty value.
     This is required because in the current version of BIND 10 DHCP, the
     default configuration values are not propagated to the configuration parsers:
     if the "data" is not set the parser will assume that this
@@ -4812,7 +4822,7 @@ should include options from the isc option space:
 &gt; <userinput>config commit</userinput>
 </screen>
     Even though the "container" option does not carry any data except
-    sub-options, the "data" field must be explictly set to an empty value.
+    sub-options, the "data" field must be explicitly set to an empty value.
     This is required because in the current version of BIND 10 DHCP, the
     default configuration values are not propagated to the configuration parsers:
     if the "data" is not set the parser will assume that this

src/bin/auth/auth_messages.mes

@@ -374,12 +374,33 @@ XFRIN (Transfer-in) process.  It is issued during server startup is an
 indication that the initialization is proceeding normally.
 
 % AUTH_ZONEMGR_COMMS error communicating with zone manager: %1
-This is a debug message output during the processing of a NOTIFY request.
+This is an internal error during the processing of a NOTIFY request.
 An error (listed in the message) has been encountered whilst communicating
 with the zone manager. The NOTIFY request will not be honored.
+This may be some temporary failure, but is generally an unexpected
+event and is quite likely a bug.  It's probably worth filing a report.
 
 % AUTH_ZONEMGR_ERROR received error response from zone manager: %1
-This is a debug message output during the processing of a NOTIFY
-request. The zone manager component has been informed of the request,
+The zone manager component has been informed of the request,
 but has returned an error response (which is included in the message). The
-NOTIFY request will not be honored.
+NOTIFY request will not be honored.  As of this writing, this can only
+happen due to a bug inside the Zonemgr implementation.  Zonemgr itself
+may log more detailed cause of this, and these are probably worth
+filing a bug report.
+
+% AUTH_ZONEMGR_NOTEXIST received NOTIFY but Zonemgr does not exist
+This is a debug message produced by the authoritative server when it
+receives a NOTIFY message but the Zonemgr component is not running at
+that time.  Not running Zonemgr is completely valid for, e.g., primary
+only servers, so this is not necessarily a problem.  If this message
+is logged even if Zonemgr is supposed to be running, it's encouraged
+to check other logs to identify why that happens.  It may or may not
+be a real problem (for example, if it's immediately after the system
+startup, it's possible that Auth has started up and is running but
+Zonemgr is not yet).  Even if this is indeed an unexpected case,
+Zonemgr should normally be restarted by the Init process, so unless
+this repeats too often it may be negligible in practice (still it's
+worth filing a bug report).  In any case, the authoritative server
+simply drops the NOTIFY message; if it's a temporary failure or
+delayed startup, subsequently resent messages will eventually reach
+Zonemgr.

src/bin/auth/auth_srv.cc

@@ -22,6 +22,7 @@
 #include <config/ccsession.h>
 
 #include <cc/data.h>
+#include <cc/proto_defs.h>
 
 #include <exceptions/exceptions.h>
 
@@ -41,7 +42,7 @@
 
 #include <asiodns/dns_service.h>
 
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/client_list.h>
 
 #include <xfr/xfrout_client.h>
@@ -789,18 +790,15 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
         return (true);
     }
 
-    // In the code that follows, we simply ignore the notify if any internal
-    // error happens rather than returning (e.g.) SERVFAIL.  RFC 1996 is
-    // silent about such cases, but there doesn't seem to be anything we can
-    // improve at the primary server side by sending an error anyway.
-    if (xfrin_session_ == NULL) {
-        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_NO_XFRIN);
-        return (false);
-    }
-
     LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RECEIVED_NOTIFY)
         .arg(question->getName()).arg(question->getClass()).arg(remote_ep);
 
+    // xfrin_session_ should have been set and never be replaced except in
+    // tests; otherwise it's an internal bug.  assert() may be too strong,
+    // but processMessage() will catch all exceptions, so there's no better
+    // way.
+    assert(xfrin_session_);
+
     const string remote_ip_address = remote_ep.getAddress().toText();
     static const string command_template_start =
         "{\"command\": [\"notify\", {\"zone_name\" : \"";
@@ -816,12 +814,24 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
                 command_template_end);
         const unsigned int seq =
             xfrin_session_->group_sendmsg(notify_command, "Zonemgr",
-                                          "*", "*");
+                                          CC_INSTANCE_WILDCARD,
+                                          CC_INSTANCE_WILDCARD, true);
         ConstElementPtr env, answer, parsed_answer;
         xfrin_session_->group_recvmsg(env, answer, false, seq);
         int rcode;
         parsed_answer = parseAnswer(rcode, answer);
-        if (rcode != 0) {
+        if (rcode == CC_REPLY_NO_RECPT) {
+            // This can happen when Zonemgr is not running.  When we support
+            // notification-based membership framework, we should check if it's
+            // supposed to be running and shouldn't even send the command if
+            // not.  Until then, we log this event at the debug level as we
+            // don't know whether it's a real trouble or intentional
+            // configuration.  (Also, when it's done, maybe we should simply
+            // propagate the exception and return SERVFAIL to suppress further
+            // NOTIFY).
+            LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_ZONEMGR_NOTEXIST);
+            return (false);
+        } else if (rcode != CC_REPLY_SUCCESS) {
             LOG_ERROR(auth_logger, AUTH_ZONEMGR_ERROR)
                       .arg(parsed_answer->str());
             return (false);

src/bin/auth/datasrc_clients_mgr.h

@@ -25,7 +25,7 @@
 
 #include <cc/data.h>
 
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/client_list.h>
 #include <datasrc/memory/zone_writer.h>
 

src/bin/auth/tests/auth_srv_unittest.cc

@@ -26,6 +26,8 @@
 #include <dns/rdataclass.h>
 #include <dns/tsig.h>
 
+#include <cc/proto_defs.h>
+
 #include <server_common/portconfig.h>
 #include <server_common/keyring.h>
 
@@ -76,7 +78,6 @@ using namespace isc::asiolink;
 using namespace isc::testutils;
 using namespace isc::server_common::portconfig;
 using namespace isc::auth::unittest;
-using isc::datasrc::memory::ZoneTableSegment;
 using isc::UnitTestUtil;
 using boost::scoped_ptr;
 using isc::auth::statistics::Counters;
@@ -262,8 +263,6 @@ updateDatabase(AuthSrv& server, const char* params) {
     installDataSrcClientLists(server, configureDataSource(config));
 }
 
-// Note: if with_static is set to true, the corresponding test should be
-// disabled in case of USE_STATIC_LINK.
 void
 updateInMemory(AuthSrv& server, const char* origin, const char* filename,
                bool with_static = true)
@@ -278,8 +277,9 @@ updateInMemory(AuthSrv& server, const char* origin, const char* filename,
         "}]";
     if (with_static) {
         spec_txt += ", \"CH\": [{"
-        "   \"type\": \"static\","
-        "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
+        "   \"type\": \"MasterFiles\","
+        "   \"cache-enable\": true,"
+        "   \"params\": {\"BIND\": \"" + string(STATIC_DSRC_FILE) + "\"}"
             "}]";
     }
     spec_txt += "}";
@@ -288,14 +288,13 @@ updateInMemory(AuthSrv& server, const char* origin, const char* filename,
     installDataSrcClientLists(server, configureDataSource(config));
 }
 
-// Note: tests using this function should be disabled in case of
-// USE_STATIC_LINK.
 void
 updateBuiltin(AuthSrv& server) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"CH\": [{"
-        "   \"type\": \"static\","
-        "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
+        "   \"type\": \"MasterFiles\","
+        "   \"cache-enable\": true,"
+        "   \"params\": {\"BIND\": \"" + string(STATIC_DSRC_FILE) + "\"}"
         "}]}"));
     installDataSrcClientLists(server, configureDataSource(config));
 }
@@ -748,11 +747,7 @@ TEST_F(AuthSrvTest, notify) {
     checkStatisticsCounters(stats_after, expect);
 }
 
-#ifdef USE_STATIC_LINK
-TEST_F(AuthSrvTest, DISABLED_notifyForCHClass) {
-#else
 TEST_F(AuthSrvTest, notifyForCHClass) {
-#endif
     // Same as the previous test, but for the CH RRClass (so we install the
     // builtin (static) data source.
     updateBuiltin(server);
@@ -868,10 +863,12 @@ TEST_F(AuthSrvTest, notifyWithErrorRcode) {
                 Opcode::NOTIFY().getCode(), QR_FLAG | AA_FLAG, 1, 0, 0, 0);
 }
 
-TEST_F(AuthSrvTest, notifyWithoutSession) {
+TEST_F(AuthSrvTest, notifyWithoutRecipient) {
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
 
-    server.setXfrinSession(NULL);
+    // Emulate the case where msgq tells auth there's no Zonemgr module.
+    notify_session.setMessage(isc::config::createAnswer(CC_REPLY_NO_RECPT,
+                                                        "no recipient"));
 
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
                                        default_qid, Name("example"),
@@ -883,6 +880,9 @@ TEST_F(AuthSrvTest, notifyWithoutSession) {
     // happens.
     server.processMessage(*io_message, *parse_message, *response_obuffer,
                           &dnsserv);
+    // want_answer should have been set to true so auth can catch it if zonemgr
+    // is not running.
+    EXPECT_TRUE(notify_session.wasAnswerWanted());
     EXPECT_FALSE(dnsserv.hasAnswer());
 }
 
@@ -996,11 +996,7 @@ TEST_F(AuthSrvTest, notifyNotAuthNoClass) {
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
 // well
-#ifdef USE_STATIC_LINK
-TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin
-#else
 TEST_F(AuthSrvTest, TSIGSigned) {
-#endif
     // Prepare key, the client message, etc
     updateBuiltin(server);
     const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
@@ -1058,11 +1054,7 @@ TEST_F(AuthSrvTest, TSIGSigned) {
 // authoritative only server in terms of performance, and it's quite likely
 // we need to drop it for the authoritative server implementation.
 // At that point we can drop this test, too.
-#ifdef USE_STATIC_LINK
-TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) {
-#else
 TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
-#endif
     updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
@@ -1090,11 +1082,7 @@ TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
 
 // The most primitive check: checking the result of the processMessage()
 // method
-#ifdef USE_STATIC_LINK
-TEST_F(AuthSrvTest, DISABLED_builtInQuery) {
-#else
 TEST_F(AuthSrvTest, builtInQuery) {
-#endif
     updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
@@ -1111,11 +1099,7 @@ TEST_F(AuthSrvTest, builtInQuery) {
 }
 
 // Same type of test as builtInQueryViaDNSServer but for an error response.
-#ifdef USE_STATIC_LINK
-TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin
-#else
-TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin
-#endif
+TEST_F(AuthSrvTest, iqueryViaDNSServer) {
     updateBuiltin(server);
     createDataFromFile("iquery_fromWire.wire");
     (*server.getDNSLookupProvider())(*io_message, parse_message,
@@ -1226,11 +1210,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
                 opcode.getCode(), QR_FLAG, 1, 0, 0, 0);
 }
 
-#ifdef USE_STATIC_LINK
-TEST_F(AuthSrvTest, DISABLED_queryWithInMemoryClientNoDNSSEC) {
-#else
 TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
-#endif
     // In this example, we do simple check that query is handled from the
     // query handler class, and confirm it returns no error and a non empty
     // answer section.  Detailed examination on the response content
@@ -1246,11 +1226,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
                 opcode.getCode(), QR_FLAG | AA_FLAG, 1, 1, 2, 1);
 }
 
-#ifdef USE_STATIC_LINK
-TEST_F(AuthSrvTest, DISABLED_queryWithInMemoryClientDNSSEC) {
-#else
 TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
-#endif
     // Similar to the previous test, but the query has the DO bit on.
     // The response should contain RRSIGs, and should have more RRs than
     // the previous case.
@@ -1265,14 +1241,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
                 opcode.getCode(), QR_FLAG | AA_FLAG, 1, 2, 3, 3);
 }
 
-TEST_F(AuthSrvTest,
-#ifdef USE_STATIC_LINK
-       DISABLED_chQueryWithInMemoryClient
-#else
-       chQueryWithInMemoryClient
-#endif
-    )
-{
+TEST_F(AuthSrvTest, chQueryWithInMemoryClient) {
     // Set up the in-memory
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
@@ -1745,9 +1714,7 @@ public:
              real_list, ThrowWhen throw_when, bool isc_exception,
              ConstRRsetPtr fake_rrset = ConstRRsetPtr()) :
         ConfigurableClientList(RRClass::IN()),
-        real_(real_list),
-        config_(Element::fromJSON("{}")),
-        ztable_segment_(ZoneTableSegment::create(*config_, RRClass::IN()))
+        real_(real_list)
     {
         BOOST_FOREACH(const DataSourceInfo& info, real_->getDataSources()) {
              const isc::datasrc::DataSourceClientPtr
@@ -1759,13 +1726,13 @@ public:
              data_sources_.push_back(
                  DataSourceInfo(client.get(),
                                 isc::datasrc::DataSourceClientContainerPtr(),
-                                false, RRClass::IN(), ztable_segment_, ""));
+                                boost::shared_ptr<
+                                isc::datasrc::internal::CacheConfig>(),
+                                RRClass::IN(), ""));
         }
     }
 private:
     const boost::shared_ptr<isc::datasrc::ConfigurableClientList> real_;
-    const ConstElementPtr config_;
-    boost::shared_ptr<ZoneTableSegment> ztable_segment_;
     vector<isc::datasrc::DataSourceClientPtr> clients_;
 };
 
@@ -1775,14 +1742,7 @@ private:
 //
 // Set the proxies to never throw, this should have the same result as
 // queryWithInMemoryClientNoDNSSEC, and serves to test the two proxy classes
-TEST_F(AuthSrvTest,
-#ifdef USE_STATIC_LINK
-       DISABLED_queryWithInMemoryClientProxy
-#else
-       queryWithInMemoryClientProxy
-#endif
-    )
-{
+TEST_F(AuthSrvTest, queryWithInMemoryClientProxy) {
     // Set real inmem client to proxy
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
     boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
@@ -1829,14 +1789,7 @@ setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
     mgr.setDataSrcClientLists(lists);
 }
 
-TEST_F(AuthSrvTest,
-#ifdef USE_STATIC_LINK
-       DISABLED_queryWithThrowingProxyServfails
-#else
-       queryWithThrowingProxyServfails
-#endif
-    )
-{
+TEST_F(AuthSrvTest, queryWithThrowingProxyServfails) {
     // Test the common cases, all of which should simply return SERVFAIL
     // Use THROW_NEVER as end marker
     ThrowWhen throws[] = { THROW_AT_FIND_ZONE,
@@ -1860,14 +1813,7 @@ TEST_F(AuthSrvTest,
 
 // Throw isc::Exception in getClass(). (Currently?) getClass is not called
 // in the processMessage path, so this should result in a normal answer
-TEST_F(AuthSrvTest,
-#ifdef USE_STATIC_LINK
-       DISABLED_queryWithInMemoryClientProxyGetClass
-#else
-       queryWithInMemoryClientProxyGetClass
-#endif
-    )
-{
+TEST_F(AuthSrvTest, queryWithInMemoryClientProxyGetClass) {
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     setupThrow(server, THROW_AT_GET_CLASS, true);
 
@@ -1880,14 +1826,7 @@ TEST_F(AuthSrvTest,
                 opcode.getCode(), QR_FLAG | AA_FLAG, 1, 1, 2, 1);
 }
 
-TEST_F(AuthSrvTest,
-#ifdef USE_STATIC_LINK
-       DISABLED_queryWithThrowingInToWire
-#else
-       queryWithThrowingInToWire
-#endif
-    )
-{
+TEST_F(AuthSrvTest, queryWithThrowingInToWire) {
     // Set up a faked data source.  It will return an empty RRset for the
     // query.
     ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"),

src/bin/auth/tests/config_unittest.cc

@@ -21,7 +21,7 @@
 
 #include <cc/data.h>
 
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 
 #include <xfr/xfrout_client.h>
 

src/bin/bind10/init.py.in

@@ -223,7 +223,7 @@ class Init:
         self.component_config = {}
         # Some time in future, it may happen that a single component has
         # multple processes (like a pipeline-like component). If so happens,
-        # name "components" may be inapropriate. But as the code isn't probably
+        # name "components" may be inappropriate. But as the code isn't probably
         # completely ready for it, we leave it at components for now. We also
         # want to support multiple instances of a single component. If it turns
         # out that we'll have a single component with multiple same processes

src/bin/bind10/tests/init_test.py.in

@@ -1595,7 +1595,7 @@ class TestInitComponents(unittest.TestCase):
         init.components[53] = component
 
         # case where the returned pid is unknown to us. nothing should
-        # happpen then.
+        # happen then.
         init.get_process_exit_status_called = False
         init._get_process_exit_status = init._get_process_exit_status_unknown_pid
         init.components_to_restart = []

src/bin/bindctl/bindcmd.py

@@ -148,9 +148,9 @@ class BindCmdInterpreter(Cmd):
         # is processed by a script that expects a specific format.
         if my_readline == sys.stdin.readline and sys.stdin.isatty():
             sys.stdout.write("""\
-WARNING: Python readline module isn't available, so the command line editor
-         (including command history management) does not work.  See BIND 10
-         guide for more details.\n\n""")
+WARNING: The Python readline module isn't available, so some command line
+         editing features (including command history management) will not
+         work.  See the BIND 10 guide for more details.\n\n""")
 
         try:
             if not self.login_to_cmdctl():

src/bin/bindctl/run_bindctl.sh.in

@@ -27,7 +27,7 @@ export PYTHONPATH
 # required by loadable python modules.
 SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
 if test $SET_ENV_LIBRARY_PATH = yes; then
-	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/threads/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
 	export @ENV_LIBRARY_PATH@
 fi
 

src/bin/cfgmgr/plugins/datasrc.spec.pre.in

@@ -18,9 +18,9 @@
                     ],
                     "CH": [
                         {
-                            "type": "static",
-                            "cache-enable": false,
-                            "params": "@@STATIC_ZONE_FILE@@"
+                            "type": "MasterFiles",
+                            "cache-enable": true,
+                            "params": {"BIND": "@@STATIC_ZONE_FILE@@"}
                         }
                     ]
                 },
@@ -68,6 +68,12 @@
                                 "item_name": "name",
                                 "item_type": "string",
                                 "item_optional": true
+                            },
+                            {
+                                "item_name": "cache-type",
+                                "item_type": "string",
+                                "item_optional": true,
+                                "item_default": "local"
                             }
                         ]
                     }

src/bin/cfgmgr/plugins/tests/tsig_keys_test.py

@@ -92,7 +92,7 @@ class TSigKeysTest(unittest.TestCase):
     def test_bad_format(self):
         """
         Test we fail on bad format. We don't really care much how here, though,
-        as this should not get in trough config manager anyway.
+        as this should not get in through config manager anyway.
         """
         self.assertNotEqual(None, tsig_keys.check({'bad_name': {}}))
         self.assertNotEqual(None, tsig_keys.check({'keys': 'not_list'}))

src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in

@@ -70,7 +70,7 @@ class TestPlugins(unittest.TestCase):
 class TestConfigManagerStartup(unittest.TestCase):
     def test_cfgmgr(self):
         # some creative module use;
-        # b10-cfgmgr has a hypen, so we use __import__
+        # b10-cfgmgr has a hyphen, so we use __import__
         # this also gives us the chance to override the imported
         # module ConfigManager in it.
         b = __import__("b10-cfgmgr")

src/bin/cmdctl/run_b10-cmdctl.sh.in

@@ -26,7 +26,7 @@ export PYTHONPATH
 # required by loadable python modules.
 SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
 if test $SET_ENV_LIBRARY_PATH = yes; then
-        @ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
+        @ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/threads/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
         export @ENV_LIBRARY_PATH@
 fi
 

src/bin/dbutil/run_dbutil.sh.in

@@ -30,7 +30,7 @@ export PYTHONPATH
 # required by loadable python modules.
 SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
 if test $SET_ENV_LIBRARY_PATH = yes; then
-	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/threads/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
 	export @ENV_LIBRARY_PATH@
 fi
 

src/bin/dhcp4/config_parser.cc

@@ -56,15 +56,6 @@ typedef isc::dhcp::DhcpConfigParser* ParserFactory(const std::string& config_id)
 /// @brief a collection of factories that creates parsers for specified element names
 typedef std::map<std::string, ParserFactory*> FactoryMap;
 
-/// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900)
-typedef std::map<std::string, uint32_t> Uint32Storage;
-
-/// @brief a collection of elements that store string values
-typedef std::map<std::string, std::string> StringStorage;
-
-/// @brief Storage for parsed boolean values.
-typedef std::map<string, bool> BooleanStorage;
-
 /// @brief Storage for option definitions.
 typedef OptionSpaceContainer<OptionDefContainer,
                              OptionDefinitionPtr> OptionDefStorage;
@@ -198,7 +189,7 @@ public:
     /// @brief Put a parsed value to the storage.
     virtual void commit() {
         if (storage_ != NULL && !param_name_.empty()) {
-            (*storage_)[param_name_] = value_;
+            storage_->setParam(param_name_, value_);
         }
     }
 
@@ -292,7 +283,7 @@ public:
         if (storage_ != NULL && !param_name_.empty()) {
             // If a given parameter already exists in the storage we override
             // its value. If it doesn't we insert a new element.
-            (*storage_)[param_name_] = value_;
+            storage_->setParam(param_name_, value_);
         }
     }
 
@@ -364,7 +355,7 @@ public:
         if (storage_ != NULL && !param_name_.empty()) {
             // If a given parameter already exists in the storage we override
             // its value. If it doesn't we insert a new element.
-            (*storage_)[param_name_] = value_;
+            storage_->setParam(param_name_, value_);
         }
     }
 
@@ -735,7 +726,7 @@ private:
     /// are invalid or insufficient this function emits an exception.
     ///
     /// @warning this function does not check if options_ storage pointer
-    /// is intitialized but this check is not needed here because it is done
+    /// is initialized but this check is not needed here because it is done
     /// in the \ref build function.
     ///
     /// @throw DhcpConfigError if parameters provided in the configuration
@@ -744,7 +735,7 @@ private:
         // Option code is held in the uint32_t storage but is supposed to
         // be uint16_t value. We need to check that value in the configuration
         // does not exceed range of uint8_t and is not zero.
-        uint32_t option_code = getParam<uint32_t>("code", uint32_values_);
+        uint32_t option_code = uint32_values_.getParam("code");
         if (option_code == 0) {
             isc_throw(DhcpConfigError, "option code must not be zero."
                       << " Option code '0' is reserved in DHCPv4.");
@@ -753,9 +744,10 @@ private:
                       << "', it must not exceed '"
                       << std::numeric_limits<uint8_t>::max() << "'");
         }
+
         // Check that the option name has been specified, is non-empty and does not
-        // contain spaces.
-        std::string option_name = getParam<std::string>("name", string_values_);
+        // contain spaces
+        std::string option_name = string_values_.getParam("name"); 
         if (option_name.empty()) {
             isc_throw(DhcpConfigError, "name of the option with code '"
                       << option_code << "' is empty");
@@ -764,7 +756,7 @@ private:
                       << "', space character is not allowed");
         }
 
-        std::string option_space = getParam<std::string>("space", string_values_);
+        std::string option_space = string_values_.getParam("space"); 
         if (!OptionSpace::validateName(option_space)) {
             isc_throw(DhcpConfigError, "invalid option space name '"
                       << option_space << "' specified for option '"
@@ -805,8 +797,8 @@ private:
         }
 
         // Get option data from the configuration database ('data' field).
-        const std::string option_data = getParam<std::string>("data", string_values_);
-        const bool csv_format = getParam<bool>("csv-format", boolean_values_);
+        const std::string option_data = string_values_.getParam("data");
+        const bool csv_format = boolean_values_.getParam("csv-format");
 
         // Transform string of hexadecimal digits into binary format.
         std::vector<uint8_t> binary;
@@ -838,7 +830,7 @@ private:
                           << " does not have a definition.");
             }
 
-            // @todo We have a limited set of option definitions intiialized at the moment.
+            // @todo We have a limited set of option definitions initialized at the moment.
             // In the future we want to initialize option definitions for all options.
             // Consequently an error will be issued if an option definition does not exist
             // for a particular option code. For now it is ok to create generic option
@@ -1080,8 +1072,9 @@ private:
 
     /// @brief Create option definition from the parsed parameters.
     void createOptionDef() {
+
         // Get the option space name and validate it.
-        std::string space = getParam<std::string>("space", string_values_);
+        std::string space = string_values_.getParam("space");
         if (!OptionSpace::validateName(space)) {
             isc_throw(DhcpConfigError, "invalid option space name '"
                       << space << "'");
@@ -1089,12 +1082,11 @@ private:
 
         // Get other parameters that are needed to create the
         // option definition.
-        std::string name = getParam<std::string>("name", string_values_);
-        uint32_t code = getParam<uint32_t>("code", uint32_values_);
-        std::string type = getParam<std::string>("type", string_values_);
-        bool array_type = getParam<bool>("array", boolean_values_);
-        std::string encapsulates = getParam<std::string>("encapsulate",
-                                                         string_values_);
+        std::string name = string_values_.getParam("name");
+        uint32_t code = uint32_values_.getParam("code");
+        std::string type = string_values_.getParam("type");
+        bool array_type = boolean_values_.getParam("array");
+        std::string encapsulates = string_values_.getParam("encapsulate");
 
         // Create option definition.
         OptionDefinitionPtr def;
@@ -1124,8 +1116,8 @@ private:
         }
         // The record-types field may carry a list of comma separated names
         // of data types that form a record.
-        std::string record_types = getParam<std::string>("record-types",
-                                                         string_values_);
+        std::string record_types = string_values_.getParam("record-types");
+
         // Split the list of record types into tokens.
         std::vector<std::string> record_tokens =
             isc::util::str::tokens(record_types, ",");
@@ -1422,13 +1414,16 @@ private:
     ///
     /// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing failed.
     void createSubnet() {
-        StringStorage::const_iterator it = string_values_.find("subnet");
-        if (it == string_values_.end()) {
+        std::string subnet_txt;
+        try {
+            subnet_txt = string_values_.getParam("subnet"); 
+        } catch (DhcpConfigError) {
+            // Rethrow with precise error.
             isc_throw(DhcpConfigError,
                       "Mandatory subnet definition in subnet missing");
         }
+
         // Remove any spaces or tabs.
-        string subnet_txt = it->second;
         boost::erase_all(subnet_txt, " ");
         boost::erase_all(subnet_txt, "\t");
 
@@ -1440,7 +1435,7 @@ private:
         size_t pos = subnet_txt.find("/");
         if (pos == string::npos) {
             isc_throw(DhcpConfigError,
-                      "Invalid subnet syntax (prefix/len expected):" << it->second);
+                      "Invalid subnet syntax (prefix/len expected):" << subnet_txt);
         }
 
         // Try to create the address object. It also validates that
@@ -1540,7 +1535,6 @@ private:
     /// @throw NotImplemented if trying to create a parser for unknown config element
     DhcpConfigParser* createSubnet4ConfigParser(const std::string& config_id) {
         FactoryMap factories;
-
         factories["valid-lifetime"] = Uint32Parser::factory;
         factories["renew-timer"] = Uint32Parser::factory;
         factories["rebind-timer"] = Uint32Parser::factory;
@@ -1571,26 +1565,21 @@ private:
     /// @throw DhcpConfigError when requested parameter is not present
     Triplet<uint32_t> getParam(const std::string& name) {
         uint32_t value = 0;
-        bool found = false;
-        Uint32Storage::iterator global = uint32_defaults.find(name);
-        if (global != uint32_defaults.end()) {
-            value = global->second;
-            found = true;
-        }
-
-        Uint32Storage::iterator local = uint32_values_.find(name);
-        if (local != uint32_values_.end()) {
-            value = local->second;
-            found = true;
-        }
-
-        if (found) {
-            return (Triplet<uint32_t>(value));
-        } else {
-            isc_throw(DhcpConfigError, "Mandatory parameter " << name
+        try {
+            // look for local value 
+            value = uint32_values_.getParam(name);
+        } catch (DhcpConfigError) {
+            try {
+                // no local, use global value 
+                value = uint32_defaults.getParam(name);
+            } catch (DhcpConfigError) {
+                isc_throw(DhcpConfigError, "Mandatory parameter " << name
                       << " missing (no global default and no subnet-"
                       << "specific value)");
+            }
         }
+
+        return (Triplet<uint32_t>(value));
     }
 
     /// storage for subnet-specific uint32 values
@@ -1859,7 +1848,7 @@ configureDhcp4Server(Dhcpv4Srv&, ConstElementPtr config_set) {
     return (answer);
 }
 
-const std::map<std::string, uint32_t>& getUint32Defaults() {
+const Uint32Storage& getUint32Defaults() {
     return (uint32_defaults);
 }
 

src/bin/dhcp4/config_parser.h

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <exceptions/exceptions.h>
+#include <dhcpsrv/dhcp_config_parser.h>
 #include <cc/data.h>
 #include <stdint.h>
 #include <string>
@@ -66,7 +67,7 @@ configureDhcp4Server(Dhcpv4Srv&,
 /// Uint32Parser works as expected.
 ///
 /// @return a reference to a global uint32 values storage.
-const std::map<std::string, uint32_t>& getUint32Defaults();
+const Uint32Storage& getUint32Defaults();
 
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -97,7 +97,7 @@ protected:
     /// @brief A dummy configuration handler that always returns success.
     ///
     /// This configuration handler does not perform configuration
-    /// parsing and always returns success. A dummy hanlder should
+    /// parsing and always returns success. A dummy handler should
     /// be installed using \ref isc::config::ModuleCCSession ctor
     /// to get the initial configuration. This initial configuration
     /// comprises values for only those elements that were modified

src/bin/dhcp4/dhcp4_messages.mes

@@ -74,7 +74,7 @@ many possible reasons for such a failure.
 % DHCP4_LEASE_ALLOC lease %1 has been allocated for client-id %2, hwaddr %3
 This debug message indicates that the server successfully granted a lease
 in response to client's REQUEST message. This is a normal behavior and
-incicates successful operation.
+indicates successful operation.
 
 % DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2
 This message indicates that the server failed to grant a lease to the

src/bin/dhcp4/dhcp4_srv.cc

@@ -57,7 +57,7 @@ static const char* SERVER_ID_FILE = "b10-dhcp4-serverid";
 // These are hardcoded parameters. Currently this is a skeleton server that only
 // grants those options and a single, fixed, hardcoded lease.
 
-Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig) {
+Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast) {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
         // First call to instance() will create IfaceMgr (it's a singleton)
@@ -67,7 +67,7 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig) {
         if (port) {
             // open sockets only if port is non-zero. Port 0 is used
             // for non-socket related testing.
-            IfaceMgr::instance().openSockets4(port);
+            IfaceMgr::instance().openSockets4(port, use_bcast);
         }
 
         string srvid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_ID_FILE);
@@ -287,9 +287,9 @@ Dhcpv4Srv::generateServerID() {
             continue;
         }
 
-        const IfaceMgr::AddressCollection addrs = iface->getAddresses();
+        const Iface::AddressCollection addrs = iface->getAddresses();
 
-        for (IfaceMgr::AddressCollection::const_iterator addr = addrs.begin();
+        for (Iface::AddressCollection::const_iterator addr = addrs.begin();
              addr != addrs.end(); ++addr) {
             if (addr->getFamily() != AF_INET) {
                 continue;
@@ -317,7 +317,7 @@ Dhcpv4Srv::writeServerID(const std::string& file_name) {
     return (true);
 }
 
-string 
+string
 Dhcpv4Srv::srvidToString(const OptionPtr& srvid) {
     if (!srvid) {
         isc_throw(BadValue, "NULL pointer passed to srvidToString()");
@@ -343,7 +343,7 @@ Dhcpv4Srv::copyDefaultFields(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     answer->setIndex(question->getIndex());
     answer->setCiaddr(question->getCiaddr());
 
-    answer->setSiaddr(IOAddress("0.0.0.0")); // explictly set this to 0
+    answer->setSiaddr(IOAddress("0.0.0.0")); // explicitly set this to 0
     answer->setHops(question->getHops());
 
     // copy MAC address
@@ -517,6 +517,28 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
 
         answer->setYiaddr(lease->addr_);
 
+        // If remote address is not set, we are dealing with a directly
+        // connected client requesting new lease. We can send response to
+        // the address assigned in the lease, but first we have to make sure
+        // that IfaceMgr supports responding directly to the client when
+        // client doesn't have address assigned to its interface yet.
+        if (answer->getRemoteAddr().toText() == "0.0.0.0") {
+            if (IfaceMgr::instance().isDirectResponseSupported()) {
+                answer->setRemoteAddr(lease->addr_);
+            } else {
+                // Since IfaceMgr does not support direct responses to
+                // clients not having IP addresses, we have to send response
+                // to broadcast. We don't check whether the use_bcast flag
+                // was set in the constructor, because this flag is only used
+                // by unit tests to prevent opening broadcast sockets, as
+                // it requires root privileges. If this function is invoked by
+                // unit tests, we expect that it sets broadcast address if
+                // direct response is not supported, so as a test can verify
+                // function's behavior, regardless of the use_bcast flag's value.
+                answer->setRemoteAddr(IOAddress("255.255.255.255"));
+            }
+        }
+
         // IP Address Lease time (type 51)
         opt = OptionPtr(new Option(Option::V4, DHO_DHCP_LEASE_TIME));
         opt->setUint32(lease->valid_lft_);

src/bin/dhcp4/dhcp4_srv.h

@@ -66,8 +66,10 @@ class Dhcpv4Srv : public boost::noncopyable {
     /// @param port specifies port number to listen on
     /// @param dbconfig Lease manager configuration string.  The default
     ///        of the "memfile" manager is used for testing.
+    /// @param use_bcast configure sockets to support broadcast messages.
     Dhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT,
-              const char* dbconfig = "type=memfile");
+              const char* dbconfig = "type=memfile",
+              const bool use_bcast = true);
 
     /// @brief Destructor. Used during DHCPv4 service shutdown.
     ~Dhcpv4Srv();
@@ -216,7 +218,7 @@ protected:
     /// @param msg_type specifies message type
     void appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type);
 
-    /// @brief Returns server-intentifier option
+    /// @brief Returns server-identifier option
     ///
     /// @return server-id option
     OptionPtr getServerID() { return serverid_; }

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -52,15 +52,14 @@ public:
 
     // Checks if global parameter of name have expected_value
     void checkGlobalUint32(string name, uint32_t expected_value) {
-        const std::map<std::string, uint32_t>& uint32_defaults = getUint32Defaults();
-        std::map<std::string, uint32_t>::const_iterator it =
-            uint32_defaults.find(name);
-        if (it == uint32_defaults.end()) {
+        const Uint32Storage& uint32_defaults = getUint32Defaults();
+        try {
+            uint32_t actual_value = uint32_defaults.getParam(name);
+            EXPECT_EQ(expected_value, actual_value);
+        } catch (DhcpConfigError) {
             ADD_FAILURE() << "Expected uint32 with name " << name
                           << " not found";
-            return;
         }
-        EXPECT_EQ(expected_value, it->second);
     }
 
     // Checks if the result of DHCP server configuration has
@@ -83,7 +82,7 @@ public:
     /// option value. These parameters are: "name", "code", "data",
     /// "csv-format" and "space".
     ///
-    /// @param param_value string holiding option parameter value to be
+    /// @param param_value string holding option parameter value to be
     /// injected into the configuration string.
     /// @param parameter name of the parameter to be configured with
     /// param value.

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -17,6 +17,7 @@
 
 #include <asiolink/io_address.h>
 #include <dhcp/dhcp4.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcp/option.h>
 #include <dhcp/option4_addrlst.h>
 #include <dhcp/option_custom.h>
@@ -44,7 +45,16 @@ namespace {
 class NakedDhcpv4Srv: public Dhcpv4Srv {
     // "Naked" DHCPv4 server, exposes internal fields
 public:
-    NakedDhcpv4Srv(uint16_t port = 0):Dhcpv4Srv(port) { }
+
+    /// @brief Constructor.
+    ///
+    /// It disables configuration of broadcast options on
+    /// sockets that are opened by the Dhcpv4Srv constructor.
+    /// Setting broadcast options requires root privileges
+    /// which is not the case when running unit tests.
+    NakedDhcpv4Srv(uint16_t port = 0)
+        : Dhcpv4Srv(port, "type=memfile", false) {
+    }
 
     using Dhcpv4Srv::processDiscover;
     using Dhcpv4Srv::processRequest;
@@ -116,10 +126,10 @@ public:
 
     /// @brief Configures options being requested in the PRL option.
     ///
-    /// The lpr-servers option is NOT configured here altough it is
+    /// The lpr-servers option is NOT configured here although it is
     /// added to the 'Parameter Request List' option in the
     /// \ref addPrlOption. When requested option is not configured
-    /// the server should not return it in its rensponse. The goal
+    /// the server should not return it in its response. The goal
     /// of not configuring the requested option is to verify that
     /// the server will not return it.
     void configureRequestedOptions() {
@@ -170,6 +180,8 @@ public:
         EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
         EXPECT_TRUE(a->getOption(DHO_DHCP_LEASE_TIME));
         EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
+        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
+        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
 
         // Check that something is offered
         EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
@@ -345,6 +357,120 @@ public:
         EXPECT_TRUE(expected_clientid->getData() == opt->getData());
     }
 
+    /// @brief Tests if Discover or Request message is processed correctly
+    ///
+    /// @param msg_type DHCPDISCOVER or DHCPREQUEST
+    /// @param client_addr client address
+    /// @param relay_addr relay address
+    void testDiscoverRequest(const uint8_t msg_type,
+                             const IOAddress& client_addr,
+                             const IOAddress& relay_addr) {
+
+        boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
+        vector<uint8_t> mac(6);
+        for (int i = 0; i < 6; i++) {
+            mac[i] = i*10;
+        }
+
+        boost::shared_ptr<Pkt4> req(new Pkt4(msg_type, 1234));
+        boost::shared_ptr<Pkt4> rsp;
+
+        req->setIface("eth0");
+        req->setIndex(17);
+        req->setHWAddr(1, 6, mac);
+        req->setRemoteAddr(IOAddress(client_addr));
+        req->setGiaddr(relay_addr);
+
+        // We are going to test that certain options are returned
+        // in the response message when requested using 'Parameter
+        // Request List' option. Let's configure those options that
+        // are returned when requested.
+        configureRequestedOptions();
+
+        if (msg_type == DHCPDISCOVER) {
+            ASSERT_NO_THROW(
+                rsp = srv->processDiscover(req);
+            );
+
+            // Should return OFFER
+            ASSERT_TRUE(rsp);
+            EXPECT_EQ(DHCPOFFER, rsp->getType());
+
+        } else {
+            ASSERT_NO_THROW(
+                rsp = srv->processRequest(req);
+            );
+
+            // Should return ACK
+            ASSERT_TRUE(rsp);
+            EXPECT_EQ(DHCPACK, rsp->getType());
+
+        }
+
+        if (relay_addr.toText() != "0.0.0.0") {
+            // This is relayed message. It should be sent brsp to relay address.
+            EXPECT_EQ(req->getGiaddr().toText(),
+                      rsp->getRemoteAddr().toText());
+
+        } else if (client_addr.toText() != "0.0.0.0") {
+            // This is a message from a client having an IP address.
+            EXPECT_EQ(req->getRemoteAddr().toText(),
+                      rsp->getRemoteAddr().toText());
+
+        } else {
+            // This is a message from a client having no IP address yet.
+            // If IfaceMgr supports direct traffic the response should
+            // be sent to the new address assigned to the client.
+            if (IfaceMgr::instance().isDirectResponseSupported()) {
+                EXPECT_EQ(rsp->getYiaddr(),
+                          rsp->getRemoteAddr().toText());
+
+            // If direct response to the client having no IP address is
+            // not supported, response should go to broadcast.
+            } else {
+                EXPECT_EQ("255.255.255.255", rsp->getRemoteAddr().toText());
+
+            }
+
+        }
+
+        messageCheck(req, rsp);
+
+        // We did not request any options so these should not be present
+        // in the RSP.
+        EXPECT_FALSE(rsp->getOption(DHO_LOG_SERVERS));
+        EXPECT_FALSE(rsp->getOption(DHO_COOKIE_SERVERS));
+        EXPECT_FALSE(rsp->getOption(DHO_LPR_SERVERS));
+
+        // Repeat the test but request some options.
+        // Add 'Parameter Request List' option.
+        addPrlOption(req);
+
+        if (msg_type == DHCPDISCOVER) {
+            ASSERT_NO_THROW(
+                rsp = srv->processDiscover(req);
+            );
+
+            // Should return non-NULL packet.
+            ASSERT_TRUE(rsp);
+            EXPECT_EQ(DHCPOFFER, rsp->getType());
+
+        } else {
+            ASSERT_NO_THROW(
+                rsp = srv->processRequest(req);
+            );
+
+            // Should return non-NULL packet.
+            ASSERT_TRUE(rsp);
+            EXPECT_EQ(DHCPACK, rsp->getType());
+
+        }
+
+        // Check that the requested options are returned.
+        optionsCheck(rsp);
+
+    }
+
     ~Dhcpv4SrvTest() {
         CfgMgr::instance().deleteSubnets4();
 
@@ -389,7 +515,7 @@ TEST_F(Dhcpv4SrvTest, basic) {
     delete naked_srv;
 }
 
-// Verifies that received DISCOVER can be processed correctly,
+// Verifies that DISCOVER received via relay can be processed correctly,
 // that the OFFER message generated in response is valid and
 // contains necessary options.
 //
@@ -397,203 +523,56 @@ TEST_F(Dhcpv4SrvTest, basic) {
 // are other tests that verify correctness of the allocation
 // engine. See DiscoverBasic, DiscoverHint, DiscoverNoClientId
 // and DiscoverInvalidHint.
-TEST_F(Dhcpv4SrvTest, processDiscover) {
-    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv(0);
-    vector<uint8_t> mac(6);
-    for (int i = 0; i < 6; i++) {
-        mac[i] = 255 - i;
-    }
-
-    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPDISCOVER, 1234));
-    boost::shared_ptr<Pkt4> offer;
-
-    pkt->setIface("eth0");
-    pkt->setIndex(17);
-    pkt->setHWAddr(1, 6, mac);
-    pkt->setRemoteAddr(IOAddress("192.0.2.56"));
-    pkt->setGiaddr(IOAddress("192.0.2.67"));
-
-    // Let's make it a relayed message
-    pkt->setHops(3);
-    pkt->setRemotePort(DHCP4_SERVER_PORT);
-
-    // We are going to test that certain options are returned
-    // (or not returned) in the OFFER message when requested
-    // using 'Parameter Request List' option. Let's configure
-    // those options that are returned when requested.
-    configureRequestedOptions();
-
-    // Should not throw
-    EXPECT_NO_THROW(
-        offer = srv->processDiscover(pkt);
-    );
-
-    // Should return something
-    ASSERT_TRUE(offer);
-
-    EXPECT_EQ(DHCPOFFER, offer->getType());
-
-    // This is relayed message. It should be sent back to relay address.
-    EXPECT_EQ(pkt->getGiaddr(), offer->getRemoteAddr());
-
-    messageCheck(pkt, offer);
-
-    // There are some options that are always present in the
-    // message, even if not requested.
-    EXPECT_TRUE(offer->getOption(DHO_DOMAIN_NAME));
-    EXPECT_TRUE(offer->getOption(DHO_DOMAIN_NAME_SERVERS));
-
-    // We did not request any options so they should not be present
-    // in the OFFER.
-    EXPECT_FALSE(offer->getOption(DHO_LOG_SERVERS));
-    EXPECT_FALSE(offer->getOption(DHO_COOKIE_SERVERS));
-    EXPECT_FALSE(offer->getOption(DHO_LPR_SERVERS));
-
-    // Add 'Parameter Request List' option.
-    addPrlOption(pkt);
-
-    // Now repeat the test but request some options.
-    EXPECT_NO_THROW(
-        offer = srv->processDiscover(pkt);
-    );
-
-    // Should return something
-    ASSERT_TRUE(offer);
-
-    EXPECT_EQ(DHCPOFFER, offer->getType());
-
-    // This is relayed message. It should be sent back to relay address.
-    EXPECT_EQ(pkt->getGiaddr(), offer->getRemoteAddr());
-
-    messageCheck(pkt, offer);
-
-    // Check that the requested options are returned.
-    optionsCheck(offer);
-
-    // Now repeat the test for directly sent message
-    pkt->setHops(0);
-    pkt->setGiaddr(IOAddress("0.0.0.0"));
-    pkt->setRemotePort(DHCP4_CLIENT_PORT);
-
-    EXPECT_NO_THROW(
-        offer = srv->processDiscover(pkt);
-    );
-
-    // Should return something
-    ASSERT_TRUE(offer);
-
-    EXPECT_EQ(DHCPOFFER, offer->getType());
-
-    // This is direct message. It should be sent back to origin, not
-    // to relay.
-    EXPECT_EQ(pkt->getRemoteAddr(), offer->getRemoteAddr());
-
-    messageCheck(pkt, offer);
+TEST_F(Dhcpv4SrvTest, processDiscoverRelay) {
+    testDiscoverRequest(DHCPDISCOVER,
+                        IOAddress("192.0.2.56"),
+                        IOAddress("192.0.2.67"));
+}
 
-    // Check that the requested options are returned.
-    optionsCheck(offer);
+// Verifies that the non-relayed DISCOVER is processed correctly when
+// client source address is specified.
+TEST_F(Dhcpv4SrvTest, processDiscoverNoRelay) {
+    testDiscoverRequest(DHCPDISCOVER,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("192.0.2.67"));
+}
 
-    delete srv;
+// Verified that the non-relayed DISCOVER is processed correctly when
+// client source address is not specified.
+TEST_F(Dhcpv4SrvTest, processDiscoverNoClientAddr) {
+    testDiscoverRequest(DHCPDISCOVER,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("0.0.0.0"));
 }
 
-// Verifies that received REQUEST can be processed correctly,
-// that the ACK message generated in response is valid and
+// Verifies that REQUEST received via relay can be processed correctly,
+// that the OFFER message generated in response is valid and
 // contains necessary options.
 //
 // Note: this test focuses on the packet correctness. There
 // are other tests that verify correctness of the allocation
-// engine. See RequestBasic.
-TEST_F(Dhcpv4SrvTest, processRequest) {
-    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv(0);
-    vector<uint8_t> mac(6);
-    for (int i = 0; i < 6; i++) {
-        mac[i] = i*10;
-    }
-
-    boost::shared_ptr<Pkt4> req(new Pkt4(DHCPREQUEST, 1234));
-    boost::shared_ptr<Pkt4> ack;
-
-    req->setIface("eth0");
-    req->setIndex(17);
-    req->setHWAddr(1, 6, mac);
-    req->setRemoteAddr(IOAddress("192.0.2.56"));
-    req->setGiaddr(IOAddress("192.0.2.67"));
-
-    // We are going to test that certain options are returned
-    // in the ACK message when requested using 'Parameter
-    // Request List' option. Let's configure those options that
-    // are returned when requested.
-    configureRequestedOptions();
-
-    // Should not throw
-    ASSERT_NO_THROW(
-        ack = srv->processRequest(req);
-    );
-
-    // Should return something
-    ASSERT_TRUE(ack);
-
-    EXPECT_EQ(DHCPACK, ack->getType());
-
-    // This is relayed message. It should be sent back to relay address.
-    EXPECT_EQ(req->getGiaddr(), ack->getRemoteAddr());
-
-    messageCheck(req, ack);
-
-    // There are some options that are always present in the
-    // message, even if not requested.
-    EXPECT_TRUE(ack->getOption(DHO_DOMAIN_NAME));
-    EXPECT_TRUE(ack->getOption(DHO_DOMAIN_NAME_SERVERS));
-
-    // We did not request any options so these should not be present
-    // in the ACK.
-    EXPECT_FALSE(ack->getOption(DHO_LOG_SERVERS));
-    EXPECT_FALSE(ack->getOption(DHO_COOKIE_SERVERS));
-    EXPECT_FALSE(ack->getOption(DHO_LPR_SERVERS));
-
-    // Add 'Parameter Request List' option.
-    addPrlOption(req);
-
-    // Repeat the test but request some options.
-    ASSERT_NO_THROW(
-        ack = srv->processRequest(req);
-    );
-
-    // Should return something
-    ASSERT_TRUE(ack);
-
-    EXPECT_EQ(DHCPACK, ack->getType());
-
-    // This is relayed message. It should be sent back to relay address.
-    EXPECT_EQ(req->getGiaddr(), ack->getRemoteAddr());
-
-    // Check that the requested options are returned.
-    optionsCheck(ack);
-
-    // Now repeat the test for directly sent message
-    req->setHops(0);
-    req->setGiaddr(IOAddress("0.0.0.0"));
-    req->setRemotePort(DHCP4_CLIENT_PORT);
-
-    EXPECT_NO_THROW(
-        ack = srv->processDiscover(req);
-    );
-
-    // Should return something
-    ASSERT_TRUE(ack);
-
-    EXPECT_EQ(DHCPOFFER, ack->getType());
-
-    // This is direct message. It should be sent back to origin, not
-    // to relay.
-    EXPECT_EQ(ack->getRemoteAddr(), req->getRemoteAddr());
-
-    messageCheck(req, ack);
+// engine. See DiscoverBasic, DiscoverHint, DiscoverNoClientId
+// and DiscoverInvalidHint.
+TEST_F(Dhcpv4SrvTest, processRequestRelay) {
+    testDiscoverRequest(DHCPREQUEST,
+                        IOAddress("192.0.2.56"),
+                        IOAddress("192.0.2.67"));
+}
 
-    // Check that the requested options are returned.
-    optionsCheck(ack);
+// Verifies that the non-relayed REQUEST is processed correctly when
+// client source address is specified.
+TEST_F(Dhcpv4SrvTest, processRequestNoRelay) {
+    testDiscoverRequest(DHCPREQUEST,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("192.0.2.67"));
+}
 
-    delete srv;
+// Verified that the non-relayed REQUEST is processed correctly when
+// client source address is not specified.
+TEST_F(Dhcpv4SrvTest, processRequestNoClientAddr) {
+    testDiscoverRequest(DHCPREQUEST,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("0.0.0.0"));
 }
 
 TEST_F(Dhcpv4SrvTest, processRelease) {

src/bin/dhcp6/config_parser.cc

@@ -66,15 +66,6 @@ typedef isc::dhcp::DhcpConfigParser* ParserFactory(const std::string& config_id)
 /// @brief Collection of factories that create parsers for specified element names
 typedef std::map<std::string, ParserFactory*> FactoryMap;
 
-/// @brief Storage for parsed boolean values.
-typedef std::map<string, bool> BooleanStorage;
-
-/// @brief Collection of elements that store uint32 values (e.g. renew-timer = 900).
-typedef std::map<string, uint32_t> Uint32Storage;
-
-/// @brief Collection of elements that store string values.
-typedef std::map<string, string> StringStorage;
-
 /// @brief Storage for option definitions.
 typedef OptionSpaceContainer<OptionDefContainer,
                              OptionDefinitionPtr> OptionDefStorage;
@@ -209,7 +200,7 @@ public:
     /// @brief Put a parsed value to the storage.
     virtual void commit() {
         if (storage_ != NULL && !param_name_.empty()) {
-            (*storage_)[param_name_] = value_;
+            storage_->setParam(param_name_, value_);
         }
     }
 
@@ -317,7 +308,7 @@ public:
         if (storage_ != NULL) {
             // If a given parameter already exists in the storage we override
             // its value. If it doesn't we insert a new element.
-            (*storage_)[param_name_] = value_;
+            storage_->setParam(param_name_, value_);
         }
     }
 
@@ -393,7 +384,7 @@ public:
         if (storage_ != NULL && !param_name_.empty()) {
             // If a given parameter already exists in the storage we override
             // its value. If it doesn't we insert a new element.
-            (*storage_)[param_name_] = value_;
+            storage_->setParam(param_name_, value_);
         }
     }
 
@@ -764,7 +755,7 @@ private:
     /// are invalid or insufficient this function emits an exception.
     ///
     /// @warning this function does not check if options_ storage pointer
-    /// is intitialized but this check is not needed here because it is done
+    /// is initialized but this check is not needed here because it is done
     /// in the \ref build function.
     ///
     /// @throw DhcpConfigError if parameters provided in the configuration
@@ -774,7 +765,7 @@ private:
         // Option code is held in the uint32_t storage but is supposed to
         // be uint16_t value. We need to check that value in the configuration
         // does not exceed range of uint16_t and is not zero.
-        uint32_t option_code = getParam<uint32_t>("code", uint32_values_);
+        uint32_t option_code = uint32_values_.getParam("code");
         if (option_code == 0) {
             isc_throw(DhcpConfigError, "option code must not be zero."
                       << " Option code '0' is reserved in DHCPv6.");
@@ -785,7 +776,7 @@ private:
         }
         // Check that the option name has been specified, is non-empty and does not
         // contain spaces.
-        std::string option_name = getParam<std::string>("name", string_values_);
+        std::string option_name = string_values_.getParam("name");
         if (option_name.empty()) {
             isc_throw(DhcpConfigError, "name of the option with code '"
                       << option_code << "' is empty");
@@ -794,7 +785,7 @@ private:
                       << "', space character is not allowed");
         }
 
-        std::string option_space = getParam<std::string>("space", string_values_);
+        std::string option_space = string_values_.getParam("space");
         if (!OptionSpace::validateName(option_space)) {
             isc_throw(DhcpConfigError, "invalid option space name '"
                       << option_space << "' specified for option '"
@@ -835,8 +826,8 @@ private:
         }
 
         // Get option data from the configuration database ('data' field).
-        const std::string option_data = getParam<std::string>("data", string_values_);
-        const bool csv_format = getParam<bool>("csv-format", boolean_values_);
+        const std::string option_data = string_values_.getParam("data");
+        const bool csv_format = boolean_values_.getParam("csv-format");
 
         // Transform string of hexadecimal digits into binary format.
         std::vector<uint8_t> binary;
@@ -868,7 +859,7 @@ private:
                           << " does not have a definition.");
             }
 
-            // @todo We have a limited set of option definitions intiialized at the moment.
+            // @todo We have a limited set of option definitions initialized at the moment.
             // In the future we want to initialize option definitions for all options.
             // Consequently an error will be issued if an option definition does not exist
             // for a particular option code. For now it is ok to create generic option
@@ -1109,7 +1100,7 @@ private:
     /// @brief Create option definition from the parsed parameters.
     void createOptionDef() {
         // Get the option space name and validate it.
-        std::string space = getParam<std::string>("space", string_values_);
+        std::string space = string_values_.getParam("space");
         if (!OptionSpace::validateName(space)) {
             isc_throw(DhcpConfigError, "invalid option space name '"
                       << space << "'");
@@ -1117,12 +1108,11 @@ private:
 
         // Get other parameters that are needed to create the
         // option definition.
-        std::string name = getParam<std::string>("name", string_values_);
-        uint32_t code = getParam<uint32_t>("code", uint32_values_);
-        std::string type = getParam<std::string>("type", string_values_);
-        bool array_type = getParam<bool>("array", boolean_values_);
-        std::string encapsulates = getParam<std::string>("encapsulate",
-                                                         string_values_);
+        std::string name = string_values_.getParam("name");
+        uint32_t code = uint32_values_.getParam("code");
+        std::string type = string_values_.getParam("type");
+        bool array_type = boolean_values_.getParam("array");
+        std::string encapsulates = string_values_.getParam("encapsulate");
 
         // Create option definition.
         OptionDefinitionPtr def;
@@ -1153,8 +1143,7 @@ private:
 
         // The record-types field may carry a list of comma separated names
         // of data types that form a record.
-        std::string record_types = getParam<std::string>("record-types",
-                                                         string_values_);
+        std::string record_types = string_values_.getParam("record-types");
         // Split the list of record types into tokens.
         std::vector<std::string> record_tokens =
             isc::util::str::tokens(record_types, ",");
@@ -1448,17 +1437,19 @@ private:
     ///
     /// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing failed.
     void createSubnet() {
-
-        // Find a subnet string.
-        StringStorage::const_iterator it = string_values_.find("subnet");
-        if (it == string_values_.end()) {
+        std::string subnet_txt;
+        try {
+            subnet_txt = string_values_.getParam("subnet");
+        } catch (DhcpConfigError) {
+            // rethrow with precise error
             isc_throw(DhcpConfigError,
                       "Mandatory subnet definition in subnet missing");
         }
+
         // Remove any spaces or tabs.
-        string subnet_txt = it->second;
         boost::erase_all(subnet_txt, " ");
         boost::erase_all(subnet_txt, "\t");
+
         // The subnet format is prefix/len. We are going to extract
         // the prefix portion of a subnet string to create IOAddress
         // object from it. IOAddress will be passed to the Subnet's
@@ -1467,7 +1458,7 @@ private:
         size_t pos = subnet_txt.find("/");
         if (pos == string::npos) {
             isc_throw(DhcpConfigError,
-                      "Invalid subnet syntax (prefix/len expected):" << it->second);
+                      "Invalid subnet syntax (prefix/len expected):" << subnet_txt);
         }
 
         // Try to create the address object. It also validates that
@@ -1487,11 +1478,11 @@ private:
 
         // Get interface name. If it is defined, then the subnet is available
         // directly over specified network interface.
-
-        string iface;
-        StringStorage::const_iterator iface_iter = string_values_.find("interface");
-        if (iface_iter != string_values_.end()) {
-            iface = iface_iter->second;
+        std::string iface;
+        try {
+            iface = string_values_.getParam("interface");
+        } catch (DhcpConfigError) {
+            // iface not mandatory so swallow the exception
         }
 
         /// @todo: Convert this to logger once the parser is working reliably
@@ -1624,26 +1615,21 @@ private:
     /// @throw DhcpConfigError when requested parameter is not present
     isc::dhcp::Triplet<uint32_t> getParam(const std::string& name) {
         uint32_t value = 0;
-        bool found = false;
-        Uint32Storage::iterator global = uint32_defaults.find(name);
-        if (global != uint32_defaults.end()) {
-            value = global->second;
-            found = true;
-        }
-
-        Uint32Storage::iterator local = uint32_values_.find(name);
-        if (local != uint32_values_.end()) {
-            value = local->second;
-            found = true;
-        }
-
-        if (found) {
-            return (isc::dhcp::Triplet<uint32_t>(value));
-        } else {
-            isc_throw(isc::dhcp::DhcpConfigError, "Mandatory parameter " << name
+        try {
+            // look for local value 
+            value = uint32_values_.getParam(name);
+        } catch (DhcpConfigError) {
+            try {
+                // no local, use global value 
+                value = uint32_defaults.getParam(name);
+            } catch (DhcpConfigError) {
+                isc_throw(DhcpConfigError, "Mandatory parameter " << name
                       << " missing (no global default and no subnet-"
                       << "specific value)");
+            }
         }
+
+        return (Triplet<uint32_t>(value));
     }
 
     /// storage for subnet-specific uint32 values

src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -95,7 +95,7 @@ protected:
     /// @brief A dummy configuration handler that always returns success.
     ///
     /// This configuration handler does not perform configuration
-    /// parsing and always returns success. A dummy hanlder should
+    /// parsing and always returns success. A dummy handler should
     /// be installed using \ref isc::config::ModuleCCSession ctor
     /// to get the initial configuration. This initial configuration
     /// comprises values for only those elements that were modified

src/bin/dhcp6/dhcp6_srv.cc

@@ -329,7 +329,7 @@ Dhcpv6Srv::generateServerID() {
         // we will grow knobs to selectively turn them on or off. Also,
         // this code is used only *once* during first start on a new machine
         // and then server-id is stored. (or at least it will be once
-        // DUID storage is implemente
+        // DUID storage is implemented)
 
         // I wish there was a this_is_a_real_physical_interface flag...
 

src/bin/dhcp6/dhcp6_srv.h

@@ -40,7 +40,7 @@ namespace dhcp {
 /// packets, processes them, manages leases assignment and generates
 /// appropriate responses.
 ///
-/// @note Only one instance of this class is instantated as it encompasses
+/// @note Only one instance of this class is instantiated as it encompasses
 ///       the whole operation of the server.  Nothing, however, enforces the
 ///       singleton status of the object.
 class Dhcpv6Srv : public boost::noncopyable {
@@ -69,7 +69,7 @@ public:
     /// @brief Destructor. Used during DHCPv6 service shutdown.
     virtual ~Dhcpv6Srv();
 
-    /// @brief Returns server-intentifier option.
+    /// @brief Returns server-indentifier option.
     ///
     /// @return server-id option
     OptionPtr getServerID() { return serverid_; }

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -88,7 +88,7 @@ public:
     /// option value. These parameters are: "name", "code", "data" and
     /// "csv-format".
     ///
-    /// @param param_value string holiding option parameter value to be
+    /// @param param_value string holding option parameter value to be
     /// injected into the configuration string.
     /// @param parameter name of the parameter to be configured with
     /// param value.

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -79,20 +79,12 @@ public:
 
 static const char* DUID_FILE = "server-id-test.txt";
 
-class Dhcpv6SrvTest : public ::testing::Test {
+// test fixture for any tests requiring blank/empty configuration
+// serves as base class for additional tests 
+class NakedDhcpv6SrvTest : public ::testing::Test {
 public:
-    /// Name of the server-id file (used in server-id tests)
-
-    // these are empty for now, but let's keep them around
-    Dhcpv6SrvTest() : rcode_(-1) {
-        subnet_ = Subnet6Ptr(new Subnet6(IOAddress("2001:db8:1::"), 48, 1000,
-                                         2000, 3000, 4000));
-        pool_ = Pool6Ptr(new Pool6(Pool6::TYPE_IA, IOAddress("2001:db8:1:1::"), 64));
-        subnet_->addPool(pool_);
-
-        CfgMgr::instance().deleteSubnets6();
-        CfgMgr::instance().addSubnet6(subnet_);
 
+    NakedDhcpv6SrvTest() : rcode_(-1) {
         // it's ok if that fails. There should not be such a file anyway
         unlink(DUID_FILE);
     }
@@ -142,25 +134,22 @@ public:
         EXPECT_TRUE(expected_clientid->getData() == tmp->getData());
     }
 
-    // Checks that server response (ADVERTISE or REPLY) contains proper IA_NA option
-    // It returns IAADDR option for each chaining with checkIAAddr method.
-    boost::shared_ptr<Option6IAAddr> checkIA_NA(const Pkt6Ptr& rsp, uint32_t expected_iaid,
-                                         uint32_t expected_t1, uint32_t expected_t2) {
-        OptionPtr tmp = rsp->getOption(D6O_IA_NA);
-        // Can't use ASSERT_TRUE() in method that returns something
-        if (!tmp) {
-            ADD_FAILURE() << "IA_NA option not present in response";
-            return (boost::shared_ptr<Option6IAAddr>());
-        }
+    // Checks if server response is a NAK
+    void checkNakResponse(const Pkt6Ptr& rsp, uint8_t expected_message_type,
+                          uint32_t expected_transid, 
+                          uint16_t expected_status_code) {
+        // Check if we get response at all
+        checkResponse(rsp, expected_message_type, expected_transid);
 
-        boost::shared_ptr<Option6IA> ia = boost::dynamic_pointer_cast<Option6IA>(tmp);
-        EXPECT_EQ(expected_iaid, ia->getIAID() );
-        EXPECT_EQ(expected_t1, ia->getT1());
-        EXPECT_EQ(expected_t2, ia->getT2());
+        // Check that IA_NA was returned 
+        OptionPtr option_ia_na = rsp->getOption(D6O_IA_NA);
+        ASSERT_TRUE(option_ia_na);
 
-        tmp = ia->getOption(D6O_IAADDR);
-        boost::shared_ptr<Option6IAAddr> addr = boost::dynamic_pointer_cast<Option6IAAddr>(tmp);
-        return (addr);
+        // check that the status is no address available
+        boost::shared_ptr<Option6IA> ia = boost::dynamic_pointer_cast<Option6IA>(option_ia_na);
+        ASSERT_TRUE(ia);
+
+        checkIA_NAStatusCode(ia, expected_status_code);
     }
 
     // Checks that server rejected IA_NA, i.e. that it has no addresses and
@@ -199,7 +188,6 @@ public:
         }
     }
 
-
     void checkMsgStatusCode(const Pkt6Ptr& msg, uint16_t expected_status) {
         boost::shared_ptr<OptionCustom> status =
             boost::dynamic_pointer_cast<OptionCustom>(msg->getOption(D6O_STATUS_CODE));
@@ -219,7 +207,71 @@ public:
         }
     }
 
-    // Check that generated IAADDR option contains expected address.
+    // Basic checks for generated response (message type and transaction-id).
+    void checkResponse(const Pkt6Ptr& rsp, uint8_t expected_message_type,
+                       uint32_t expected_transid) {
+        ASSERT_TRUE(rsp);
+        EXPECT_EQ(expected_message_type, rsp->getType());
+        EXPECT_EQ(expected_transid, rsp->getTransid());
+    }
+
+    virtual ~NakedDhcpv6SrvTest() {
+        // Let's clean up if there is such a file.
+        unlink(DUID_FILE);
+    };
+
+    // A DUID used in most tests (typically as client-id)
+    DuidPtr duid_;
+
+    int rcode_;
+    ConstElementPtr comment_;
+};
+
+// Provides suport for tests against a preconfigured subnet6                       
+// extends upon NakedDhcp6SrvTest
+class Dhcpv6SrvTest : public NakedDhcpv6SrvTest {
+public:
+    /// Name of the server-id file (used in server-id tests)
+
+    // these are empty for now, but let's keep them around
+    Dhcpv6SrvTest() {
+        subnet_ = Subnet6Ptr(new Subnet6(IOAddress("2001:db8:1::"), 48, 1000,
+                                         2000, 3000, 4000));
+        pool_ = Pool6Ptr(new Pool6(Pool6::TYPE_IA, IOAddress("2001:db8:1:1::"), 64));
+        subnet_->addPool(pool_);
+
+        CfgMgr::instance().deleteSubnets6();
+        CfgMgr::instance().addSubnet6(subnet_);
+    }
+
+    // Checks that server response (ADVERTISE or REPLY) contains proper IA_NA option
+    // It returns IAADDR option for each chaining with checkIAAddr method.
+    boost::shared_ptr<Option6IAAddr> checkIA_NA(const Pkt6Ptr& rsp, uint32_t expected_iaid,
+                                            uint32_t expected_t1, uint32_t expected_t2) {
+        OptionPtr tmp = rsp->getOption(D6O_IA_NA);
+        // Can't use ASSERT_TRUE() in method that returns something
+        if (!tmp) {
+            ADD_FAILURE() << "IA_NA option not present in response";
+            return (boost::shared_ptr<Option6IAAddr>());
+        }
+ 
+        boost::shared_ptr<Option6IA> ia = boost::dynamic_pointer_cast<Option6IA>(tmp);
+        if (!ia) {
+            ADD_FAILURE() << "IA_NA cannot convert option ptr to Option6";
+            return (boost::shared_ptr<Option6IAAddr>());
+        }
+
+        EXPECT_EQ(expected_iaid, ia->getIAID());
+        EXPECT_EQ(expected_t1, ia->getT1());
+        EXPECT_EQ(expected_t2, ia->getT2());
+ 
+        tmp = ia->getOption(D6O_IAADDR);
+        boost::shared_ptr<Option6IAAddr> addr = boost::dynamic_pointer_cast<Option6IAAddr>(tmp);
+        return (addr);
+    }
+
+    // Check that generated IAADDR option contains expected address
+    // and lifetime values match the configured subnet
     void checkIAAddr(const boost::shared_ptr<Option6IAAddr>& addr,
                      const IOAddress& expected_addr,
                      uint32_t /* expected_preferred */,
@@ -235,15 +287,8 @@ public:
         EXPECT_EQ(addr->getValid(), subnet_->getValid());
     }
 
-    // Basic checks for generated response (message type and transaction-id).
-    void checkResponse(const Pkt6Ptr& rsp, uint8_t expected_message_type,
-                       uint32_t expected_transid) {
-        ASSERT_TRUE(rsp);
-        EXPECT_EQ(expected_message_type, rsp->getType());
-        EXPECT_EQ(expected_transid, rsp->getTransid());
-    }
-
     // Checks if the lease sent to client is present in the database
+    // and is valid when checked agasint the configured subnet
     Lease6Ptr checkLease(const DuidPtr& duid, const OptionPtr& ia_na,
                          boost::shared_ptr<Option6IAAddr> addr) {
         boost::shared_ptr<Option6IA> ia = boost::dynamic_pointer_cast<Option6IA>(ia_na);
@@ -265,9 +310,6 @@ public:
 
     ~Dhcpv6SrvTest() {
         CfgMgr::instance().deleteSubnets6();
-
-        // Let's clean up if there is such a file.
-        unlink(DUID_FILE);
     };
 
     // A subnet used in most tests
@@ -275,13 +317,132 @@ public:
 
     // A pool used in most tests
     Pool6Ptr pool_;
+};
 
-    // A DUID used in most tests (typically as client-id)
-    DuidPtr duid_;
+// This test verifies that incoming SOLICIT can be handled properly when
+// there are no subnets configured. 
+//
+// This test sends a SOLICIT and the expected response 
+// is an ADVERTISE with STATUS_NoAddrsAvail and no address provided in the 
+// response
+TEST_F(NakedDhcpv6SrvTest, SolicitNoSubnet) {
+    NakedDhcpv6Srv srv(0);
+
+    Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
+    sol->setRemoteAddr(IOAddress("fe80::abcd"));
+    sol->addOption(generateIA(234, 1500, 3000));
+    OptionPtr clientid = generateClientId();
+    sol->addOption(clientid);
+
+    // Pass it to the server and get an advertise
+    Pkt6Ptr reply = srv.processSolicit(sol);
+
+    // check that we get the right NAK
+    checkNakResponse (reply, DHCPV6_ADVERTISE, 1234, STATUS_NoAddrsAvail);
+}
+
+// This test verifies that incoming REQUEST can be handled properly when
+// there are no subnets configured. 
+//
+// This test sends a REQUEST and the expected response 
+// is an REPLY with STATUS_NoAddrsAvail and no address provided in the 
+// response
+TEST_F(NakedDhcpv6SrvTest, RequestNoSubnet) {
+    NakedDhcpv6Srv srv(0);
+
+    // Let's create a REQUEST
+    Pkt6Ptr req = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234));
+    req->setRemoteAddr(IOAddress("fe80::abcd"));
+    boost::shared_ptr<Option6IA> ia = generateIA(234, 1500, 3000);
+
+    // with a hint
+    IOAddress hint("2001:db8:1:1::dead:beef");
+    OptionPtr hint_opt(new Option6IAAddr(D6O_IAADDR, hint, 300, 500));
+    ia->addOption(hint_opt);
+    req->addOption(ia);
+    OptionPtr clientid = generateClientId();
+    req->addOption(clientid);
+
+    // server-id is mandatory in REQUEST
+    req->addOption(srv.getServerID());
+
+    // Pass it to the server and hope for a REPLY
+    Pkt6Ptr reply = srv.processRequest(req);
+
+    // check that we get the right NAK
+    checkNakResponse (reply, DHCPV6_REPLY, 1234, STATUS_NoAddrsAvail);
+}
+
+// This test verifies that incoming RENEW can be handled properly, even when
+// no subnets are configured.
+//
+// This test sends a RENEW and the expected response 
+// is an REPLY with STATUS_NoBinding and no address provided in the 
+// response
+TEST_F(NakedDhcpv6SrvTest, RenewNoSubnet) {
+    NakedDhcpv6Srv srv(0);
+
+    const IOAddress addr("2001:db8:1:1::cafe:babe");
+    const uint32_t iaid = 234;
+
+    // Generate client-id also duid_
+    OptionPtr clientid = generateClientId();
+
+    // Let's create a RENEW
+    Pkt6Ptr req = Pkt6Ptr(new Pkt6(DHCPV6_RENEW, 1234));
+    req->setRemoteAddr(IOAddress("fe80::abcd"));
+    boost::shared_ptr<Option6IA> ia = generateIA(iaid, 1500, 3000);
+
+    OptionPtr renewed_addr_opt(new Option6IAAddr(D6O_IAADDR, addr, 300, 500));
+    ia->addOption(renewed_addr_opt);
+    req->addOption(ia);
+    req->addOption(clientid);
+
+    // Server-id is mandatory in RENEW
+    req->addOption(srv.getServerID());
+
+    // Pass it to the server and hope for a REPLY
+    Pkt6Ptr reply = srv.processRenew(req);
+
+    // check that we get the right NAK
+    checkNakResponse (reply, DHCPV6_REPLY, 1234, STATUS_NoBinding);
+}
+
+// This test verifies that incoming RELEASE can be handled properly, even when
+// no subnets are configured.
+//
+// This test sends a RELEASE and the expected response 
+// is an REPLY with STATUS_NoBinding and no address provided in the 
+// response
+TEST_F(NakedDhcpv6SrvTest, ReleaseNoSubnet) {
+    NakedDhcpv6Srv srv(0);
+
+    const IOAddress addr("2001:db8:1:1::cafe:babe");
+    const uint32_t iaid = 234;
+
+    // Generate client-id also duid_
+    OptionPtr clientid = generateClientId();
+
+    // Let's create a RELEASE
+    Pkt6Ptr req = Pkt6Ptr(new Pkt6(DHCPV6_RELEASE, 1234));
+    req->setRemoteAddr(IOAddress("fe80::abcd"));
+    boost::shared_ptr<Option6IA> ia = generateIA(iaid, 1500, 3000);
+
+    OptionPtr released_addr_opt(new Option6IAAddr(D6O_IAADDR, addr, 300, 500));
+    ia->addOption(released_addr_opt);
+    req->addOption(ia);
+    req->addOption(clientid);
+
+    // Server-id is mandatory in RELEASE
+    req->addOption(srv.getServerID());
+
+    // Pass it to the server and hope for a REPLY
+    Pkt6Ptr reply = srv.processRelease(req);
+
+    // check that we get the right NAK
+    checkNakResponse (reply, DHCPV6_REPLY, 1234, STATUS_NoBinding);
+}
 
-    int rcode_;
-    ConstElementPtr comment_;
-};
 
 // Test verifies that the Dhcpv6_srv class can be instantiated. It checks a mode
 // without open sockets and with sockets opened on a high port (to not require

src/bin/loadzone/run_loadzone.sh.in

@@ -25,7 +25,7 @@ export PYTHONPATH
 # required by loadable python modules.
 SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
 if test $SET_ENV_LIBRARY_PATH = yes; then
-	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/threads/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
 	export @ENV_LIBRARY_PATH@
 fi
 

src/bin/msgq/msgq.py.in

@@ -790,7 +790,7 @@ class MsgQ:
             if not self.running:
                 return
 
-            # TODO: Any config handlig goes here.
+            # TODO: Any config handling goes here.
 
             return isc.config.create_answer(0)
 

src/bin/msgq/tests/msgq_run_test.py

@@ -72,12 +72,21 @@ class MsgqRunTest(unittest.TestCase):
         # Start msgq
         self.__msgq = subprocess.Popen([MSGQ_PATH, '-s', SOCKET_PATH],
                                        close_fds=True)
-        # Wait for it to become ready (up to the alarm-set timeout)
-        while not os.path.exists(SOCKET_PATH):
-            # Just a short wait, so we don't hog CPU, but don't wait too long
-            time.sleep(0.01)
         # Some testing data
         self.__no_recpt = {"result": [-1, "No such recipient"]}
+        # Wait for it to become ready (up to the alarm-set timeout)
+        connection = None
+        while not connection:
+            try:
+                # If the msgq is ready, this'll succeed. If not, it'll throw
+                # session error.
+                connection = isc.cc.session.Session(SOCKET_PATH)
+            except isc.cc.session.SessionError:
+                time.sleep(0.1) # Retry after a short time
+        # We have the connection now, that means it works. Close this
+        # connection, we won't use it. Each test gets enough new connections
+        # of its own.
+        connection.close()
 
     def __message(self, data):
         """

src/bin/msgq/tests/msgq_test.py

@@ -186,7 +186,7 @@ class MsgQTest(unittest.TestCase):
         The test is not exhaustive as it doesn't test all combination
         of existence of the recipient, addressing schemes, want_answer
         header and the reply header. It is not needed, these should
-        be mostly independant. That means, for example, if the message
+        be mostly independent. That means, for example, if the message
         is a reply and there's no recipient to send it to, the error
         would not be generated no matter if we addressed the recipient
         by lname or group. If we included everything, the test would
@@ -338,7 +338,7 @@ class BadSocket:
         self.send_exception = send_exception
 
     # completely wrap all calls and member access
-    # (except explicitely overridden ones)
+    # (except explicitly overridden ones)
     def __getattr__(self, name, *args):
         attr = getattr(self.socket, name)
         if isinstance(attr, collections.Callable):
@@ -834,7 +834,7 @@ class SocketTests(unittest.TestCase):
         self.assertIsNone(self.__killed_socket)
 
     def test_send_data_interrupt(self):
-        '''send() is interruptted. send_data() returns 0, sock isn't killed.'''
+        '''send() is interrupted. send_data() returns 0, sock isn't killed.'''
         expected_blockings = []
         for eno in [errno.EAGAIN, errno.EWOULDBLOCK, errno.EINTR]:
             self.__sock_error.errno = eno

src/bin/stats/stats.py.in

@@ -194,7 +194,7 @@ class Stats:
         '''Constructor
 
         module_ccsession_class is parameterized so that test can specify
-        a mocked class to test the behavior without involing network I/O.
+        a mocked class to test the behavior without involving network I/O.
         In other cases this parameter shouldn't be specified.
 
         '''

src/bin/stats/stats_httpd.py.in

@@ -407,7 +407,7 @@ class StatsHttpd:
         old_config = self.config.copy()
         self.load_config(new_config)
         # If the http sockets aren't opened or
-        # if new_config doesn't have'listen_on', it returns
+        # if new_config doesn't have 'listen_on', it returns
         if len(self.httpd) == 0 or 'listen_on' not in new_config:
             return isc.config.ccsession.create_answer(0)
         self.close_httpd()

src/bin/stats/tests/b10-stats_test.py

@@ -376,7 +376,7 @@ class TestStats(unittest.TestCase):
                              'report_time': 42})),
                           ('update_module', ())], call_log)
 
-        # Then update faked timestamp so the intial polling will happen, and
+        # Then update faked timestamp so the initial polling will happen, and
         # confirm that.
         call_log = []
         stats.get_timestamp = lambda: 10

src/bin/xfrin/tests/xfrin_test.py

@@ -2393,7 +2393,7 @@ class TestXfrinProcess(unittest.TestCase):
                                    master_addrinfo, tsig_key)
 
         # An awkward check that would specifically identify an old bug
-        # where initialziation of XfrinConnection._tsig_ctx_creator caused
+        # where initialization of XfrinConnection._tsig_ctx_creator caused
         # self reference and subsequently led to reference leak.
         orig_ref = sys.getrefcount(conn)
         conn._tsig_ctx_creator = None

src/bin/xfrin/xfrin.py.in

@@ -188,7 +188,7 @@ def get_soa_serial(soa_rdata):
 
 class XfrinState:
     '''
-    The states of the incomding *XFR state machine.
+    The states of the incoming *XFR state machine.
 
     We (will) handle both IXFR and AXFR with a single integrated state
     machine because they cannot be distinguished immediately - an AXFR
@@ -270,7 +270,7 @@ class XfrinState:
     can be used as singleton objects.  For now, however, we always instantiate
     a new object for every state transition, partly because the introduction
     of singleton will make a code bit complicated, and partly because
-    the overhead of object instantiotion wouldn't be significant for xfrin.
+    the overhead of object instantiation wouldn't be significant for xfrin.
 
     '''
     def set_xfrstate(self, conn, new_state):

src/bin/zonemgr/zonemgr.py.in

@@ -191,7 +191,7 @@ class ZonemgrRefresh:
         self._set_zone_retry_timer(zone_name_class)
 
     def zone_handle_notify(self, zone_name_class, master):
-        """Handle an incomding NOTIFY message via the Auth module.
+        """Handle an incoming NOTIFY message via the Auth module.
 
         It returns True if the specified zone matches one of the locally
         configured list of secondary zones; otherwise returns False.

src/lib/acl/loader.h

@@ -329,7 +329,7 @@ public:
         const List &list(description->listValue());
         boost::shared_ptr<ACL<Context, Action> > result(
             new ACL<Context, Action>(default_action_));
-        // Run trough the list of elements
+        // Run through the list of elements
         for (List::const_iterator i(list.begin()); i != list.end(); ++i) {
             Map map;
             try {
@@ -417,7 +417,7 @@ private:
             }
             default: {
                 // This is the AND-abbreviated form. We need to create an
-                // AND (or "ALL") operator, loop trough the whole map and
+                // AND (or "ALL") operator, loop through the whole map and
                 // fill it in. We do a small trick - we create bunch of
                 // single-item maps, call this loader recursively (therefore
                 // it will get into the "case 1" branch, where there is

src/lib/cache/tests/testdata/message_nxdomain_large_ttl.wire

@@ -19,7 +19,7 @@ b1fe 8583
 ## Authority
 ##
 # example.org: type SOA, class IN, mname ns1.example.org
-# TTL: 3 Hourse, 1 second (10801seconds)
+# TTL: 3 Hours, 1 second (10801 seconds)
 c0 0e 00 06 00 01 00 00 2a 31 00 22 03 6e 73 31 c0
 0e 05 61 64 6d 69 6e c0 0e 00 00 04 d2 00 00 0e
 10 00 00 07 08 00 24 ea 00 00 00 2a 31

src/lib/config/ccsession.cc

@@ -165,7 +165,7 @@ namespace {
 // getValue() (main problem described in ticket #993)
 // This returns either the value set for the given relative id,
 // or its default value
-// (intentially defined here so this interface does not get
+// (intentionally defined here so this interface does not get
 // included in ConfigData as it is)
 ConstElementPtr getValueOrDefault(ConstElementPtr config_part,
                                   const std::string& relative_id,

src/lib/config/ccsession.h

@@ -313,12 +313,12 @@ public:
      *                  spec_is_filename is true (the default), then a
      *                  filename is assumed, otherwise a module name.
      * \param handler The handler functor called whenever there's a change.
-     *                Called once initally from this function. May be NULL
+     *                Called once initially from this function. May be NULL
      *                if you don't want any handler to be called and you're
      *                fine with requesting the data through
      *                getRemoteConfigValue() each time.
      *
-     *                The handler should not throw, or it'll fall trough and
+     *                The handler should not throw, or it'll fall through and
      *                the exception will get into strange places, probably
      *                aborting the application.
      * \param spec_is_filename Says if spec_name is filename or module name.
@@ -375,25 +375,29 @@ public:
         return (session_.group_sendmsg(msg, group, instance, to, want_answer));
     };
 
-    /**
-     * Receive a message from the underlying CC session.
-     * This has the same interface as isc::cc::Session::group_recvmsg()
-     *
-     * \param envelope see isc::cc::Session::group_recvmsg()
-     * \param msg see isc::cc::Session::group_recvmsg()
-     * \param nonblock see isc::cc::Session::group_recvmsg()
-     * \param seq see isc::cc::Session::group_recvmsg()
-     * \return see isc::cc::Session::group_recvmsg()
-     */
+    /// \brief Receive a message from the underlying CC session.
+    /// This has the same interface as isc::cc::Session::group_recvmsg()
+    ///
+    /// NOTE: until #2804 is resolved this method wouldn't work except in
+    /// very limited cases; don't try to use it until then.
+    ///
+    /// \param envelope see isc::cc::Session::group_recvmsg()
+    /// \param msg see isc::cc::Session::group_recvmsg()
+    /// \param nonblock see isc::cc::Session::group_recvmsg()
+    /// \param seq see isc::cc::Session::group_recvmsg()
+    /// \return see isc::cc::Session::group_recvmsg()
     bool groupRecvMsg(isc::data::ConstElementPtr& envelope,
                       isc::data::ConstElementPtr& msg,
                       bool nonblock = true,
                       int seq = -1) {
         return (session_.group_recvmsg(envelope, msg, nonblock, seq));
-    };
+    }
 
     /// \brief Send a command message and wait for the answer.
     ///
+    /// NOTE: until #2804 is resolved this method wouldn't work except in
+    /// very limited cases; don't try to use it until then.
+    ///
     /// This is mostly a convenience wrapper around groupSendMsg
     /// and groupRecvMsg, with some error handling.
     ///

src/lib/cryptolink/cryptolink.h

@@ -101,7 +101,7 @@ class CryptoLinkImpl;
 /// There is only one way to access it, through getCryptoLink(), which
 /// returns a reference to the initialized library. On the first call,
 /// it will be initialized automatically. You can however initialize it
-/// manually through a call to the initalize(), before your first call
+/// manually through a call to initialize(), before your first call
 /// to getCryptoLink. Any subsequent call to initialize() will be a
 /// noop.
 ///

src/lib/datasrc/.gitignore

@@ -3,3 +3,5 @@
 /datasrc_config.h
 /datasrc_config.h.pre
 /static.zone
+/sqlite3_datasrc_messages.cc
+/sqlite3_datasrc_messages.h

src/lib/datasrc/Makefile.am

@@ -24,8 +24,7 @@ CLEANFILES += datasrc_config.h
 CLEANFILES += static.zone
 
 lib_LTLIBRARIES = libb10-datasrc.la
-libb10_datasrc_la_SOURCES = data_source.h
-libb10_datasrc_la_SOURCES += exceptions.h
+libb10_datasrc_la_SOURCES = exceptions.h
 libb10_datasrc_la_SOURCES += zone.h zone_finder.h zone_finder.cc
 libb10_datasrc_la_SOURCES += zone_finder_context.cc
 libb10_datasrc_la_SOURCES += zone_iterator.h
@@ -39,10 +38,11 @@ libb10_datasrc_la_SOURCES += master_loader_callbacks.h
 libb10_datasrc_la_SOURCES += master_loader_callbacks.cc
 libb10_datasrc_la_SOURCES += rrset_collection_base.h rrset_collection_base.cc
 libb10_datasrc_la_SOURCES += zone_loader.h zone_loader.cc
+libb10_datasrc_la_SOURCES += cache_config.h cache_config.cc
 nodist_libb10_datasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 libb10_datasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
 
-pkglib_LTLIBRARIES = sqlite3_ds.la static_ds.la
+pkglib_LTLIBRARIES = sqlite3_ds.la
 
 sqlite3_ds_la_SOURCES = sqlite3_accessor.h sqlite3_accessor.cc
 sqlite3_ds_la_SOURCES += sqlite3_accessor_link.cc
@@ -53,12 +53,6 @@ sqlite3_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 sqlite3_ds_la_LIBADD += libb10-datasrc.la
 sqlite3_ds_la_LIBADD += $(SQLITE_LIBS)
 
-static_ds_la_SOURCES = static_datasrc_link.cc
-static_ds_la_SOURCES += static_datasrc.h
-static_ds_la_LDFLAGS = -module -avoid-version
-static_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-static_ds_la_LIBADD += libb10-datasrc.la
-
 libb10_datasrc_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 libb10_datasrc_la_LIBADD += $(top_builddir)/src/lib/dns/libb10-dns++.la
 libb10_datasrc_la_LIBADD += $(top_builddir)/src/lib/log/libb10-log.la

src/lib/datasrc/cache_config.cc

@@ -0,0 +1,198 @@
+// Copyright (C) 2013  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/cache_config.h>
+#include <datasrc/client.h>
+#include <datasrc/memory/load_action.h>
+#include <datasrc/memory/zone_data_loader.h>
+
+#include <util/memory_segment.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+
+#include <cc/data.h>
+#include <exceptions/exceptions.h>
+
+#include <boost/bind.hpp>
+
+#include <cassert>
+#include <map>
+#include <string>
+
+using namespace isc::data;
+
+namespace isc {
+namespace datasrc {
+namespace internal {
+
+namespace {
+bool
+getEnabledFromConf(const Element& conf) {
+    return (conf.contains("cache-enable") &&
+            conf.get("cache-enable")->boolValue());
+}
+
+std::string
+getSegmentTypeFromConf(const Element& conf) {
+    // If cache-type is not explicitly configured, use the default type.
+    // (Ideally we should retrieve the default from the spec).
+    if (!conf.contains("cache-type")) {
+        return ("local");
+    }
+    return (conf.get("cache-type")->stringValue());
+}
+}
+
+CacheConfig::CacheConfig(const std::string& datasrc_type,
+                         const DataSourceClient* datasrc_client,
+                         const Element& datasrc_conf,
+                         bool allowed) :
+    enabled_(allowed && getEnabledFromConf(datasrc_conf)),
+    segment_type_(getSegmentTypeFromConf(datasrc_conf)),
+    datasrc_client_(datasrc_client)
+{
+    ConstElementPtr params = datasrc_conf.get("params");
+    if (!params) {
+        params.reset(new NullElement());
+    }
+    if (datasrc_type == "MasterFiles") {
+        if (datasrc_client_) {
+            isc_throw(InvalidParameter,
+                      "data source client is given for MasterFiles");
+        }
+
+        if (!enabled_) {
+            isc_throw(CacheConfigError,
+                      "The cache must be enabled for the MasterFiles type");
+        }
+
+        typedef std::map<std::string, ConstElementPtr> ZoneToFile;
+        const ZoneToFile& zone_to_file = params->mapValue();
+        ZoneToFile::const_iterator const it_end = zone_to_file.end();
+        for (ZoneToFile::const_iterator it = zone_to_file.begin();
+             it != it_end;
+             ++it)
+        {
+            zone_config_[dns::Name(it->first)] = it->second->stringValue();
+        }
+    } else {
+        if (!datasrc_client_) {
+            isc_throw(InvalidParameter,
+                      "data source client is missing for data source type: "
+                      << datasrc_type);
+        }
+        if (!enabled_) {
+            return;
+        }
+
+        if (!datasrc_conf.contains("cache-zones")) {
+            isc_throw(NotImplemented, "Auto-detection of zones "
+                      "to cache is not yet implemented, supply "
+                      "cache-zones parameter");
+            // TODO: Auto-detect list of all zones in the
+            // data source.
+        }
+
+        const ConstElementPtr zones = datasrc_conf.get("cache-zones");
+        for (size_t i = 0; i < zones->size(); ++i) {
+            const dns::Name zone_name(zones->get(i)->stringValue());
+            if (!zone_config_.insert(Zones::value_type(zone_name,
+                                                       "")).second) {
+                isc_throw(CacheConfigError, "Duplicate cache zone: " <<
+                          zone_name);
+            }
+        }
+    }
+}
+
+namespace {
+
+// We would like to use boost::bind for this. However, the loadZoneData takes
+// a reference, while we have a shared pointer to the iterator -- and we need
+// to keep it alive as long as the ZoneWriter is alive. Therefore we can't
+// really just dereference it and pass it, since it would get destroyed once
+// the getCachedZoneWriter would end. This class holds the shared pointer
+// alive, otherwise is mostly simple.
+//
+// It might be doable with nested boost::bind, but it would probably look
+// more awkward and complicated than this.
+class IteratorLoader {
+public:
+    IteratorLoader(const dns::RRClass& rrclass, const dns::Name& name,
+                   const ZoneIteratorPtr& iterator) :
+        rrclass_(rrclass),
+        name_(name),
+        iterator_(iterator)
+    {}
+    memory::ZoneData* operator()(util::MemorySegment& segment) {
+        return (memory::loadZoneData(segment, rrclass_, name_, *iterator_));
+    }
+private:
+    const dns::RRClass rrclass_;
+    const dns::Name name_;
+    ZoneIteratorPtr iterator_;
+};
+
+// We can't use the loadZoneData function directly in boost::bind, since
+// it is overloaded and the compiler can't choose the correct version
+// reliably and fails. So we simply wrap it into an unique name.
+memory::ZoneData*
+loadZoneDataFromFile(util::MemorySegment& segment, const dns::RRClass& rrclass,
+                     const dns::Name& name, const std::string& filename)
+{
+    return (memory::loadZoneData(segment, rrclass, name, filename));
+}
+
+} // unnamed namespace
+
+memory::LoadAction
+CacheConfig::getLoadAction(const dns::RRClass& rrclass,
+                           const dns::Name& zone_name) const
+{
+    // First, check if the specified zone is configured to be cached.
+    Zones::const_iterator found = zone_config_.find(zone_name);
+    if (found == zone_config_.end()) {
+        return (memory::LoadAction());
+    }
+
+    if (!found->second.empty()) {
+        // This is "MasterFiles" data source.
+        return (boost::bind(loadZoneDataFromFile, _1, rrclass, zone_name,
+                            found->second));
+    }
+
+    // Otherwise there must be a "source" data source (ensured by constructor)
+    assert(datasrc_client_);
+
+    // If the specified zone name does not exist in our client of the source,
+    // DataSourceError is thrown, which is exactly the result what we
+    // want, so no need to handle it.
+    ZoneIteratorPtr iterator(datasrc_client_->getIterator(zone_name));
+    if (!iterator) {
+        // This shouldn't happen for a compliant implementation of
+        // DataSourceClient, but we'll protect ourselves from buggy
+        // implementations.
+        isc_throw(Unexpected, "getting LoadAction for " << zone_name
+                  << "/" << rrclass << " resulted in Null zone iterator");
+    }
+
+    // Wrap the iterator into the correct functor (which keeps it alive as
+    // long as it is needed).
+    return (IteratorLoader(rrclass, zone_name, iterator));
+}
+
+} // namespace internal
+} // namespace datasrc
+} // namespace isc

src/lib/datasrc/cache_config.h

@@ -0,0 +1,218 @@
+// Copyright (C) 2013  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_CACHE_CONFIG_H
+#define DATASRC_CACHE_CONFIG_H
+
+#include <exceptions/exceptions.h>
+
+#include <dns/dns_fwd.h>
+#include <cc/data.h>
+#include <datasrc/memory/load_action.h>
+
+#include <boost/noncopyable.hpp>
+
+#include <map>
+#include <string>
+
+namespace isc {
+namespace datasrc {
+class DataSourceClient;
+
+namespace internal {
+
+/// \brief Exception thrown for configuration error related to in-memory cache.
+class CacheConfigError : public Exception {
+public:
+    CacheConfigError(const char* file, size_t line, const char* what) :
+        Exception(file, line, what)
+    {}
+};
+
+/// \brief Configuration for in-memory cache of a data source.
+///
+/// This class understands and validates the configuration parameters for
+/// \c DataSourceClient related to in-memory cache, and converts it to native,
+/// type-safe objects for the convenience of the user of this class.
+/// Specifically, it allows the user to get the underlying memory segment
+/// type for the cache as a string and to iterate over zone names to be
+/// cached in memory.
+///
+/// It also provides unified interface for getting \c memory::LoadAction
+/// object that can be used for loading zones, regardless of the underlying
+/// data source properties, i.e., whether it's special "MasterFiles" type
+/// or other generic data sources.
+///
+/// This class is publicly defined so it can be tested directly, but
+/// it's essentially private to the \c ConfigurableClientList class.
+/// It's therefore defined in an "internal" namespace, and isn't expected
+/// to be used by other classes or user applications.  Likewise, this file
+/// is not expected to be installed with other publicly usable header files.
+///
+/// It's defined as noncopyable, simply because it's not expected to be
+/// copied in the intended usage for \c ConfigurableClientList.  Prohibiting
+/// copies will help avoid unexpected disruption due to accidental copy and
+/// sharing internal resources as a result of that.
+class CacheConfig : boost::noncopyable {
+public:
+    /// \brief Constructor.
+    ///
+    /// It performs the following validation on the given configuration:
+    /// - For the "MasterFiles" type
+    ///   - datasrc_client_ must not be provided (must be null); throws
+    ///     InvalidParameter otherwise.
+    ///   - cache must be enabled: "cache-enable" configuration item exists
+    ///     and is true, and allowed parameter is true, too; throws
+    ///     CacheConfigError otherwise.
+    ///   - "params" configuration item must be provided and of a map type,
+    ///     and each map entry maps a string to another string; throws
+    ///     data::TypeError otherwise.
+    ///   - the key string of each map entry must be a valid textual
+    ///     representation of a domain name.  Otherwise corresponding
+    ///     exception from the dns::Name class will be thrown.
+    /// - For other types
+    ///   - datasrc_client_ must be provided (must not be null); throws
+    ///     InvalidParameter otherwise.
+    ///   - (Unless cache is disabled) "cache-zones" configuration item must
+    ///     exist and must be a list of strings; throws data::TypeError
+    ///     otherwise.
+    ///   - Each string value of cache-zones entries must be a valid textual
+    ///     representation of a domain name.  Otherwise corresponding
+    ///     exception from the dns::Name class will be thrown.
+    ///   - Names in the list must not have duplicates;
+    ///     throws CacheConfigError otherwise.
+    ///
+    /// For other data source types than "MasterFiles", cache can be disabled.
+    /// In this case cache-zones configuration item is simply ignored, even
+    /// it contains an error that would otherwise trigger an exception.
+    ///
+    /// The specified set of zones (directly in "params" in case of
+    /// "MasterFile", and specified in "cache-zones" for others) can be
+    /// empty.
+    ///
+    /// This constructor also identifies the underlying memory segment type
+    /// used for the cache.  It's given via the "cache-type" configuration
+    /// item if defined; otherwise it defaults to "local".
+    ///
+    /// \throw InvalidParameter Program error at the caller side rather than
+    /// in the configuration (see above)
+    /// \throw CacheConfigError There is a semantics error in the given
+    /// configuration (see above)
+    /// \throw data::TypeError Invalid type of data is found in the
+    /// configuration (see above)
+    /// \throw Other Exceptions from the dns::Name class when conversion from
+    /// text fails (see above)
+    ///
+    /// \param datasrc_type Type of data source. This must be the "type"
+    /// value of the data source configuration.
+    /// \param datasrc_client Client of the underlying data source for the
+    /// cache, if it's used; for MasterFiles types it's null.
+    /// \param datasrc_conf Configuration element for the data source.
+    /// This must be the value of, e.g., data_sources/classes/IN[0] of
+    /// BIND 10 configuration.
+    /// \param allowed Whether in-memory cache is allowed by the process.
+    /// This must be derived from the allow_cache parameter of
+    /// \c ConfigurableClientList::configure().
+    CacheConfig(const std::string& datasrc_type,
+                const DataSourceClient* datasrc_client,
+                const data::Element& datasrc_conf,
+                bool allowed);
+
+    /// \brief Return if the cache is enabled.
+    ///
+    /// The cache is considered enabled iff the "cache-enable" configuration
+    /// item (given on construction) existed and was set to true, and
+    /// the \c allowed parameter to the constructor was true.
+    ///
+    /// \throw None
+    bool isEnabled() const { return (enabled_); }
+
+    /// \brief Return the memory segment type to be used for the zone table.
+    ///
+    /// \throw None
+    const std::string& getSegmentType() const { return (segment_type_); }
+
+    /// \brief Return a \c LoadAction functor to load zone data into memory.
+    ///
+    /// This method returns an appropriate \c LoadAction functor that can be
+    /// passed to a \c memory::ZoneWriter object to load data of the specified
+    /// zone into memory.  The source of the zone data differs depending on
+    /// the cache configuration (either a master file or another data source),
+    /// but this method hides the details and works as a unified interface
+    /// for the caller.
+    ///
+    /// If the specified zone is not configured to be cached, it returns an
+    /// empty functor (which can be evaluated to be \c false as a boolean).
+    /// It doesn't throw an exception in this case because the expected caller
+    /// of this method would handle such a case internally.
+    ///
+    /// \throw DataSourceError error happens in the underlying data source
+    /// storing the cache data.  Most commonly it's because the specified zone
+    /// doesn't exist there.
+    /// \throw Unexpected Unexpected error happens in the underlying data
+    /// source storing the cache data.  This shouldn't happen as long as the
+    /// data source implementation meets the public API requirement.
+    ///
+    /// \param rrclass The RR class of the zone
+    /// \param zone_name The origin name of the zone
+    /// \return A \c LoadAction functor to load zone data or an empty functor
+    /// (see above).
+    memory::LoadAction getLoadAction(const dns::RRClass& rrlcass,
+                                     const dns::Name& zone_name) const;
+
+    /// \brief Read only iterator type over configured cached zones.
+    ///
+    /// \note This initial version exposes the internal data structure (i.e.
+    /// map from name to string) through this public iterator type for
+    /// simplicity.  In terms of data encapsulation it's better to introduce
+    /// a custom iterator type that only goes through the conceptual list
+    /// of zone names, but due to the limitation of the expected user of this
+    /// class that would probably be premature generalization.  In future,
+    /// we might want to allow getting the list of zones directly from the
+    /// underlying data source.  If and when that happens we should introduce
+    /// a custom type.  In any case, the user of this class should only
+    /// use the typedef, not the original map iterator.  It should also
+    /// use this iterator as a forward iterator (datasource-based iterator
+    /// wouldn't be able to be bidirectional), and it shouldn't use the
+    /// value of the map entry (a string, specifying a path to master file
+    /// for MasterFiles data source).
+    typedef std::map<dns::Name, std::string>::const_iterator ConstZoneIterator;
+
+    /// \brief Return the beginning of cached zones in the form of iterator.
+    ConstZoneIterator begin() const { return (zone_config_.begin()); }
+
+    /// \brief Return the end of cached zones in the form of iterator.
+    ConstZoneIterator end() const { return (zone_config_.end()); }
+
+private:
+    const bool enabled_; // if the use of in-memory zone table is enabled
+    const std::string segment_type_;
+    // client of underlying data source, will be NULL for MasterFile datasrc
+    const DataSourceClient* datasrc_client_;
+
+    // Maps each of zones to be cached to a string.  For "MasterFiles" type
+    // of data source, the string is a path to the master zone file; for
+    // others it's an empty string.
+    typedef std::map<dns::Name, std::string> Zones;
+    Zones zone_config_;
+};
+}
+}
+}
+
+#endif  // DATASRC_CACHE_CONFIG_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/client.h

@@ -206,7 +206,7 @@ public:
     ///
     /// The default implementation throws isc::NotImplemented. This allows
     /// for easy and fast deployment of minimal custom data sources, where
-    /// the user/implementator doesn't have to care about anything else but
+    /// the user/implementer doesn't have to care about anything else but
     /// the actual queries. Also, in some cases, it isn't possible to traverse
     /// the zone from logic point of view (eg. dynamically generated zone
     /// data).

src/lib/datasrc/client_list.cc

@@ -13,16 +13,17 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 
-#include "client_list.h"
-#include "exceptions.h"
-#include "client.h"
-#include "factory.h"
-#include "memory/memory_client.h"
-#include "memory/zone_table_segment.h"
-#include "memory/zone_writer.h"
-#include "memory/zone_data_loader.h"
-#include "memory/zone_data_updater.h"
-#include "logger.h"
+#include <datasrc/client_list.h>
+#include <datasrc/exceptions.h>
+#include <datasrc/client.h>
+#include <datasrc/factory.h>
+#include <datasrc/cache_config.h>
+#include <datasrc/memory/memory_client.h>
+#include <datasrc/memory/zone_table_segment.h>
+#include <datasrc/memory/zone_writer.h>
+#include <datasrc/memory/zone_data_loader.h>
+#include <datasrc/memory/zone_data_updater.h>
+#include <datasrc/logger.h>
 #include <dns/masterload.h>
 #include <util/memory_segment_local.h>
 
@@ -30,6 +31,7 @@
 #include <set>
 #include <boost/foreach.hpp>
 #include <boost/bind.hpp>
+#include <boost/scoped_ptr.hpp>
 
 using namespace isc::data;
 using namespace isc::dns;
@@ -47,28 +49,18 @@ namespace datasrc {
 
 ConfigurableClientList::DataSourceInfo::DataSourceInfo(
     DataSourceClient* data_src_client,
-    const DataSourceClientContainerPtr& container, bool has_cache,
-    const RRClass& rrclass, const shared_ptr<ZoneTableSegment>& segment,
-    const string& name) :
+    const DataSourceClientContainerPtr& container,
+    boost::shared_ptr<internal::CacheConfig> cache_conf,
+    const RRClass& rrclass, const string& name) :
     data_src_client_(data_src_client),
     container_(container),
-    name_(name)
+    name_(name),
+    cache_conf_(cache_conf)
 {
-    if (has_cache) {
-        cache_.reset(new InMemoryClient(segment, rrclass));
-        ztable_segment_ = segment;
-    }
-}
-
-ConfigurableClientList::DataSourceInfo::DataSourceInfo(
-    const RRClass& rrclass, const shared_ptr<ZoneTableSegment>& segment,
-    bool has_cache, const string& name) :
-    data_src_client_(NULL),
-    name_(name)
-{
-    if (has_cache) {
-        cache_.reset(new InMemoryClient(segment, rrclass));
-        ztable_segment_ = segment;
+    if (cache_conf_ && cache_conf_->isEnabled()) {
+        ztable_segment_.reset(ZoneTableSegment::create(
+                                  rrclass, cache_conf_->getSegmentType()));
+        cache_.reset(new InMemoryClient(ztable_segment_, rrclass));
     }
 }
 
@@ -94,8 +86,6 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
     size_t i(0); // Outside of the try to be able to access it in the catch
     try {
         vector<DataSourceInfo> new_data_sources;
-        shared_ptr<ZoneTableSegment> ztable_segment(
-            ZoneTableSegment::create(*config, rrclass_));
         set<string> used_names;
         for (; i < config->size(); ++i) {
             // Extract the parameters
@@ -110,111 +100,69 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
             if (paramConf == ConstElementPtr()) {
                 paramConf.reset(new NullElement());
             }
-            const bool want_cache(allow_cache &&
-                                  dconf->contains("cache-enable") &&
-                                  dconf->get("cache-enable")->boolValue());
             // Get the name (either explicit, or guess)
             const ConstElementPtr name_elem(dconf->get("name"));
             const string name(name_elem ? name_elem->stringValue() : type);
             if (!used_names.insert(name).second) {
-                isc_throw(ConfigurationError, "Duplicit name in client list: "
+                isc_throw(ConfigurationError, "Duplicate name in client list: "
                           << name);
             }
 
-            if (type == "MasterFiles") {
-                // In case the cache is not allowed, we just skip the master
-                // files (at least for now)
-                if (!allow_cache) {
-                    // We're not going to load these zones. Issue warnings about it.
-                    const map<string, ConstElementPtr>
-                        zones_files(paramConf->mapValue());
-                    for (map<string, ConstElementPtr>::const_iterator
-                         it(zones_files.begin()); it != zones_files.end();
-                         ++it) {
-                        LOG_WARN(logger, DATASRC_LIST_NOT_CACHED).
-                            arg(it->first).arg(rrclass_);
-                    }
-                    continue;
-                }
-                if (!want_cache) {
-                    isc_throw(ConfigurationError, "The cache must be enabled "
-                              "for the MasterFiles type");
-                }
-                new_data_sources.push_back(DataSourceInfo(rrclass_,
-                                                          ztable_segment,
-                                                          true, name));
-            } else {
-                // Ask the factory to create the data source for us
-                const DataSourcePair ds(this->getDataSourceClient(type,
-                                                                  paramConf));
-                // And put it into the vector
-                new_data_sources.push_back(DataSourceInfo(ds.first, ds.second,
-                                                          want_cache, rrclass_,
-                                                          ztable_segment,
-                                                          name));
+            // Create a client for the underling data source via factory.
+            // If it's our internal type of data source, this is essentially
+            // no-op.  In the latter case, it's of no use unless cache is
+            // allowed; we simply skip building it in that case.
+            const DataSourcePair dsrc_pair = getDataSourceClient(type,
+                                                                 paramConf);
+            if (!allow_cache && !dsrc_pair.first) {
+                LOG_WARN(logger, DATASRC_LIST_NOT_CACHED).
+                    arg(name).arg(rrclass_);
+                continue;
             }
 
-            if (want_cache) {
-                if (!dconf->contains("cache-zones") && type != "MasterFiles") {
-                    isc_throw(isc::NotImplemented, "Auto-detection of zones "
-                              "to cache is not yet implemented, supply "
-                              "cache-zones parameter");
-                    // TODO: Auto-detect list of all zones in the
-                    // data source.
-                }
-
-                // List the zones we are loading
-                vector<string> zones_origins;
-                if (type == "MasterFiles") {
-                    const map<string, ConstElementPtr>
-                        zones_files(paramConf->mapValue());
-                    for (map<string, ConstElementPtr>::const_iterator
-                         it(zones_files.begin()); it != zones_files.end();
-                         ++it) {
-                        zones_origins.push_back(it->first);
-                    }
-                } else {
-                    const ConstElementPtr zones(dconf->get("cache-zones"));
-                    for (size_t i(0); i < zones->size(); ++i) {
-                        zones_origins.push_back(zones->get(i)->stringValue());
-                    }
+            // Build in-memory cache configuration, and create a set of
+            // related objects including the in-memory zone table for the
+            // cache.
+            boost::shared_ptr<internal::CacheConfig> cache_conf(
+                new internal::CacheConfig(type, dsrc_pair.first, *dconf,
+                                          allow_cache));
+            new_data_sources.push_back(DataSourceInfo(dsrc_pair.first,
+                                                      dsrc_pair.second,
+                                                      cache_conf, rrclass_,
+                                                      name));
+
+            // If cache is disabled we are done for this data source.
+            // Otherwise load zones into the in-memory cache.
+            if (!cache_conf->isEnabled()) {
+                continue;
+            }
+            internal::CacheConfig::ConstZoneIterator end_of_zones =
+                cache_conf->end();
+            for (internal::CacheConfig::ConstZoneIterator zone_it =
+                     cache_conf->begin();
+                 zone_it != end_of_zones;
+                 ++zone_it)
+            {
+                const Name& zname = zone_it->first;
+                memory::LoadAction load_action;
+                try {
+                    load_action = cache_conf->getLoadAction(rrclass_, zname);
+                } catch (const DataSourceError&) {
+                    isc_throw(ConfigurationError, "Data source error for "
+                              "loading a zone (possibly non-existent) "
+                              << zname << "/" << rrclass_);
                 }
-
-                const shared_ptr<InMemoryClient>
-                    cache(new_data_sources.back().cache_);
-                const DataSourceClient* const
-                    client(new_data_sources.back().data_src_client_);
-                for (vector<string>::const_iterator it(zones_origins.begin());
-                     it != zones_origins.end(); ++it) {
-                    const Name origin(*it);
-                    if (type == "MasterFiles") {
-                        try {
-                            cache->load(origin,
-                                        paramConf->get(*it)->stringValue());
-                        } catch (const ZoneLoaderException& e) {
-                            LOG_ERROR(logger, DATASRC_LOAD_FROM_FILE_ERROR)
-                                .arg(origin).arg(e.what());
-                        }
-                    } else {
-                        ZoneIteratorPtr iterator;
-                        try {
-                            iterator = client->getIterator(origin);
-                        } catch (const DataSourceError&) {
-                            isc_throw(ConfigurationError, "Unable to "
-                                      "cache non-existent zone "
-                                      << origin);
-                        }
-                        if (!iterator) {
-                            isc_throw(isc::Unexpected, "Got NULL iterator "
-                                      "for zone " << origin);
-                        }
-                        try {
-                            cache->load(origin, *iterator);
-                        } catch (const ZoneLoaderException& e) {
-                            LOG_ERROR(logger, DATASRC_LOAD_FROM_ITERATOR_ERROR)
-                                .arg(origin).arg(e.what());
-                        }
-                    }
+                assert(load_action); // in this loop this should be always true
+                boost::scoped_ptr<memory::ZoneWriter> writer;
+                try {
+                    writer.reset(new_data_sources.back().ztable_segment_->
+                                 getZoneWriter(load_action, zname, rrclass_));
+                    writer->load();
+                    writer->install();
+                    writer->cleanup();
+                } catch (const ZoneLoaderException& e) {
+                    LOG_ERROR(logger, DATASRC_LOAD_ZONE_ERROR)
+                        .arg(zname).arg(rrclass_).arg(name).arg(e.what());
                 }
             }
         }
@@ -227,6 +175,9 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
     } catch (const TypeError& te) {
         isc_throw(ConfigurationError, "Malformed configuration at data source "
                   "no. " << i << ": " << te.what());
+    } catch (const internal::CacheConfigError& ex) {
+        // convert to the "public" exception type.
+        isc_throw(ConfigurationError, ex.what());
     }
 }
 
@@ -375,46 +326,6 @@ ConfigurableClientList::reload(const Name& name) {
     return (ZONE_SUCCESS);
 }
 
-namespace {
-
-// We would like to use boost::bind for this. However, the loadZoneData takes
-// a reference, while we have a shared pointer to the iterator -- and we need
-// to keep it alive as long as the ZoneWriter is alive. Therefore we can't
-// really just dereference it and pass it, since it would get destroyed once
-// the getCachedZoneWriter would end. This class holds the shared pointer
-// alive, otherwise is mostly simple.
-//
-// It might be doable with nested boost::bind, but it would probably look
-// more awkward and complicated than this.
-class IteratorLoader {
-public:
-    IteratorLoader(const RRClass& rrclass, const Name& name,
-                   const ZoneIteratorPtr& iterator) :
-        rrclass_(rrclass),
-        name_(name),
-        iterator_(iterator)
-    {}
-    memory::ZoneData* operator()(util::MemorySegment& segment) {
-        return (memory::loadZoneData(segment, rrclass_, name_, *iterator_));
-    }
-private:
-    const RRClass rrclass_;
-    const Name name_;
-    ZoneIteratorPtr iterator_;
-};
-
-// We can't use the loadZoneData function directly in boost::bind, since
-// it is overloaded and the compiler can't choose the correct version
-// reliably and fails. So we simply wrap it into an unique name.
-memory::ZoneData*
-loadZoneDataFromFile(util::MemorySegment& segment, const RRClass& rrclass,
-                     const Name& name, const string& filename)
-{
-    return (memory::loadZoneData(segment, rrclass, name, filename));
-}
-
-}
-
 ConfigurableClientList::ZoneWriterPair
 ConfigurableClientList::getCachedZoneWriter(const Name& name) {
     if (!allow_cache_) {
@@ -426,36 +337,15 @@ ConfigurableClientList::getCachedZoneWriter(const Name& name) {
     if (!result.finder) {
         return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr()));
     }
-    // Try to get the in-memory cache for the zone. If there's none,
-    // we can't provide the result.
-    if (!result.info->cache_) {
+
+    // Then get the appropriate load action and create a zone writer.
+    // Note that getCacheConfig() must return non NULL in this module (only
+    // tests could set it to a bogus value).
+    const memory::LoadAction load_action =
+        result.info->getCacheConfig()->getLoadAction(rrclass_, name);
+    if (!load_action) {
         return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr()));
     }
-    memory::LoadAction load_action;
-    DataSourceClient* client(result.info->data_src_client_);
-    if (client != NULL) {
-        // Now finally provide the writer.
-        // If it does not exist in client,
-        // DataSourceError is thrown, which is exactly the result what we
-        // want, so no need to handle it.
-        ZoneIteratorPtr iterator(client->getIterator(name));
-        if (!iterator) {
-            isc_throw(isc::Unexpected, "Null iterator from " << name);
-        }
-        // And wrap the iterator into the correct functor (which
-        // keeps it alive as long as it is needed).
-        load_action = IteratorLoader(rrclass_, name, iterator);
-    } else {
-        // The MasterFiles special case
-        const string filename(result.info->cache_->getFileName(name));
-        if (filename.empty()) {
-            isc_throw(isc::Unexpected, "Confused about missing both filename "
-                      "and data source");
-        }
-        // boost::bind is enough here.
-        load_action = boost::bind(loadZoneDataFromFile, _1, rrclass_, name,
-                                  filename);
-    }
     return (ZoneWriterPair(ZONE_SUCCESS,
                            ZoneWriterPtr(
                                result.info->ztable_segment_->
@@ -470,6 +360,10 @@ ConfigurableClientList::getDataSourceClient(const string& type,
                                             const ConstElementPtr&
                                             configuration)
 {
+    if (type == "MasterFiles") {
+        return (DataSourcePair(0, DataSourceClientContainerPtr()));
+    }
+
     DataSourceClientContainerPtr
         container(new DataSourceClientContainer(type, configuration));
     return (DataSourcePair(&container->getInstance(), container));

src/lib/datasrc/client_list.h

@@ -46,6 +46,10 @@ class InMemoryClient;
 class ZoneWriter;
 }
 
+namespace internal {
+class CacheConfig;
+}
+
 /// \brief Segment status of the cache
 ///
 /// Describes the status in which the memory segment for the in-memory cache of
@@ -170,7 +174,7 @@ public:
 
         /// \brief Negative answer constructor.
         ///
-        /// This conscructs a result for negative answer. Both pointers are
+        /// This constructs a result for negative answer. Both pointers are
         /// NULL, and exact_match_ is false.
         FindResult() :
             dsrc_client_(NULL),
@@ -338,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.
     };
@@ -397,19 +403,11 @@ public:
     ///
     /// \todo The content yet to be defined.
     struct DataSourceInfo {
-        // Plays a role of default constructor too (for vector)
-        DataSourceInfo(const dns::RRClass& rrclass,
-                       const boost::shared_ptr
-                           <isc::datasrc::memory::ZoneTableSegment>&
-                               ztable_segment,
-                       bool has_cache = false,
-                       const std::string& name = std::string());
         DataSourceInfo(DataSourceClient* data_src_client,
                        const DataSourceClientContainerPtr& container,
-                       bool has_cache, const dns::RRClass& rrclass,
-                       const boost::shared_ptr
-                           <isc::datasrc::memory::ZoneTableSegment>&
-                               ztable_segment, const std::string& name);
+                       boost::shared_ptr<internal::CacheConfig> cache_conf,
+                       const dns::RRClass& rrclass,
+                       const std::string& name);
         DataSourceClient* data_src_client_;
         DataSourceClientContainerPtr container_;
 
@@ -422,6 +420,14 @@ public:
         boost::shared_ptr<memory::InMemoryClient> cache_;
         boost::shared_ptr<memory::ZoneTableSegment> ztable_segment_;
         std::string name_;
+
+        const internal::CacheConfig* getCacheConfig() const {
+            return (cache_conf_.get());
+        }
+    private:
+        // this is kept private for now.  When it needs to be accessed,
+        // we'll add a read-only getter method.
+        boost::shared_ptr<internal::CacheConfig> cache_conf_;
     };
 
     /// \brief The collection of data sources.
@@ -441,6 +447,13 @@ public:
     /// Also, derived classes could want to create the data source clients
     /// in a different way, though inheriting this class is not recommended.
     ///
+    /// Some types of data sources can be internal to the \c ClientList
+    /// implementation and do not require a corresponding dynamic module
+    /// loaded via \c DataSourceClientContainer.  In such a case, this method
+    /// simply returns a pair of null pointers.  It will help the caller reduce
+    /// type dependent processing.  Currently, "MasterFiles" is considered to
+    /// be this type of data sources.
+    ///
     /// The parameters are the same as of the constructor.
     /// \return Pair containing both the data source client and the container.
     ///     The container might be NULL in the derived class, it is

src/lib/datasrc/data_source.h

@@ -1,68 +0,0 @@
-// Copyright (C) 2009  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 DATA_SOURCE_H
-#define DATA_SOURCE_H
-
-#include <stdint.h>
-
-#include <vector>
-
-#include <boost/shared_ptr.hpp>
-
-#include <exceptions/exceptions.h>
-
-#include <dns/name.h>
-#include <dns/rrclass.h>
-#include <cc/data.h>
-
-namespace isc {
-
-namespace dns {
-class Name;
-class RRType;
-class RRset;
-class RRsetList;
-}
-
-namespace datasrc {
-
-/// This exception represents Backend-independent errors relating to
-/// data source operations.
-class DataSourceError : public Exception {
-public:
-    DataSourceError(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) {}
-};
-
-/// \brief No such serial number when obtaining difference iterator
-///
-/// Thrown if either the zone/start serial number or zone/end serial number
-/// combination does not exist in the differences table.  (Note that this
-/// includes the case where the differences table contains no records related
-/// to that zone.)
-class NoSuchSerial : public DataSourceError {
-public:
-    NoSuchSerial(const char* file, size_t line, const char* what) :
-        DataSourceError(file, line, what) {}
-};
-
-}
-}
-
-#endif
-
-// Local Variables:
-// mode: c++
-// End:

src/lib/datasrc/database.cc

@@ -17,7 +17,7 @@
 #include <vector>
 
 #include <datasrc/database.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/zone_iterator.h>
 #include <datasrc/rrset_collection_base.h>
 
@@ -30,7 +30,7 @@
 #include <dns/rdataclass.h>
 #include <dns/nsec3hash.h>
 
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/logger.h>
 
 #include <boost/foreach.hpp>
@@ -1644,17 +1644,23 @@ DatabaseUpdater::addRRset(const AbstractRRset& rrset) {
                 { cvtr.getName(), cvtr.getType(), cvtr.getTTL(), rdata_txt };
             accessor_->addRecordDiff(zone_id_, serial_.getValue(),
                                      Accessor::DIFF_ADD, journal);
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ADDDIFF).
+                arg(cvtr.getName()).arg(cvtr.getType()).arg(rdata_txt);
         }
         if (nsec3_type) {
             const string nsec3_columns[Accessor::ADD_NSEC3_COLUMN_COUNT] =
                 { cvtr.getNSEC3Name(), cvtr.getTTL(), cvtr.getType(),
                   rdata_txt };
             accessor_->addNSEC3RecordToZone(nsec3_columns);
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ADDNSEC3).
+                arg(cvtr.getNSEC3Name()).arg(rdata_txt);
         } else {
             const string columns[Accessor::ADD_COLUMN_COUNT] =
                 { cvtr.getName(), cvtr.getRevName(), cvtr.getTTL(),
                   cvtr.getType(), sigtype, rdata_txt };
             accessor_->addRecordToZone(columns);
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ADDRR).
+                arg(cvtr.getName()).arg(cvtr.getType()).arg(rdata_txt);
         }
     }
 }
@@ -1698,14 +1704,22 @@ DatabaseUpdater::deleteRRset(const AbstractRRset& rrset) {
                 { cvtr.getName(), cvtr.getType(), cvtr.getTTL(), rdata_txt };
             accessor_->addRecordDiff(zone_id_, serial_.getValue(),
                                      Accessor::DIFF_DELETE, journal);
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_DELETEDIFF).
+                arg(cvtr.getName()).arg(cvtr.getType()).arg(rdata_txt);
         }
-        const string params[Accessor::DEL_PARAM_COUNT] =
-            { nsec3_type ? cvtr.getNSEC3Name() : cvtr.getName(),
-              cvtr.getType(), rdata_txt };
         if (nsec3_type) {
+            const string params[Accessor::DEL_NSEC3_PARAM_COUNT] =
+                { cvtr.getNSEC3Name(), cvtr.getType(), rdata_txt };
             accessor_->deleteNSEC3RecordInZone(params);
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_DELETENSEC3).
+                arg(cvtr.getNSEC3Name()).arg(rdata_txt);
         } else {
+            const string params[Accessor::DEL_PARAM_COUNT] =
+                { cvtr.getName(), cvtr.getType(), rdata_txt,
+                  cvtr.getRevName() };
             accessor_->deleteRecordInZone(params);
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_DELETERR).
+                arg(cvtr.getName()).arg(cvtr.getType()).arg(rdata_txt);
         }
     }
 }

src/lib/datasrc/database.h

@@ -24,7 +24,7 @@
 #include <dns/rrset.h>
 #include <dns/rrtype.h>
 
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/client.h>
 #include <datasrc/zone.h>
 #include <datasrc/logger.h>
@@ -116,18 +116,42 @@ public:
         ADD_NSEC3_COLUMN_COUNT = 4 ///< Number of columns
     };
 
-    /// \brief Definitions of the fields to be passed to deleteRecordInZone()
-    /// and deleteNSEC3RecordInZone()
+    /// \brief Definitions of the fields to be passed to deleteRecordInZone().
     ///
     /// Each derived implementation of deleteRecordInZone() should expect
     /// the "params" array to be filled with the values as described in this
     /// enumeration, in this order.
+    ///
+    /// DEL_RNAME is included in case the reversed form is more convenient
+    /// for the underlying implementation to identify the record to be
+    /// deleted (reversed names are generally easier to sort, which may help
+    /// perform the search faster).  It's up to the underlying implementation
+    /// which one (or both) it uses for the search.   DEL_NAME and DEL_RNAME
+    /// are mutually convertible with the understanding of DNS names, and
+    /// in that sense redundant.  But both are provided so the underlying
+    /// implementation doesn't have to deal with DNS level concepts.
     enum DeleteRecordParams {
-        DEL_NAME = 0, ///< The owner name of the record (a domain name)
-                      ///< or the hash label for deleteNSEC3RecordInZone()
+        DEL_NAME = 0, ///< The owner name of the record (a domain name).
         DEL_TYPE = 1, ///< The RRType of the record (A/NS/TXT etc.)
         DEL_RDATA = 2, ///< Full text representation of the record's RDATA
-        DEL_PARAM_COUNT = 3 ///< Number of parameters
+        DEL_RNAME = 3, ///< As DEL_NAME, but with the labels of domain name
+                       ///< in reverse order (eg. org.example.).
+        DEL_PARAM_COUNT = 4 ///< Number of parameters
+    };
+
+    /// \brief Definitions of the fields to be passed to
+    /// deleteNSEC3RecordInZone().
+    ///
+    /// Each derived implementation of deleteNSEC3RecordInZone() should expect
+    /// the "params" array to be filled with the values as described in this
+    /// enumeration, in this order.
+    enum DeleteNSEC3RecordParams {
+        DEL_NSEC3_HASH = 0, ///< The hash (1st) label of the owren name,
+                            ///< excluding the dot character.
+        DEL_NSEC3_TYPE = 1, ///< The type of RR. Either RRSIG or NSEC3.
+        DEL_NSEC3_RDATA = 2, ///< Full text representation of the record's
+                             ///<  RDATA. Must match the one in the database.
+        DEL_NSEC3_PARAM_COUNT = 3 ///< Number of parameters.
     };
 
     /// \brief Operation mode when adding a record diff.
@@ -161,7 +185,7 @@ public:
     ///
     /// This method looks up a zone for the given name in the database. It
     /// should match only exact zone name (eg. name is equal to the zone's
-    /// apex), as the DatabaseClient will loop trough the labels itself and
+    /// apex), as the DatabaseClient will loop through the labels itself and
     /// find the most suitable zone.
     ///
     /// It is not specified if and what implementation of this method may
@@ -314,7 +338,7 @@ public:
     /// \note In case there are multiple NSEC3 chains and they collide
     ///     (unlikely, but it can happen), this can return multiple NSEC3
     ///     records.
-    /// \exception any Since any implementaion can be used, the caller should
+    /// \exception any Since any implementation can be used, the caller should
     ///     expect any exception to be thrown.
     /// \exception isc::NotImplemented in case the database does not support
     ///     NSEC3
@@ -576,11 +600,8 @@ public:
     /// \c addRecordToZone() and \c addNSEC3RecordToZone(), and the same
     /// notes apply to this method.
     ///
-    /// This method uses the same set of parameters to specify the record
-    /// to be deleted as \c deleteRecordInZone(), but the \c DEL_NAME column
-    /// is expected to only store the hash label of the owner name.
-    /// This is the same as \c ADD_NSEC3_HASH column for
-    /// \c addNSEC3RecordToZone().
+    /// This method uses the \c DeleteNSEC3RecordParams enum to specify the
+    /// values.
     ///
     /// \exception DataSourceError Invalid call without starting a transaction,
     /// or other internal database error.
@@ -590,7 +611,7 @@ public:
     /// \param params An array of strings that defines a record to be deleted
     /// from the NSEC3 namespace of the zone.
     virtual void deleteNSEC3RecordInZone(
-        const std::string (&params)[DEL_PARAM_COUNT]) = 0;
+        const std::string (&params)[DEL_NSEC3_PARAM_COUNT]) = 0;
 
     /// \brief Start a general transaction.
     ///
@@ -868,7 +889,7 @@ public:
     /// database.
     ///
     /// Application should not come directly in contact with this class
-    /// (it should handle it trough generic ZoneFinder pointer), therefore
+    /// (it should handle it through generic ZoneFinder pointer), therefore
     /// it could be completely hidden in the .cc file. But it is provided
     /// to allow testing and for rare cases when a database needs slightly
     /// different handling, so it can be subclassed.

src/lib/datasrc/datasrc_messages.mes

@@ -83,11 +83,37 @@ with the content. The problem does not stop the new version from being used
 but it should still be checked and fixed. See the message to know what exactly
 is wrong with the data.
 
+% DATASRC_DATABASE_ADDDIFF updated diff table for add: %1 %2 %3
+Debug message. A difference record for adding a record to the zone is being
+appended to the difference table. The name, type and rdata of the record is
+logged.
+
+% DATASRC_DATABASE_ADDNSEC3 added NSEC3 RR: %1 %2
+Debug message. A new NSEC3 record is added to the table. The hash and the rdata
+is logged.
+
+% DATASRC_DATABASE_ADDRR added RR: %1 %2 %3
+Debug message. A new resource record is added to the table. The name, type and
+rdata is logged.
+
 % DATASRC_DATABASE_COVER_NSEC_UNSUPPORTED %1 doesn't support DNSSEC when asked for NSEC data covering %2
 The datasource tried to provide an NSEC proof that the named domain does not
 exist, but the database backend doesn't support DNSSEC. No proof is included
 in the answer as a result.
 
+% DATASRC_DATABASE_DELETEDIFF updated diff table for delete: %1 %2 %3
+Debug message. A difference record for removing a record from the zone is being
+appended to the difference table. The name, type and rdata of the record is
+logged.
+
+% DATASRC_DATABASE_DELETENSEC3 deleted NSEC3 RR: %1 %2
+Debug message. An NSEC3 record is removed from the table. The name, type and
+rdata is logged.
+
+% DATASRC_DATABASE_DELETERR deleted RR: %1 %2 %3
+Debug message. A resource record is removed from the table. The name, type and
+rdata is logged.
+
 % DATASRC_DATABASE_FINDNSEC3 Looking for NSEC3 for %1 in %2 mode
 Debug information. A search in an database data source for NSEC3 that
 matches or covers the given name is being started.
@@ -321,22 +347,19 @@ not contain RRs the requested type.  AN NXRRSET indication is returned.
 A debug message indicating that a query for the given name and RR type is being
 processed.
 
-% DATASRC_LIST_NOT_CACHED zone %1/%2 not cached, cache disabled globally. Will not be available.
-The process disabled caching of RR data completely. However, the given zone
-is provided as a master file and it can be served from memory cache only.
-Therefore, the zone will not be available for this process. If this is
-a problem, you should move the zone to some database backend (sqlite3, for
-example) and use it from there.
-
-% DATASRC_LOAD_FROM_FILE_ERROR Error loading zone %1: %2
-An error was found in the zone data when it was being loaded from a
-file. The zone was not loaded. The specific error is shown in the
-message, and should be addressed.
-
-% DATASRC_LOAD_FROM_ITERATOR_ERROR Error loading zone %1: %2
-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_LIST_NOT_CACHED zones in data source %1 for class %2 not cached, cache disabled globally. Will not be available.
+The process disabled caching of RR data completely. However, this data source
+is provided from a master file and it can be served from memory cache only.
+Therefore, the entire data source will not be available for this process. If
+this is a problem, you should configure the zones of that data source to some
+database backend (sqlite3, for example) and use it from there.
+
+% DATASRC_LOAD_ZONE_ERROR Error loading zone %1/%2 on data source %3: %4
+During data source configuration, an error was found in the zone data
+when it was being loaded in to memory on the shown data source.  This
+particular zone was not loaded, but data source configuration
+continues, possibly loading other zones into memory. 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

src/lib/datasrc/exceptions.h

@@ -20,6 +20,26 @@
 namespace isc {
 namespace datasrc {
 
+/// This exception represents Backend-independent errors relating to
+/// data source operations.
+class DataSourceError : public Exception {
+public:
+    DataSourceError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/// \brief No such serial number when obtaining difference iterator
+///
+/// Thrown if either the zone/start serial number or zone/end serial number
+/// combination does not exist in the differences table.  (Note that this
+/// includes the case where the differences table contains no records related
+/// to that zone.)
+class NoSuchSerial : public DataSourceError {
+public:
+    NoSuchSerial(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/factory.cc

@@ -14,7 +14,7 @@
 
 #include "factory.h"
 
-#include "data_source.h"
+#include "exceptions.h"
 #include "database.h"
 #include "sqlite3_accessor.h"
 

src/lib/datasrc/factory.h

@@ -15,7 +15,7 @@
 #ifndef DATA_SOURCE_FACTORY_H
 #define DATA_SOURCE_FACTORY_H 1
 
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/client.h>
 
 #include <cc/data.h>

src/lib/datasrc/memory/domaintree.h

@@ -1684,7 +1684,7 @@ DomainTree<T>::previousNode(DomainTreeNodeChain<T>& node_path) const {
         }
     }
 
-    // Exchange the node at the top of the path, as we move horizontaly
+    // Exchange the node at the top of the path, as we move horizontally
     // through the domain tree
     node_path.pop();
     node_path.push(node);

src/lib/datasrc/memory/memory_client.cc

@@ -18,15 +18,11 @@
 #include <datasrc/memory/logger.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/rdataset.h>
-#include <datasrc/memory/segment_object_holder.h>
 #include <datasrc/memory/treenode_rrset.h>
 #include <datasrc/memory/zone_finder.h>
-#include <datasrc/memory/zone_data_loader.h>
 #include <datasrc/memory/zone_table_segment.h>
 
-#include <util/memory_segment_local.h>
-
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/factory.h>
 #include <datasrc/result.h>
 
@@ -34,12 +30,8 @@
 #include <dns/rdataclass.h>
 #include <dns/rrclass.h>
 
-#include <algorithm>
 #include <utility>
-#include <cctype>
-#include <cassert>
 
-using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 using namespace isc::datasrc::memory;
@@ -49,86 +41,14 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
-using detail::SegmentObjectHolder;
 using boost::shared_ptr;
 
-namespace { // unnamed namespace
-
-// A helper internal class used by the memory client, used for deleting
-// filenames stored in an internal tree.
-class FileNameDeleter {
-public:
-    FileNameDeleter() {}
-
-    void operator()(std::string* filename) const {
-        delete filename;
-    }
-};
-
-} // end of unnamed namespace
-
 InMemoryClient::InMemoryClient(shared_ptr<ZoneTableSegment> ztable_segment,
                                RRClass rrclass) :
     ztable_segment_(ztable_segment),
-    rrclass_(rrclass),
-    zone_count_(0),
-    file_name_tree_(FileNameTree::create(
-        ztable_segment_->getMemorySegment(), false))
+    rrclass_(rrclass)
 {}
 
-InMemoryClient::~InMemoryClient() {
-    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
-    FileNameDeleter deleter;
-    FileNameTree::destroy(mem_sgmt, file_name_tree_, deleter);
-}
-
-result::Result
-InMemoryClient::loadInternal(const isc::dns::Name& zone_name,
-                             const std::string& filename,
-                             ZoneData* zone_data)
-{
-    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
-    SegmentObjectHolder<ZoneData, RRClass> holder(
-        mem_sgmt, zone_data, rrclass_);
-
-    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_ADD_ZONE).
-        arg(zone_name).arg(rrclass_);
-
-    // Set the filename in file_name_tree_ now, so that getFileName()
-    // can use it (during zone reloading).
-    FileNameNode* node(NULL);
-    switch (file_name_tree_->insert(mem_sgmt, zone_name, &node)) {
-    case FileNameTree::SUCCESS:
-    case FileNameTree::ALREADYEXISTS:
-        // These are OK
-        break;
-    default:
-        // Can Not Happen
-        assert(false);
-    }
-    // node must point to a valid node now
-    assert(node != NULL);
-
-    const std::string* tstr = node->setData(new std::string(filename));
-    delete tstr;
-
-    ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
-    const ZoneTable::AddResult result(zone_table->addZone(mem_sgmt, rrclass_,
-                                                          zone_name,
-                                                          holder.release()));
-    if (result.code == result::SUCCESS) {
-        // Only increment the zone count if the zone doesn't already
-        // exist.
-        ++zone_count_;
-    }
-    // Destroy the old instance of the zone if there was any
-    if (result.zone_data != NULL) {
-        ZoneData::destroy(mem_sgmt, result.zone_data, rrclass_);
-    }
-
-    return (result.code);
-}
-
 RRClass
 InMemoryClient::getClass() const {
     return (rrclass_);
@@ -136,7 +56,8 @@ InMemoryClient::getClass() const {
 
 unsigned int
 InMemoryClient::getZoneCount() const {
-    return (zone_count_);
+    const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
+    return (zone_table->getZoneCount());
 }
 
 isc::datasrc::DataSourceClient::FindResult
@@ -162,39 +83,6 @@ InMemoryClient::findZoneData(const isc::dns::Name& zone_name) {
     return (result.zone_data);
 }
 
-result::Result
-InMemoryClient::load(const isc::dns::Name& zone_name,
-                     const std::string& filename)
-{
-    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_LOAD).arg(zone_name).
-        arg(filename);
-
-    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
-    ZoneData* zone_data = loadZoneData(mem_sgmt, rrclass_, zone_name,
-                                       filename);
-    return (loadInternal(zone_name, filename, zone_data));
-}
-
-result::Result
-InMemoryClient::load(const isc::dns::Name& zone_name, ZoneIterator& iterator) {
-    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
-    ZoneData* zone_data = loadZoneData(mem_sgmt, rrclass_, zone_name,
-                                       iterator);
-    return (loadInternal(zone_name, string(), zone_data));
-}
-
-const std::string
-InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
-    const FileNameNode* node(NULL);
-    const FileNameTree::Result result = file_name_tree_->find(zone_name,
-                                                              &node);
-    if (result == FileNameTree::EXACTMATCH) {
-        return (*node->getData());
-    } else {
-        return (std::string());
-    }
-}
-
 namespace {
 
 class MemoryIterator : public ZoneIterator {
@@ -369,7 +257,7 @@ InMemoryClient::getUpdater(const isc::dns::Name&, bool, bool) const {
     isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
 }
 
-pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
 InMemoryClient::getJournalReader(const isc::dns::Name&, uint32_t,
                                  uint32_t) const
 {

src/lib/datasrc/memory/memory_client.h

@@ -55,7 +55,7 @@ class ZoneTableSegment;
 class InMemoryClient : public DataSourceClient {
 public:
     ///
-    /// \name Constructors and Destructor.
+    /// \name Constructor.
     ///
     //@{
 
@@ -66,9 +66,6 @@ public:
     /// It never throws an exception otherwise.
     InMemoryClient(boost::shared_ptr<ZoneTableSegment> ztable_segment,
                    isc::dns::RRClass rrclass);
-
-    /// The destructor.
-    ~InMemoryClient();
     //@}
 
     /// \brief Returns the class of the data source client.
@@ -81,68 +78,6 @@ public:
     /// \return The number of zones stored in the client.
     virtual unsigned int getZoneCount() const;
 
-    /// \brief Load zone from masterfile.
-    ///
-    /// This loads data from masterfile specified by filename. It replaces
-    /// current content. The masterfile parsing ability is kind of limited,
-    /// see isc::dns::masterLoad.
-    ///
-    /// This throws isc::dns::MasterLoadError or AddError if there are
-    /// problems with loading (missing file, malformed data, unexpected
-    /// zone, etc. - see isc::dns::masterLoad for details).
-    ///
-    /// In case of internal problems, NullRRset or AssertError could
-    /// be thrown, but they should not be expected. Exceptions caused by
-    /// allocation may be thrown as well.
-    ///
-    /// If anything is thrown, the previous content is preserved (so it can
-    /// be used to update the data, but if user makes a typo, the old one
-    /// is kept).
-    ///
-    /// \param filename The master file to load.
-    ///
-    /// \todo We may need to split it to some kind of build and commit/abort.
-    ///     This will probably be needed when a better implementation of
-    ///     configuration reloading is written.
-    result::Result load(const isc::dns::Name& zone_name,
-                        const std::string& filename);
-
-    /// \brief Load zone from another data source.
-    ///
-    /// This is similar to the other version, but zone's RRsets are provided
-    /// by an iterator of another data source.  On successful load, the
-    /// internal filename will be cleared.
-    ///
-    /// This implementation assumes the iterator produces combined RRsets,
-    /// that is, there should exactly one RRset for the same owner name and
-    /// RR type.  This means the caller is expected to create the iterator
-    /// with \c separate_rrs being \c false.  This implementation also assumes
-    /// RRsets of different names are not mixed; so if the iterator produces
-    /// an RRset of a different name than that of the previous RRset, that
-    /// previous name must never appear in the subsequent sequence of RRsets.
-    /// Note that the iterator API does not ensure this.  If the underlying
-    /// implementation does not follow it, load() will fail.  Note, however,
-    /// that this whole interface is tentative.  in-memory zone loading will
-    /// have to be revisited fundamentally, and at that point this restriction
-    /// probably won't matter.
-    result::Result load(const isc::dns::Name& zone_name,
-                        ZoneIterator& iterator);
-
-    /// Return the master file name of the zone
-    ///
-    /// This method returns the name of the zone's master file to be loaded.
-    /// The returned string will be an empty unless the data source client has
-    /// successfully loaded the \c zone_name zone from a file before.
-    ///
-    /// This method should normally not throw an exception.  But the creation
-    /// of the return string may involve a resource allocation, and if it
-    /// fails, the corresponding standard exception will be thrown.
-    ///
-    /// \return The name of the zone file corresponding to the zone, or
-    /// an empty string if the client hasn't loaded the \c zone_name
-    /// zone from a file before.
-    const std::string getFileName(const isc::dns::Name& zone_name) const;
-
     /// Returns a \c ZoneFinder result that best matches the given name.
     ///
     /// This derived version of the method never throws an exception.
@@ -180,20 +115,8 @@ public:
                      uint32_t end_serial) const;
 
 private:
-    // Some type aliases
-    typedef DomainTree<std::string> FileNameTree;
-    typedef DomainTreeNode<std::string> FileNameNode;
-
-    // Common process for zone load. Registers filename internally and
-    // adds the ZoneData to the ZoneTable.
-    result::Result loadInternal(const isc::dns::Name& zone_name,
-                                const std::string& filename,
-                                ZoneData* zone_data);
-
     boost::shared_ptr<ZoneTableSegment> ztable_segment_;
     const isc::dns::RRClass rrclass_;
-    unsigned int zone_count_;
-    FileNameTree* file_name_tree_;
 };
 
 } // namespace memory

src/lib/datasrc/memory/memory_messages.mes

@@ -16,6 +16,10 @@ $NAMESPACE isc::datasrc::memory
 
 # \brief Messages for the data source memory library
 
+% DATASRC_MEMORY_ANY_SUCCESS ANY query for '%1' successful
+Debug information. The domain was found and an ANY type query is being answered
+by providing everything found inside the domain.
+
 % DATASRC_MEMORY_BAD_NSEC3_NAME NSEC3 record has a bad owner name '%1'
 The software refuses to load NSEC3 records into a wildcard domain or
 the owner name has two or more labels below the zone origin.
@@ -23,6 +27,71 @@ It isn't explicitly forbidden, but no sane zone wouldn have such names
 for NSEC3.  BIND 9 also refuses NSEC3 at wildcard, so this behavior is
 compatible with BIND 9.
 
+% DATASRC_MEMORY_CHECK_ERROR post-load check of zone %1/%2 failed: %3
+The zone was loaded into the data source successfully, but the content fails
+basic sanity checks. See the message if you want to know what exactly is wrong
+with the data. The data can not be used and previous version, if any, will be
+preserved.
+
+% DATASRC_MEMORY_CHECK_WARNING %1/%2: %3
+The zone was loaded into the data source successfully, but there's some problem
+with the content. The problem does not stop the new version from being used
+(though there may be other problems that do, see DATASRC_MEMORY_CHECK_ERROR),
+but it should still be checked and fixed. See the message to know what exactly
+is wrong with the data.
+
+% DATASRC_MEMORY_CNAME CNAME at the domain '%1'
+Debug information. The requested domain is an alias to a different domain,
+returning the CNAME instead.
+
+% DATASRC_MEMORY_DELEG_FOUND delegation found at '%1'
+Debug information. A delegation point was found above the requested record.
+
+% DATASRC_MEMORY_DNAME_ENCOUNTERED encountered a DNAME
+Debug information. While searching for the requested domain, a DNAME was
+encountered on the way.  This may lead to redirection to a different domain and
+stop the search.
+
+% DATASRC_MEMORY_DNAME_FOUND DNAME found at '%1'
+Debug information. A DNAME was found instead of the requested information.
+
+% DATASRC_MEMORY_DOMAIN_EMPTY requested domain '%1' is empty
+Debug information. The requested domain exists in the tree of domains, but
+it is empty. Therefore it doesn't contain the requested resource type.
+
+% DATASRC_MEMORY_EXACT_DELEGATION delegation at the exact domain '%1'
+Debug information. There's a NS record at the requested domain. This means
+this zone is not authoritative for the requested domain, but a delegation
+should be followed. The requested domain is an apex of some zone.
+
+% DATASRC_MEMORY_FINDNSEC3 finding NSEC3 for %1, mode %2
+Debug information. A search in an in-memory data source for NSEC3 that
+matches or covers the given name is being started.
+
+% DATASRC_MEMORY_FINDNSEC3_COVER found a covering NSEC3 for %1: %2
+Debug information. An NSEC3 that covers the given name is found and
+being returned.  The found NSEC3 RRset is also displayed.
+
+% DATASRC_MEMORY_FINDNSEC3_MATCH found a matching NSEC3 for %1 at label count %2: %3
+Debug information. An NSEC3 that matches (a possibly superdomain of)
+the given name is found and being returned.  When the shown label
+count is smaller than that of the given name, the matching NSEC3 is
+for a superdomain of the given name (see DATASRC_MEMORY_FINDNSEC3_TRYHASH).
+The found NSEC3 RRset is also displayed.
+
+% DATASRC_MEMORY_FINDNSEC3_TRYHASH looking for NSEC3 for %1 at label count %2 (hash %3)
+Debug information. In an attempt of finding an NSEC3 for the give name,
+(a possibly superdomain of) the name is hashed and searched for in the
+NSEC3 name space.  When the shown label count is smaller than that of the
+shown name, the search tries the superdomain name that share the shown
+(higher) label count of the shown name (e.g., for
+www.example.com. with shown label count of 3, example.com. is being
+tried).
+
+% DATASRC_MEMORY_FIND_TYPE_AT_ORIGIN origin query for type %1 in in-memory zone %2/%3 successful
+Debug information.  A specific type RRset is requested at a zone origin
+of an in-memory zone and it is found.
+
 % DATASRC_MEMORY_MEM_ADD_RRSET adding RRset '%1/%2' into zone '%3'
 Debug information. An RRset is being added to the in-memory data source.
 
@@ -57,9 +126,13 @@ RRset is split into multiple locations is not supported yet.
 Debug information. A zone object for this zone is being searched for in the
 in-memory data source.
 
-% DATASRC_MEMORY_MEM_LOAD loading zone '%1' from file '%2'
+% DATASRC_MEMORY_MEM_LOAD_FROM_FILE loading zone '%1/%2' from file '%3'
 Debug information. The content of master file is being loaded into the memory.
 
+% DATASRC_MEMORY_MEM_LOAD_FROM_DATASRC loading zone '%1/%2' from other data source
+Debug information. The content of another  data source is being loaded
+into the memory.
+
 % DATASRC_MEMORY_MEM_NO_NSEC3PARAM NSEC3PARAM is missing for NSEC3-signed zone %1/%2
 The in-memory data source has loaded a zone signed with NSEC3 RRs,
 but it doesn't have a NSEC3PARAM RR at the zone origin.  It's likely that
@@ -89,33 +162,15 @@ explicitly forbidden, but the protocol is ambiguous about how this should
 behave and BIND 9 refuses that as well. Please describe your intention using
 different tools.
 
-% DATASRC_MEMORY_CHECK_ERROR post-load check of zone %1/%2 failed: %3
-The zone was loaded into the data source successfully, but the content fails
-basic sanity checks. See the message if you want to know what exactly is wrong
-with the data. The data can not be used and previous version, if any, will be
-preserved.
-
-% DATASRC_MEMORY_CHECK_WARNING %1/%2: %3
-The zone was loaded into the data source successfully, but there's some problem
-with the content. The problem does not stop the new version from being used
-(though there may be other problems that do, see DATASRC_MEMORY_CHECK_ERROR),
-but it should still be checked and fixed. See the message to know what exactly
-is wrong with the data.
-
-% DATASRC_MEMORY_DNAME_ENCOUNTERED encountered a DNAME
-Debug information. While searching for the requested domain, a DNAME was
-encountered on the way.  This may lead to redirection to a different domain and
-stop the search.
+% DATASRC_MEMORY_NOT_FOUND requested domain '%1' not found
+Debug information. The requested domain does not exist.
 
 % DATASRC_MEMORY_NS_ENCOUNTERED encountered a NS
 Debug information. While searching for the requested domain, a NS was
 encountered on the way (a delegation). This may lead to stop of the search.
 
-% DATASRC_MEMORY_DNAME_FOUND DNAME found at '%1'
-Debug information. A DNAME was found instead of the requested information.
-
-% DATASRC_MEMORY_DELEG_FOUND delegation found at '%1'
-Debug information. A delegation point was found above the requested record.
+% DATASRC_MEMORY_SUCCESS query for '%1/%2' successful
+Debug information. The requested record was found.
 
 % DATASRC_MEMORY_SUPER_STOP stopped as '%1' is superdomain of a zone node, meaning it's empty
 Debug information. The search stopped because the requested domain was
@@ -128,54 +183,3 @@ doesn't have the requested record type).
 Debug information. A domain above wildcard was reached, but there's something
 below the requested domain. Therefore the wildcard doesn't apply here.  This
 behaviour is specified by RFC 1034, section 4.3.3.
-
-% DATASRC_MEMORY_NOT_FOUND requested domain '%1' not found
-Debug information. The requested domain does not exist.
-
-% DATASRC_MEMORY_FIND_TYPE_AT_ORIGIN origin query for type %1 in in-memory zone %2/%3 successful
-Debug information.  A specific type RRset is requested at a zone origin
-of an in-memory zone and it is found.
-
-% DATASRC_MEMORY_DOMAIN_EMPTY requested domain '%1' is empty
-Debug information. The requested domain exists in the tree of domains, but
-it is empty. Therefore it doesn't contain the requested resource type.
-
-% DATASRC_MEMORY_EXACT_DELEGATION delegation at the exact domain '%1'
-Debug information. There's a NS record at the requested domain. This means
-this zone is not authoritative for the requested domain, but a delegation
-should be followed. The requested domain is an apex of some zone.
-
-% DATASRC_MEMORY_ANY_SUCCESS ANY query for '%1' successful
-Debug information. The domain was found and an ANY type query is being answered
-by providing everything found inside the domain.
-
-% DATASRC_MEMORY_SUCCESS query for '%1/%2' successful
-Debug information. The requested record was found.
-
-% DATASRC_MEMORY_CNAME CNAME at the domain '%1'
-Debug information. The requested domain is an alias to a different domain,
-returning the CNAME instead.
-
-% DATASRC_MEMORY_FINDNSEC3 finding NSEC3 for %1, mode %2
-Debug information. A search in an in-memory data source for NSEC3 that
-matches or covers the given name is being started.
-
-% DATASRC_MEMORY_FINDNSEC3_COVER found a covering NSEC3 for %1: %2
-Debug information. An NSEC3 that covers the given name is found and
-being returned.  The found NSEC3 RRset is also displayed.
-
-% DATASRC_MEMORY_FINDNSEC3_MATCH found a matching NSEC3 for %1 at label count %2: %3
-Debug information. An NSEC3 that matches (a possibly superdomain of)
-the given name is found and being returned.  When the shown label
-count is smaller than that of the given name, the matching NSEC3 is
-for a superdomain of the given name (see DATASRC_MEMORY_FINDNSEC3_TRYHASH).
-The found NSEC3 RRset is also displayed.
-
-% DATASRC_MEMORY_FINDNSEC3_TRYHASH looking for NSEC3 for %1 at label count %2 (hash %3)
-Debug information. In an attempt of finding an NSEC3 for the give name,
-(a possibly superdomain of) the name is hashed and searched for in the
-NSEC3 name space.  When the shown label count is smaller than that of the
-shown name, the search tries the superdomain name that share the shown
-(higher) label count of the shown name (e.g., for
-www.example.com. with shown label count of 3, example.com. is being
-tried).

src/lib/datasrc/memory/rdata_serialization.h

@@ -364,7 +364,7 @@ struct RdataEncodeSpec;
 /// from the field sequence, you'll need to build the complete
 /// wire-format data, and then construct a dns::Rdata object from it.
 ///
-/// To use it, contstruct it with the data you got from RDataEncoder,
+/// To use it, construct it with the data you got from RDataEncoder,
 /// provide it with callbacks and then iterate through the data.
 /// The callbacks are called with the data fields contained in the
 /// data.

src/lib/datasrc/memory/zone_data_loader.cc

@@ -253,6 +253,9 @@ loadZoneData(util::MemorySegment& mem_sgmt,
              const isc::dns::Name& zone_name,
              const std::string& zone_file)
 {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_LOAD_FROM_FILE).
+        arg(zone_name).arg(rrclass).arg(zone_file);
+
      return (loadZoneDataInternal(mem_sgmt, rrclass, zone_name,
                                  boost::bind(masterLoaderWrapper,
                                              zone_file.c_str(),
@@ -266,6 +269,9 @@ loadZoneData(util::MemorySegment& mem_sgmt,
              const isc::dns::Name& zone_name,
              ZoneIterator& iterator)
 {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_LOAD_FROM_DATASRC).
+        arg(zone_name).arg(rrclass);
+
     return (loadZoneDataInternal(mem_sgmt, rrclass, zone_name,
                                  boost::bind(generateRRsetFromIterator,
                                              &iterator, _1)));

src/lib/datasrc/memory/zone_finder.cc

@@ -18,7 +18,7 @@
 #include <datasrc/memory/rdata_serialization.h>
 
 #include <datasrc/zone_finder.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <dns/labelsequence.h>
 #include <dns/name.h>
 #include <dns/rrset.h>

src/lib/datasrc/memory/zone_table.cc

@@ -12,14 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <util/memory_segment.h>
-
-#include <dns/name.h>
-
-#include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table.h>
+#include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/domaintree.h>
 #include <datasrc/memory/segment_object_holder.h>
+#include <datasrc/memory/logger.h>
+
+#include <util/memory_segment.h>
+
+#include <dns/name.h>
 
 #include <boost/function.hpp>
 #include <boost/bind.hpp>
@@ -70,6 +71,9 @@ ZoneTable::AddResult
 ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
                    const Name& zone_name, ZoneData* content)
 {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_ADD_ZONE).
+        arg(zone_name).arg(rrclass_);
+
     if (content == NULL) {
         isc_throw(isc::BadValue, "Zone content must not be NULL");
     }
@@ -94,6 +98,7 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     if (old != NULL) {
         return (AddResult(result::EXIST, old));
     } else {
+        ++zone_count_;
         return (AddResult(result::SUCCESS, NULL));
     }
 }

src/lib/datasrc/memory/zone_table.h

@@ -104,6 +104,7 @@ private:
     /// It never throws an exception otherwise.
     ZoneTable(const dns::RRClass& rrclass, ZoneTableTree* zones) :
         rrclass_(rrclass),
+        zone_count_(0),
         zones_(zones)
     {}
 
@@ -139,6 +140,11 @@ public:
     /// is undefined if this condition isn't met).
     static void destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable);
 
+    /// \brief Return the number of zones contained in the zone table.
+    ///
+    /// \throw None.
+    size_t getZoneCount() const { return (zone_count_); }
+
     /// Add a new zone to the \c ZoneTable.
     ///
     /// This method adds a given zone data to the internal table.
@@ -187,6 +193,7 @@ public:
 
 private:
     const dns::RRClass rrclass_;
+    size_t zone_count_;
     boost::interprocess::offset_ptr<ZoneTableTree> zones_;
 };
 }

src/lib/datasrc/memory/zone_table_segment.cc

@@ -15,6 +15,8 @@
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_table_segment_local.h>
 
+#include <string>
+
 using namespace isc::dns;
 
 namespace isc {
@@ -22,13 +24,15 @@ namespace datasrc {
 namespace memory {
 
 ZoneTableSegment*
-ZoneTableSegment::create(const isc::data::Element&, const RRClass& rrclass) {
-    /// FIXME: For now, we always return ZoneTableSegmentLocal. This
-    /// should be updated eventually to parse the passed Element
-    /// argument and construct a corresponding ZoneTableSegment
-    /// implementation.
-
-    return (new ZoneTableSegmentLocal(rrclass));
+ZoneTableSegment::create(const RRClass& rrclass, const std::string& type) {
+    // This will be a few sequences of if-else and hardcoded.  Not really
+    // sophisticated, but we don't expect to have too many types at the moment.
+    // Until that it becomes a real issue we won't be too smart.
+    if (type == "local") {
+        return (new ZoneTableSegmentLocal(rrclass));
+    }
+    isc_throw(UnknownSegmentType, "Zone table segment type not supported: "
+              << type);
 }
 
 void

src/lib/datasrc/memory/zone_table_segment.h

@@ -15,15 +15,20 @@
 #ifndef ZONE_TABLE_SEGMENT_H
 #define ZONE_TABLE_SEGMENT_H
 
+#include <exceptions/exceptions.h>
+
 #include <dns/rrclass.h>
+
 #include <datasrc/memory/zone_table.h>
-#include "load_action.h"
+#include <datasrc/memory/load_action.h>
+
 #include <cc/data.h>
 #include <util/memory_segment.h>
 
 #include <boost/interprocess/offset_ptr.hpp>
 
-#include <stdlib.h>
+#include <cstdlib>
+#include <string>
 
 namespace isc {
 // Some forward declarations
@@ -35,6 +40,15 @@ namespace datasrc {
 namespace memory {
 class ZoneWriter;
 
+/// \brief Exception thrown when unknown or unsupported type of zone table
+/// segment is specified.
+class UnknownSegmentType : public Exception {
+public:
+    UnknownSegmentType(const char* file, size_t line, const char* what) :
+        Exception(file, line, what)
+    {}
+};
+
 /// \brief Memory-management independent entry point that contains a
 /// pointer to a zone table in memory.
 ///
@@ -97,26 +111,14 @@ public:
     /// dynamically-allocated object. The caller is responsible for
     /// destroying it with \c ZoneTableSegment::destroy().
     ///
-    /// FIXME: For now, we always return ZoneTableSegmentLocal
-    /// regardless of the passed \c config.
+    /// \throw UnknownSegmentType The memory segment type specified in
+    /// \c config is not known or not supported in this implementation.
     ///
-    /// \param config The configuration based on which a derived object
-    ///               is returned.
-    /// \return Returns a ZoneTableSegment object
-    static ZoneTableSegment* create(const isc::data::Element& config,
-                                    const isc::dns::RRClass& rrclass);
-
-    /// \brief Temporary/Testing version of create.
-    ///
-    /// This exists as a temporary solution during the migration phase
-    /// towards using the ZoneTableSegment. It doesn't take a config,
-    /// but a memory segment instead. If you can, you should use the
-    /// other version, this one will be gone soon.
-    ///
-    /// \param segment The memory segment to use.
-    /// \return Returns a new ZoneTableSegment object.
-    /// \todo Remove this method.
-    static ZoneTableSegment* create(isc::util::MemorySegment& segment);
+    /// \param rrclass The RR class of the zones to be maintained in the table.
+    /// \param type The memory segment type used for the zone table segment.
+    /// \return Returns a ZoneTableSegment object of the specified type.
+    static ZoneTableSegment* create(const isc::dns::RRClass& rrclass,
+                                    const std::string& type);
 
     /// \brief Destroy a ZoneTableSegment
     ///
@@ -147,3 +149,7 @@ public:
 } // namespace isc
 
 #endif // ZONE_TABLE_SEGMENT_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/sqlite3_accessor.cc

@@ -25,7 +25,7 @@
 #include <datasrc/sqlite3_accessor.h>
 #include <datasrc/sqlite3_datasrc_messages.h>
 #include <datasrc/logger.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/factory.h>
 #include <datasrc/database.h>
 #include <util/filename.h>
@@ -104,7 +104,9 @@ const char* const text_statements[NUM_STATEMENTS] = {
     "INSERT INTO records "      // ADD_RECORD
         "(zone_id, name, rname, ttl, rdtype, sigtype, rdata) "
         "VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
-    "DELETE FROM records WHERE zone_id=?1 AND name=?2 " // DEL_RECORD
+    // DEL_RECORD:
+    // Delete based on the reverse name, as that one has an index.
+    "DELETE FROM records WHERE zone_id=?1 AND rname=?2 " // DEL_RECORD
         "AND rdtype=?3 AND rdata=?4",
 
     // ITERATE_RECORDS:
@@ -1295,19 +1297,27 @@ SQLite3Accessor::deleteRecordInZone(const string (&params)[DEL_PARAM_COUNT]) {
         isc_throw(DataSourceError, "deleting record in SQLite3 "
                   "data source without transaction");
     }
-    doUpdate<const string (&)[DEL_PARAM_COUNT]>(
-        *dbparameters_, DEL_RECORD, params, "delete record from zone");
+    // We don't pass all the parameters to the query, one name (reserve one
+    // in this case) is sufficient. Pass only the needed ones.
+    const size_t SQLITE3_DEL_PARAM_COUNT = DEL_PARAM_COUNT - 1;
+    const string sqlite3_params[SQLITE3_DEL_PARAM_COUNT] = {
+        params[DEL_RNAME],
+        params[DEL_TYPE],
+        params[DEL_RDATA]
+    };
+    doUpdate<const string (&)[SQLITE3_DEL_PARAM_COUNT]>(
+        *dbparameters_, DEL_RECORD, sqlite3_params, "delete record from zone");
 }
 
 void
 SQLite3Accessor::deleteNSEC3RecordInZone(
-    const string (&params)[DEL_PARAM_COUNT])
+    const string (&params)[DEL_NSEC3_PARAM_COUNT])
 {
     if (!dbparameters_->updating_zone) {
         isc_throw(DataSourceError, "deleting NSEC3-related record in SQLite3 "
                   "data source without transaction");
     }
-    doUpdate<const string (&)[DEL_PARAM_COUNT]>(
+    doUpdate<const string (&)[DEL_NSEC3_PARAM_COUNT]>(
         *dbparameters_, DEL_NSEC3_RECORD, params,
         "delete NSEC3 record from zone");
 }

src/lib/datasrc/sqlite3_accessor.h

@@ -17,7 +17,7 @@
 #define DATASRC_SQLITE3_ACCESSOR_H
 
 #include <datasrc/database.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 
 #include <exceptions/exceptions.h>
 
@@ -230,7 +230,7 @@ public:
         const std::string (&params)[DEL_PARAM_COUNT]);
 
     virtual void deleteNSEC3RecordInZone(
-        const std::string (&params)[DEL_PARAM_COUNT]);
+        const std::string (&params)[DEL_NSEC3_PARAM_COUNT]);
 
     /// This derived version of the method prepares an SQLite3 statement
     /// for adding the diff first time it's called, and if it fails throws
@@ -250,7 +250,7 @@ public:
     virtual std::string findPreviousName(int zone_id, const std::string& rname)
         const;
 
-    /// \brief Conrete implemantion of the pure virtual method of
+    /// \brief Concrete implementation of the pure virtual method of
     /// DatabaseAccessor
     virtual std::string findPreviousNSEC3Hash(int zone_id,
                                               const std::string& hash) const;

src/lib/datasrc/static_datasrc.h

@@ -1,50 +0,0 @@
-// Copyright (C) 2013  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_STATIC_H
-#define DATASRC_STATIC_H
-
-#include <datasrc/database.h>
-#include <cc/data.h>
-
-#include <string>
-
-namespace isc {
-namespace datasrc {
-
-/// \brief Creates an instance of the static datasource client
-///
-/// Currently the configuration passed here must be a StringElement,
-/// containing the path to a zone file for the BIND./CH zone.
-///
-/// \param config The configuration for the datasource instance (see above)
-/// \param error This string will be set to an error message if an error occurs
-///              during initialization
-/// \return An instance of the static datasource client, or NULL if there was
-///         an error
-extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config,
-                                            std::string& error);
-
-/// \brief Destroy the instance created by createInstance()
-extern "C" void destroyInstance(DataSourceClient* instance);
-
-}
-}
-
-#endif  // DATASRC_STATIC_H
-
-// Local Variables:
-// mode: c++
-// End:

src/lib/datasrc/static_datasrc_link.cc

@@ -1,68 +0,0 @@
-// 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 "client.h"
-#include "static_datasrc.h"
-#include <datasrc/memory/memory_client.h>
-#include <datasrc/memory/zone_table_segment.h>
-
-#include <cc/data.h>
-#include <dns/rrclass.h>
-
-#include <memory>
-#include <exception>
-
-using namespace isc::data;
-using namespace isc::dns;
-using namespace boost;
-using namespace std;
-
-namespace isc {
-namespace datasrc {
-
-DataSourceClient*
-createInstance(ConstElementPtr config, string& error) {
-    try {
-        // FIXME: Fix the config that should be passed to
-        // ZoneTableSegment::create() when it actually uses the config
-        // to do something.
-        shared_ptr<memory::ZoneTableSegment> ztable_segment(
-            memory::ZoneTableSegment::create(isc::data::NullElement(),
-                                             RRClass::CH()));
-        // Create the data source
-        auto_ptr<memory::InMemoryClient> client
-            (new memory::InMemoryClient(ztable_segment, RRClass::CH()));
-
-        // Fill it with data
-        const string path(config->stringValue());
-        client->load(Name("BIND"), path);
-
-        return (client.release());
-    }
-    catch (const std::exception& e) {
-        error = e.what();
-    }
-    catch (...) {
-        error = "Unknown exception";
-    }
-    return (NULL);
-}
-
-void
-destroyInstance(DataSourceClient* instance) {
-    delete instance;
-}
-
-}
-}

src/lib/datasrc/tests/Makefile.am

@@ -48,6 +48,7 @@ common_ldadd += $(GTEST_LDADD) $(SQLITE_LIBS)
 run_unittests_SOURCES = $(common_sources)
 
 run_unittests_SOURCES += test_client.h test_client.cc
+run_unittests_SOURCES += mock_client.h mock_client.cc
 run_unittests_SOURCES += logger_unittest.cc
 run_unittests_SOURCES += client_unittest.cc
 run_unittests_SOURCES += database_unittest.h database_unittest.cc
@@ -58,6 +59,7 @@ run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += client_list_unittest.cc
 run_unittests_SOURCES += master_loader_callbacks_test.cc
 run_unittests_SOURCES += zone_loader_unittest.cc
+run_unittests_SOURCES += cache_config_unittest.cc
 
 # We need the actual module implementation in the tests (they are not part
 # of libdatasrc)

src/lib/datasrc/tests/cache_config_unittest.cc

@@ -0,0 +1,312 @@
+// Copyright (C) 2013  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/cache_config.h>
+#include <datasrc/exceptions.h>
+#include <datasrc/memory/load_action.h>
+#include <datasrc/memory/zone_data.h>
+#include <datasrc/tests/mock_client.h>
+
+#include <cc/data.h>
+#include <util/memory_segment_local.h>
+#include <dns/name.h>
+#include <dns/rrclass.h>
+
+#include <gtest/gtest.h>
+
+#include <iterator>             // for std::distance
+
+using namespace isc::datasrc;
+using namespace isc::data;
+using namespace isc::dns;
+using isc::datasrc::unittest::MockDataSourceClient;
+using isc::datasrc::internal::CacheConfig;
+using isc::datasrc::internal::CacheConfigError;
+using isc::datasrc::memory::LoadAction;
+using isc::datasrc::memory::ZoneData;
+
+namespace {
+
+const char* zones[] = {
+    "example.org.",
+    "example.com.",
+    "null.org",                 // test for bad iterator case
+    NULL
+};
+
+class CacheConfigTest : public ::testing::Test {
+protected:
+    CacheConfigTest() :
+        mock_client_(zones),
+        master_config_(Element::fromJSON(
+                           "{\"cache-enable\": true,"
+                           " \"params\": "
+                           "  {\".\": \"" TEST_DATA_DIR "/root.zone\"}"
+                           "}")),
+        mock_config_(Element::fromJSON("{\"cache-enable\": true,"
+                                       " \"cache-zones\": [\".\"]}"))
+    {}
+
+    virtual void TearDown() {
+        EXPECT_TRUE(msgmt_.allMemoryDeallocated());
+    }
+
+    MockDataSourceClient mock_client_;
+    const ConstElementPtr master_config_; // valid config for MasterFiles
+    const ConstElementPtr mock_config_; // valid config for MasterFiles
+    isc::util::MemorySegmentLocal msgmt_;
+};
+
+size_t
+countZones(const CacheConfig& cache_config) {
+    return (std::distance(cache_config.begin(), cache_config.end()));
+}
+
+TEST_F(CacheConfigTest, constructMasterFiles) {
+    // A simple case: configuring a MasterFiles table with a single zone
+    const CacheConfig cache_conf("MasterFiles", 0, *master_config_, true);
+    EXPECT_EQ(1, countZones(cache_conf));
+
+    // With multiple zones.  Note that the constructor doesn't check if the
+    // file exists, so they can be anything.
+    const ConstElementPtr config_elem_multi(
+        Element::fromJSON("{\"cache-enable\": true,"
+                          " \"params\": "
+                          "{\"example.com\": \"file1\","
+                          " \"example.org\": \"file2\","
+                          " \"example.info\": \"file3\"}"
+                          "}"));
+    const CacheConfig cache_conf2("MasterFiles", 0, *config_elem_multi, true);
+    EXPECT_EQ(3, countZones(cache_conf2));
+
+    // A bit unusual, but acceptable case: empty parameters, so no zones.
+    const CacheConfig cache_conf3("MasterFiles", 0,
+                                  *Element::fromJSON("{\"cache-enable\": true,"
+                                                     " \"params\": {}}"),
+                                  true);
+    EXPECT_EQ(0, countZones(cache_conf3));
+}
+
+TEST_F(CacheConfigTest, badConstructMasterFiles) {
+    // no "params"
+    EXPECT_THROW(CacheConfig("MasterFiles", 0,
+                             *Element::fromJSON("{\"cache-enable\": true}"),
+                             true),
+                 isc::data::TypeError);
+
+    // no "cache-enable"
+    EXPECT_THROW(CacheConfig("MasterFiles", 0,
+                             *Element::fromJSON("{\"params\": {}}"), true),
+                 CacheConfigError);
+    // cache disabled for MasterFiles
+    EXPECT_THROW(CacheConfig("MasterFiles", 0,
+                             *Element::fromJSON("{\"cache-enable\": false,"
+                                                " \"params\": {}}"), true),
+                 CacheConfigError);
+    // cache enabled but not "allowed"
+    EXPECT_THROW(CacheConfig("MasterFiles", 0,
+                             *Element::fromJSON("{\"cache-enable\": false,"
+                                                " \"params\": {}}"), false),
+                 CacheConfigError);
+    // type error for cache-enable
+    EXPECT_THROW(CacheConfig("MasterFiles", 0,
+                             *Element::fromJSON("{\"cache-enable\": 1,"
+                                                " \"params\": {}}"), true),
+                 isc::data::TypeError);
+
+    // "params" is not a map
+    EXPECT_THROW(CacheConfig("MasterFiles", 0,
+                             *Element::fromJSON("{\"cache-enable\": true,"
+                                                " \"params\": []}"), true),
+                 isc::data::TypeError);
+
+    // bogus zone name
+    const ConstElementPtr bad_config(Element::fromJSON(
+                                         "{\"cache-enable\": true,"
+                                         " \"params\": "
+                                         "{\"bad..name\": \"file1\"}}"));
+    EXPECT_THROW(CacheConfig("MasterFiles", 0, *bad_config, true),
+                 isc::dns::EmptyLabel);
+
+    // file name is not a string
+    const ConstElementPtr bad_config2(Element::fromJSON(
+                                          "{\"cache-enable\": true,"
+                                          " \"params\": {\".\": 1}}"));
+    EXPECT_THROW(CacheConfig("MasterFiles", 0, *bad_config2, true),
+                 isc::data::TypeError);
+
+    // Specify data source client (must be null for MasterFiles)
+    EXPECT_THROW(CacheConfig("MasterFiles", &mock_client_,
+                             *Element::fromJSON("{\"cache-enable\": true,"
+                                                " \"params\": {}}"), true),
+                 isc::InvalidParameter);
+}
+
+TEST_F(CacheConfigTest, getLoadActionWithMasterFiles) {
+    uint8_t labels_buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+
+    const CacheConfig cache_conf("MasterFiles", 0, *master_config_, true);
+
+    // Check getLoadAction.  Since it returns a mere functor, we can only
+    // check the behavior by actually calling it.  For the purpose of this
+    // test, it should suffice if we confirm the call succeeds and shows
+    // some reasonably valid behavior (we'll check the origin name for that).
+    LoadAction action = cache_conf.getLoadAction(RRClass::IN(),
+                                                 Name::ROOT_NAME());
+    ZoneData* zone_data = action(msgmt_);
+    ASSERT_TRUE(zone_data);
+    EXPECT_EQ(".", zone_data->getOriginNode()->
+              getAbsoluteLabels(labels_buf).toText());
+    ZoneData::destroy(msgmt_, zone_data, RRClass::IN());
+
+    // If the specified zone name is not configured to be cached,
+    // getLoadAction returns empty (false) functor.
+    EXPECT_FALSE(cache_conf.getLoadAction(RRClass::IN(), Name("example.com")));
+}
+
+TEST_F(CacheConfigTest, constructWithMock) {
+    // Performing equivalent set of tests as constructMasterFiles
+
+    // Configure with a single zone.
+    const CacheConfig cache_conf("mock", &mock_client_, *mock_config_, true);
+    EXPECT_EQ(1, countZones(cache_conf));
+    EXPECT_TRUE(cache_conf.isEnabled());
+
+    // Configure with multiple zones.
+    const ConstElementPtr config_elem_multi(
+        Element::fromJSON("{\"cache-enable\": true,"
+                          " \"cache-zones\": "
+                          "[\"example.com\", \"example.org\",\"example.info\"]"
+                          "}"));
+    const CacheConfig cache_conf2("mock", &mock_client_, *config_elem_multi,
+                                  true);
+    EXPECT_EQ(3, countZones(cache_conf2));
+
+    // Empty
+    const CacheConfig cache_conf3(
+        "mock", &mock_client_,
+        *Element::fromJSON("{\"cache-enable\": true,"
+                           " \"cache-zones\": []}"), true);
+    EXPECT_EQ(0, countZones(cache_conf3));
+
+    // disabled.  value of cache-zones are ignored.
+    const ConstElementPtr config_elem_disabled(
+        Element::fromJSON("{\"cache-enable\": false,"
+                          " \"cache-zones\": [\"example.com\"]}"));
+    EXPECT_FALSE(CacheConfig("mock", &mock_client_, *config_elem_disabled,
+                             true).isEnabled());
+    // enabled but not "allowed".  same effect.
+    EXPECT_FALSE(CacheConfig("mock", &mock_client_,
+                             *Element::fromJSON("{\"cache-enable\": true,"
+                                                " \"cache-zones\": []}"),
+                             false).isEnabled());
+}
+
+TEST_F(CacheConfigTest, badConstructWithMock) {
+    // no "cache-zones" (may become valid in future, but for now "notimp")
+    EXPECT_THROW(CacheConfig("mock", &mock_client_,
+                             *Element::fromJSON("{\"cache-enable\": true}"),
+                             true),
+                 isc::NotImplemented);
+
+    // "cache-zones" is not a list
+    EXPECT_THROW(CacheConfig("mock", &mock_client_,
+                             *Element::fromJSON("{\"cache-enable\": true,"
+                                                " \"cache-zones\": {}}"),
+                             true),
+                 isc::data::TypeError);
+
+    // "cache-zone" entry is not a string
+    EXPECT_THROW(CacheConfig("mock", &mock_client_,
+                             *Element::fromJSON("{\"cache-enable\": true,"
+                                                " \"cache-zones\": [1]}"),
+                             true),
+                 isc::data::TypeError);
+
+    // bogus zone name
+    const ConstElementPtr bad_config(Element::fromJSON(
+                                         "{\"cache-enable\": true,"
+                                         " \"cache-zones\": [\"bad..\"]}"));
+    EXPECT_THROW(CacheConfig("mock", &mock_client_, *bad_config, true),
+                 isc::dns::EmptyLabel);
+
+    // duplicate zone name (note that comparison is case insensitive)
+    const ConstElementPtr dup_config(Element::fromJSON(
+                                         "{\"cache-enable\": true,"
+                                         " \"cache-zones\": "
+                                         " [\"example\", \"EXAMPLE\"]}"));
+    EXPECT_THROW(CacheConfig("mock", &mock_client_, *dup_config, true),
+                 CacheConfigError);
+
+    // datasrc is null
+    EXPECT_THROW(CacheConfig("mock", 0, *mock_config_, true),
+                 isc::InvalidParameter);
+}
+
+TEST_F(CacheConfigTest, getLoadActionWithMock) {
+    uint8_t labels_buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+
+    // Similar to MasterFiles counterpart, but using underlying source
+    // data source.
+
+    // Note: there's a mismatch between this configuration and the actual
+    // mock data source content: example.net doesn't exist in the data source.
+    const ConstElementPtr config(Element::fromJSON(
+                                     "{\"cache-enable\": true,"
+                                     " \"cache-zones\": [\"example.org\","
+                                     " \"example.net\", \"null.org\"]}"));
+    const CacheConfig cache_conf("mock", &mock_client_, *config, true);
+    LoadAction action = cache_conf.getLoadAction(RRClass::IN(),
+                                                 Name("example.org"));
+    ZoneData* zone_data = action(msgmt_);
+    ASSERT_TRUE(zone_data);
+    EXPECT_EQ("example.org.", zone_data->getOriginNode()->
+              getAbsoluteLabels(labels_buf).toText());
+    ZoneData::destroy(msgmt_, zone_data, RRClass::IN());
+
+    // Zone not configured for the cache
+    EXPECT_FALSE(cache_conf.getLoadAction(RRClass::IN(), Name("example.com")));
+
+    // Zone configured for the cache but doesn't exist in the underling data
+    // source.
+    EXPECT_THROW(cache_conf.getLoadAction(RRClass::IN(), Name("example.net")),
+                 DataSourceError);
+
+    // buggy data source client: it returns a null pointer from getIterator.
+    EXPECT_THROW(cache_conf.getLoadAction(RRClass::IN(), Name("null.org")),
+                 isc::Unexpected);
+}
+
+TEST_F(CacheConfigTest, getSegmentType) {
+    // Default type
+    EXPECT_EQ("local",
+              CacheConfig("MasterFiles", 0,
+                          *master_config_, true).getSegmentType());
+
+    // If we explicitly configure it, that value should be used.
+    ConstElementPtr config(Element::fromJSON("{\"cache-enable\": true,"
+                                             " \"cache-type\": \"mapped\","
+                                             " \"params\": {}}" ));
+    EXPECT_EQ("mapped",
+              CacheConfig("MasterFiles", 0, *config, true).getSegmentType());
+
+    // Wrong types: should be rejected at construction time
+    ConstElementPtr badconfig(Element::fromJSON("{\"cache-enable\": true,"
+                                                " \"cache-type\": 1,"
+                                                " \"params\": {}}"));
+    EXPECT_THROW(CacheConfig("MasterFiles", 0, *badconfig, true),
+                 isc::data::TypeError);
+}
+
+}

src/lib/datasrc/tests/client_list_unittest.cc

@@ -14,13 +14,16 @@
 
 #include <datasrc/client_list.h>
 #include <datasrc/client.h>
+#include <datasrc/cache_config.h>
 #include <datasrc/zone_iterator.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_finder.h>
 #include <datasrc/memory/zone_writer.h>
 
+#include <datasrc/tests/mock_client.h>
+
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
 #include <dns/rdataclass.h>
@@ -33,6 +36,7 @@
 #include <fstream>
 
 using namespace isc::datasrc;
+using isc::datasrc::unittest::MockDataSourceClient;
 using isc::datasrc::memory::InMemoryClient;
 using isc::datasrc::memory::ZoneTableSegment;
 using isc::datasrc::memory::InMemoryZoneFinder;
@@ -45,162 +49,6 @@ using namespace std;
 
 namespace {
 
-// A test data source. It pretends it has some zones.
-class MockDataSourceClient : public DataSourceClient {
-public:
-    class Finder : public ZoneFinder {
-    public:
-        Finder(const Name& origin) :
-            origin_(origin)
-        {}
-        Name getOrigin() const { return (origin_); }
-        // The rest is not to be called, so just have them
-        RRClass getClass() const {
-            isc_throw(isc::NotImplemented, "Not implemented");
-        }
-        shared_ptr<Context> find(const Name&, const RRType&,
-                                 const FindOptions)
-        {
-            isc_throw(isc::NotImplemented, "Not implemented");
-        }
-        shared_ptr<Context> findAll(const Name&,
-                                    vector<ConstRRsetPtr>&,
-                                    const FindOptions)
-        {
-            isc_throw(isc::NotImplemented, "Not implemented");
-        }
-        FindNSEC3Result findNSEC3(const Name&, bool) {
-            isc_throw(isc::NotImplemented, "Not implemented");
-        }
-    private:
-        Name origin_;
-    };
-    class Iterator : public ZoneIterator {
-    public:
-        Iterator(const Name& origin, bool include_a) :
-            origin_(origin),
-            soa_(new RRset(origin_, RRClass::IN(), RRType::SOA(),
-                           RRTTL(3600)))
-        {
-            // The RData here is bogus, but it is not used to anything. There
-            // just needs to be some.
-            soa_->addRdata(rdata::generic::SOA(Name::ROOT_NAME(),
-                                               Name::ROOT_NAME(),
-                                               0, 0, 0, 0, 0));
-            rrsets_.push_back(soa_);
-
-            RRsetPtr rrset(new RRset(origin_, RRClass::IN(), RRType::NS(),
-                                     RRTTL(3600)));
-            rrset->addRdata(rdata::generic::NS(Name::ROOT_NAME()));
-            rrsets_.push_back(rrset);
-
-            if (include_a) {
-                 // Dummy A rrset. This is used for checking zone data
-                 // after reload.
-                 rrset.reset(new RRset(Name("tstzonedata").concatenate(origin_),
-                                       RRClass::IN(), RRType::A(),
-                                       RRTTL(3600)));
-                 rrset->addRdata(rdata::in::A("192.0.2.1"));
-                 rrsets_.push_back(rrset);
-            }
-
-            rrsets_.push_back(ConstRRsetPtr());
-
-            it_ = rrsets_.begin();
-        }
-        virtual isc::dns::ConstRRsetPtr getNextRRset() {
-            ConstRRsetPtr result = *it_;
-            ++it_;
-            return (result);
-        }
-        virtual isc::dns::ConstRRsetPtr getSOA() const {
-            return (soa_);
-        }
-    private:
-        const Name origin_;
-        const RRsetPtr soa_;
-        std::vector<ConstRRsetPtr> rrsets_;
-        std::vector<ConstRRsetPtr>::const_iterator it_;
-    };
-    // Constructor from a list of zones.
-    MockDataSourceClient(const char* zone_names[]) :
-        have_a_(true), use_baditerator_(true)
-    {
-        for (const char** zone(zone_names); *zone; ++zone) {
-            zones.insert(Name(*zone));
-        }
-    }
-    // Constructor from configuration. The list of zones will be empty, but
-    // it will keep the configuration inside for further inspection.
-    MockDataSourceClient(const string& type,
-                         const ConstElementPtr& configuration) :
-        type_(type),
-        configuration_(configuration),
-        have_a_(true), use_baditerator_(true)
-    {
-        EXPECT_NE("MasterFiles", type) << "MasterFiles is a special case "
-            "and it never should be created as a data source client";
-        if (configuration_->getType() == Element::list) {
-            for (size_t i(0); i < configuration_->size(); ++i) {
-                zones.insert(Name(configuration_->get(i)->stringValue()));
-            }
-        }
-    }
-    virtual FindResult findZone(const Name& name) const {
-        if (zones.empty()) {
-            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
-        }
-        set<Name>::const_iterator it(zones.upper_bound(name));
-        if (it == zones.begin()) {
-            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
-        }
-        --it;
-        NameComparisonResult compar(it->compare(name));
-        const ZoneFinderPtr finder(new Finder(*it));
-        switch (compar.getRelation()) {
-            case NameComparisonResult::EQUAL:
-                return (FindResult(result::SUCCESS, finder));
-            case NameComparisonResult::SUPERDOMAIN:
-                return (FindResult(result::PARTIALMATCH, finder));
-            default:
-                return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
-        }
-    }
-    // These methods are not used. They just need to be there to have
-    // complete vtable.
-    virtual ZoneUpdaterPtr getUpdater(const Name&, bool, bool) const {
-        isc_throw(isc::NotImplemented, "Not implemented");
-    }
-    virtual pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
-        getJournalReader(const Name&, uint32_t, uint32_t) const
-    {
-        isc_throw(isc::NotImplemented, "Not implemented");
-    }
-    virtual ZoneIteratorPtr getIterator(const Name& name, bool) const {
-        if (use_baditerator_ && name == Name("noiter.org")) {
-            isc_throw(isc::NotImplemented, "Asked not to be implemented");
-        } else if (use_baditerator_ && name == Name("null.org")) {
-            return (ZoneIteratorPtr());
-        } else {
-            FindResult result(findZone(name));
-            if (result.code == isc::datasrc::result::SUCCESS) {
-                return (ZoneIteratorPtr(new Iterator(name, have_a_)));
-            } else {
-                isc_throw(DataSourceError, "No such zone");
-            }
-        }
-    }
-    void disableA() { have_a_ = false; }
-    void disableBadIterator() { use_baditerator_ = false; }
-    const string type_;
-    const ConstElementPtr configuration_;
-private:
-    set<Name> zones;
-    bool have_a_; // control the iterator behavior whether to include A record
-    bool use_baditerator_; // whether to use bogus zone iterators for tests
-};
-
-
 // The test version is the same as the normal version. We, however, add
 // some methods to dig directly in the internals, for the tests.
 class TestedList : public ConfigurableClientList {
@@ -219,6 +67,9 @@ public:
         if (type == "error") {
             isc_throw(DataSourceError, "The error data source type");
         }
+        if (type == "MasterFiles") {
+            return (DataSourcePair(0, DataSourceClientContainerPtr()));
+        }
         shared_ptr<MockDataSourceClient>
             ds(new MockDataSourceClient(type, configuration));
         // Make sure it is deleted when the test list is deleted.
@@ -269,8 +120,7 @@ public:
             "   \"params\": [\"example.org\", \"example.com\", "
             "                \"noiter.org\", \"null.org\"]"
             "}]")),
-        config_(Element::fromJSON("{}")),
-        ztable_segment_(ZoneTableSegment::create(*config_, rrclass_))
+        ztable_segment_(ZoneTableSegment::create(rrclass_, "local"))
     {
         for (size_t i(0); i < ds_count; ++ i) {
             shared_ptr<MockDataSourceClient>
@@ -278,34 +128,56 @@ public:
             ds_.push_back(ds);
             ds_info_.push_back(ConfigurableClientList::DataSourceInfo(
                                    ds.get(), DataSourceClientContainerPtr(),
-                                   false, rrclass_, ztable_segment_, ""));
+                                   boost::shared_ptr<internal::CacheConfig>(),
+                                   rrclass_, ""));
         }
     }
 
     // Install a "fake" cached zone using a temporary underlying data source
-    // client.
-    void prepareCache(size_t index, const Name& zone) {
-        // Prepare the temporary data source client
-        const char* zones[2];
-        const std::string zonename_txt = zone.toText();
-        zones[0] = zonename_txt.c_str();
-        zones[1] = NULL;
-        MockDataSourceClient mock_client(zones);
+    // 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 =
+            static_cast<MockDataSourceClient*>(dsrc_info.data_src_client_);
+
         // Disable some default features of the mock to distinguish the
         // temporary case from normal case.
-        mock_client.disableA();
-        mock_client.disableBadIterator();
-
-        // Create cache from the temporary data source, and push it to the
-        // client list.
-        const shared_ptr<InMemoryClient> cache(
-            new InMemoryClient(ztable_segment_, rrclass_));
-        cache->load(zone, *mock_client.getIterator(zone, false));
+        mock_client->disableA();
+        mock_client->disableBadIterator();
+
+        // 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\": " +
+                              string(enabled ? "true," : "false,") +
+                              " \"cache-zones\": "
+                              "   [\"" + zone.toText() + "\"]}");
+        boost::shared_ptr<internal::CacheConfig> cache_conf(
+            new internal::CacheConfig("mock", mock_client, *cache_conf_elem,
+                                      true));
+        dsrc_info = ConfigurableClientList::DataSourceInfo(
+            dsrc_info.data_src_client_,
+            dsrc_info.container_,
+            cache_conf, rrclass_, dsrc_info.name_);
+
+        // Load the data into the zone table.
+        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
+        }
 
-        ConfigurableClientList::DataSourceInfo& dsrc_info =
-                list_->getDataSources()[index];
-        dsrc_info.cache_ = cache;
-        dsrc_info.ztable_segment_ = ztable_segment_;
+        // On completion of load revert to the previous state of underlying
+        // data source.
+        mock_client->enableA();
+        mock_client->enableBadIterator();
     }
     // Check the positive result is as we expect it.
     void positiveResult(const ClientList::FindResult& result,
@@ -382,7 +254,7 @@ public:
     const ClientList::FindResult negative_result_;
     vector<shared_ptr<MockDataSourceClient> > ds_;
     vector<ConfigurableClientList::DataSourceInfo> ds_info_;
-    const ConstElementPtr config_elem_, config_elem_zones_, config_;
+    const ConstElementPtr config_elem_, config_elem_zones_;
     shared_ptr<ZoneTableSegment> ztable_segment_;
 };
 
@@ -1024,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.
@@ -1043,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);
@@ -1076,7 +959,7 @@ TYPED_TEST(ReloadTest, reloadNoSuchZone) {
 // Check we gracefuly throw an exception when a zone disappeared in
 // the underlying data source when we want to reload it
 TYPED_TEST(ReloadTest, reloadZoneGone) {
-    this->list_->configure(this->config_elem_, true);
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("example.org");
     // We put in a cache for non-existent zone. This emulates being loaded
     // and then the zone disappearing. We prefill the cache, so we can check
@@ -1086,6 +969,10 @@ TYPED_TEST(ReloadTest, reloadZoneGone) {
     EXPECT_EQ(ZoneFinder::SUCCESS,
               this->list_->find(name).finder_->find(name,
                                                     RRType::SOA())->code);
+    // Remove the zone from the data source.
+    static_cast<MockDataSourceClient*>(
+        this->list_->getDataSources()[0].data_src_client_)->eraseZone(name);
+
     // The zone is not there, so abort the reload.
     EXPECT_THROW(this->doReload(name), DataSourceError);
     // The (cached) zone is not hurt.

src/lib/datasrc/tests/database_unittest.cc

@@ -25,7 +25,7 @@
 #include <datasrc/database.h>
 #include <datasrc/zone.h>
 #include <datasrc/zone_finder.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/zone_iterator.h>
 
 #include <testutils/dnsmessage_test.h>
@@ -167,7 +167,8 @@ public:
     virtual void addNSEC3RecordToZone(const string (&)[ADD_NSEC3_COLUMN_COUNT])
     {}
     virtual void deleteRecordInZone(const string (&)[DEL_PARAM_COUNT]) {}
-    virtual void deleteNSEC3RecordInZone(const string (&)[DEL_PARAM_COUNT]) {}
+    virtual void deleteNSEC3RecordInZone(const string
+                                         (&)[DEL_NSEC3_PARAM_COUNT]) {}
     virtual void addRecordDiff(int, uint32_t, DiffOperation,
                                const std::string (&)[DIFF_PARAM_COUNT]) {}
 
@@ -634,9 +635,8 @@ private:
     };
 
     // Common subroutine for deleteRecordinZone and deleteNSEC3RecordInZone.
-    void deleteRecord(Domains& domains,
-                      const string (&params)[DEL_PARAM_COUNT])
-    {
+    template<size_t param_count>
+    void deleteRecord(Domains& domains, const string (&params)[param_count]) {
         vector<vector<string> >& records =
             domains[params[DatabaseAccessor::DEL_NAME]];
         records.erase(remove_if(records.begin(), records.end(),
@@ -655,7 +655,7 @@ public:
     }
 
     virtual void deleteNSEC3RecordInZone(
-        const string (&params)[DEL_PARAM_COUNT])
+        const string (&params)[DEL_NSEC3_PARAM_COUNT])
     {
         deleteRecord(*update_nsec3_namespace_, params);
     }

src/lib/datasrc/tests/factory_unittest.cc

@@ -16,7 +16,7 @@
 
 #include <datasrc/datasrc_config.h>
 #include <datasrc/factory.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/sqlite3_accessor.h>
 
 #include <dns/rrclass.h>
@@ -28,8 +28,6 @@ using namespace isc::datasrc;
 using namespace isc::data;
 
 std::string SQLITE_DBFILE_EXAMPLE_ORG = TEST_DATA_DIR "/example.org.sqlite3";
-const std::string STATIC_DS_FILE = TEST_DATA_DIR "/static.zone";
-const std::string STATIC_BAD_DS_FILE = TEST_DATA_DIR "/static-bad.zone";
 const std::string ROOT_ZONE_FILE = TEST_DATA_DIR "/root.zone";
 
 namespace {
@@ -166,55 +164,5 @@ TEST(FactoryTest, badType) {
                                            DataSourceError);
 }
 
-// Check the static data source can be loaded.
-TEST(FactoryTest, staticDS) {
-    // The only configuration is the file to load.
-    const ConstElementPtr config(new StringElement(STATIC_DS_FILE));
-    // Get the data source
-    DataSourceClientContainer dsc("static", config);
-    // And try getting something out to see if it really works.
-    DataSourceClient::FindResult
-        result(dsc.getInstance().findZone(isc::dns::Name("BIND")));
-    ASSERT_EQ(result::SUCCESS, result.code);
-    EXPECT_EQ(isc::dns::Name("BIND"), result.zone_finder->getOrigin());
-    EXPECT_EQ(isc::dns::RRClass::CH(), result.zone_finder->getClass());
-    const isc::dns::ConstRRsetPtr
-        version(result.zone_finder->find(isc::dns::Name("VERSION.BIND"),
-                                         isc::dns::RRType::TXT())->rrset);
-    ASSERT_NE(isc::dns::ConstRRsetPtr(), version);
-    EXPECT_EQ(isc::dns::Name("VERSION.BIND"), version->getName());
-    EXPECT_EQ(isc::dns::RRClass::CH(), version->getClass());
-    EXPECT_EQ(isc::dns::RRType::TXT(), version->getType());
-}
-
-// Check that file not containing BIND./CH is rejected
-TEST(FactoryTest, staticDSBadFile) {
-    // The only configuration is the file to load.
-    const ConstElementPtr config(new StringElement(STATIC_BAD_DS_FILE));
-    // See it does not want the file
-    EXPECT_THROW(DataSourceClientContainer("static", config), DataSourceError);
-}
-
-// Check that some bad configs are rejected
-TEST(FactoryTest, staticDSBadConfig) {
-    const char* configs[] = {
-        // The file does not exist
-        "\"/does/not/exist\"",
-        // Bad types
-        "null",
-        "42",
-        "{}",
-        "[]",
-        "true",
-        NULL
-    };
-    for (const char** config(configs); *config; ++config) {
-        SCOPED_TRACE(*config);
-        EXPECT_THROW(DataSourceClientContainer("static",
-                                               Element::fromJSON(*config)),
-                     DataSourceError);
-    }
-}
-
 } // end anonymous namespace
 

src/lib/datasrc/tests/memory/Makefile.am

@@ -21,6 +21,7 @@ if HAVE_GTEST
 TESTS += run_unittests
 
 run_unittests_SOURCES = run_unittests.cc
+run_unittests_SOURCES += zone_loader_util.h zone_loader_util.cc
 run_unittests_SOURCES += rdata_serialization_unittest.cc
 run_unittests_SOURCES += rdataset_unittest.cc
 run_unittests_SOURCES += domaintree_unittest.cc

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

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <datasrc/tests/memory/zone_loader_util.h>
+
 #include <exceptions/exceptions.h>
 
 #include <util/memory_segment_local.h>
@@ -26,7 +28,7 @@
 #include <dns/masterload.h>
 
 #include <datasrc/result.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table.h>
 #include <datasrc/memory/zone_data_updater.h>
@@ -42,6 +44,7 @@
 
 #include <boost/lexical_cast.hpp>
 #include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
 
 #include <new>                  // for bad_alloc
 
@@ -53,6 +56,7 @@ using namespace isc::datasrc::memory;
 using namespace isc::testutils;
 using boost::shared_ptr;
 using std::vector;
+using isc::datasrc::memory::test::loadZoneIntoTable;
 
 namespace {
 
@@ -169,26 +173,22 @@ protected:
                              zclass_, mem_sgmt_)),
                          client_(new InMemoryClient(ztable_segment_, zclass_))
     {}
-    ~MemoryClientTest() {
-        delete client_;
-    }
     void TearDown() {
-        delete client_;
-        client_ = NULL;
+        client_.reset();
         ztable_segment_.reset();
         EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated()); // catch any leak here.
     }
     const RRClass zclass_;
     test::MemorySegmentTest mem_sgmt_;
     shared_ptr<ZoneTableSegment> ztable_segment_;
-    InMemoryClient* client_;
+    boost::scoped_ptr<InMemoryClient> client_;
 };
 
 TEST_F(MemoryClientTest, loadRRsetDoesntMatchOrigin) {
     // Attempting to load example.org to example.com zone should result
     // in an exception.
-    EXPECT_THROW(client_->load(Name("example.com"),
-                               TEST_DATA_DIR "/example.org-empty.zone"),
+    EXPECT_THROW(loadZoneData(mem_sgmt_, zclass_, Name("example.com"),
+                              TEST_DATA_DIR "/example.org-empty.zone"),
                  ZoneLoaderException);
 }
 
@@ -196,8 +196,8 @@ TEST_F(MemoryClientTest, loadErrorsInParsingZoneMustNotLeak1) {
     // Attempting to load broken example.org zone should result in an
     // exception. This should not leak ZoneData and other such
     // allocations.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR "/example.org-broken1.zone"),
+    EXPECT_THROW(loadZoneData(mem_sgmt_, zclass_, Name("example.org"),
+                              TEST_DATA_DIR "/example.org-broken1.zone"),
                  ZoneLoaderException);
     // Teardown checks for memory segment leaks
 }
@@ -206,50 +206,45 @@ TEST_F(MemoryClientTest, loadErrorsInParsingZoneMustNotLeak2) {
     // Attempting to load broken example.org zone should result in an
     // exception. This should not leak ZoneData and other such
     // allocations.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR "/example.org-broken2.zone"),
+    EXPECT_THROW(loadZoneData(mem_sgmt_, zclass_, Name("example.org"),
+                              TEST_DATA_DIR "/example.org-broken2.zone"),
                  ZoneLoaderException);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadNonExistentZoneFile) {
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR "/somerandomfilename"),
+    EXPECT_THROW(loadZoneData(mem_sgmt_, zclass_, Name("example.org"),
+                              TEST_DATA_DIR "/somerandomfilename"),
                  ZoneLoaderException);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadEmptyZoneFileThrows) {
     // When an empty zone file is loaded, the origin doesn't even have
-    // an SOA RR. This condition should be avoided, and hence load()
-    // should throw when an empty zone is loaded.
-
-    EXPECT_EQ(0, client_->getZoneCount());
-
-    EXPECT_THROW(client_->load(Name("."),
-                               TEST_DATA_DIR "/empty.zone"),
+    // an SOA RR. This condition should be avoided, and hence it results in
+    // an exception.
+    EXPECT_THROW(loadZoneData(mem_sgmt_, zclass_, Name("."),
+                              TEST_DATA_DIR "/empty.zone"),
                  ZoneValidationError);
-
-    EXPECT_EQ(0, client_->getZoneCount());
-
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, load) {
     // This is a simple load check for a "full" and correct zone that
     // should not result in any exceptions.
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org.zone");
-    const ZoneData* zone_data =
-        client_->findZoneData(Name("example.org"));
+    ZoneData* zone_data = loadZoneData(mem_sgmt_, zclass_,
+                                       Name("example.org"),
+                                       TEST_DATA_DIR
+                                       "/example.org.zone");
     ASSERT_NE(static_cast<const ZoneData*>(NULL), zone_data);
     EXPECT_FALSE(zone_data->isSigned());
     EXPECT_FALSE(zone_data->isNSEC3Signed());
+    ZoneData::destroy(mem_sgmt_, zone_data, zclass_);
 }
 
 TEST_F(MemoryClientTest, loadFromIterator) {
-    client_->load(Name("example.org"),
-                  *MockIterator::makeIterator(rrset_data));
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      *MockIterator::makeIterator(rrset_data));
 
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
 
@@ -281,21 +276,23 @@ TEST_F(MemoryClientTest, loadFromIterator) {
     // Iterating past the end should result in an exception
     EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected);
 
+    // NOTE: The rest of the tests is not actually about InMemoryClient
+
     // Loading the zone with an iterator separating RRs of the same
     // RRset should not fail. It is acceptable to load RRs of the same
     // type again.
-    client_->load(Name("example.org"),
-                  *MockIterator::makeIterator(
-                      rrset_data_separated));
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      *MockIterator::makeIterator(rrset_data_separated));
 
     // Similar to the previous case, but with separated RRSIGs.
-    client_->load(Name("example.org"),
-                  *MockIterator::makeIterator(
-                      rrset_data_sigseparated));
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      *MockIterator::makeIterator(rrset_data_sigseparated));
 
     // Emulating bogus iterator implementation that passes empty RRSIGs.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               *MockIterator::makeIterator(rrset_data, true)),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   *MockIterator::makeIterator(rrset_data,
+                                                               true)),
                  isc::Unexpected);
 }
 
@@ -316,16 +313,16 @@ TEST_F(MemoryClientTest, loadMemoryAllocationFailures) {
             // fail (due to MemorySegmentTest throwing) and we check for
             // leaks when this happens.
             InMemoryClient client2(ztable_segment, zclass_);
-            client2.load(Name("example.org"),
-                         TEST_DATA_DIR "/example.org.zone");
+            loadZoneIntoTable(*ztable_segment, Name("example.org"), zclass_,
+                              TEST_DATA_DIR "/example.org.zone");
         }, std::bad_alloc);
     }
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadNSEC3Signed) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-nsec3-signed.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-nsec3-signed.zone");
     const ZoneData* zone_data =
         client_->findZoneData(Name("example.org"));
     ASSERT_NE(static_cast<const ZoneData*>(NULL), zone_data);
@@ -336,8 +333,8 @@ TEST_F(MemoryClientTest, loadNSEC3Signed) {
 TEST_F(MemoryClientTest, loadNSEC3EmptySalt) {
     // Load NSEC3 with empty ("-") salt. This should not throw or crash
     // or anything.
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-nsec3-empty-salt.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-nsec3-empty-salt.zone");
     const ZoneData* zone_data =
         client_->findZoneData(Name("example.org"));
     ASSERT_NE(static_cast<const ZoneData*>(NULL), zone_data);
@@ -346,8 +343,8 @@ TEST_F(MemoryClientTest, loadNSEC3EmptySalt) {
 }
 
 TEST_F(MemoryClientTest, loadNSEC3SignedNoParam) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-nsec3-signed-no-param.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-nsec3-signed-no-param.zone");
     const ZoneData* zone_data =
         client_->findZoneData(Name("example.org"));
     ASSERT_NE(static_cast<const ZoneData*>(NULL), zone_data);
@@ -360,14 +357,14 @@ TEST_F(MemoryClientTest, loadReloadZone) {
     // doesn't increase.
     EXPECT_EQ(0, client_->getZoneCount());
 
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-empty.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-empty.zone");
     EXPECT_EQ(1, client_->getZoneCount());
 
     // Reload zone with same data
 
-    client_->load(Name("example.org"),
-                  client_->getFileName(Name("example.org")));
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-empty.zone");
     EXPECT_EQ(1, client_->getZoneCount());
 
     const ZoneData* zone_data =
@@ -396,8 +393,8 @@ TEST_F(MemoryClientTest, loadReloadZone) {
 
     // Reload zone with different data
 
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-rrsigs.zone");
     EXPECT_EQ(1, client_->getZoneCount());
 
     zone_data = client_->findZoneData(Name("example.org"));
@@ -441,15 +438,14 @@ TEST_F(MemoryClientTest, loadReloadZone) {
 TEST_F(MemoryClientTest, loadDuplicateType) {
     // This should not result in any exceptions (multiple records of the
     // same name, type are present, one after another in sequence).
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-duplicate-type.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-duplicate-type.zone");
 
     // This should not result in any exceptions (multiple records of the
     // same name, type are present, but not one after another in
     // sequence).
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR
-                  "/example.org-duplicate-type-bad.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-duplicate-type-bad.zone");
 
     const ZoneData* zone_data =
         client_->findZoneData(Name("example.org"));
@@ -479,104 +475,116 @@ TEST_F(MemoryClientTest, loadDuplicateType) {
 
 TEST_F(MemoryClientTest, loadMultipleCNAMEThrows) {
     // Multiple CNAME RRs should throw.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-multiple-cname.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-multiple-cname.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadMultipleDNAMEThrows) {
     // Multiple DNAME RRs should throw.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-multiple-dname.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-multiple-dname.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadMultipleNSEC3Throws) {
     // Multiple NSEC3 RRs should throw.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-multiple-nsec3.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-multiple-nsec3.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadMultipleNSEC3PARAMThrows) {
     // Multiple NSEC3PARAM RRs should throw.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-multiple-nsec3param.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-multiple-nsec3param.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadOutOfZoneThrows) {
     // Out of zone names should throw.
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-out-of-zone.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-out-of-zone.zone"),
                  ZoneLoaderException);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadWildcardNSThrows) {
     // Wildcard NS names should throw
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-wildcard-ns.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-wildcard-ns.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadWildcardDNAMEThrows) {
     // Wildcard DNAME names should throw
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-wildcard-dname.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-wildcard-dname.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadWildcardNSEC3Throws) {
     // Wildcard NSEC3 names should throw
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-wildcard-nsec3.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-wildcard-nsec3.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadNSEC3WithFewerLabelsThrows) {
     // NSEC3 names with labels != (origin_labels + 1) should throw
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-nsec3-fewer-labels.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-nsec3-fewer-labels.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadNSEC3WithMoreLabelsThrows) {
     // NSEC3 names with labels != (origin_labels + 1) should throw
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-nsec3-more-labels.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-nsec3-more-labels.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadCNAMEAndNotNSECThrows) {
     // CNAME and not NSEC should throw
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-cname-and-not-nsec-1.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-cname-and-not-nsec-1.zone"),
                  ZoneDataUpdater::AddError);
 
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-cname-and-not-nsec-2.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-cname-and-not-nsec-2.zone"),
                  ZoneDataUpdater::AddError);
 
     // Teardown checks for memory segment leaks
@@ -584,41 +592,41 @@ TEST_F(MemoryClientTest, loadCNAMEAndNotNSECThrows) {
 
 TEST_F(MemoryClientTest, loadDNAMEAndNSApex1) {
     // DNAME + NS (apex) is OK
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR
-                  "/example.org-dname-ns-apex-1.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-dname-ns-apex-1.zone");
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadDNAMEAndNSApex2) {
     // DNAME + NS (apex) is OK (reverse order)
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR
-                  "/example.org-dname-ns-apex-2.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-dname-ns-apex-2.zone");
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadDNAMEAndNSNonApex1) {
     // DNAME + NS (non-apex) must throw
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-dname-ns-nonapex-1.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-dname-ns-nonapex-1.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadDNAMEAndNSNonApex2) {
     // DNAME + NS (non-apex) must throw (reverse order)
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-dname-ns-nonapex-2.zone"),
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   TEST_DATA_DIR
+                                   "/example.org-dname-ns-nonapex-2.zone"),
                  ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, loadRRSIGs) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-rrsigs.zone");
     EXPECT_EQ(1, client_->getZoneCount());
 }
 
@@ -642,37 +650,31 @@ TEST_F(MemoryClientTest, loadRRSIGsRdataMixedCoveredTypes) {
 
     rrsets_vec.push_back(rrset);
 
-    EXPECT_THROW(
-        client_->load(Name("example.org"),
-                      *MockVectorIterator::makeIterator(rrsets_vec)),
-        ZoneDataUpdater::AddError);
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   *MockVectorIterator::makeIterator(
+                                       rrsets_vec)),
+                 ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, getZoneCount) {
     EXPECT_EQ(0, client_->getZoneCount());
-    client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-empty.zone");
+    // We've updated the zone table already in the client, so the count
+    // should also be incremented indirectly.
     EXPECT_EQ(1, client_->getZoneCount());
 }
 
-TEST_F(MemoryClientTest, getFileNameForNonExistentZone) {
-    // Zone "example.org." doesn't exist
-    EXPECT_TRUE(client_->getFileName(Name("example.org.")).empty());
-}
-
-TEST_F(MemoryClientTest, getFileName) {
-    client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
-    EXPECT_EQ(TEST_DATA_DIR "/example.org-empty.zone",
-              client_->getFileName(Name("example.org")));
-}
-
 TEST_F(MemoryClientTest, getIteratorForNonExistentZone) {
     // Zone "." doesn't exist
     EXPECT_THROW(client_->getIterator(Name(".")), DataSourceError);
 }
 
 TEST_F(MemoryClientTest, getIterator) {
-    client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-empty.zone");
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
 
     // First we have the SOA
@@ -693,8 +695,8 @@ TEST_F(MemoryClientTest, getIterator) {
 }
 
 TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-multiple.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-multiple.zone");
 
     // separate_rrs = false
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
@@ -746,8 +748,8 @@ TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
 
 // Test we get RRSIGs and NSEC3s too for iterating with separate RRs
 TEST_F(MemoryClientTest, getIteratorSeparateSigned) {
-    client_->load(Name("example.org"),
-                       TEST_DATA_DIR "/example.org-nsec3-signed.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-nsec3-signed.zone");
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org"), true));
     bool seen_rrsig = false, seen_nsec3 = false;
     for (ConstRRsetPtr rrset = iterator->getNextRRset();
@@ -764,7 +766,8 @@ TEST_F(MemoryClientTest, getIteratorSeparateSigned) {
 }
 
 TEST_F(MemoryClientTest, getIteratorGetSOAThrowsNotImplemented) {
-    client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-empty.zone");
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
 
     // This method is not implemented.
@@ -780,16 +783,17 @@ TEST_F(MemoryClientTest, addEmptyRRsetThrows) {
     rrsets_vec.push_back(RRsetPtr(new RRset(Name("example.org"), zclass_,
                                             RRType::A(), RRTTL(3600))));
 
-    EXPECT_THROW(
-        client_->load(Name("example.org"),
-                      *MockVectorIterator::makeIterator(rrsets_vec)),
-        ZoneDataUpdater::AddError);
+    EXPECT_THROW(loadZoneIntoTable(*ztable_segment_, Name("example.org"),
+                                   zclass_,
+                                   *MockVectorIterator::makeIterator(
+                                       rrsets_vec)),
+                 ZoneDataUpdater::AddError);
     // Teardown checks for memory segment leaks
 }
 
 TEST_F(MemoryClientTest, findZoneData) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/example.org-rrsigs.zone");
 
     const ZoneData* zone_data = client_->findZoneData(Name("example.com"));
     EXPECT_EQ(static_cast<const ZoneData*>(NULL), zone_data);

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

@@ -12,8 +12,9 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include "memory_segment_test.h"
-#include "zone_table_segment_test.h"
+#include <datasrc/tests/memory/memory_segment_test.h>
+#include <datasrc/tests/memory/zone_table_segment_test.h>
+#include <datasrc/tests/memory/zone_loader_util.h>
 
 // NOTE: this faked_nsec3 inclusion (and all related code below)
 // was ported during #2109 for the convenience of implementing #2218
@@ -21,14 +22,14 @@
 // In #2219 the original is expected to be removed, and this file should
 // probably be moved here (and any leftover code not handled in #2218 should
 // be cleaned up)
-#include "../../tests/faked_nsec3.h"
+#include <datasrc/tests/faked_nsec3.h>
 
 #include <datasrc/memory/zone_finder.h>
 #include <datasrc/memory/zone_data_updater.h>
 #include <datasrc/memory/rdata_serialization.h>
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/memory_client.h>
-#include <datasrc/data_source.h>
+#include <datasrc/exceptions.h>
 #include <datasrc/client.h>
 #include <testutils/dnsmessage_test.h>
 
@@ -1610,12 +1611,13 @@ TEST_F(InMemoryZoneFinderTest, findOrphanRRSIG) {
 // \brief testcase for #2504 (Problem in inmem NSEC denial of existence
 // handling)
 TEST_F(InMemoryZoneFinderTest, NSECNonExistentTest) {
+    const Name name("example.com.");
     shared_ptr<ZoneTableSegment> ztable_segment(
          new ZoneTableSegmentTest(class_, mem_sgmt_));
+    loadZoneIntoTable(*ztable_segment, name, class_,
+                      TEST_DATA_DIR "/2504-test.zone");
     InMemoryClient client(ztable_segment, class_);
-    Name name("example.com.");
 
-    client.load(name, TEST_DATA_DIR "/2504-test.zone");
     DataSourceClient::FindResult result(client.findZone(name));
 
     // Check for a non-existing name
@@ -1771,16 +1773,17 @@ TEST_F(InMemoryZoneFinderNSEC3Test, findNSEC3MissingOrigin) {
      DefaultNSEC3HashCreator creator;
      setNSEC3HashCreator(&creator);
 
+     const Name name("example.com.");
      shared_ptr<ZoneTableSegment> ztable_segment(
           new ZoneTableSegmentTest(class_, mem_sgmt_));
+     loadZoneIntoTable(*ztable_segment, name, class_,
+                       TEST_DATA_DIR "/2503-test.zone");
      InMemoryClient client(ztable_segment, class_);
-     Name name("example.com.");
 
-     client.load(name, TEST_DATA_DIR "/2503-test.zone");
      DataSourceClient::FindResult result(client.findZone(name));
 
      // Check for a non-existing name
-     Name search_name("nonexist.example.com.");
+     const Name search_name("nonexist.example.com.");
      ZoneFinder::FindNSEC3Result find_result(
           result.zone_finder->findNSEC3(search_name, true));
      // findNSEC3() must have completed (not throw or assert). Because