Browse Source

[2833] revise ZoneTableSegment::create() so it takes ZoneTableConfig

some non trivial adjusments were needed for its users.
JINMEI Tatuya 12 years ago
parent
commit
2cc9047529

+ 2 - 5
src/bin/auth/tests/auth_srv_unittest.cc

@@ -1745,9 +1745,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
@@ -1764,8 +1762,7 @@ public:
     }
 private:
     const boost::shared_ptr<isc::datasrc::ConfigurableClientList> real_;
-    const ConstElementPtr config_;
-    boost::shared_ptr<ZoneTableSegment> ztable_segment_;
+    boost::shared_ptr<ZoneTableSegment> ztable_segment_; // can be null
     vector<isc::datasrc::DataSourceClientPtr> clients_;
 };
 

+ 6 - 3
src/lib/datasrc/client_list.cc

@@ -122,8 +122,11 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
 
             shared_ptr<ZoneTableSegment> ztable_segment;
             if (want_cache) {
-                ztable_segment.reset(ZoneTableSegment::create(*dconf, // XXX
-                                                              rrclass_));
+                // For now we use dummy config
+                internal::ZoneTableConfig ztconfig("MasterFiles", 0,
+                                                   *Element::fromJSON("{\"params\": {}}"));
+                ztable_segment.reset(ZoneTableSegment::create(rrclass_,
+                                                              ztconfig));
             }
 
             if (type == "MasterFiles") {
@@ -191,7 +194,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                     client(new_data_sources.back().data_src_client_);
 
                 // temporary, just validate it
-                internal::ZoneTableConfig ztconfig(type, new_data_sources.back().data_src_client_, *dconf);
+                internal::ZoneTableConfig ztconfig(type, client, *dconf);
 
                 for (vector<string>::const_iterator it(zones_origins.begin());
                      it != zones_origins.end(); ++it) {

+ 4 - 1
src/lib/datasrc/memory/zone_table_segment.cc

@@ -14,6 +14,7 @@
 
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_table_segment_local.h>
+#include <datasrc/zone_table_config.h>
 
 using namespace isc::dns;
 
@@ -22,7 +23,9 @@ namespace datasrc {
 namespace memory {
 
 ZoneTableSegment*
-ZoneTableSegment::create(const isc::data::Element&, const RRClass& rrclass) {
+ZoneTableSegment::create(const RRClass& rrclass,
+                         const internal::ZoneTableConfig&)
+{
     /// FIXME: For now, we always return ZoneTableSegmentLocal. This
     /// should be updated eventually to parse the passed Element
     /// argument and construct a corresponding ZoneTableSegment

+ 13 - 3
src/lib/datasrc/memory/zone_table_segment.h

@@ -16,8 +16,10 @@
 #define ZONE_TABLE_SEGMENT_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>
 
@@ -32,6 +34,9 @@ class Name;
 class RRClass;
 }
 namespace datasrc {
+namespace internal {
+class ZoneTableConfig;
+}
 namespace memory {
 class ZoneWriter;
 
@@ -103,8 +108,9 @@ public:
     /// \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);
+    static ZoneTableSegment* create(
+        const isc::dns::RRClass& rrclass,
+        const isc::datasrc::internal::ZoneTableConfig& config);
 
     /// \brief Destroy a ZoneTableSegment
     ///
@@ -135,3 +141,7 @@ public:
 } // namespace isc
 
 #endif // ZONE_TABLE_SEGMENT_H
+
+// Local Variables:
+// mode: c++
+// End:

+ 54 - 12
src/lib/datasrc/static_datasrc_link.cc

@@ -12,8 +12,11 @@
 // 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/static_datasrc.h>
+#include <datasrc/client.h>
+#include <datasrc/zone_table_config.h>
+
+#include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/zone_table_segment.h>
 
@@ -30,30 +33,69 @@ using namespace std;
 
 namespace isc {
 namespace datasrc {
+namespace {
+// XXX: this is a hack: we need to make sure the zone table config is valid
+// throughout the zone table segment, but there's no way to keep it alive
+// within this factory.  So we use a custom segment that internally creates
+// and hold the config.  Actually, we shouldn't need a separate data source
+// client implementation for "static"; the generic "MasterFiles" data source
+// with pre-generated configuration should suffice.  When it's done, we can
+// remove this loadable module with this hack.
+class ZoneTableSegmentStatic : public memory::ZoneTableSegment {
+public:
+    ZoneTableSegmentStatic(const string& zone_file) :
+        memory::ZoneTableSegment(RRClass::CH()),
+        ztconfig_("MasterFiles", 0, *data::Element::fromJSON(
+                      "{\"params\": {\"BIND\": \"" + zone_file + "\"}}")),
+        ztsegment_(memory::ZoneTableSegment::create(RRClass::CH(), ztconfig_))
+    {}
+
+    virtual ~ZoneTableSegmentStatic() {
+        memory::ZoneTableSegment::destroy(ztsegment_);
+    }
+
+    virtual memory::ZoneTableHeader& getHeader() {
+        return (ztsegment_->getHeader());
+    }
+    virtual const memory::ZoneTableHeader& getHeader() const {
+        return (ztsegment_->getHeader());
+    }
+    virtual isc::util::MemorySegment& getMemorySegment() {
+        return (ztsegment_->getMemorySegment());
+    }
+    virtual memory::ZoneWriter* getZoneWriter(
+        const memory::LoadAction& load_action,
+        const dns::Name& origin,
+        const dns::RRClass& rrclass)
+    {
+        return (ztsegment_->getZoneWriter(load_action, origin, rrclass));
+    }
+
+private:
+    const internal::ZoneTableConfig ztconfig_;
+    memory::ZoneTableSegment* ztsegment_; // actual segment
+};
+}
 
 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.
+        // Set up the zone table.
+        const string path(config->stringValue());
         shared_ptr<memory::ZoneTableSegment> ztable_segment(
-            memory::ZoneTableSegment::create(isc::data::NullElement(),
-                                             RRClass::CH()));
+            new ZoneTableSegmentStatic(path));
+
         // 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) {
+    } catch (const std::exception& e) {
         error = e.what();
-    }
-    catch (...) {
+    } catch (...) {
         error = "Unknown exception";
     }
     return (NULL);

+ 5 - 3
src/lib/datasrc/tests/client_list_unittest.cc

@@ -16,6 +16,7 @@
 #include <datasrc/client.h>
 #include <datasrc/zone_iterator.h>
 #include <datasrc/data_source.h>
+#include <datasrc/zone_table_config.h>
 #include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_finder.h>
@@ -116,8 +117,8 @@ public:
             "   \"params\": [\"example.org\", \"example.com\", "
             "                \"noiter.org\", \"null.org\"]"
             "}]")),
-        config_(Element::fromJSON("{}")),
-        ztable_segment_(ZoneTableSegment::create(*config_, rrclass_))
+        ztconfig_("MasterFiles", 0, *Element::fromJSON("{\"params\": {}}")),
+        ztable_segment_(ZoneTableSegment::create(rrclass_, ztconfig_))
     {
         for (size_t i(0); i < ds_count; ++ i) {
             shared_ptr<MockDataSourceClient>
@@ -229,7 +230,8 @@ 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_;
+    const internal::ZoneTableConfig ztconfig_;
     shared_ptr<ZoneTableSegment> ztable_segment_;
 };
 

+ 4 - 2
src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc

@@ -14,6 +14,7 @@
 
 #include <datasrc/memory/zone_writer_local.h>
 #include <datasrc/memory/zone_table_segment_local.h>
+#include <datasrc/zone_table_config.h>
 #include <util/memory_segment_local.h>
 
 #include <gtest/gtest.h>
@@ -31,8 +32,8 @@ namespace {
 class ZoneTableSegmentTest : public ::testing::Test {
 protected:
     ZoneTableSegmentTest() :
-        ztable_segment_(ZoneTableSegment::create(isc::data::NullElement(),
-                                                 RRClass::IN()))
+        ztconf_("MasterFiles", 0, *Element::fromJSON("{\"params\": {}}")),
+        ztable_segment_(ZoneTableSegment::create(RRClass::IN(), ztconf_))
     {}
 
     void TearDown() {
@@ -40,6 +41,7 @@ protected:
         ztable_segment_ = NULL;
     }
 
+    const isc::datasrc::internal::ZoneTableConfig ztconf_;
     ZoneTableSegment* ztable_segment_;
 };
 

+ 5 - 2
src/lib/datasrc/tests/memory/zone_writer_unittest.cc

@@ -15,6 +15,7 @@
 #include <datasrc/memory/zone_writer_local.h>
 #include <datasrc/memory/zone_table_segment_local.h>
 #include <datasrc/memory/zone_data.h>
+#include <datasrc/zone_table_config.h>
 
 #include <cc/data.h>
 #include <dns/rrclass.h>
@@ -29,6 +30,7 @@ using boost::scoped_ptr;
 using boost::bind;
 using isc::dns::RRClass;
 using isc::dns::Name;
+using isc::data::Element;
 using namespace isc::datasrc::memory;
 
 namespace {
@@ -41,8 +43,8 @@ public:
         // FIXME: The NullElement probably isn't the best one, but we don't
         // know how the config will look, so it just fills the argument
         // (which is currently ignored)
-        segment_(ZoneTableSegment::create(isc::data::NullElement(),
-                                          RRClass::IN())),
+        ztconf_("MasterFiles", 0, *Element::fromJSON("{\"params\": {}}")),
+        segment_(ZoneTableSegment::create(RRClass::IN(), ztconf_)),
         writer_(new
             ZoneWriterLocal(dynamic_cast<ZoneTableSegmentLocal*>(segment_.
                                                                  get()),
@@ -58,6 +60,7 @@ public:
         writer_.reset();
     }
 protected:
+    const isc::datasrc::internal::ZoneTableConfig ztconf_;
     scoped_ptr<ZoneTableSegment> segment_;
     scoped_ptr<ZoneWriterLocal> writer_;
     bool load_called_;

+ 21 - 8
src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -23,6 +23,7 @@
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
+#include <datasrc/zone_table_config.h>
 
 #include "test_client.h"
 #include <testutils/dnsmessage_test.h>
@@ -46,6 +47,7 @@ using namespace isc::dns;
 using namespace isc::datasrc;
 using isc::datasrc::memory::InMemoryClient;
 using isc::datasrc::memory::ZoneTableSegment;
+using isc::datasrc::internal::ZoneTableConfig;
 using namespace isc::testutils;
 
 namespace {
@@ -59,15 +61,16 @@ typedef shared_ptr<DataSourceClient> DataSourceClientPtr;
 // This is the type used as the test parameter.  Note that this is
 // intentionally a plain old type (i.e. a function pointer), not a class;
 // otherwise it could cause initialization fiasco at the instantiation time.
-typedef DataSourceClientPtr (*ClientCreator)(RRClass, const Name&);
+typedef DataSourceClientPtr (*ClientCreator)(RRClass, const Name&,
+                                             const ZoneTableConfig&);
 
 // Creator for the in-memory client to be tested
 DataSourceClientPtr
-createInMemoryClient(RRClass zclass, const Name& zname)
+createInMemoryClient(RRClass zclass, const Name& zname,
+                     const ZoneTableConfig& ztconfig)
 {
-    const ElementPtr config(Element::fromJSON("{}"));
     shared_ptr<ZoneTableSegment> ztable_segment(
-        ZoneTableSegment::create(*config, zclass));
+        ZoneTableSegment::create(zclass, ztconfig));
     shared_ptr<InMemoryClient> client(new InMemoryClient(ztable_segment,
                                                          zclass));
     client->load(zname, TEST_ZONE_FILE);
@@ -100,8 +103,11 @@ createSQLite3Client(RRClass zclass, const Name& zname, stringstream& ss) {
     return (client);
 }
 
+// ZoneTableConfig is in-memory specific, unused here.
 DataSourceClientPtr
-createSQLite3ClientWithNS(RRClass zclass, const Name& zname) {
+createSQLite3ClientWithNS(RRClass zclass, const Name& zname,
+                          const ZoneTableConfig&)
+{
     stringstream ss("ns.example.com. 3600 IN A 192.0.2.7");
     return (createSQLite3Client(zclass, zname, ss));
 }
@@ -112,8 +118,15 @@ class ZoneFinderContextTest :
         public ::testing::TestWithParam<ClientCreator>
 {
 protected:
-    ZoneFinderContextTest() : qclass_(RRClass::IN()), qzone_("example.org") {
-        client_ = (*GetParam())(qclass_, qzone_);
+    ZoneFinderContextTest() :
+        qclass_(RRClass::IN()), qzone_("example.org"),
+        ztconfig_("MasterFiles", 0, *Element::fromJSON(
+                      "{\"params\": "
+                      " {\"" + qzone_.toText() + "\": "
+                      "  \"" + TEST_ZONE_FILE + "\"}"
+                      "}"))
+    {
+        client_ = (*GetParam())(qclass_, qzone_, ztconfig_);
         REQUESTED_A.push_back(RRType::A());
         REQUESTED_AAAA.push_back(RRType::AAAA());
         REQUESTED_BOTH.push_back(RRType::A());
@@ -126,9 +139,9 @@ protected:
 
     const RRClass qclass_;
     const Name qzone_;
+    const ZoneTableConfig ztconfig_; // used for in-memory data source
     DataSourceClientPtr client_;
     ZoneFinderPtr finder_;
-
     vector<RRType> requested_types_;
     vector<RRType> REQUESTED_A;
     vector<RRType> REQUESTED_AAAA;

+ 41 - 14
src/lib/datasrc/tests/zone_loader_unittest.cc

@@ -14,6 +14,7 @@
 
 #include <datasrc/zone_loader.h>
 #include <datasrc/data_source.h>
+#include <datasrc/zone_table_config.h>
 #include <datasrc/rrset_collection_base.h>
 
 #include <datasrc/memory/zone_table_segment.h>
@@ -31,11 +32,14 @@
 #include <boost/shared_ptr.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <boost/foreach.hpp>
+
+#include <cassert>
 #include <string>
 #include <vector>
 
 using namespace isc::dns;
 using namespace isc::datasrc;
+using isc::data::Element;
 using boost::shared_ptr;
 using std::string;
 using std::vector;
@@ -287,13 +291,34 @@ MockClient::getUpdater(const Name& name, bool replace, bool journaling) const {
 class ZoneLoaderTest : public ::testing::Test {
 protected:
     ZoneLoaderTest() :
-        rrclass_(RRClass::IN()),
-        ztable_segment_(memory::ZoneTableSegment::
-                        create(isc::data::NullElement(), rrclass_)),
-        source_client_(ztable_segment_, rrclass_)
-    {}
+        rrclass_(RRClass::IN())
+    {
+        // Use ROOT_NAME as a placeholder; it will be ignored if filename is
+        // null.
+        prepareSource(Name::ROOT_NAME(), 0);
+    }
     void prepareSource(const Name& zone, const char* filename) {
-        source_client_.load(zone, string(TEST_DATA_DIR) + "/" + filename);
+        // Cleanup the existing data in the right order
+        source_client_.reset();
+        ztable_segment_.reset();
+        ztconfig_.reset();
+
+        // (re)configure zone table, then (re)construct the in-memory client
+        // with it.
+        string ztconf_txt = "{\"params\": {";
+        if (filename) {
+            ztconf_txt += "\"" + zone.toText() + "\": \"" + filename + "\"";
+        }
+        ztconf_txt += "}}";
+        ztconfig_.reset(new internal::ZoneTableConfig(
+                            "MasterFiles", 0, *Element::fromJSON(ztconf_txt)));
+        ztable_segment_.reset(memory::ZoneTableSegment::create(rrclass_,
+                                                               *ztconfig_));
+        source_client_.reset(new memory::InMemoryClient(ztable_segment_,
+                                                        rrclass_));
+        if (filename) {
+            source_client_->load(zone, string(TEST_DATA_DIR) + "/" + filename);
+        }
     }
 private:
     const RRClass rrclass_;
@@ -301,11 +326,13 @@ private:
     // from. It is still easier than setting up sqlite3 client, since
     // we have this one in the linked library.
 
+    boost::scoped_ptr<internal::ZoneTableConfig> ztconfig_;
+
     // FIXME: We should be destroying it by ZoneTableSegment::destroy.
     // But the shared pointer won't let us, will it?
     shared_ptr<memory::ZoneTableSegment> ztable_segment_;
 protected:
-    memory::InMemoryClient source_client_;
+    boost::scoped_ptr<memory::InMemoryClient> source_client_;
     // This one is mocked. It will help us see what is happening inside.
     // Also, mocking it is simpler than setting up an sqlite3 client.
     MockClient destination_client_;
@@ -314,7 +341,7 @@ protected:
 // Use the loader to load an unsigned zone.
 TEST_F(ZoneLoaderTest, copyUnsigned) {
     prepareSource(Name::ROOT_NAME(), "root.zone");
-    ZoneLoader loader(destination_client_, Name::ROOT_NAME(), source_client_);
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(), *source_client_);
     // It gets the updater directly in the constructor
     ASSERT_EQ(1, destination_client_.provided_updaters_.size());
     EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
@@ -355,7 +382,7 @@ TEST_F(ZoneLoaderTest, copyUnsigned) {
 // Try loading incrementally.
 TEST_F(ZoneLoaderTest, copyUnsignedIncremental) {
     prepareSource(Name::ROOT_NAME(), "root.zone");
-    ZoneLoader loader(destination_client_, Name::ROOT_NAME(), source_client_);
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(), *source_client_);
 
     // Try loading few RRs first.
     loader.loadIncremental(10);
@@ -390,7 +417,7 @@ TEST_F(ZoneLoaderTest, copyUnsignedIncremental) {
 TEST_F(ZoneLoaderTest, copySigned) {
     prepareSource(Name("example.org"), "example.org.nsec3-signed");
     ZoneLoader loader(destination_client_, Name("example.org"),
-                      source_client_);
+                      *source_client_);
     loader.load();
 
     // All the RRs are there, including the ones in NSEC3 namespace
@@ -416,13 +443,13 @@ TEST_F(ZoneLoaderTest, copyMissingDestination) {
     destination_client_.missing_zone_ = true;
     prepareSource(Name::ROOT_NAME(), "root.zone");
     EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
-                            source_client_), DataSourceError);
+                            *source_client_), DataSourceError);
 }
 
 // If the source zone does not exist, it throws
 TEST_F(ZoneLoaderTest, copyMissingSource) {
     EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
-                            source_client_), DataSourceError);
+                            *source_client_), DataSourceError);
 }
 
 // The class of the source and destination are different
@@ -430,7 +457,7 @@ TEST_F(ZoneLoaderTest, classMismatch) {
     destination_client_.rrclass_ = RRClass::CH();
     prepareSource(Name::ROOT_NAME(), "root.zone");
     EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
-                            source_client_), isc::InvalidParameter);
+                            *source_client_), isc::InvalidParameter);
 }
 
 // Load an unsigned zone, all at once
@@ -593,7 +620,7 @@ TEST_F(ZoneLoaderTest, loadCheckWarn) {
 TEST_F(ZoneLoaderTest, copyCheckWarn) {
     prepareSource(Name("example.org"), "checkwarn.zone");
     ZoneLoader loader(destination_client_, Name("example.org"),
-                      source_client_);
+                      *source_client_);
     EXPECT_TRUE(loader.loadIncremental(10));
     // The messages go to the log. We don't have an easy way to examine them.
     // But the zone was committed and contains all 3 RRs

+ 1 - 1
src/lib/datasrc/zone_table_config.cc

@@ -29,7 +29,7 @@ namespace datasrc {
 namespace internal {
 
 ZoneTableConfig::ZoneTableConfig(const std::string& datasrc_type,
-                                 DataSourceClient* datasrc_client,
+                                 const DataSourceClient* datasrc_client,
                                  const Element& datasrc_conf) :
     datasrc_client_(datasrc_client)
 {

+ 2 - 2
src/lib/datasrc/zone_table_config.h

@@ -47,7 +47,7 @@ namespace internal {
 class ZoneTableConfig {
 public:
     ZoneTableConfig(const std::string& datasrc_type,
-                    DataSourceClient* datasrc_client,
+                    const DataSourceClient* datasrc_client,
                     const data::Element& datasrc_conf);
 
     /// Return corresponding \c LoadAction for the given name of zone.
@@ -66,7 +66,7 @@ public:
 
 private:
     // client of underlying data source, will be NULL for MasterFile datasrc
-    DataSourceClient* datasrc_client_;
+    const DataSourceClient* datasrc_client_;
     Zones zone_config_;
 };
 }