Merge #451 (MemoryZone::load)

git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@4057 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/auth/config.cc

@@ -151,16 +151,21 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
             isc_throw(AuthConfigError, "Missing zone file for zone: "
                       << origin->str());
         }
-        const result::Result result = memory_datasrc_->addZone(
-            ZonePtr(new MemoryZone(rrclass_, Name(origin->stringValue()))));
+        shared_ptr<MemoryZone> new_zone(new MemoryZone(rrclass_,
+            Name(origin->stringValue())));
+        const result::Result result = memory_datasrc_->addZone(new_zone);
         if (result == result::EXIST) {
             isc_throw(AuthConfigError, "zone "<< origin->str()
                       << " already exists");
         }
 
-        // TODO
-        // then load the zone from 'file', which is currently not implemented.
-        //
+        /*
+         * TODO: Once we have better reloading of configuration (something
+         * else than throwing everything away and loading it again), we will
+         * need the load method to be split into some kind of build and
+         * commit/abort parts.
+         */
+        new_zone->load(file->stringValue());
     }
 }
 

src/bin/auth/tests/config_unittest.cc

@@ -17,6 +17,7 @@
 #include <exceptions/exceptions.h>
 
 #include <dns/rrclass.h>
+#include <dns/masterload.h>
 
 #include <cc/data.h>
 
@@ -143,33 +144,42 @@ TEST_F(MemoryDatasrcConfigTest, addZeroZone) {
 }
 
 TEST_F(MemoryDatasrcConfigTest, addOneZone) {
-    parser->build(Element::fromJSON(
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
                       "[{\"type\": \"memory\","
                       "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"}]}]"));
-    parser->commit();
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    // Check it actually loaded something
+    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(rrclass)->findZone(
+        Name("ns.example.com.")).zone->find(Name("ns.example.com."),
+        RRType::A()).code);
 }
 
 TEST_F(MemoryDatasrcConfigTest, addMultiZones) {
-    parser->build(Element::fromJSON(
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
                       "[{\"type\": \"memory\","
                       "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"},"
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"},"
                       "              {\"origin\": \"example.org\","
-                      "               \"file\": \"example.org.zone\"},"
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.org.zone\"},"
                       "              {\"origin\": \"example.net\","
-                      "               \"file\": \"example.net.zone\"}]}]"));
-    parser->commit();
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.net.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(3, server.getMemoryDataSrc(rrclass)->getZoneCount());
 }
 
 TEST_F(MemoryDatasrcConfigTest, replace) {
-    parser->build(Element::fromJSON(
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
                       "[{\"type\": \"memory\","
                       "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"}]}]"));
-    parser->commit();
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::SUCCESS,
               server.getMemoryDataSrc(rrclass)->findZone(
@@ -179,31 +189,69 @@ TEST_F(MemoryDatasrcConfigTest, replace) {
     // should replace the old one.
     delete parser;
     parser = createAuthConfigParser(server, "datasources"); 
-    parser->build(Element::fromJSON(
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
                       "[{\"type\": \"memory\","
                       "  \"zones\": [{\"origin\": \"example.org\","
-                      "               \"file\": \"example.org.zone\"},"
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.org.zone\"},"
                       "              {\"origin\": \"example.net\","
-                      "               \"file\": \"example.net.zone\"}]}]"));
-    parser->commit();
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.net.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(2, server.getMemoryDataSrc(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::NOTFOUND,
               server.getMemoryDataSrc(rrclass)->findZone(
                   Name("example.com")).code);
 }
 
+TEST_F(MemoryDatasrcConfigTest, exception) {
+    // Load a zone
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.com\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
+    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(isc::datasrc::result::SUCCESS,
+              server.getMemoryDataSrc(rrclass)->findZone(
+                  Name("example.com")).code);
+
+    // create a new parser, and try to load something. It will throw,
+    // the given master file should not exist
+    delete parser;
+    parser = createAuthConfigParser(server, "datasources");
+    EXPECT_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.org\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.org.zone\"},"
+                      "              {\"origin\": \"example.net\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/nonexistent.zone\"}]}]")), isc::dns::MasterLoadError);
+    // As that one throwed exception, it is not expected from us to
+    // commit it
+
+    // The original should be untouched
+    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(isc::datasrc::result::SUCCESS,
+              server.getMemoryDataSrc(rrclass)->findZone(
+                  Name("example.com")).code);
+}
+
 TEST_F(MemoryDatasrcConfigTest, remove) {
-    parser->build(Element::fromJSON(
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
                       "[{\"type\": \"memory\","
                       "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"}]}]"));
-    parser->commit();
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
 
     delete parser;
     parser = createAuthConfigParser(server, "datasources"); 
-    parser->build(Element::fromJSON("[]"));
-    parser->commit();
+    EXPECT_NO_THROW(parser->build(Element::fromJSON("[]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
 }
 
@@ -212,9 +260,11 @@ TEST_F(MemoryDatasrcConfigTest, adDuplicateZones) {
                      Element::fromJSON(
                          "[{\"type\": \"memory\","
                          "  \"zones\": [{\"origin\": \"example.com\","
-                         "               \"file\": \"example.zone\"},"
+                         "               \"file\": \"" TEST_DATA_DIR
+                         "/example.zone\"},"
                          "              {\"origin\": \"example.com\","
-                         "               \"file\": \"example.com.zone\"}]}]")),
+                         "               \"file\": \"" TEST_DATA_DIR
+                         "/example.com.zone\"}]}]")),
                  AuthConfigError);
 }
 

src/lib/datasrc/memory_datasrc.cc

@@ -15,9 +15,11 @@
 #include <map>
 #include <cassert>
 #include <boost/shared_ptr.hpp>
+#include <boost/bind.hpp>
 
 #include <dns/name.h>
 #include <dns/rrclass.h>
+#include <dns/masterload.h>
 
 #include <datasrc/memory_datasrc.h>
 #include <datasrc/rbtree.h>
@@ -65,7 +67,7 @@ struct MemoryZone::MemoryZoneImpl {
      * access is without the impl_-> and it will get inlined anyway.
      */
     // Implementation of MemoryZone::add
-    result::Result add(const ConstRRsetPtr& rrset) {
+    result::Result add(const ConstRRsetPtr& rrset, DomainTree* domains) {
         // Sanitize input
         if (!rrset) {
             isc_throw(NullRRset, "The rrset provided is NULL");
@@ -80,7 +82,7 @@ struct MemoryZone::MemoryZoneImpl {
         }
         // Get the node
         DomainNode* node;
-        switch (domains_.insert(name, &node)) {
+        switch (domains->insert(name, &node)) {
             // Just check it returns reasonable results
             case DomainTree::SUCCEED:
             case DomainTree::ALREADYEXIST:
@@ -111,6 +113,22 @@ struct MemoryZone::MemoryZoneImpl {
         }
     }
 
+    /*
+     * Same as above, but it checks the return value and if it already exists,
+     * it throws.
+     */
+    void addFromLoad(const ConstRRsetPtr& set, DomainTree* domains) {
+            switch (add(set, domains)) {
+                case result::EXIST:
+                    isc_throw(dns::MasterLoadError, "Duplicate rrset: " <<
+                        set->toText());
+                case result::SUCCESS:
+                    return;
+                default:
+                    assert(0);
+            }
+    }
+
     // Implementation of MemoryZone::find
     FindResult find(const Name& name, RRType type) const {
         // Get the node
@@ -172,7 +190,19 @@ MemoryZone::find(const Name& name, const RRType& type) const {
 
 result::Result
 MemoryZone::add(const ConstRRsetPtr& rrset) {
-    return (impl_->add(rrset));
+    return (impl_->add(rrset, &impl_->domains_));
+}
+
+
+void
+MemoryZone::load(const string& filename) {
+    // Load it into a temporary tree
+    MemoryZoneImpl::DomainTree tmp;
+    masterLoad(filename.c_str(), getOrigin(), getClass(),
+        boost::bind(&MemoryZoneImpl::addFromLoad, impl_, _1, &tmp));
+    // If it went well, put it inside
+    tmp.swap(impl_->domains_);
+    // And let the old data die with tmp
 }
 
 /// Implementation details for \c MemoryDataSrc hidden from the public

src/lib/datasrc/memory_datasrc.h

@@ -97,6 +97,30 @@ public:
         { }
     };
 
+    /// \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 if there is problem with loading
+    /// (missing file, malformed, it contains different zone, etc - see
+    /// isc::dns::masterLoad for details).
+    ///
+    /// In case of internal problems, OutOfZone, 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.
+    void load(const std::string& filename);
 private:
     /// \name Hidden private data
     //@{

src/lib/datasrc/rbtree.h

@@ -294,6 +294,16 @@ public:
     Result insert(const isc::dns::Name& name, RBNode<T>** inserted_node);
     //@}
 
+    /// \brief Swaps two tree's contents.
+    ///
+    /// This acts the same as many std::*.swap functions, exchanges the
+    /// contents. This doesn't throw anything.
+    void swap(RBTree<T>& other) {
+        std::swap(root_, other.root_);
+        std::swap(NULLNODE, other.NULLNODE);
+        std::swap(node_count_, other.node_count_);
+    }
+
 private:
     /// \name RBTree balance functions
     //@{

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -17,6 +17,7 @@
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
+#include <dns/masterload.h>
 
 #include <datasrc/memory_datasrc.h>
 
@@ -181,18 +182,28 @@ public:
      * \param name The name to ask for.
      * \param rrtype The RRType to ask of.
      * \param result The expected code of the result.
+     * \param check_answer Should a check against equality of the answer be
+     *     done?
      * \param answer The expected rrset, if any should be returned.
+     * \param zone Check different MemoryZone object than zone_ (if NULL,
+     *     uses zone_)
      */
     void findTest(const Name& name, const RRType& rrtype, Zone::Result result,
-        const ConstRRsetPtr& answer = ConstRRsetPtr())
+        bool check_answer = true,
+        const ConstRRsetPtr& answer = ConstRRsetPtr(), MemoryZone *zone = NULL)
     {
+        if (!zone) {
+            zone = &zone_;
+        }
         // The whole block is inside, because we need to check the result and
         // we can't assign to FindResult
         EXPECT_NO_THROW({
-            Zone::FindResult find_result(zone_.find(name, rrtype));
+            Zone::FindResult find_result(zone->find(name, rrtype));
             // Check it returns correct answers
             EXPECT_EQ(result, find_result.code);
-            EXPECT_EQ(answer, find_result.rrset);
+            if (check_answer) {
+                EXPECT_EQ(answer, find_result.rrset);
+            }
         });
     }
 };
@@ -248,8 +259,8 @@ TEST_F(MemoryZoneTest, find) {
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_a_)));
 
     // These two should be successful
-    findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_);
-    findTest(ns_name_, RRType::A(), Zone::SUCCESS, rr_ns_a_);
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, true, rr_ns_);
+    findTest(ns_name_, RRType::A(), Zone::SUCCESS, true, rr_ns_a_);
 
     // These domain exist but don't have the provided RRType
     findTest(origin_, RRType::AAAA(), Zone::NXRRSET);
@@ -260,4 +271,34 @@ TEST_F(MemoryZoneTest, find) {
     findTest(Name("example.net"), RRType::A(), Zone::NXDOMAIN);
 }
 
+TEST_F(MemoryZoneTest, load) {
+    // Put some data inside the zone
+    EXPECT_NO_THROW(EXPECT_EQ(result::SUCCESS, zone_.add(rr_ns_)));
+    // Loading with different origin should fail
+    EXPECT_THROW(zone_.load(TEST_DATA_DIR "/root.zone"), MasterLoadError);
+    // See the original data is still there, survived the exception
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, true, rr_ns_);
+    // Create correct zone
+    MemoryZone rootzone(class_, Name("."));
+    // Try putting something inside
+    EXPECT_NO_THROW(EXPECT_EQ(result::SUCCESS, rootzone.add(rr_ns_aaaa_)));
+    // Load the zone. It should overwrite/remove the above RRset
+    EXPECT_NO_THROW(rootzone.load(TEST_DATA_DIR "/root.zone"));
+
+    // Now see there are some rrsets (we don't look inside, though)
+    findTest(Name("."), RRType::SOA(), Zone::SUCCESS, false, ConstRRsetPtr(),
+        &rootzone);
+    findTest(Name("."), RRType::NS(), Zone::SUCCESS, false, ConstRRsetPtr(),
+        &rootzone);
+    findTest(Name("a.root-servers.net."), RRType::A(), Zone::SUCCESS, false,
+        ConstRRsetPtr(), &rootzone);
+    // But this should no longer be here
+    findTest(ns_name_, RRType::AAAA(), Zone::NXDOMAIN, true, ConstRRsetPtr(),
+        &rootzone);
+
+    // Try loading zone that is wrong in a different way
+    EXPECT_THROW(zone_.load(TEST_DATA_DIR "/duplicate_rrset.zone"),
+        MasterLoadError);
+}
+
 }

src/lib/datasrc/tests/testdata/duplicate_rrset.zone

@@ -0,0 +1,4 @@
+example.org.    3600 IN SOA ns1.example.org. admin.example.org. 1234 3600 1800 2419200 7200
+example.org.    3600 IN NS ns1.example.org.
+example.org.    3600 IN MX 10 mail.example.org.
+example.org.    3600 IN NS ns2.example.org.

src/lib/testutils/testdata/Makefile.am

@@ -18,6 +18,7 @@ EXTRA_DIST += shortquestion_fromWire
 EXTRA_DIST += shortresponse_fromWire
 EXTRA_DIST += simplequery_fromWire.spec
 EXTRA_DIST += simpleresponse_fromWire.spec
+EXTRA_DIST += example.com.zone example.net.zone example.org.zone example.zone
 
 EXTRA_DIST += example.com
 EXTRA_DIST += example.sqlite3

src/lib/testutils/testdata/example.com.zone

@@ -0,0 +1,3 @@
+example.com.    3600    IN  SOA ns.example.com. admin.example.com. 1234 3600 1800 2419200 7200
+example.com.    3600    IN  NS ns.example.com.
+ns.example.com.	3600    IN  A 192.0.2.1

src/lib/testutils/testdata/example.net.zone

@@ -0,0 +1,3 @@
+example.net.    3600    IN  SOA ns.example.net. admin.example.net. 1234 3600 1800 2419200 7200
+example.net.    3600    IN  NS ns.example.net.
+ns.example.net.	3600    IN  A 192.0.2.1

src/lib/testutils/testdata/example.org.zone

@@ -0,0 +1,3 @@
+example.org.    3600    IN  SOA ns.example.org. admin.example.org. 1234 3600 1800 2419200 7200
+example.org.    3600    IN  NS ns.example.org.
+ns.example.org.	3600    IN  A 192.0.2.1

src/lib/testutils/testdata/example.zone

@@ -0,0 +1,3 @@
+example.com.    3600    IN  SOA ns.example.com. admin.example.com. 1234 3600 1800 2419200 7200
+example.com.    3600    IN  NS ns.example.com.
+ns.example.com.	3600    IN  A 192.0.2.1