Merge branch 'trac461'

src/lib/datasrc/rbtree.h

@@ -54,7 +54,7 @@ operator-(const isc::dns::Name& super_name, const isc::dns::Name& sub_name) {
 }
 }
 
-template <typename T>
+template <typename T, bool returnEmptyNode>
 class RBTree;
 
 /// \brief \c RBNode is used by RBTree to store any data related to one domain
@@ -82,7 +82,8 @@ class RBNode : public boost::noncopyable {
 private:
     /// The RBNode is meant for use from within RBTree, so it has access to
     /// it.
-    friend class RBTree<T>;
+    template <typename U, bool returnEmptyNode>
+    friend class RBTree;
 
     /// \name Constructors
     ///
@@ -238,6 +239,7 @@ template <typename T>
 RBNode<T>::~RBNode() {
 }
 
+
 // note: the following class description is documented using multiline comments
 // because the verbatim diagram contain a backslash, which could be interpreted
 // as escape of newline in singleline comment.
@@ -256,6 +258,12 @@ RBNode<T>::~RBNode() {
  *  - Decreases the memory footprint, as it doesn't store the suffix labels
  *      multiple times.
  *
+ *  Depending on different usage, rbtree will support different search policy.
+ *  Whether return empty node to end user is one policy among them. Search
+ *  policy is as the last template parameter, the default policy will NOT
+ *  return empty node to end user, pass ture will get empty node during find
+ *  is needed
+ *
  * \anchor diagram
  *
  * with the following names:
@@ -285,10 +293,6 @@ RBNode<T>::~RBNode() {
                                     / \
                                    o   q
    \endverbatim
- *  \note open problems:
- *  - current \c find() function only returns non-empty nodes, so there is no
- *    difference between find a non existent name and a name corresponding to
- *    an empty non-terminal nodes, but in DNS query logic, they are different
  *  \todo
  *  - add remove interface
  *  - add iterator to iterate over the whole \c RBTree.  This may be necessary,
@@ -296,7 +300,7 @@ RBNode<T>::~RBNode() {
  *  - since \c RBNode only has down pointer without up pointer, the node path
  *    during finding should be recorded for later use
  */
-template <typename T>
+template <typename T, bool returnEmptyNode = false>
 class RBTree : public boost::noncopyable {
     friend class RBNode<T>;
 public:
@@ -531,21 +535,22 @@ private:
     unsigned int node_count_;
 };
 
-template <typename T>
-RBTree<T>::RBTree() {
+template <typename T, bool S>
+RBTree<T,S>::RBTree() {
     NULLNODE = RBNode<T>::NULL_NODE();
     root_ = NULLNODE;
     node_count_ = 0;
 }
 
-template <typename T>
-RBTree<T>::~RBTree() {
+template <typename T, bool S>
+RBTree<T,S>::~RBTree() {
     deleteHelper(root_);
     assert(node_count_ == 0);
 }
 
-template <typename T>
-void RBTree<T> ::deleteHelper(RBNode<T> *root) {
+template <typename T, bool S>
+void 
+RBTree<T,S>::deleteHelper(RBNode<T> *root) {
     if (root == NULLNODE) {
         return;
     }
@@ -574,9 +579,10 @@ void RBTree<T> ::deleteHelper(RBNode<T> *root) {
     --node_count_;
 }
 
-template <typename T> template <typename CBARG>
-typename RBTree<T>::Result
-RBTree<T>::find(const isc::dns::Name& name, RBNode<T>** node,
+template <typename T, bool S>
+template <typename CBARG>
+typename RBTree<T,S>::Result
+RBTree<T,S>::find(const isc::dns::Name& name, RBNode<T>** node,
                 bool (*callback)(const RBNode<T>&, CBARG),
                 CBARG callback_arg) const
 {
@@ -584,9 +590,10 @@ RBTree<T>::find(const isc::dns::Name& name, RBNode<T>** node,
     return (findHelper(name, &up_node, node, callback, callback_arg));
 }
 
-template <typename T> template <typename CBARG>
-typename RBTree<T>::Result
-RBTree<T>::find(const isc::dns::Name& name, const RBNode<T>** node,
+template <typename T, bool S>
+template <typename CBARG>
+typename RBTree<T,S>::Result
+RBTree<T,S>::find(const isc::dns::Name& name, const RBNode<T>** node,
                 bool (*callback)(const RBNode<T>&, CBARG),
                 CBARG callback_arg) const
 {
@@ -600,13 +607,14 @@ RBTree<T>::find(const isc::dns::Name& name, const RBNode<T>** node,
     return (ret);
 }
 
-template <typename T> template <typename CBARG>
-typename RBTree<T>::Result
-RBTree<T>::findHelper(const isc::dns::Name& target_name,
-                      const RBNode<T>** up_node,
-                      RBNode<T>** target,
-                      bool (*callback)(const RBNode<T>&, CBARG),
-                      CBARG callback_arg) const
+template <typename T, bool returnEmptyNode>
+template <typename CBARG>
+typename RBTree<T,returnEmptyNode>::Result
+RBTree<T,returnEmptyNode>::findHelper(const isc::dns::Name& target_name,
+                                      const RBNode<T>** up_node,
+                                      RBNode<T>** target,
+                                      bool (*callback)(const RBNode<T>&, CBARG),
+                                      CBARG callback_arg) const
 {
     using namespace helper;
 
@@ -621,7 +629,7 @@ RBTree<T>::findHelper(const isc::dns::Name& target_name,
         const isc::dns::NameComparisonResult::NameRelation relation =
             compare_result.getRelation();
         if (relation == isc::dns::NameComparisonResult::EQUAL) {
-            if (!node->isEmpty()) {
+            if (returnEmptyNode || !node->isEmpty()) {
                 *target = node;
                 ret = EXACTMATCH;
             }
@@ -634,7 +642,7 @@ RBTree<T>::findHelper(const isc::dns::Name& target_name,
                 node = (compare_result.getOrder() < 0) ?
                     node->left_ : node->right_;
             } else if (relation == isc::dns::NameComparisonResult::SUBDOMAIN) {
-                if (!node->isEmpty()) {
+                if (returnEmptyNode || !node->isEmpty()) {
                     ret = RBTree<T>::PARTIALMATCH;
                     *target = node;
                     if (callback != NULL && node->callback_required_) {
@@ -655,9 +663,11 @@ RBTree<T>::findHelper(const isc::dns::Name& target_name,
     return (ret);
 }
 
-template <typename T>
-typename RBTree<T>::Result
-RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
+
+template <typename T, bool returnEmptyNode>
+typename RBTree<T,returnEmptyNode>::Result
+RBTree<T,returnEmptyNode>::insert(const isc::dns::Name& target_name,
+                                  RBNode<T>** new_node) {
     using namespace helper;
     RBNode<T>* parent = NULLNODE;
     RBNode<T>* current = root_;
@@ -674,7 +684,12 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
             if (new_node != NULL) {
                 *new_node = current;
             }
-            return (ALREADYEXISTS);
+
+            if (current->isEmpty() && !returnEmptyNode) {
+                return (SUCCESS);
+            } else {
+                return (ALREADYEXISTS);
+            }
         } else {
             const int common_label_count = compare_result.getCommonLabels();
             if (common_label_count == 1) {
@@ -730,9 +745,10 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
     return (SUCCESS);
 }
 
-template <typename T>
+
+template <typename T, bool S>
 void
-RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
+RBTree<T,S>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
     using namespace helper;
     const isc::dns::Name sub_name = node.name_ - base_name;
     // using auto_ptr here is to avoid memory leak in case of exception raised
@@ -752,9 +768,10 @@ RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
     down_node.release();
 }
 
-template <typename T>
+
+template <typename T, bool S>
 void
-RBTree<T>::insertRebalance(RBNode<T>** root, RBNode<T>* node) {
+RBTree<T,S>::insertRebalance(RBNode<T>** root, RBNode<T>* node) {
 
     RBNode<T>* uncle;
     while (node != *root && node->parent_->color_ == RBNode<T>::RED) {
@@ -798,9 +815,9 @@ RBTree<T>::insertRebalance(RBNode<T>** root, RBNode<T>* node) {
 }
 
 
-template <typename T>
+template <typename T, bool S>
 RBNode<T>*
-RBTree<T>::leftRotate(RBNode<T>** root, RBNode<T>* node) {
+RBTree<T,S>::leftRotate(RBNode<T>** root, RBNode<T>* node) {
     RBNode<T>* right = node->right_;
     node->right_ = right->left_;
     if (right->left_ != NULLNODE)
@@ -823,9 +840,9 @@ RBTree<T>::leftRotate(RBNode<T>** root, RBNode<T>* node) {
     return (node);
 }
 
-template <typename T>
+template <typename T, bool S>
 RBNode<T>*
-RBTree<T>::rightRotate(RBNode<T>** root, RBNode<T>* node) {
+RBTree<T,S>::rightRotate(RBNode<T>** root, RBNode<T>* node) {
     RBNode<T>* left = node->left_;
     node->left_ = left->right_;
     if (left->right_ != NULLNODE)
@@ -847,17 +864,18 @@ RBTree<T>::rightRotate(RBNode<T>** root, RBNode<T>* node) {
     return (node);
 }
 
-template <typename T>
+
+template <typename T, bool S>
 void
-RBTree<T>::dumpTree(std::ostream& os, unsigned int depth) const {
+RBTree<T,S>::dumpTree(std::ostream& os, unsigned int depth) const {
     indent(os, depth);
     os << "tree has " << node_count_ << " node(s)\n";
     dumpTreeHelper(os, root_, depth);
 }
 
-template <typename T>
+template <typename T, bool S>
 void
-RBTree<T>::dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
+RBTree<T,S>::dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
                           unsigned int depth) const
 {
     if (node == NULLNODE) {
@@ -882,13 +900,15 @@ RBTree<T>::dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
     dumpTreeHelper(os, node->right_, depth + 1);
 }
 
-template <typename T>
+template <typename T, bool S>
 void
-RBTree<T>::indent(std::ostream& os, unsigned int depth) {
+RBTree<T,S>::indent(std::ostream& os, unsigned int depth) {
     static const unsigned int INDENT_FOR_EACH_DEPTH = 5;
     os << std::string(depth * INDENT_FOR_EACH_DEPTH, ' ');
 }
 
+
+
 }
 }
 

src/lib/datasrc/tests/rbtree_unittest.cc

@@ -51,30 +51,21 @@ namespace {
 class RBTreeTest : public::testing::Test {
 protected:
     RBTreeTest() : rbtree() {
-        rbtree.insert(Name("c"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(1)));
-        rbtree.insert(Name("b"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(2)));
-        rbtree.insert(Name("a"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(3)));
-        rbtree.insert(Name("x.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(4)));
-        rbtree.insert(Name("z.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(5)));
-        rbtree.insert(Name("g.h"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(6)));
-        rbtree.insert(Name("i.g.h"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(7)));
-        rbtree.insert(Name("o.w.y.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(8)));
-        rbtree.insert(Name("j.z.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(9)));
-        rbtree.insert(Name("p.w.y.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(10)));
-        rbtree.insert(Name("q.w.y.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(11)));
+        const char * domain_names[] = {"c", "b", "a", "x.d.e.f", "z.d.e.f", "g.h", "i.g.h", 
+            "o.w.y.d.e.f", "j.z.d.e.f", "p.w.y.d.e.f", "q.w.y.d.e.f"};
+        int name_count = sizeof(domain_names) / sizeof(domain_names[0]);
+        for (int i = 0; i < name_count; ++i) {
+            rbtree.insert(Name(domain_names[i]), &rbtnode);
+            rbtnode->setData(RBNode<int>::NodeDataPtr(new int(i + 1)));
+
+            rbtree_expose_empty_node.insert(Name(domain_names[i]), &rbtnode);
+            rbtnode->setData(RBNode<int>::NodeDataPtr(new int(i + 1)));
+
+        }
     }
+
     RBTree<int> rbtree;
+    RBTree<int, true> rbtree_expose_empty_node;
     RBNode<int>* rbtnode;
     const RBNode<int>* crbtnode;
 };
@@ -90,32 +81,64 @@ TEST_F(RBTreeTest, setGetData) {
 }
 
 TEST_F(RBTreeTest, insertNames) {
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("d.e.f"), &rbtnode));
+    //if don't expose empty node, even the node already exsit which is caused by node fission
+    //we will return succeed
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("d.e.f"), &rbtnode));
     EXPECT_EQ(Name("d.e.f"), rbtnode->getName());
     EXPECT_EQ(13, rbtree.getNodeCount());
 
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, 
+            rbtree_expose_empty_node.insert(Name("d.e.f"), &rbtnode));
+    EXPECT_EQ(Name("d.e.f"), rbtnode->getName());
+    EXPECT_EQ(13, rbtree_expose_empty_node.getNodeCount());
+
+
+    //insert not exist node
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("."), &rbtnode));
     EXPECT_EQ(Name("."), rbtnode->getName());
     EXPECT_EQ(14, rbtree.getNodeCount());
 
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("."), &rbtnode));
+    EXPECT_EQ(Name("."), rbtnode->getName());
+    EXPECT_EQ(14, rbtree_expose_empty_node.getNodeCount());
+    
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("example.com"), &rbtnode));
     EXPECT_EQ(15, rbtree.getNodeCount());
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(12)));
 
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(15, rbtree_expose_empty_node.getNodeCount());
+    rbtnode->setData(RBNode<int>::NodeDataPtr(new int(12)));
+
+
     // return ALREADYEXISTS, since node "example.com" already has been explicitly inserted
     EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("example.com"), &rbtnode));
     EXPECT_EQ(15, rbtree.getNodeCount());
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree_expose_empty_node.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(15, rbtree_expose_empty_node.getNodeCount());
+
 
     // split the node "d.e.f"
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("k.e.f"), &rbtnode));
     EXPECT_EQ(Name("k"), rbtnode->getName());
     EXPECT_EQ(17, rbtree.getNodeCount());
 
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("k.e.f"), &rbtnode));
+    EXPECT_EQ(Name("k"), rbtnode->getName());
+    EXPECT_EQ(17, rbtree_expose_empty_node.getNodeCount());
+
+
     // split the node "g.h"
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("h"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("h"), &rbtnode));
     EXPECT_EQ(Name("h"), rbtnode->getName());
     EXPECT_EQ(18, rbtree.getNodeCount());
 
+    //node fission will create node "h"
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree_expose_empty_node.insert(Name("h"), &rbtnode));
+    EXPECT_EQ(Name("h"), rbtnode->getName());
+    EXPECT_EQ(18, rbtree_expose_empty_node.getNodeCount());
+
+
     // add child domain
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("m.p.w.y.d.e.f"), &rbtnode));
     EXPECT_EQ(Name("m"), rbtnode->getName());
@@ -124,18 +147,35 @@ TEST_F(RBTreeTest, insertNames) {
     EXPECT_EQ(Name("n"), rbtnode->getName());
     EXPECT_EQ(20, rbtree.getNodeCount());
 
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("m.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(Name("m"), rbtnode->getName());
+    EXPECT_EQ(19, rbtree_expose_empty_node.getNodeCount());
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("n.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(Name("n"), rbtnode->getName());
+    EXPECT_EQ(20, rbtree_expose_empty_node.getNodeCount());
+
+
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("l.a"), &rbtnode));
     EXPECT_EQ(Name("l"), rbtnode->getName());
     EXPECT_EQ(21, rbtree.getNodeCount());
 
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("l.a"), &rbtnode));
+    EXPECT_EQ(Name("l"), rbtnode->getName());
+    EXPECT_EQ(21, rbtree_expose_empty_node.getNodeCount());
+
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("r.d.e.f"), &rbtnode));
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("s.d.e.f"), &rbtnode));
     EXPECT_EQ(23, rbtree.getNodeCount());
 
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("r.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("s.d.e.f"), &rbtnode));
+    EXPECT_EQ(23, rbtree_expose_empty_node.getNodeCount());
+
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("h.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("h.w.y.d.e.f"), &rbtnode));
 
     // add more nodes one by one to cover leftRotate and rightRotate
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("f"), &rbtnode));
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("m"), &rbtnode));
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("nm"), &rbtnode));
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("om"), &rbtnode));
@@ -146,8 +186,21 @@ TEST_F(RBTreeTest, insertNames) {
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("i"), &rbtnode));
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("ae"), &rbtnode));
     EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("n"), &rbtnode));
+    
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree_expose_empty_node.insert(Name("f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("m"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("nm"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("om"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("k"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("l"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("fe"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("ge"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("i"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("ae"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("n"), &rbtnode));
 }
 
+   
 TEST_F(RBTreeTest, findName) {
     // find const rbtnode
     // exact match
@@ -196,7 +249,7 @@ TEST_F(RBTreeTest, callback) {
                                                   &subrbtnode));
     subrbtnode->setData(RBNode<int>::NodeDataPtr(new int(2)));
     RBNode<int>* parentrbtnode;
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("example"),
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("example"),
                                                        &parentrbtnode));
     //  the chilld/parent nodes shouldn't "inherit" the callback flag.
     // "rbtnode" may be invalid due to the insertion, so we need to re-find