Resolved issue CiscoDevNet#800

sdk/cpp/CHANGES.md

@@ -5,6 +5,7 @@
 
 #### Resolved GitHub issues
   * NETCONF provider should raise more appropriate exceptions ([#774](https://github.com/CiscoDevNet/ydk-gen/issues/774))
+  * YList test fails when list entity added before keys are initialized ([#800](https://github.com/CiscoDevNet/ydk-gen/issues/800))
 
 
 ### 2019-05-15 version 0.8.3

sdk/cpp/core/src/common_utilities.cpp

@@ -24,6 +24,7 @@
 #include <set>
 
 #include "common_utilities.hpp"
+#include "entity_util.hpp"
 #include "entity_data_node_walker.hpp"
 #include "xml_subtree_codec.hpp"
 #include "json_subtree_codec.hpp"
@@ -112,10 +113,10 @@ string entity_vector_to_string(vector<Entity*> & entity_list)
 static shared_ptr<Entity>
 find_child_entity(shared_ptr<Entity> parent_entity, Entity & filter_entity)
 {
-    auto filter_absolute_path = filter_entity.get_absolute_path();
-    auto parent_absolute_path = parent_entity->get_absolute_path();
+    auto filter_absolute_path = absolute_path(filter_entity);
+    auto parent_absolute_path = absolute_path(*parent_entity);
 
-    if (filter_absolute_path == parent_entity->get_absolute_path()) {
+    if (filter_absolute_path == parent_absolute_path) {
         YLOG_DEBUG("find_child_entity: Filter matches with parent entity, returning");
         return parent_entity;
     }
@@ -130,8 +131,9 @@ find_child_entity(shared_ptr<Entity> parent_entity, Entity & filter_entity)
     auto filter_segment_path = filter_entity.get_segment_path();
     auto it = children.find(filter_segment_path);
     if (it != children.end()) {
-    	YLOG_DEBUG("Got child with matching segment path; absolute path is '{}'", it->second->get_absolute_path());
-        if (it->second->get_absolute_path() == filter_absolute_path)
+        auto child_abs_path = absolute_path(*it->second);
+        YLOG_DEBUG("Got child with matching segment path; absolute path is '{}'", child_abs_path);
+        if (child_abs_path == filter_absolute_path)
             return it->second;
     }
     YLOG_DEBUG("No matching child found");
@@ -146,38 +148,38 @@ find_child_entity(shared_ptr<Entity> parent_entity, Entity & filter_entity)
 shared_ptr<Entity>
 get_child_entity_from_top(shared_ptr<Entity> top_entity, Entity & filter_entity)
 {
-	shared_ptr<Entity> child_entity;
-	if (filter_entity.is_top_level_class) {
-        if (filter_entity.get_absolute_path() == top_entity->get_absolute_path()) {
+    shared_ptr<Entity> child_entity;
+    if (filter_entity.is_top_level_class) {
+        if (absolute_path(filter_entity) == absolute_path(*top_entity)) {
         	return top_entity;
         }
         else {
-            YLOG_ERROR("get_child_entity_from_top: The filter '{}' points to a different top-entity", filter_entity.get_absolute_path());
+            YLOG_ERROR("get_child_entity_from_top: The filter '{}' points to a different top-entity", absolute_path(filter_entity));
         }
     }
     else {
-        YLOG_DEBUG("Searching for child entity matching non-top level filter '{}'", filter_entity.get_absolute_path());
+        YLOG_DEBUG("Searching for child entity matching non-top level filter '{}'", absolute_path(filter_entity));
         child_entity = find_child_entity(top_entity, filter_entity);
         if (child_entity != nullptr) {
-        	YLOG_DEBUG("Found matching child entity '{}'", child_entity->get_absolute_path());
+            YLOG_DEBUG("Found matching child entity '{}'", absolute_path(*child_entity));
             child_entity->parent = nullptr;
         }
         else {
         	YLOG_DEBUG("Matching child entity was not found");
         }
     }
-	return child_entity;
+    return child_entity;
 }
 
 shared_ptr<Entity> get_top_entity_from_filter(Entity & filter)
 {
     if (filter.parent == nullptr) {
-    	if (filter.is_top_level_class)
+        if (filter.is_top_level_class)
             return filter.clone_ptr();
-    	else {
-    		YLOG_ERROR("get_top_entity_from_filter: Could not traverse from filter '{}' up to top-entity", filter.get_absolute_path());
-    	    return nullptr;
-    	}
+        else {
+            YLOG_ERROR("get_top_entity_from_filter: Could not traverse from filter '{}' up to top-entity", absolute_path(filter));
+            return nullptr;
+        }
     }
 
     return get_top_entity_from_filter(*(filter.parent));
@@ -313,9 +315,7 @@ execute_rpc(ydk::ServiceProvider & provider, vector<Entity*> & filter_list,
 
     // Build resulting list of entities
     for (Entity* entity : filter_list) {
-        string internal_key = entity->get_absolute_path();
-        if (internal_key.empty())
-        	internal_key = entity->get_segment_path();
+        string internal_key = absolute_path(*entity);
         shared_ptr<path::DataNode> datanode;
         for (auto dn_entry : path_to_datanode) {
             if (internal_key.find(dn_entry.first) == 0) {

sdk/cpp/core/src/entity_data_node_walker.cpp

@@ -237,7 +237,7 @@ void get_entity_from_data_node(path::DataNode * node, shared_ptr<Entity> entity)
         {
             YLOG_DEBUG("Going into child {} in parent {}", child_name, node->get_path());
             shared_ptr<Entity> child_entity;
-            if(data_node_is_list(*child_data_node))
+            if (data_node_is_list(*child_data_node))
             {
                 child_entity = entity->get_child_by_name(child_name, get_segment_path(child_data_node->get_path()));
             }
@@ -257,6 +257,11 @@ void get_entity_from_data_node(path::DataNode * node, shared_ptr<Entity> entity)
             }
             child_entity->parent = entity.get();
             get_entity_from_data_node(child_data_node.get(), child_entity);
+            if (data_node_is_list(*child_data_node) &&
+                child_entity->ylist && child_entity->ylist->ylist_key_names.size() > 0)
+            {
+                child_entity->ylist->review(child_entity);
+            }
         }
     }
 }

sdk/cpp/core/src/netconf_service.cpp

@@ -291,6 +291,10 @@ get_from_list(NetconfServiceProvider& provider, DataStore source, vector<Entity*
     size_t bypass_validation = 0;
     string filter_string = "";
     for (auto entity : filter_list) {
+        YFilter original_yfilter = entity->yfilter;
+        if (!entity->is_top_level_class && original_yfilter == YFilter::not_set) {
+            entity->yfilter = YFilter::read;
+        }
         auto top_entity = get_top_entity(entity);
         if (top_entity->is_top_level_class) {
             filter_string += get_xml_subtree_filter_payload(*top_entity, provider);
@@ -300,6 +304,7 @@ get_from_list(NetconfServiceProvider& provider, DataStore source, vector<Entity*
         else {
             filter_string += get_data_payload(*entity, provider);
         }
+        entity->yfilter = original_yfilter;
     }
     rpc->get_input_node().create_datanode("filter", filter_string);
 
@@ -410,11 +415,16 @@ get_entity(NetconfServiceProvider& provider, DataStore source, Entity& filter, c
         create_input_leaf(rpc->get_input_node(), source, "source");
     }
 
+    YFilter original_yfilter = filter.yfilter;
+    if (!filter.is_top_level_class && original_yfilter == YFilter::not_set) {
+        filter.yfilter = YFilter::read;
+    }
     auto top_entity = get_top_entity(&filter);
     string filter_string = (top_entity->is_top_level_class) ?
         get_xml_subtree_filter_payload(*top_entity, provider) :
         get_data_payload(filter, provider);
     rpc->get_input_node().create_datanode("filter", filter_string);
+    filter.yfilter = original_yfilter;
 
     if (filter.ignore_validation && top_entity->is_top_level_class) {
         // Bypass libyang validation

sdk/cpp/core/src/types.hpp

@@ -105,6 +105,8 @@ struct EntityPath {
 
 typedef void (*augment_capabilities_function)();
 
+class YList;
+
 class Entity {
   public:
     Entity();
@@ -154,6 +156,7 @@ class Entity {
     bool ignore_validation;
     std::vector<std::string> ylist_key_names;
     std::string ylist_key;
+    YList* ylist;
 
     std::string get_ylist_key() const;
 };
@@ -369,13 +372,15 @@ class YList
 
     void append(std::shared_ptr<Entity> ep);
     void extend(std::initializer_list<std::shared_ptr<Entity>> ep_list);
+    void review(std::shared_ptr<Entity> ep);
     std::string build_key(std::shared_ptr<Entity> ep);
 
     std::shared_ptr<Entity> pop(const std::string& key);
     std::shared_ptr<Entity> pop(const std::size_t item);
 
-  private:
     std::vector<std::string> ylist_key_names;
+
+  private:
     std::map<std::string,std::shared_ptr<Entity>> entity_map;
     std::vector<std::string> key_vector;
     Entity* parent;

sdk/cpp/core/src/value_list.cpp

@@ -293,9 +293,9 @@ YList::build_key(shared_ptr<Entity> ep)
     ostringstream value_buffer;
     string key;
     vector< pair<string, LeafData> > name_leaf_data_vector = ep->get_name_leaf_data();
-    for (auto key : ylist_key_names) {
+    for (auto ylist_key : ylist_key_names) {
         for (auto name_leaf_data : name_leaf_data_vector) {
-            if (key == name_leaf_data.first) {
+            if (ylist_key == name_leaf_data.first) {
                 key = value_buffer.str();
                 if (key.length() > 0) {
                     value_buffer << ",";
@@ -326,6 +326,21 @@ YList::append(shared_ptr<Entity> ep)
     }
     entity_map[key] = ep;
     ep->ylist_key = key;
+    ep->ylist = this;
+}
+
+void
+YList::review(shared_ptr<Entity> ep)
+{
+    string key = build_key(ep);
+    if (key != ep->ylist_key) {
+        pop(ep->ylist_key);
+        if (!entity_map[key]) {
+            key_vector.push_back(key);
+        }
+        entity_map[key] = ep;
+        ep->ylist_key = key;
+    }
 }
 
 void

sdk/cpp/core/tests/test_entity.cpp

@@ -503,25 +503,25 @@ TEST_CASE("test_ylist")
     delete list_holder;
 }
 
-//TODO Test for issue #800 to be resolved
-//TEST_CASE("test_ylist_race")
-//{
-//	TestEntity* list_holder = new TestEntity();
-//
-//    YList ylist = YList(list_holder, {"name"});
-//
-//    // Append test1 to the YList before key value is defined
-//    auto test1 = std::make_shared<TestEntity>();
-//    ylist.append(test1);
-//
-//    ylist[0]->name = "test1";
-//    ylist[0]->enabled = true;
-//
-//    auto keys = ylist.keys();
-//    REQUIRE(vector_to_string(keys) == R"("test1")");
-//
-//    auto ep = ylist["test1"];
-//    REQUIRE(ep != nullptr);
-//
-//    delete list_holder;
-//}
+TEST_CASE("test_ylist_race")
+{
+    TestEntity* list_holder = new TestEntity();
+
+    YList ylist = YList(list_holder, {"name"});
+
+    // Append test1 to the YList before key value is defined
+    auto test1 = std::make_shared<TestEntity>();
+    ylist.append(test1);
+
+    test1->name = "test1";
+    test1->enabled = true;
+    ylist.review(test1);
+
+    auto keys = ylist.keys();
+    REQUIRE(vector_to_string(keys) == R"("test1")");
+
+    auto ep = ylist["test1"];
+    REQUIRE(ep != nullptr);
+
+    delete list_holder;
+}

sdk/cpp/tests/test_crud.cpp

@@ -352,13 +352,23 @@ TEST_CASE( "crud_delete_container" )
     // Create presence container
     auto runner = ydktest_sanity::Runner();
     runner.runner_2 = make_shared<ydktest_sanity::Runner::Runner2>();
+    runner.runner_2->parent = &runner;
     runner.runner_2->some_leaf = "some-leaf";
     auto res = crud.create(provider, runner);
     REQUIRE(res);
 
-    // Delete presence container
+    // Read presence container
     runner = ydktest_sanity::Runner();
     runner.runner_2 = make_shared<ydktest_sanity::Runner::Runner2>();
+    runner.runner_2->parent = &runner;
+    auto r2_ent = crud.read(provider, *runner.runner_2);
+    REQUIRE(r2_ent != nullptr);
+    auto r2 = dynamic_cast<ydktest_sanity::Runner::Runner2*>(r2_ent.get());
+    REQUIRE(r2->some_leaf.value == "some-leaf");
+
+    runner = ydktest_sanity::Runner();
+    runner.runner_2 = make_shared<ydktest_sanity::Runner::Runner2>();
+    runner.runner_2->parent = &runner;
     res = crud.delete_(provider, *runner.runner_2);
     REQUIRE(res);
 
@@ -367,3 +377,42 @@ TEST_CASE( "crud_delete_container" )
     res = crud.delete_(provider, runner);
     REQUIRE(res);
 }
+
+TEST_CASE( "crud_read_non_top_container" )
+{
+    ydk::path::Repository repo{TEST_HOME};
+    NetconfServiceProvider provider{repo, "127.0.0.1", "admin", "admin", 12022};
+    CrudService crud{};
+
+    // CLEANUP
+    auto native = ydktest_sanity::Native();
+    bool reply = crud.delete_(provider, native);
+    REQUIRE(reply);
+
+    // CREATE
+    auto loopback = make_shared<ydktest_sanity::Native::Interface::Loopback>();
+    loopback->name = "2222";
+    native.interface->loopback.append(loopback);
+    auto address = make_shared<ydktest_sanity::Native::Interface::Loopback::Ipv4::Address>();
+    address->ip = "2.2.2.2";
+    address->netmask = "255.255.255.255";
+    loopback->ipv4->address.append(address);
+    reply = crud.create(provider, native);
+    REQUIRE(reply);
+
+    // READ
+    auto rnative = ydktest_sanity::Native();
+    loopback = make_shared<ydktest_sanity::Native::Interface::Loopback>();
+    loopback->name = "2222";
+    rnative.interface->loopback.append(loopback);
+    auto read_entity_wrapper = crud.read(provider, *loopback->ipv4);
+    REQUIRE(read_entity_wrapper != nullptr);
+    auto ipv4 = dynamic_cast<ydktest_sanity::Native::Interface::Loopback::Ipv4*>(read_entity_wrapper.get());
+    auto ip_address = dynamic_cast<ydktest_sanity::Native::Interface::Loopback::Ipv4::Address*>(ipv4->address["2.2.2.2"].get());
+    REQUIRE(ip_address->netmask.value == "255.255.255.255");
+
+    // CLEANUP
+    auto dnative = ydktest_sanity::Native();
+    reply = crud.delete_(provider, dnative);
+    REQUIRE(reply);
+}

sdk/python/core/tests/test_non_top_operations.py

@@ -90,6 +90,9 @@ def test_iden_list(self):
         self.assertIsNone(runner_read)
 
     def test_delete_container(self):
+        import logging
+        from test_utils import enable_logging
+        enable_logging(logging.DEBUG)
         # Build loopback configuration
         address = Native.Interface.Loopback.Ipv4.Address()
         address.ip = "2.2.2.2"
@@ -106,6 +109,15 @@ def test_delete_container(self):
         result = crud.create(self.ncc, native)
         self.assertTrue(result)
 
+        # Read ipv4 configuration
+        native = Native()
+        loopback = Native.Interface.Loopback()
+        loopback.name = 2222
+        native.interface.loopback.append(loopback)
+        ipv4_config = crud.read(self.ncc, loopback.ipv4)
+        self.assertIsNotNone(ipv4_config)
+        self.assertEqual(ipv4_config.address['2.2.2.2'].netmask, "255.255.255.255")
+
         # Remove ipv4 configuration
         native = Native()
         loopback = Native.Interface.Loopback()
@@ -118,6 +130,7 @@ def test_delete_container(self):
         native = Native()
         result = crud.delete(self.ncc, native)
         self.assertEqual(result, True)
+        enable_logging(logging.ERROR)
 
 
 if __name__ == '__main__':