[2440] update add methods of RdataEncoder to return bool depending on duplicate

This is necessary to extend RdataSet class so it calculates the correct
number of RDTA/RRSIGs excluding duplicates.

src/lib/datasrc/memory/rdata_serialization.cc

@@ -520,7 +520,7 @@ RdataEncoder::start(RRClass rrclass, RRType rrtype, const void* old_data,
     impl_->old_sig_len_ = total_len;
 }
 
-void
+bool
 RdataEncoder::addRdata(const Rdata& rdata) {
     if (impl_->encode_spec_ == NULL) {
         isc_throw(InvalidOperation,
@@ -532,16 +532,18 @@ RdataEncoder::addRdata(const Rdata& rdata) {
     ConstRdataPtr rdatap = createRdata(*impl_->current_type_,
                                        *impl_->current_class_, rdata);
     if (impl_->rdatas_.find(rdatap) != impl_->rdatas_.end()) {
-        return;
+        return (false);
     }
 
     impl_->field_composer_.startRdata();
     rdata.toWire(impl_->field_composer_);
     impl_->field_composer_.endRdata();
     impl_->rdatas_.insert(rdatap);
+
+    return (true);
 }
 
-void
+bool
 RdataEncoder::addSIGRdata(const Rdata& sig_rdata) {
     if (impl_->encode_spec_ == NULL) {
         isc_throw(InvalidOperation,
@@ -552,7 +554,7 @@ RdataEncoder::addSIGRdata(const Rdata& sig_rdata) {
     ConstRdataPtr rdatap = createRdata(RRType::RRSIG(), *impl_->current_class_,
                                        sig_rdata);
     if (impl_->rrsigs_.find(rdatap) != impl_->rrsigs_.end()) {
-        return;
+        return (false);
     }
 
     const size_t cur_pos = impl_->rrsig_buffer_.getLength();
@@ -564,6 +566,8 @@ RdataEncoder::addSIGRdata(const Rdata& sig_rdata) {
     }
     impl_->rrsigs_.insert(rdatap);
     impl_->rrsig_lengths_.push_back(rrsig_datalen);
+
+    return (true);
 }
 
 size_t

src/lib/datasrc/memory/rdata_serialization.h

@@ -155,6 +155,9 @@ public:
     /// \param rrtype The RR type of RDATA to be encoded in the session.
     void start(dns::RRClass rrclass, dns::RRType rrtype);
 
+    /// \brief Start the encoding session in the merge mode.
+    ///
+    /// TBD for details.
     void start(dns::RRClass rrclass, dns::RRType rrtype,
                const void* old_data, size_t rdata_count, size_t sig_count);
 
@@ -169,6 +172,14 @@ public:
     /// to some extent, but the check is not complete; this is generally
     /// the responsibility of the caller.
     ///
+    /// This method checks if the given RDATA is a duplicate of already
+    /// added one (including ones encoded in the old data if the session
+    /// began with the merge mode).  If it's a duplicate this method ignores
+    /// the given RDATA and returns false; otherwise it returns true.
+    /// The check is based on the comparison in the "canonical form" as
+    /// described in RFC4034 Section 6.2.  In particular, domain name fields
+    /// of the RDATA are generally compared in case-insensitive manner.
+    ///
     /// The caller can destroy \c rdata after this call is completed.
     ///
     /// \note This implementation does not support RDATA (or any subfield of
@@ -189,7 +200,9 @@ public:
     /// \throw std::bad_alloc Internal memory allocation failure.
     ///
     /// \param rdata An RDATA to be encoded in the session.
-    void addRdata(const dns::rdata::Rdata& rdata);
+    /// \return true if the given RDATA was added to encode; false if
+    /// it's a duplicate and ignored.
+    bool addRdata(const dns::rdata::Rdata& rdata);
 
     /// \brief Add an RRSIG RDATA for encoding.
     ///
@@ -205,6 +218,13 @@ public:
     /// it could even accept any type of RDATA as opaque data.  It's caller's
     /// responsibility to ensure the assumption.
     ///
+    /// This method checks if the given RRSIG RDATA is a duplicate of already
+    /// added one (including ones encoded in the old data if the session
+    /// began with the merge mode).  If it's a duplicate this method ignores
+    /// the given RRSIG and returns false; otherwise it returns true.
+    /// The check is based on the comparison in the "canonical form" as
+    /// described in RFC4034 Section 6.2.
+    ///
     /// The caller can destroy \c rdata after this call is completed.
     ///
     /// \note Like addRdata(), this implementation does not support
@@ -219,7 +239,9 @@ public:
     ///
     /// \param sig_rdata An RDATA to be encoded in the session.  Supposed to
     /// be of type RRSIG.
-    void addSIGRdata(const dns::rdata::Rdata& sig_rdata);
+    /// \return true if the given RRSIG RDATA was added to encode; false if
+    /// it's a duplicate and ignored.
+    bool addSIGRdata(const dns::rdata::Rdata& sig_rdata);
 
     /// \brief Return the length of space for encoding for the session.
     ///

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

@@ -611,16 +611,28 @@ checkEncode(RRClass rrclass, RRType rrtype,
         encoder_.start(rrclass, rrtype);
     }
     size_t count = 0;
+    std::vector<ConstRdataPtr> encoded; // for duplicate check include old
     BOOST_FOREACH(const ConstRdataPtr& rdata, rdata_list) {
         if (++count > old_rdata_count) {
-            encoder_.addRdata(*rdata);
+            const bool uniq =
+                (std::find_if(encoded.begin(), encoded.end(),
+                              boost::bind(rdataMatch, rdata, _1)) ==
+                 encoded.end());
+            EXPECT_EQ(uniq, encoder_.addRdata(*rdata));
         }
+        encoded.push_back(rdata); // we need to remember old rdata too
     }
     count = 0;
+    encoded.clear();
     BOOST_FOREACH(const ConstRdataPtr& rdata, rrsig_list) {
         if (++count > old_rrsig_count) {
-            encoder_.addSIGRdata(*rdata);
+            const bool uniq =
+                (std::find_if(encoded.begin(), encoded.end(),
+                              boost::bind(rdataMatch, rdata, _1)) ==
+                 encoded.end());
+            EXPECT_EQ(uniq, encoder_.addSIGRdata(*rdata));
         }
+        encoded.push_back(rdata);
     }
     const size_t storage_len = encoder_.getStorageLength();
     encodeWrapper(storage_len);