Browse Source

[2377] Change the callback interface

As we pass exactly one RR each time, RRset is overkill. We just pass
bunch of parameters to the callback.
Michal 'vorner' Vaner 12 years ago
parent
commit
b548f4ed6c

+ 15 - 5
src/lib/datasrc/master_loader_callbacks.cc

@@ -48,6 +48,19 @@ logWarning(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
         arg(name).arg(rrclass).arg(reason);
         arg(name).arg(rrclass).arg(reason);
 }
 }
 
 
+void
+addRR(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
+      const isc::dns::RRType& type, const isc::dns::RRTTL& ttl,
+      const isc::dns::rdata::RdataPtr& data, ZoneUpdater* updater)
+{
+    // We get description of one RR. The updater takes RRset, so we
+    // wrap it up and push there. It should collate the RRsets of the
+    // same name and type together, since the addRRset should "merge".
+    isc::dns::BasicRRset rrset(name, rrclass, type, ttl);
+    rrset.addRdata(data);
+    updater->addRRset(rrset);
+}
+
 }
 }
 
 
 isc::dns::MasterLoaderCallbacks
 isc::dns::MasterLoaderCallbacks
@@ -61,12 +74,9 @@ createMasterLoaderCallbacks(const isc::dns::Name& name,
                                                         rrclass, _1, _2, _3)));
                                                         rrclass, _1, _2, _3)));
 }
 }
 
 
-isc::dns::AddRRsetCallback
+isc::dns::AddRRCallback
 createMasterLoaderAddCallback(ZoneUpdater& updater) {
 createMasterLoaderAddCallback(ZoneUpdater& updater) {
-    return (boost::bind(&ZoneUpdater::addRRset, &updater,
-                        // The callback provides a shared pointer, we
-                        // need the object. This bind unpacks the object.
-                        boost::bind(&isc::dns::RRsetPtr::operator*, _1)));
+    return (boost::bind(addRR, _1, _2, _3, _4, _5, &updater));
 }
 }
 
 
 }
 }

+ 1 - 1
src/lib/datasrc/master_loader_callbacks.h

@@ -58,7 +58,7 @@ createMasterLoaderCallbacks(const isc::dns::Name& name,
 /// \param updater The zone updater to use.
 /// \param updater The zone updater to use.
 /// \return The callback to be passed to MasterLoader.
 /// \return The callback to be passed to MasterLoader.
 /// \throw std::bad_alloc when allocation fails.
 /// \throw std::bad_alloc when allocation fails.
-isc::dns::AddRRsetCallback
+isc::dns::AddRRCallback
 createMasterLoaderAddCallback(ZoneUpdater& updater);
 createMasterLoaderAddCallback(ZoneUpdater& updater);
 
 
 }
 }

+ 16 - 7
src/lib/datasrc/tests/master_loader_callbacks_test.cc

@@ -18,6 +18,7 @@
 #include <dns/rrset.h>
 #include <dns/rrset.h>
 #include <dns/rrclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
 #include <dns/rrttl.h>
+#include <dns/rdata.h>
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
@@ -40,8 +41,8 @@ public:
     // the correct ones, according to a predefined set in a list.
     // the correct ones, according to a predefined set in a list.
     virtual void addRRset(const isc::dns::AbstractRRset& rrset) {
     virtual void addRRset(const isc::dns::AbstractRRset& rrset) {
         ASSERT_FALSE(expected_rrsets_.empty());
         ASSERT_FALSE(expected_rrsets_.empty());
-        // In our tests, pointer equality is enough.
-        EXPECT_EQ(expected_rrsets_.front().get(), &rrset);
+
+        EXPECT_EQ(expected_rrsets_.front().get()->toText(), rrset.toText());
         // And remove this RRset, as it has been used.
         // And remove this RRset, as it has been used.
         expected_rrsets_.pop_front();
         expected_rrsets_.pop_front();
     }
     }
@@ -67,14 +68,22 @@ protected:
                                                isc::dns::RRClass::IN(), &ok_))
                                                isc::dns::RRClass::IN(), &ok_))
     {}
     {}
     // Generate a new RRset, put it to the updater and return it.
     // Generate a new RRset, put it to the updater and return it.
-    isc::dns::RRsetPtr generateRRset() {
+    void generateRRset(isc::dns::AddRRCallback callback) {
         const isc::dns::RRsetPtr
         const isc::dns::RRsetPtr
             result(new isc::dns::RRset(isc::dns::Name("example.org"),
             result(new isc::dns::RRset(isc::dns::Name("example.org"),
                                        isc::dns::RRClass::IN(),
                                        isc::dns::RRClass::IN(),
                                        isc::dns::RRType::A(),
                                        isc::dns::RRType::A(),
                                        isc::dns::RRTTL(3600)));
                                        isc::dns::RRTTL(3600)));
+        const isc::dns::rdata::RdataPtr
+            data(isc::dns::rdata::createRdata(isc::dns::RRType::A(),
+                                              isc::dns::RRClass::IN(),
+                                              "192.0.2.1"));
+
+        result->addRdata(data);
         updater_.expected_rrsets_.push_back(result);
         updater_.expected_rrsets_.push_back(result);
-        return (result);
+
+        callback(result->getName(), result->getClass(), result->getType(),
+                 result->getTTL(), data);
     }
     }
     // An updater to be passed to the context
     // An updater to be passed to the context
     MockUpdater updater_;
     MockUpdater updater_;
@@ -112,11 +121,11 @@ TEST_F(MasterLoaderCallbackTest, callbacks) {
 
 
 // Try adding some RRsets.
 // Try adding some RRsets.
 TEST_F(MasterLoaderCallbackTest, addRRset) {
 TEST_F(MasterLoaderCallbackTest, addRRset) {
-    isc::dns::AddRRsetCallback
+    isc::dns::AddRRCallback
         callback(createMasterLoaderAddCallback(updater_));
         callback(createMasterLoaderAddCallback(updater_));
     // Put some of them in.
     // Put some of them in.
-    EXPECT_NO_THROW(callback(generateRRset()));
-    EXPECT_NO_THROW(callback(generateRRset()));
+    EXPECT_NO_THROW(generateRRset(callback));
+    EXPECT_NO_THROW(generateRRset(callback));
     // They all get pushed there right away, so there are none in the queue
     // They all get pushed there right away, so there are none in the queue
     EXPECT_TRUE(updater_.expected_rrsets_.empty());
     EXPECT_TRUE(updater_.expected_rrsets_.empty());
 }
 }

+ 4 - 12
src/lib/dns/master_loader.cc

@@ -18,7 +18,6 @@
 #include <dns/rrttl.h>
 #include <dns/rrttl.h>
 #include <dns/rrclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 #include <dns/rrtype.h>
-#include <dns/rrset.h>
 #include <dns/rdata.h>
 #include <dns/rdata.h>
 
 
 using std::string;
 using std::string;
@@ -32,7 +31,7 @@ public:
                      const Name& zone_origin,
                      const Name& zone_origin,
                      const RRClass& zone_class,
                      const RRClass& zone_class,
                      const MasterLoaderCallbacks& callbacks,
                      const MasterLoaderCallbacks& callbacks,
-                     const AddRRsetCallback& add_callback,
+                     const AddRRCallback& add_callback,
                      MasterLoader::Options options) :
                      MasterLoader::Options options) :
         lexer_(),
         lexer_(),
         zone_origin_(zone_origin),
         zone_origin_(zone_origin),
@@ -88,13 +87,7 @@ public:
             // the Rdata. The errors should have been reported by
             // the Rdata. The errors should have been reported by
             // callbacks_ already, so we just need to not report the RR
             // callbacks_ already, so we just need to not report the RR
             if (data != rdata::RdataPtr()) {
             if (data != rdata::RdataPtr()) {
-                // Create the RRset. We don't need the RRSIG, so we are good
-                // with the basic one
-                const RRsetPtr rrset(new BasicRRset(name, rrclass, rrtype,
-                                                    ttl));
-                rrset->addRdata(data);
-                // OK now, so give the RRset with single RR to the caller
-                add_callback_(rrset);
+                add_callback_(name, rrclass, rrtype, ttl, data);
 
 
                 // Good, we loaded another one
                 // Good, we loaded another one
                 ++count;
                 ++count;
@@ -108,16 +101,15 @@ private:
     const Name& zone_origin_;
     const Name& zone_origin_;
     const RRClass zone_class_;
     const RRClass zone_class_;
     MasterLoaderCallbacks callbacks_;
     MasterLoaderCallbacks callbacks_;
-    AddRRsetCallback add_callback_;
+    AddRRCallback add_callback_;
     MasterLoader::Options options_;
     MasterLoader::Options options_;
-    RRsetPtr current_rrset_;
 };
 };
 
 
 MasterLoader::MasterLoader(const char* master_file,
 MasterLoader::MasterLoader(const char* master_file,
                            const Name& zone_origin,
                            const Name& zone_origin,
                            const RRClass& zone_class,
                            const RRClass& zone_class,
                            const MasterLoaderCallbacks& callbacks,
                            const MasterLoaderCallbacks& callbacks,
-                           const AddRRsetCallback& add_callback,
+                           const AddRRCallback& add_callback,
                            Options options)
                            Options options)
 {
 {
     impl_ = new MasterLoaderImpl(master_file, zone_origin,
     impl_ = new MasterLoaderImpl(master_file, zone_origin,

+ 1 - 1
src/lib/dns/master_loader.h

@@ -33,7 +33,7 @@ public:
                  const Name& zone_origin,
                  const Name& zone_origin,
                  const RRClass& zone_class,
                  const RRClass& zone_class,
                  const MasterLoaderCallbacks& callbacks,
                  const MasterLoaderCallbacks& callbacks,
-                 const AddRRsetCallback& add_callback,
+                 const AddRRCallback& add_callback,
                  Options options = DEFAULT);
                  Options options = DEFAULT);
     ~MasterLoader();
     ~MasterLoader();
 
 

+ 12 - 4
src/lib/dns/master_loader_callbacks.h

@@ -23,9 +23,14 @@
 
 
 namespace isc {
 namespace isc {
 namespace dns {
 namespace dns {
-
-class AbstractRRset;
-typedef boost::shared_ptr<AbstractRRset> RRsetPtr;
+class Name;
+class RRClass;
+class RRType;
+class RRTTL;
+namespace rdata {
+class Rdata;
+typedef boost::shared_ptr<Rdata> RdataPtr;
+}
 
 
 /// \brief Type of callback to add a RRset.
 /// \brief Type of callback to add a RRset.
 ///
 ///
@@ -36,7 +41,10 @@ typedef boost::shared_ptr<AbstractRRset> RRsetPtr;
 /// \param RRset The rrset to add. It does not contain the accompanying
 /// \param RRset The rrset to add. It does not contain the accompanying
 ///     RRSIG (if the zone is signed), they are reported with separate
 ///     RRSIG (if the zone is signed), they are reported with separate
 ///     calls to the callback.
 ///     calls to the callback.
-typedef boost::function<void(const RRsetPtr& rrset)> AddRRsetCallback;
+typedef boost::function<void(const Name& name, const RRClass& rrclass,
+                             const RRType& rrtype, const RRTTL& rrttl,
+                             const rdata::RdataPtr& rdata)>
+    AddRRCallback;
 
 
 /// \brief Set of issue callbacks for a loader.
 /// \brief Set of issue callbacks for a loader.
 ///
 ///

+ 7 - 2
src/lib/dns/tests/master_loader_unittest.cc

@@ -55,7 +55,11 @@ public:
         }
         }
     }
     }
 
 
-    void addRRset(const RRsetPtr& rrset) {
+    void addRRset(const Name& name, const RRClass& rrclass,
+                  const RRType& rrtype, const RRTTL& rrttl,
+                  const rdata::RdataPtr& data) {
+        const RRsetPtr rrset(new BasicRRset(name, rrclass, rrtype, rrttl));
+        rrset->addRdata(data);
         rrsets_.push_back(rrset);
         rrsets_.push_back(rrset);
     }
     }
 
 
@@ -66,7 +70,8 @@ public:
                                         file).c_str(), origin, rrclass,
                                         file).c_str(), origin, rrclass,
                                        callbacks_,
                                        callbacks_,
                                        boost::bind(&MasterLoaderTest::addRRset,
                                        boost::bind(&MasterLoaderTest::addRRset,
-                                                   this, _1), options));
+                                                   this, _1, _2, _3, _4, _5),
+                                       options));
     }
     }
 
 
     // Check the next RR in the ones produced by the loader
     // Check the next RR in the ones produced by the loader