From 977967a1a0473bb563d8c3565e2654a950bfb72c Mon Sep 17 00:00:00 2001 From: ygorelik Date: Mon, 22 Jul 2019 12:22:35 -0700 Subject: [PATCH] Resolved issue #800 --- sdk/cpp/CHANGES.md | 1 + sdk/cpp/core/src/common_utilities.cpp | 40 +++++++-------- sdk/cpp/core/src/entity_data_node_walker.cpp | 7 ++- sdk/cpp/core/src/netconf_service.cpp | 10 ++++ sdk/cpp/core/src/types.hpp | 7 ++- sdk/cpp/core/src/value_list.cpp | 19 ++++++- sdk/cpp/core/tests/test_entity.cpp | 44 ++++++++-------- sdk/cpp/tests/test_crud.cpp | 51 ++++++++++++++++++- .../core/tests/test_non_top_operations.py | 13 +++++ 9 files changed, 145 insertions(+), 47 deletions(-) diff --git a/sdk/cpp/CHANGES.md b/sdk/cpp/CHANGES.md index 075719b38..ac8767e4b 100644 --- a/sdk/cpp/CHANGES.md +++ b/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 diff --git a/sdk/cpp/core/src/common_utilities.cpp b/sdk/cpp/core/src/common_utilities.cpp index d0be2b920..0af07fab4 100644 --- a/sdk/cpp/core/src/common_utilities.cpp +++ b/sdk/cpp/core/src/common_utilities.cpp @@ -24,6 +24,7 @@ #include #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_list) static shared_ptr find_child_entity(shared_ptr 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 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 parent_entity, Entity & filter_entity) shared_ptr get_child_entity_from_top(shared_ptr top_entity, Entity & filter_entity) { - shared_ptr child_entity; - if (filter_entity.is_top_level_class) { - if (filter_entity.get_absolute_path() == top_entity->get_absolute_path()) { + shared_ptr 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 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 & 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 datanode; for (auto dn_entry : path_to_datanode) { if (internal_key.find(dn_entry.first) == 0) { diff --git a/sdk/cpp/core/src/entity_data_node_walker.cpp b/sdk/cpp/core/src/entity_data_node_walker.cpp index a353f4049..b3e87c919 100644 --- a/sdk/cpp/core/src/entity_data_node_walker.cpp +++ b/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) { YLOG_DEBUG("Going into child {} in parent {}", child_name, node->get_path()); shared_ptr 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) } 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); + } } } } diff --git a/sdk/cpp/core/src/netconf_service.cpp b/sdk/cpp/core/src/netconf_service.cpp index 4315356f0..3ccb8c0cb 100644 --- a/sdk/cpp/core/src/netconf_service.cpp +++ b/sdk/cpp/core/src/netconf_service.cpp @@ -291,6 +291,10 @@ get_from_list(NetconfServiceProvider& provider, DataStore source, vectoryfilter; + 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, vectoryfilter = 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 diff --git a/sdk/cpp/core/src/types.hpp b/sdk/cpp/core/src/types.hpp index 0a135e845..0a47eb16f 100644 --- a/sdk/cpp/core/src/types.hpp +++ b/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 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 ep); void extend(std::initializer_list> ep_list); + void review(std::shared_ptr ep); std::string build_key(std::shared_ptr ep); std::shared_ptr pop(const std::string& key); std::shared_ptr pop(const std::size_t item); - private: std::vector ylist_key_names; + + private: std::map> entity_map; std::vector key_vector; Entity* parent; diff --git a/sdk/cpp/core/src/value_list.cpp b/sdk/cpp/core/src/value_list.cpp index f9114ed3a..ba8c81103 100644 --- a/sdk/cpp/core/src/value_list.cpp +++ b/sdk/cpp/core/src/value_list.cpp @@ -293,9 +293,9 @@ YList::build_key(shared_ptr ep) ostringstream value_buffer; string key; vector< pair > 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 ep) } entity_map[key] = ep; ep->ylist_key = key; + ep->ylist = this; +} + +void +YList::review(shared_ptr 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 diff --git a/sdk/cpp/core/tests/test_entity.cpp b/sdk/cpp/core/tests/test_entity.cpp index 35eb5d1e8..08baf76f0 100644 --- a/sdk/cpp/core/tests/test_entity.cpp +++ b/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(); -// 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(); + 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; +} diff --git a/sdk/cpp/tests/test_crud.cpp b/sdk/cpp/tests/test_crud.cpp index 9a32ea9a9..d0897cfe9 100644 --- a/sdk/cpp/tests/test_crud.cpp +++ b/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(); + 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(); + runner.runner_2->parent = &runner; + auto r2_ent = crud.read(provider, *runner.runner_2); + REQUIRE(r2_ent != nullptr); + auto r2 = dynamic_cast(r2_ent.get()); + REQUIRE(r2->some_leaf.value == "some-leaf"); + + runner = ydktest_sanity::Runner(); + runner.runner_2 = make_shared(); + 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(); + loopback->name = "2222"; + native.interface->loopback.append(loopback); + auto address = make_shared(); + 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(); + 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(read_entity_wrapper.get()); + auto ip_address = dynamic_cast(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); +} diff --git a/sdk/python/core/tests/test_non_top_operations.py b/sdk/python/core/tests/test_non_top_operations.py index 0e523b7e5..d7c591f24 100644 --- a/sdk/python/core/tests/test_non_top_operations.py +++ b/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__':