[svn] commit: r2298 - in /branches/trac192/src/lib/datasrc: data_source.cc data_source.h tests/datasrc_unittest.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Jun 28 01:07:26 UTC 2010
Author: jinmei
Date: Mon Jun 28 01:07:25 2010
New Revision: 2298
Log:
updated DataSrcMatch class implementation based on review discussions.
with some updates/cleanups:
- gave more detailed doc
- added more tests, one of which seems to identify a bug
- constify as much as possible
Modified:
branches/trac192/src/lib/datasrc/data_source.cc
branches/trac192/src/lib/datasrc/data_source.h
branches/trac192/src/lib/datasrc/tests/datasrc_unittest.cc
Modified: branches/trac192/src/lib/datasrc/data_source.cc
==============================================================================
--- branches/trac192/src/lib/datasrc/data_source.cc (original)
+++ branches/trac192/src/lib/datasrc/data_source.cc Mon Jun 28 01:07:25 2010
@@ -62,12 +62,11 @@
const isc::dns::RRType& t = isc::dns::RRType::ANY()) :
top_source_(ts),
dsm_(((t == RRType::DS() && n.getLabelCount() != 1)
- ? n.split(1, n.getLabelCount() - 1)
- : n),
- c, t)
+ ? n.split(1, n.getLabelCount() - 1) : n),
+ c)
{}
- Name* getEnclosingZone() {
+ const Name* getEnclosingZone() {
if (dsm_.getEnclosingZone() == NULL) {
top_source_->findClosestEnclosure(dsm_);
}
@@ -326,7 +325,7 @@
// so now we proceed with the low-level data source lookup, and cache
// whatever we find.
const DataSrc* ds = zoneinfo.getDataSource();
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
if (ds == NULL) {
task.flags |= DataSrc::NO_SUCH_ZONE;
@@ -505,7 +504,7 @@
// referrals.
inline bool
hasDelegation(Query& q, QueryTaskPtr task, ZoneInfo& zoneinfo) {
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
if (zonename == NULL) {
if (task->state == QueryTask::GETANSWER) {
q.message().setRcode(Rcode::REFUSED());
@@ -573,7 +572,7 @@
Message& m = q.message();
RRsetList soa;
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
QueryTask newtask(q, *zonename, RRType::SOA(), QueryTask::SIMPLE_QUERY);
RETERR(doQueryTask(newtask, zoneinfo, soa));
if (newtask.flags != 0) {
@@ -603,7 +602,7 @@
inline DataSrc::Result
getNsec3(Query& q, ZoneInfo& zoneinfo, string& hash, RRsetPtr& target) {
const DataSrc* ds = zoneinfo.getDataSource();
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
if (ds == NULL) {
q.message().setRcode(Rcode::SERVFAIL());
@@ -622,7 +621,7 @@
DataSrc::Result result;
RRsetList nsec3param;
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
QueryTask newtask(q, *zonename, RRType::NSEC3PARAM(),
QueryTask::SIMPLE_QUERY);
result = doQueryTask(newtask, zoneinfo, nsec3param);
@@ -654,7 +653,7 @@
inline DataSrc::Result
proveNX(Query& q, QueryTaskPtr task, ZoneInfo& zoneinfo, const bool wildcard) {
Message& m = q.message();
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
ConstNsec3ParamPtr nsec3 = getNsec3Param(q, zoneinfo);
if (nsec3 != NULL) {
@@ -748,7 +747,7 @@
return (DataSrc::SUCCESS);
}
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
const int nlen = task->qname.getLabelCount();
const int diff = nlen - zonename->getLabelCount();
if (diff < 1) {
@@ -889,7 +888,7 @@
// Query found a referral; let's find out if that was expected--
// i.e., if an NS was at the zone apex, or if we were querying
// specifically for, and found, a DS, NSEC, or DNAME record.
- Name* zonename = zoneinfo.getEnclosingZone();
+ const Name* const zonename = zoneinfo.getEnclosingZone();
if ((task->flags & REFERRAL) != 0 &&
(zonename->getLabelCount() == task->qname.getLabelCount() ||
((task->qtype == RRType::NSEC() ||
Modified: branches/trac192/src/lib/datasrc/data_source.h
==============================================================================
--- branches/trac192/src/lib/datasrc/data_source.h (original)
+++ branches/trac192/src/lib/datasrc/data_source.h Mon Jun 28 01:07:25 2010
@@ -27,7 +27,6 @@
#include <dns/name.h>
#include <dns/rrclass.h>
-#include <dns/rrtype.h>
#include <cc/data.h>
namespace isc {
@@ -297,79 +296,107 @@
std::vector<ConstDataSrcPtr> data_sources;
};
-/// \brief Information about the zone for a given query
+/// \brief Information about the zone along with the %data source that best
+/// matches a give name and RR class.
///
-/// The \c DataSrcMatch object is created with the query name and query
-/// class (and optionally, query type) for an ongoing query, and the
-/// top-level data source. When it becomes necessary to find the
-/// closest enclosing zone matching the query and the concrete data
-/// source which is authoritative for that zone, then the \c DataSrcMatch
-/// object will search through data sources to make that determination.
-/// (This is done only when needed, so that the search can be avoided
-/// when the query can be answered from the cache.)
+/// A \c DataSrcMatch object is created with a domain name and RR class to
+/// hold the search state of looking for the zone and the %data source that
+/// stores the zone that best match the given name and RR class.
+/// The application of this class passes an object of \c DataSrcMatch to
+/// one or more ^data sources via their \c findClosestEnclosure() method.
+/// The %data source searches its content for the given key, and update
+/// the state if it finds a better zone than the currently recorded one.
+///
+/// The state of a \c DataSrcMatch object should be updated if and only if:
+/// - The specified RR class and the RR class of the %data source are the
+// same, or the specified RR class is ANY; and
+/// - There is no matching %data source and name found (which is probably
+/// wrong, see below), or the given enclosing name gives a longer match
+/// than the currently stored enclosing name against the specified name.
class DataSrcMatch {
///
/// \name Constructors, Assignment Operator and Destructor.
///
- /// Note: The copy constructor and the assignment operator are intentionally
- /// defined as private.
+ /// Note: The copy constructor and the assignment operator are
+ /// intentionally defined as private.
//@{
private:
DataSrcMatch(const DataSrcMatch& source);
DataSrcMatch& operator=(const DataSrcMatch& source);
public:
- /// \brief Constructor
- ///
- /// Queries for the DS rrtype must be sent to the parent zone, not
- /// to the zone matching the query name. Therefore, if a query type of
- /// DS is specified here, the query name is altered, so that when
- /// we search for the closest enclosing zone, we'll find the right one.
- ///
- /// If no type is specified, a default of ANY() is used.
- ///
- /// \param ds Pointer to the top-level data source
- /// \param qname Query name
- /// \param qclass Query class
- /// \param qtype Query type
- DataSrcMatch(const isc::dns::Name& qname,
- const isc::dns::RRClass& qclass,
- const isc::dns::RRType& qtype = isc::dns::RRType::ANY()) :
+ /// \brief The constructor.
+ ///
+ /// This constructor normally doesn't throw an exception. However,
+ /// it creates a copy of the given name object, which may require memory
+ /// allocation, and if it fails the corresponding standard exception will
+ /// be thrown.
+ ///
+ /// \param name The domain name to be matched.
+ /// \param rrclass The RR class to be matched
+ DataSrcMatch(const isc::dns::Name& name,
+ const isc::dns::RRClass& rrclass) :
closest_name_(NULL), best_source_(NULL),
- qname_(qname), qclass_(qclass), qtype_(qtype)
+ name_(name), rrclass_(rrclass)
{}
~DataSrcMatch();
//@}
/// \name Getter and Setter Methods
//@{
- /// \brief Returns the query name.
- const isc::dns::Name& getName() const { return (qname_); }
-
- /// \brief Returns the query class.
- const isc::dns::RRClass& getClass() const { return (qclass_); }
-
- /// \brief Returns the best enclosing zone name found for the query so far.
+ /// \brief Returns the name to be matched.
+ const isc::dns::Name& getName() const { return (name_); }
+
+ /// \brief Returns the RR class to be matched.
+ ///
+ /// This method never throws an exception.
+ const isc::dns::RRClass& getClass() const { return (rrclass_); }
+
+ /// \brief Returns the best enclosing zone name found for the given
+ // name and RR class so far.
///
/// \return A pointer to the zone apex \c Name, NULL if none found yet.
- isc::dns::Name* getEnclosingZone() { return (closest_name_); }
-
- /// \brief Returns the best data source found for the query so far.
- ///
- /// \return A pointer to a concrete data source, NULL if none found yet.
- const DataSrc* getDataSource() { return (best_source_); }
+ ///
+ /// This method never throws an exception.
+ const isc::dns::Name* getEnclosingZone() const { return (closest_name_); }
+
+ /// \brief Returns the best %data source found for the given name and
+ /// RR class so far.
+ ///
+ /// This method never throws an exception.
+ ///
+ /// \return A pointer to a concrete %data source, NULL if none found yet.
+ const DataSrc* getDataSource() const { return (best_source_); }
//@}
- /// \brief Called by a concrete data source's
- /// <code>findClosestEnclosure()</code> routine to store the
- /// best match for the query that has found so far.
+ /// \brief Update the object state with better information if possible.
+ ///
+ /// This method is intended to be called by a concrete %data source's
+ /// \c findClosestEnclosure() method to store the best match for
+ /// the given name and class that has been found so far.
+ ///
+ /// It compares the best name (if found) and \c container, and if the
+ /// latter gives a longer match, it will install the given %data source
+ /// and the enclosing name as the best match;
+ /// if there is no known pair of %data source and enclosing name,
+ /// this method will install the given pair unconditionally.
+ /// (which is probably BAD);
+ /// otherwise this method does nothing.
+ ///
+ /// In any case, if a new pair of %data source and enclosing name are
+ /// installed, a new name object will be internally allocated.
+ /// And, if memory allocation fails the corresponding standard exception
+ /// will be thrown.
+ ///
+ /// \param new_source A candidate %data source that gives a better match.
+ /// \param container The enclosing name of the matching zone in
+ /// \c new_source.
void update(const DataSrc& new_source, const isc::dns::Name& container);
private:
isc::dns::Name* closest_name_;
const DataSrc* best_source_;
- const isc::dns::Name qname_;
- const isc::dns::RRClass& qclass_;
- const isc::dns::RRType& qtype_;
+ const isc::dns::Name name_;
+ const isc::dns::RRClass& rrclass_;
};
class Nsec3Param {
Modified: branches/trac192/src/lib/datasrc/tests/datasrc_unittest.cc
==============================================================================
--- branches/trac192/src/lib/datasrc/tests/datasrc_unittest.cc (original)
+++ branches/trac192/src/lib/datasrc/tests/datasrc_unittest.cc Mon Jun 28 01:07:25 2010
@@ -868,28 +868,104 @@
EXPECT_EQ(0, ds.dataSrcCount());
}
-TEST_F(DataSrcTest, DataSrcMatch) {
- MetaDataSrc ds;
- ConstDataSrcPtr tsp = ConstDataSrcPtr(new TestDataSrc);
- ds.addDataSrc(tsp);
+class DataSrcMatchTest : public ::testing::Test {
+protected:
+ DataSrcMatchTest() {
+ datasrc1.init();
+ }
+ // test data source serves example.com/IN.
+ TestDataSrc datasrc1;
+ // this data source is dummy. Its content doesn't matter in the tests.
+ TestDataSrc datasrc2;
+};
+
+TEST_F(DataSrcMatchTest, match) {
DataSrcMatch match(Name("very.very.long.example.com"), RRClass::IN());
- ds.findClosestEnclosure(match);
+ datasrc1.findClosestEnclosure(match);
EXPECT_EQ(Name("example.com"), *match.getEnclosingZone());
- EXPECT_EQ(tsp.get(), match.getDataSource());
-}
-
-TEST_F(DataSrcTest, DISABLED_DataSrcMatchByType) {
- MetaDataSrc ds;
- ConstDataSrcPtr tsp = ConstDataSrcPtr(new TestDataSrc);
- ds.addDataSrc(tsp);
-
- DataSrcMatch match1(Name("sql1.example.com"), RRClass::IN(), RRType::NS());
- ds.findClosestEnclosure(match1);
- EXPECT_EQ(Name("sql1.example.com"), *match1.getEnclosingZone());
-
- DataSrcMatch match2(Name("sql1.example.com"), RRClass::IN(), RRType::DS());
- ds.findClosestEnclosure(match2);
- EXPECT_EQ(Name("example.com"), *match2.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
+}
+
+TEST_F(DataSrcMatchTest, matchWithWrongClass) {
+ DataSrcMatch match(Name("very.very.long.example.com"), RRClass::CH());
+ datasrc1.findClosestEnclosure(match);
+ EXPECT_EQ(NULL, match.getEnclosingZone());
+ EXPECT_EQ(NULL, match.getDataSource());
+}
+
+TEST_F(DataSrcMatchTest, matchWithAnyClass) {
+ DataSrcMatch match(Name("very.very.long.example.com"), RRClass::ANY());
+ datasrc1.findClosestEnclosure(match);
+ EXPECT_EQ(Name("example.com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
+}
+
+TEST_F(DataSrcMatchTest, updateWithWrongClass) {
+ DataSrcMatch match(Name("www.example.com"), RRClass::CH());
+ match.update(datasrc2, Name("com"));
+ EXPECT_EQ(Name("com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc2, match.getDataSource());
+
+ // datasrc1 gives a better enclosing name, but RR class doesn't match.
+ // hmm...what should we do? the current implementation simply ignores
+ // the mismatch of the class, but it doesn't seem to be correct.
+ EXPECT_EQ(RRClass::IN(), datasrc1.getClass());
+ match.update(datasrc1, Name("example.com"));
+ EXPECT_EQ(Name("example.com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
+}
+
+TEST_F(DataSrcMatchTest, updateAgainstAnyClass) {
+ DataSrcMatch match(Name("www.example.com"), RRClass::ANY());
+ match.update(datasrc2, Name("com"));
+ EXPECT_EQ(Name("com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc2, match.getDataSource());
+
+ // see the previous test. in this case the given class for search is
+ // ANY, so update should be okay. but the fact is that the implementation
+ // simply ignores the RR class.
+ EXPECT_EQ(RRClass::IN(), datasrc1.getClass());
+ match.update(datasrc1, Name("example.com"));
+ EXPECT_EQ(Name("example.com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
+}
+
+TEST_F(DataSrcMatchTest, updateWithNoMatch) {
+ DataSrcMatch match(Name("www.example.com"), RRClass::IN());
+ match.update(datasrc1, Name("com"));
+ EXPECT_EQ(Name("com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
+
+ // An attempt of update with a name that doesn't match. This attempt
+ // should be ignored.
+ match.update(datasrc2, Name("example.org"));
+ EXPECT_EQ(Name("com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
+}
+
+// This test currently fails.
+TEST_F(DataSrcMatchTest, DISABLED_initialUpdateWithNoMatch) {
+ DataSrcMatch match(Name("www.example.com"), RRClass::IN());
+
+ // An initial attempt of update with a name that doesn't match.
+ // Should be ignored.
+ match.update(datasrc1, Name("example.org"));
+ EXPECT_EQ(NULL, match.getEnclosingZone());
+ EXPECT_EQ(NULL, match.getDataSource());
+}
+
+TEST_F(DataSrcMatchTest, updateWithShorterMatch) {
+ DataSrcMatch match(Name("www.example.com"), RRClass::IN());
+
+ match.update(datasrc1, Name("example.com"));
+ EXPECT_EQ(Name("example.com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
+
+ // An attempt of update with a name that gives a shorter match.
+ // This attempt should be ignored.
+ match.update(datasrc2, Name("com"));
+ EXPECT_EQ(Name("example.com"), *match.getEnclosingZone());
+ EXPECT_EQ(&datasrc1, match.getDataSource());
}
#if 0 // currently fails
More information about the bind10-changes
mailing list