From d3992d320583cf3536695f02173fc86f8117d2e3 Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe Date: Wed, 15 Oct 2025 16:19:35 +0100 Subject: [PATCH 1/8] validate_resource utility --- Development/nmos/control_protocol_utils.cpp | 14 +++++++ Development/nmos/control_protocol_utils.h | 3 ++ .../nmos/test/control_protocol_utils_test.cpp | 40 +++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/Development/nmos/control_protocol_utils.cpp b/Development/nmos/control_protocol_utils.cpp index a9fd83d2..5e235c99 100644 --- a/Development/nmos/control_protocol_utils.cpp +++ b/Development/nmos/control_protocol_utils.cpp @@ -985,6 +985,20 @@ namespace nmos } } + // Validate that the resource has been correctly constructed according to the class_descriptor + bool validate_resource(const control_protocol_resource& resource, const experimental::control_class_descriptor& class_descriptor) + { + // Validate properties + for (const auto& property_descriptor : class_descriptor.property_descriptors.as_array()) + { + if (!resource.data.has_field(nmos::fields::nc::name(property_descriptor))) + { + return false; + } + } + return true; + } + resources::const_iterator find_resource_by_role_path(const resources& resources, const web::json::array& role_path_) { auto role_path = role_path_; diff --git a/Development/nmos/control_protocol_utils.h b/Development/nmos/control_protocol_utils.h index 7e11c4f2..cf06b5c8 100644 --- a/Development/nmos/control_protocol_utils.h +++ b/Development/nmos/control_protocol_utils.h @@ -126,6 +126,9 @@ namespace nmos // method parameters constraints validation, may throw nmos::control_protocol_exception void method_parameters_contraints_validation(const web::json::value& arguments, const web::json::value& nc_method_descriptor, get_control_protocol_datatype_descriptor_handler get_control_protocol_datatype_descriptor); + // Object validation functions + bool validate_resource(const control_protocol_resource& resource, const experimental::control_class_descriptor& class_descriptor); + resources::const_iterator find_touchpoint_resource(const resources& resources, const resource& resource); // insert 'value changed', 'sequence item added', 'sequence item changed' or 'sequence item removed' notification events into all grains whose subscriptions match the specified version, type and "pre" or "post" values diff --git a/Development/nmos/test/control_protocol_utils_test.cpp b/Development/nmos/test/control_protocol_utils_test.cpp index ffdf7694..c1d095b6 100644 --- a/Development/nmos/test/control_protocol_utils_test.cpp +++ b/Development/nmos/test/control_protocol_utils_test.cpp @@ -1177,3 +1177,43 @@ BST_TEST_CASE(testFindTouchpointResources) BST_CHECK_EQUAL(touchpoint, resources.end()); } } + + +BST_TEST_CASE(testValidateResource) +{ + using web::json::value_of; + using web::json::value; + + nmos::experimental::control_protocol_state control_protocol_state; + auto get_control_protocol_class_descriptor = nmos::make_get_control_protocol_class_descriptor_handler(control_protocol_state); + + auto root_block = nmos::make_root_block(); + auto oid = nmos::root_block_oid; + // root, ClassManager + auto class_manager = nmos::make_class_manager(++oid, control_protocol_state); + auto receiver_block_oid = ++oid; + // root, receivers + auto receivers = nmos::make_block(receiver_block_oid, nmos::root_block_oid, U("receivers"), U("Receivers"), U("Receivers block")); + + // root, receivers, mon1 + auto monitor1 = nmos::make_receiver_monitor(++oid, true, receiver_block_oid, U("mon1"), U("monitor 1"), U("monitor 1"), value_of({ {nmos::nc::details::make_touchpoint_nmos({nmos::ncp_touchpoint_resource_types::receiver, U("id_1")})} })); + + auto block_class_descriptor = get_control_protocol_class_descriptor(nmos::nc_block_class_id); + + BST_CHECK(nmos::nc::validate_resource(root_block, block_class_descriptor)); + BST_CHECK(nmos::nc::validate_resource(receivers, block_class_descriptor)); + + auto class_manager_class_descriptor = get_control_protocol_class_descriptor(nmos::nc_class_manager_class_id); + + BST_CHECK(nmos::nc::validate_resource(class_manager, class_manager_class_descriptor)); + + auto receiver_monitor_class_descriptor = get_control_protocol_class_descriptor(nmos::nc_receiver_monitor_class_id); + + BST_CHECK(nmos::nc::validate_resource(monitor1, receiver_monitor_class_descriptor)); + + // Negative tests + BST_CHECK(!nmos::nc::validate_resource(root_block, receiver_monitor_class_descriptor)); + BST_CHECK(!nmos::nc::validate_resource(receivers, class_manager_class_descriptor)); + BST_CHECK(!nmos::nc::validate_resource(class_manager, block_class_descriptor)); + BST_CHECK(!nmos::nc::validate_resource(monitor1, block_class_descriptor)); +} From c475b5d2489f0890163b8483a36f9544590fdc78 Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe Date: Wed, 5 Nov 2025 16:19:57 +0000 Subject: [PATCH 2/8] Throw exception on badly constructed Device Model object --- Development/nmos/control_protocol_utils.cpp | 2 +- Development/nmos/test/control_protocol_utils_test.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Development/nmos/control_protocol_utils.cpp b/Development/nmos/control_protocol_utils.cpp index 100b2b29..ee4132b2 100644 --- a/Development/nmos/control_protocol_utils.cpp +++ b/Development/nmos/control_protocol_utils.cpp @@ -993,7 +993,7 @@ namespace nmos { if (!resource.data.has_field(nmos::fields::nc::name(property_descriptor))) { - return false; + throw control_protocol_exception("missing control resource property" + utility::us2s(nmos::fields::nc::name(property_descriptor))); } } return true; diff --git a/Development/nmos/test/control_protocol_utils_test.cpp b/Development/nmos/test/control_protocol_utils_test.cpp index c7044856..7efda0a5 100644 --- a/Development/nmos/test/control_protocol_utils_test.cpp +++ b/Development/nmos/test/control_protocol_utils_test.cpp @@ -1288,8 +1288,8 @@ BST_TEST_CASE(testValidateResource) BST_CHECK(nmos::nc::validate_resource(monitor1, receiver_monitor_class_descriptor)); // Negative tests - BST_CHECK(!nmos::nc::validate_resource(root_block, receiver_monitor_class_descriptor)); - BST_CHECK(!nmos::nc::validate_resource(receivers, class_manager_class_descriptor)); - BST_CHECK(!nmos::nc::validate_resource(class_manager, block_class_descriptor)); - BST_CHECK(!nmos::nc::validate_resource(monitor1, block_class_descriptor)); + BST_CHECK_THROW(nmos::nc::validate_resource(root_block, receiver_monitor_class_descriptor), nmos::control_protocol_exception); + BST_CHECK_THROW(nmos::nc::validate_resource(receivers, class_manager_class_descriptor), nmos::control_protocol_exception); + BST_CHECK_THROW(nmos::nc::validate_resource(class_manager, block_class_descriptor), nmos::control_protocol_exception); + BST_CHECK_THROW(nmos::nc::validate_resource(monitor1, block_class_descriptor), nmos::control_protocol_exception); } From 03f4d80e787b1864acee0134fb28643768206cb7 Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe <64410119+jonathan-r-thorpe@users.noreply.github.com> Date: Mon, 10 Nov 2025 14:16:11 +0000 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: Simon Lo --- Development/nmos/control_protocol_utils.cpp | 6 +++--- Development/nmos/control_protocol_utils.h | 2 +- Development/nmos/test/control_protocol_utils_test.cpp | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Development/nmos/control_protocol_utils.cpp b/Development/nmos/control_protocol_utils.cpp index ee4132b2..1c1907b9 100644 --- a/Development/nmos/control_protocol_utils.cpp +++ b/Development/nmos/control_protocol_utils.cpp @@ -986,12 +986,12 @@ namespace nmos } // Validate that the resource has been correctly constructed according to the class_descriptor - bool validate_resource(const control_protocol_resource& resource, const experimental::control_class_descriptor& class_descriptor) + void validate_resource(const resource& resource, const experimental::control_class_descriptor& class_descriptor) { // Validate properties for (const auto& property_descriptor : class_descriptor.property_descriptors.as_array()) { - if (!resource.data.has_field(nmos::fields::nc::name(property_descriptor))) + if ((resource.data.is_null()) || (!resource.data.has_field(nmos::fields::nc::name(property_descriptor)))) { throw control_protocol_exception("missing control resource property" + utility::us2s(nmos::fields::nc::name(property_descriptor))); } @@ -1313,7 +1313,7 @@ namespace nmos { // A monitor is expected to go through a period of instability upon activation. Therefore, on monitor activation // domain specific statuses offering an Inactive option MUST transition immediately to the Healthy state. - // Furthermore, after activation, as long as the monitor isn’t being deactivated, it MUST delay the reporting + // Furthermore, after activation, as long as the monitor isnÂ’t being deactivated, it MUST delay the reporting // of non Healthy states for the duration specified by statusReportingDelay, and then transition to any other appropriate state. const auto& found = find_resource(resources, utility::s2us(std::to_string(oid))); if (resources.end() != found && nc::is_status_monitor(nc::details::parse_class_id(nmos::fields::nc::class_id(found->data)))) diff --git a/Development/nmos/control_protocol_utils.h b/Development/nmos/control_protocol_utils.h index a36b7524..8a32b0f8 100644 --- a/Development/nmos/control_protocol_utils.h +++ b/Development/nmos/control_protocol_utils.h @@ -127,7 +127,7 @@ namespace nmos void method_parameters_contraints_validation(const web::json::value& arguments, const web::json::value& nc_method_descriptor, get_control_protocol_datatype_descriptor_handler get_control_protocol_datatype_descriptor); // Object validation functions - bool validate_resource(const control_protocol_resource& resource, const experimental::control_class_descriptor& class_descriptor); + void validate_resource(const resource& resource, const experimental::control_class_descriptor& class_descriptor); resources::const_iterator find_touchpoint_resource(const resources& resources, const resource& resource); diff --git a/Development/nmos/test/control_protocol_utils_test.cpp b/Development/nmos/test/control_protocol_utils_test.cpp index 7efda0a5..8ece86b8 100644 --- a/Development/nmos/test/control_protocol_utils_test.cpp +++ b/Development/nmos/test/control_protocol_utils_test.cpp @@ -1276,16 +1276,16 @@ BST_TEST_CASE(testValidateResource) auto block_class_descriptor = get_control_protocol_class_descriptor(nmos::nc_block_class_id); - BST_CHECK(nmos::nc::validate_resource(root_block, block_class_descriptor)); - BST_CHECK(nmos::nc::validate_resource(receivers, block_class_descriptor)); + BST_CHECK_NO_THROW(nmos::nc::validate_resource(root_block, block_class_descriptor)); + BST_CHECK_NO_THROW(nmos::nc::validate_resource(receivers, block_class_descriptor)); auto class_manager_class_descriptor = get_control_protocol_class_descriptor(nmos::nc_class_manager_class_id); - BST_CHECK(nmos::nc::validate_resource(class_manager, class_manager_class_descriptor)); + BST_CHECK_NO_THROW(nmos::nc::validate_resource(class_manager, class_manager_class_descriptor)); auto receiver_monitor_class_descriptor = get_control_protocol_class_descriptor(nmos::nc_receiver_monitor_class_id); - BST_CHECK(nmos::nc::validate_resource(monitor1, receiver_monitor_class_descriptor)); + BST_CHECK_NO_THROW(nmos::nc::validate_resource(monitor1, receiver_monitor_class_descriptor)); // Negative tests BST_CHECK_THROW(nmos::nc::validate_resource(root_block, receiver_monitor_class_descriptor), nmos::control_protocol_exception); From eac763d4c13db11d48d20323b56f0f75cefd9138 Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe Date: Mon, 10 Nov 2025 14:34:10 +0000 Subject: [PATCH 4/8] Remove boolean return from void function --- Development/nmos/control_protocol_utils.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Development/nmos/control_protocol_utils.cpp b/Development/nmos/control_protocol_utils.cpp index 1c1907b9..fe42511d 100644 --- a/Development/nmos/control_protocol_utils.cpp +++ b/Development/nmos/control_protocol_utils.cpp @@ -996,7 +996,6 @@ namespace nmos throw control_protocol_exception("missing control resource property" + utility::us2s(nmos::fields::nc::name(property_descriptor))); } } - return true; } resources::const_iterator find_resource_by_role_path(const resources& resources, const web::json::array& role_path_) From 80d9959ea8a93b3cc7d450610fae01784491492d Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe <64410119+jonathan-r-thorpe@users.noreply.github.com> Date: Mon, 10 Nov 2025 17:00:47 +0000 Subject: [PATCH 5/8] Update Development/nmos/control_protocol_utils.cpp Co-authored-by: Simon Lo --- Development/nmos/control_protocol_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Development/nmos/control_protocol_utils.cpp b/Development/nmos/control_protocol_utils.cpp index fe42511d..34b4cf94 100644 --- a/Development/nmos/control_protocol_utils.cpp +++ b/Development/nmos/control_protocol_utils.cpp @@ -1312,7 +1312,7 @@ namespace nmos { // A monitor is expected to go through a period of instability upon activation. Therefore, on monitor activation // domain specific statuses offering an Inactive option MUST transition immediately to the Healthy state. - // Furthermore, after activation, as long as the monitor isnÂ’t being deactivated, it MUST delay the reporting + // Furthermore, after activation, as long as the monitor isn't being deactivated, it MUST delay the reporting // of non Healthy states for the duration specified by statusReportingDelay, and then transition to any other appropriate state. const auto& found = find_resource(resources, utility::s2us(std::to_string(oid))); if (resources.end() != found && nc::is_status_monitor(nc::details::parse_class_id(nmos::fields::nc::class_id(found->data)))) From b4ea2d9743317890a0dd5a8e6e38096f5e634f70 Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe Date: Wed, 12 Nov 2025 14:28:15 +0000 Subject: [PATCH 6/8] Add guard when getting a property --- Development/nmos/control_protocol_methods.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Development/nmos/control_protocol_methods.cpp b/Development/nmos/control_protocol_methods.cpp index 337cd43a..8393d3ba 100644 --- a/Development/nmos/control_protocol_methods.cpp +++ b/Development/nmos/control_protocol_methods.cpp @@ -24,7 +24,7 @@ namespace nmos // find the relevant nc_property_descriptor const auto& property = nc::find_property_descriptor(details::parse_property_id(property_id), details::parse_class_id(nmos::fields::nc::class_id(resource.data)), get_control_protocol_class_descriptor); - if (!property.is_null()) + if (!property.is_null() && resource.data.has_field(nmos::fields::nc::name(property))) { return details::make_method_result({is_deprecated ? nmos::nc_method_status::method_deprecated : nmos::fields::nc::is_deprecated(property) ? nc_method_status::property_deprecated : nc_method_status::ok}, resource.data.at(nmos::fields::nc::name(property))); } From 633c9b42710529650cd659640b99edd10914c5ab Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe Date: Wed, 12 Nov 2025 15:10:53 +0000 Subject: [PATCH 7/8] Validate control protocol object methods. --- Development/nmos/control_protocol_utils.cpp | 11 +++- .../nmos/test/control_protocol_utils_test.cpp | 62 ++++++++++++++++++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/Development/nmos/control_protocol_utils.cpp b/Development/nmos/control_protocol_utils.cpp index 34b4cf94..e9fae7f8 100644 --- a/Development/nmos/control_protocol_utils.cpp +++ b/Development/nmos/control_protocol_utils.cpp @@ -993,7 +993,16 @@ namespace nmos { if ((resource.data.is_null()) || (!resource.data.has_field(nmos::fields::nc::name(property_descriptor)))) { - throw control_protocol_exception("missing control resource property" + utility::us2s(nmos::fields::nc::name(property_descriptor))); + throw control_protocol_exception("missing control resource property: " + utility::us2s(nmos::fields::nc::name(property_descriptor))); + } + } + + // Validate methods + for (const auto& method_descriptor : class_descriptor.method_descriptors) + { + if (!method_descriptor.second) + { + throw control_protocol_exception("method not implemented: " + utility::us2s(nmos::fields::nc::name(method_descriptor.first))); } } } diff --git a/Development/nmos/test/control_protocol_utils_test.cpp b/Development/nmos/test/control_protocol_utils_test.cpp index 8ece86b8..1b777036 100644 --- a/Development/nmos/test/control_protocol_utils_test.cpp +++ b/Development/nmos/test/control_protocol_utils_test.cpp @@ -1255,7 +1255,7 @@ BST_TEST_CASE(testFindMembersByClassId) } } -BST_TEST_CASE(testValidateResource) +BST_TEST_CASE(testValidateResourceProperties) { using web::json::value_of; using web::json::value; @@ -1293,3 +1293,63 @@ BST_TEST_CASE(testValidateResource) BST_CHECK_THROW(nmos::nc::validate_resource(class_manager, block_class_descriptor), nmos::control_protocol_exception); BST_CHECK_THROW(nmos::nc::validate_resource(monitor1, block_class_descriptor), nmos::control_protocol_exception); } + +BST_TEST_CASE(testValidateResourceMethods) +{ + using web::json::value_of; + using web::json::value; + + nmos::experimental::control_protocol_state control_protocol_state; + auto get_control_protocol_class_descriptor = nmos::make_get_control_protocol_class_descriptor_handler(control_protocol_state); + + auto root_block = nmos::make_root_block(); + + auto example_method_with_no_args = [](nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, slog::base_gate& gate) + { + return nmos::nc::details::make_method_result({ is_deprecated ? nmos::nc_method_status::method_deprecated : nmos::nc_method_status::ok }); + }; + + auto control_class_id = nmos::nc::make_class_id(nmos::nc_worker_class_id, 0, { 2 }); + // Define class descriptor with method and associated method - should succeed on validate + { + std::vector property_descriptors = {}; + + std::vector method_descriptors = + { + { nmos::experimental::make_control_class_method_descriptor(U("Example method with no arguments"), { 3, 1 }, U("MethodNoArgs"), U("NcMethodResult"), {}, false, example_method_with_no_args) } + }; + + auto control_class_descriptor = nmos::experimental::make_control_class_descriptor(U(""), nmos::nc::make_class_id(nmos::nc_worker_class_id, 0, { 2 }), U("ControlClass"), property_descriptors, method_descriptors, {}); + + auto make_control_object = [&](nmos::nc_oid oid) + { + auto data = nmos::nc::details::make_worker(control_class_id, oid, true, 1, U("role"), value::string(U("")), U(""), value::null(), value::null(), true); + return nmos::control_protocol_resource{ nmos::is12_versions::v1_0, nmos::types::nc_worker, std::move(data), true }; + }; + + auto control_object = make_control_object(3); + + BST_CHECK_NO_THROW(nmos::nc::validate_resource(control_object, control_class_descriptor)); + } + // Define class descriptor with method with no associated method - should throw on validate + { + std::vector property_descriptors = {}; + + std::vector method_descriptors = + { + { nmos::experimental::make_control_class_method_descriptor(U("Example method with no arguments"), { 3, 1 }, U("MethodNoArgs"), U("NcMethodResult"), {}, false, nullptr) } + }; + + auto control_class_descriptor = nmos::experimental::make_control_class_descriptor(U(""), nmos::nc::make_class_id(nmos::nc_worker_class_id, 0, { 2 }), U("ControlClass"), property_descriptors, method_descriptors, {}); + + auto make_control_object = [&](nmos::nc_oid oid) + { + auto data = nmos::nc::details::make_worker(control_class_id, oid, true, 1, U("role"), value::string(U("")), U(""), value::null(), value::null(), true); + return nmos::control_protocol_resource{ nmos::is12_versions::v1_0, nmos::types::nc_worker, std::move(data), true }; + }; + + auto control_object = make_control_object(3); + + BST_CHECK_THROW(nmos::nc::validate_resource(control_object, control_class_descriptor), nmos::control_protocol_exception); + } +} From b6379b97b193d12fb321e8a3f72736157c9c6e75 Mon Sep 17 00:00:00 2001 From: jonathan-r-thorpe <64410119+jonathan-r-thorpe@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:02:17 +0000 Subject: [PATCH 8/8] Update Development/nmos/control_protocol_utils.h Co-authored-by: Simon Lo --- Development/nmos/control_protocol_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Development/nmos/control_protocol_utils.h b/Development/nmos/control_protocol_utils.h index 8a32b0f8..517bf2a9 100644 --- a/Development/nmos/control_protocol_utils.h +++ b/Development/nmos/control_protocol_utils.h @@ -126,7 +126,7 @@ namespace nmos // method parameters constraints validation, may throw nmos::control_protocol_exception void method_parameters_contraints_validation(const web::json::value& arguments, const web::json::value& nc_method_descriptor, get_control_protocol_datatype_descriptor_handler get_control_protocol_datatype_descriptor); - // Object validation functions + // Validate that the resource has been correctly constructed according to the class_descriptor void validate_resource(const resource& resource, const experimental::control_class_descriptor& class_descriptor); resources::const_iterator find_touchpoint_resource(const resources& resources, const resource& resource);