diff --git a/score/mw/com/impl/BUILD b/score/mw/com/impl/BUILD index d677fab28..0e55d0e7f 100644 --- a/score/mw/com/impl/BUILD +++ b/score/mw/com/impl/BUILD @@ -260,6 +260,7 @@ cc_library( "//score/mw/com:__subpackages__", ], deps = [ + ":field_tags", ":method_type", ":skeleton_event", ":skeleton_field_base", @@ -284,6 +285,7 @@ cc_library( "//score/mw/com:__subpackages__", ], deps = [ + ":field_tags", ":flag_owner", ":proxy_base", ":proxy_event", @@ -323,6 +325,7 @@ cc_library( "//score/mw/com:__subpackages__", ], deps = [ + ":field_tags", ":method_type", ":proxy_event", ":proxy_event_binding", @@ -720,7 +723,19 @@ cc_library( cc_library( name = "method_type", srcs = ["method_type.cpp"], - hdrs = ["method_type.h"], + hdrs = [ + "method_identifier.h", + "method_type.h", + ], + features = COMPILER_WARNING_FEATURES, + tags = ["FFI"], + visibility = ["//score/mw/com:__subpackages__"], +) + +cc_library( + name = "field_tags", + srcs = ["field_tags.cpp"], + hdrs = ["field_tags.h"], features = COMPILER_WARNING_FEATURES, tags = ["FFI"], visibility = ["//score/mw/com:__subpackages__"], @@ -1048,6 +1063,24 @@ cc_gtest_unit_test( ], ) +cc_gtest_unit_test( + name = "method_type_test", + srcs = ["method_type_test.cpp"], + features = COMPILER_WARNING_FEATURES, + deps = [ + ":method_type", + ], +) + +cc_gtest_unit_test( + name = "method_identifier_test", + srcs = ["method_identifier_test.cpp"], + features = COMPILER_WARNING_FEATURES, + deps = [ + ":method_type", + ], +) + cc_gtest_unit_test( name = "sample_reference_tracker_test", srcs = ["sample_reference_tracker_test.cpp"], @@ -1116,9 +1149,12 @@ cc_gtest_unit_test( deps = [ ":impl", ":runtime_mock", + "//score/mw/com/impl/bindings/mock_binding", + "//score/mw/com/impl/configuration/test:configuration_store", "//score/mw/com/impl/plumbing:proxy_field_binding_factory_mock", "//score/mw/com/impl/test:binding_factory_resources", "//score/mw/com/impl/test:proxy_resources", + "//score/mw/com/impl/test:runtime_mock_guard", ], ) @@ -1339,6 +1375,8 @@ cc_unit_test_suites_for_host_and_qnx( ":skeleton_base_test", ":unit_test", ":traits_test", + ":method_type_test", + ":method_identifier_test", ":unit_test_runtime_single_exec", ], test_suites_from_sub_packages = [ diff --git a/score/mw/com/impl/bindings/lola/proxy_method.h b/score/mw/com/impl/bindings/lola/proxy_method.h index 32ea30b6a..3c660744e 100644 --- a/score/mw/com/impl/bindings/lola/proxy_method.h +++ b/score/mw/com/impl/bindings/lola/proxy_method.h @@ -33,6 +33,7 @@ namespace score::mw::com::impl::lola { class Proxy; +class ProxyMethodAttorney; class ProxyMethod : public ProxyMethodBinding { @@ -72,6 +73,8 @@ class ProxyMethod : public ProxyMethodBinding bool IsSubscribed() const; private: + friend class ProxyMethodAttorney; + QualityType asil_level_; IRuntime& lola_runtime_; TypeErasedCallQueue::TypeErasedElementInfo type_erased_element_info_; diff --git a/score/mw/com/impl/bindings/lola/proxy_method_handling_test.cpp b/score/mw/com/impl/bindings/lola/proxy_method_handling_test.cpp index 0898e8e47..e49a71d0b 100644 --- a/score/mw/com/impl/bindings/lola/proxy_method_handling_test.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_method_handling_test.cpp @@ -113,6 +113,33 @@ const LolaServiceTypeDeployment kLolaServiceTypeDeploymentWithMethods{ {}, {{kDummyMethodName0, kDummyMethodId0}, {kDummyMethodName1, kDummyMethodId1}, {kDummyMethodName2, kDummyMethodId2}}}; +const ConfigurationStore kConfigurationStore{InstanceSpecifier::Create(std::string{"my_instance_spec"}).value(), + make_ServiceIdentifierType("foo"), + QualityType::kASIL_B, + kLolaServiceTypeDeploymentWithMethods, + kLolaServiceInstanceDeploymentWithMethods}; + +// Deployment configuration that includes fields alongside methods. +const std::string kDummyFieldName{"my_dummy_field"}; +constexpr LolaServiceElementId kDummyFieldId{20U}; + +const LolaServiceInstanceDeployment kLolaServiceInstanceDeploymentWithFields{ + kLolaInstanceId, + {}, + {{kDummyFieldName, LolaFieldInstanceDeployment{{1U}, {3U}, 1U, true, 0}}}, + {{kDummyMethodName0, LolaMethodInstanceDeployment{kDummyQueueSize0}}}}; +const LolaServiceTypeDeployment kLolaServiceTypeDeploymentWithFields{kLolaServiceId, + {}, + {{kDummyFieldName, kDummyFieldId}}, + {{kDummyMethodName0, kDummyMethodId0}}}; + +const ConfigurationStore kConfigurationStoreWithFields{ + InstanceSpecifier::Create(std::string{"my_field_instance_spec"}).value(), + make_ServiceIdentifierType("foo"), + QualityType::kASIL_B, + kLolaServiceTypeDeploymentWithFields, + kLolaServiceInstanceDeploymentWithFields}; + const std::optional kEmptyInArgsTypeErasedDataInfo{}; const std::optional kEmptyReturnTypeTypeErasedDataInfo{}; const DataTypeSizeInfo kValidInArgsTypeErasedDataInfo{16U, 16U}; @@ -213,6 +240,19 @@ class ProxyMethodHandlingFixture : public ProxyMockedMemoryFixture return *this; } + ProxyMethodHandlingFixture& WithRegisteredProxyMethodsWithType( + std::vector> + methods_to_register) + { + for (auto& [method_id, method_type, type_erased_element_info] : methods_to_register) + { + const ProxyMethodInstanceIdentifier proxy_method_instance_identifier{proxy_->GetProxyInstanceIdentifier(), + {method_id, method_type}}; + proxy_method_storage_.emplace_back(*proxy_, proxy_method_instance_identifier, type_erased_element_info); + } + return *this; + } + void StopOfferService() { ASSERT_TRUE(find_service_handler_.has_value()); @@ -1051,5 +1091,76 @@ TEST_F(ProxyMethodHandlingFixture, EnablingMethodThatDoesNotContainQueueSizeInCo SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED(score::cpp::ignore = proxy_->SetupMethods()); } +class ProxyFieldMethodHandlingFixture : public ProxyMethodHandlingFixture +{ + public: + ProxyFieldMethodHandlingFixture& GivenAProxyWithFields() + { + InitialiseProxyWithConstructor(kConfigurationStoreWithFields.GetInstanceIdentifier()); + SCORE_LANGUAGE_FUTURECPP_ASSERT(proxy_ != nullptr); + return *this; + } + + // Returns the size SetupMethods asked SharedMemoryFactory::Create for, or 0 if Create wasn't called. + std::size_t CaptureShmSizeRequestedBySetupMethods() + { + std::size_t captured_size{0U}; + EXPECT_CALL(shared_memory_factory_mock_guard_.mock_, Create(_, _, _, _, _)) + .Times(AtMost(1)) + .WillRepeatedly(DoAll(SaveArg<2>(&captured_size), Return(mock_method_memory_resource_))); + const auto result = proxy_->SetupMethods(); + EXPECT_TRUE(result.has_value()); + return captured_size; + } +}; + +TEST_F(ProxyFieldMethodHandlingFixture, SetupMethodsIncludesFieldGetMethodWhenRegistered) +{ + // Given a proxy with a field deployment and a registered Get ProxyMethod + GivenAProxyWithFields().GivenAMockedSharedMemoryResource().WithRegisteredProxyMethodsWithType( + {{kDummyFieldId, MethodType::kGet, kEmptyTypeErasedInfo}}); + + // When SetupMethods runs + // Then it asks for shm with a non-zero size, proving the kGet method got folded into the layout + EXPECT_GT(CaptureShmSizeRequestedBySetupMethods(), 0U); +} + +TEST_F(ProxyFieldMethodHandlingFixture, SetupMethodsIncludesFieldSetMethodWhenRegistered) +{ + // Given a proxy with a field deployment and a registered Set ProxyMethod + GivenAProxyWithFields().GivenAMockedSharedMemoryResource().WithRegisteredProxyMethodsWithType( + {{kDummyFieldId, MethodType::kSet, kEmptyTypeErasedInfo}}); + + // When SetupMethods runs + // Then same as the Get case - the kSet branch contributes to the layout + EXPECT_GT(CaptureShmSizeRequestedBySetupMethods(), 0U); +} + +TEST_F(ProxyFieldMethodHandlingFixture, SetupMethodsIncludesBothFieldGetAndSetMethodsWhenBothRegistered) +{ + // Given a proxy with a field deployment and BOTH Get and Set registered + GivenAProxyWithFields().GivenAMockedSharedMemoryResource().WithRegisteredProxyMethodsWithType( + {{kDummyFieldId, MethodType::kGet, kEmptyTypeErasedInfo}, + {kDummyFieldId, MethodType::kSet, kEmptyTypeErasedInfo}}); + + // When SetupMethods runs + // Then the two-entry layout asks for strictly more than a one-entry layout would (base + one slot). + // That catches a regression where only one of Get/Set is exercised. + using MethodDataElement = decltype(MethodData::method_call_queues_)::value_type; + EXPECT_GT(CaptureShmSizeRequestedBySetupMethods(), sizeof(MethodData) + sizeof(MethodDataElement)); +} + +TEST_F(ProxyFieldMethodHandlingFixture, SetupMethodsDoesNotCreateShmWhenNoFieldMethodsRegistered) +{ + // Given a proxy with a field deployment but no registered Get/Set ProxyMethods + GivenAProxyWithFields().GivenAMockedSharedMemoryResource(); + + // When SetupMethods runs + // Then nothing to register -> no shm allocation + EXPECT_CALL(shared_memory_factory_mock_guard_.mock_, Create(_, _, _, _, _)).Times(0); + const auto result = proxy_->SetupMethods(); + EXPECT_TRUE(result.has_value()); +} + } // namespace } // namespace score::mw::com::impl::lola diff --git a/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp b/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp index bd05f8aec..d19459d89 100644 --- a/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp +++ b/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp @@ -1082,5 +1082,144 @@ TEST_F(SkeletonOnServiceMethodsSubscribedFixture, EXPECT_TRUE(scoped_handler_result.has_value()); } +TEST_F(SkeletonOnServiceMethodsSubscribedFixture, SubscribeMethodsSkipsFieldGetMethodNotOnSkeleton) +{ + // TODO: Rework this once the get support is added. + + // Given a skeleton with only regular methods offered and fake method data that includes a field + // Get method the skeleton doesn't actually serve + GivenASkeletonWithTwoMethods().WhichCapturesRegisteredMethodSubscribedHandlers().WhichIsOffered(); + FakeMethodData fake_method_data_with_field_get{ + {{foo_unique_method_id_, kFooTypeErasedElementInfo}, + {UniqueMethodIdentifier{test::kFooFieldId, MethodType::kGet}, kDumbTypeErasedElementInfo}}}; + ON_CALL(*mock_method_memory_resource_qm_, getUsableBaseAddress()) + .WillByDefault(Return(static_cast(&fake_method_data_with_field_get.method_data_))); + const ProxyMethodInstanceIdentifier field_get_identifier_qm{ + proxy_instance_identifier_qm_, UniqueMethodIdentifier{test::kFooFieldId, MethodType::kGet}}; + EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(_, foo_proxy_method_identifier_qm_, _, _)); + EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(_, field_get_identifier_qm, _, _)).Times(0); + + // When the proxy subscribes + ASSERT_TRUE(captured_method_subscribed_handler_qm_.has_value()); + const auto result = std::invoke(captured_method_subscribed_handler_qm_.value(), + proxy_instance_identifier_qm_, + test::kAllowedQmMethodConsumer, + kDummyPid); + + // Then subscription succeeds - the regular method is registered and the field Get is silently skipped + EXPECT_TRUE(result.has_value()); +} + +TEST_F(SkeletonOnServiceMethodsSubscribedFixture, SubscribeMethodsSkipsFieldSetMethodNotOnSkeleton) +{ + // Given a skeleton with only regular methods offered and fake method data that includes a field + // Set method the skeleton doesn't actually serve + GivenASkeletonWithTwoMethods().WhichCapturesRegisteredMethodSubscribedHandlers().WhichIsOffered(); + FakeMethodData fake_method_data_with_field_set{ + {{foo_unique_method_id_, kFooTypeErasedElementInfo}, + {UniqueMethodIdentifier{test::kFooFieldId, MethodType::kSet}, kDumbTypeErasedElementInfo}}}; + ON_CALL(*mock_method_memory_resource_qm_, getUsableBaseAddress()) + .WillByDefault(Return(static_cast(&fake_method_data_with_field_set.method_data_))); + const ProxyMethodInstanceIdentifier field_set_identifier_qm{ + proxy_instance_identifier_qm_, UniqueMethodIdentifier{test::kFooFieldId, MethodType::kSet}}; + EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(_, foo_proxy_method_identifier_qm_, _, _)); + EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(_, field_set_identifier_qm, _, _)).Times(0); + + // When the proxy subscribes + ASSERT_TRUE(captured_method_subscribed_handler_qm_.has_value()); + const auto result = std::invoke(captured_method_subscribed_handler_qm_.value(), + proxy_instance_identifier_qm_, + test::kAllowedQmMethodConsumer, + kDummyPid); + + // Then subscription succeeds - the regular method is registered and the field Set is silently skipped + EXPECT_TRUE(result.has_value()); +} + +TEST_F(SkeletonOnServiceMethodsSubscribedFixture, + SubscribeMethodsContinuesPastSkippedFieldMethodToRegularMethodsThatFollow) +{ + // Given a skeleton with foo and dumb offered + GivenASkeletonWithTwoMethods().WhichCapturesRegisteredMethodSubscribedHandlers().WhichIsOffered(); + + // Fake method data places a field Get BEFORE both regular methods. If the skeleton broke out of + // the subscribe loop on the skipped field method, foo and dumb would never be registered. + FakeMethodData fake_method_data_with_field_first{ + {{UniqueMethodIdentifier{test::kFooFieldId, MethodType::kGet}, kDumbTypeErasedElementInfo}, + {foo_unique_method_id_, kFooTypeErasedElementInfo}, + {dumb_unique_method_id_, kDumbTypeErasedElementInfo}}}; + + ON_CALL(*mock_method_memory_resource_qm_, getUsableBaseAddress()) + .WillByDefault(Return(static_cast(&fake_method_data_with_field_first.method_data_))); + + // Expect both regular methods to be registered; the field Get in between must be skipped via continue + const ProxyMethodInstanceIdentifier field_get_identifier_qm{ + proxy_instance_identifier_qm_, UniqueMethodIdentifier{test::kFooFieldId, MethodType::kGet}}; + EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(_, foo_proxy_method_identifier_qm_, _, _)); + EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(_, dumb_proxy_method_identifier_qm_, _, _)); + EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(_, field_get_identifier_qm, _, _)).Times(0); + + // When the proxy subscribes + ASSERT_TRUE(captured_method_subscribed_handler_qm_.has_value()); + const auto result = std::invoke(captured_method_subscribed_handler_qm_.value(), + proxy_instance_identifier_qm_, + test::kAllowedQmMethodConsumer, + kDummyPid); + + // Then subscription succeeds overall + EXPECT_TRUE(result.has_value()); +} + +/// Tests for VerifyAllMethodsRegistered skipping kGet methods. + +class SkeletonFieldMethodHandlingFixture : public SkeletonMethodHandlingFixture +{ + public: + SkeletonFieldMethodHandlingFixture& WithAFieldGetMethodWithoutHandler() + { + const UniqueMethodIdentifier get_method_id{test::kFooFieldId, MethodType::kGet}; + get_field_method_ = std::make_unique(*skeleton_, get_method_id); + return *this; + } + + SkeletonFieldMethodHandlingFixture& WithAFieldSetMethodWithoutHandler() + { + const UniqueMethodIdentifier set_method_id{test::kFooFieldId, MethodType::kSet}; + set_field_method_ = std::make_unique(*skeleton_, set_method_id); + return *this; + } + + std::unique_ptr get_field_method_{nullptr}; + std::unique_ptr set_field_method_{nullptr}; +}; + +TEST_F(SkeletonFieldMethodHandlingFixture, VerifyAllMethodsRegisteredReturnsTrueWhenGetMethodHasNoHandler) +{ + // TODO: Rework this once the get support is added. + + // Given a skeleton with regular methods that have handlers registered, plus a field Get method without one + GivenASkeletonWithTwoMethods(); + WithAFieldGetMethodWithoutHandler(); + + // When verifying all methods are registered + const auto result = skeleton_->VerifyAllMethodsRegistered(); + + // Then verification succeeds because kGet methods are skipped + EXPECT_TRUE(result); +} + +TEST_F(SkeletonFieldMethodHandlingFixture, VerifyAllMethodsRegisteredReturnsFalseWhenSetMethodHasNoHandler) +{ + // Given a skeleton with regular methods that have handlers registered, plus a field Set method. + GivenASkeletonWithTwoMethods(); + WithAFieldSetMethodWithoutHandler(); + + // When verifying all methods are registered + const auto result = skeleton_->VerifyAllMethodsRegistered(); + + // Then verification fails because kSet methods are NOT skipped + EXPECT_FALSE(result); +} + } // namespace } // namespace score::mw::com::impl::lola diff --git a/score/mw/com/impl/field_tags.cpp b/score/mw/com/impl/field_tags.cpp new file mode 100644 index 000000000..f5cae1c66 --- /dev/null +++ b/score/mw/com/impl/field_tags.cpp @@ -0,0 +1,13 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/field_tags.h" diff --git a/score/mw/com/impl/field_tags.h b/score/mw/com/impl/field_tags.h new file mode 100644 index 000000000..cc8cc25f5 --- /dev/null +++ b/score/mw/com/impl/field_tags.h @@ -0,0 +1,62 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef PLATFORM_AAS_MW_COM_IMPL_FIELD_TAGS_H +#define PLATFORM_AAS_MW_COM_IMPL_FIELD_TAGS_H + +#include + +namespace score::mw::com::impl +{ + +/// \brief Tag types used on ProxyField/SkeletonField level to accomplish overload-resolution for various signatures, +/// which depend on the availability of Get/Set/Notifier. A field must enable WithGetter or WithNotifier +/// (a write-only field would be invisible to consumers). +struct WithGetter +{ +}; + +struct WithSetter +{ +}; + +struct WithNotifier +{ +}; + +namespace detail +{ + +template +struct contains_type : std::disjunction...> +{ +}; + +// SFINAE only works when the condition depends on the function-template parameters, not the class-template +// parameters. The std::is_same line ties substitution to a function-template +// parameter so the check fires at overload resolution instead of class instantiation. +template +struct is_tag_enabled : std::conjunction, std::is_same> +{ +}; + +/// \brief Marker used as a disambiguator on test-only ctors to keep them distinct from production overloads. +/// Lives in detail to signal "not part of the public API"; tests reach into the detail namespace knowingly. +struct WithTestTag +{ +}; + +} // namespace detail + +} // namespace score::mw::com::impl + +#endif // PLATFORM_AAS_MW_COM_IMPL_FIELD_TAGS_H diff --git a/score/mw/com/impl/generic_proxy_event.cpp b/score/mw/com/impl/generic_proxy_event.cpp index da5499356..15e670f66 100644 --- a/score/mw/com/impl/generic_proxy_event.cpp +++ b/score/mw/com/impl/generic_proxy_event.cpp @@ -21,9 +21,9 @@ namespace score::mw::com::impl GenericProxyEvent::GenericProxyEvent(ProxyBase& base, const std::string_view event_name) : ProxyEventBase{base, + event_name, ProxyBaseView{base}.GetBinding(), - GenericProxyEventBindingFactory::Create(base, event_name), - event_name} + GenericProxyEventBindingFactory::Create(base, event_name)} { ProxyBaseView proxy_base_view{base}; if (!binding_base_) @@ -34,9 +34,9 @@ GenericProxyEvent::GenericProxyEvent(ProxyBase& base, const std::string_view eve } GenericProxyEvent::GenericProxyEvent(ProxyBase& base, - std::unique_ptr proxy_binding, - const std::string_view event_name) - : ProxyEventBase{base, ProxyBaseView{base}.GetBinding(), std::move(proxy_binding), event_name} + const std::string_view event_name, + std::unique_ptr proxy_binding) + : ProxyEventBase{base, event_name, ProxyBaseView{base}.GetBinding(), std::move(proxy_binding)} { ProxyBaseView proxy_base_view{base}; if (!binding_base_) diff --git a/score/mw/com/impl/generic_proxy_event.h b/score/mw/com/impl/generic_proxy_event.h index c6e08b186..60e3ea7d3 100644 --- a/score/mw/com/impl/generic_proxy_event.h +++ b/score/mw/com/impl/generic_proxy_event.h @@ -47,8 +47,8 @@ class GenericProxyEvent : public ProxyEventBase /// /// \param proxy_binding The binding that shall be associated with this proxy. explicit GenericProxyEvent(ProxyBase& base, - std::unique_ptr proxy_binding, - const std::string_view event_name); + const std::string_view event_name, + std::unique_ptr proxy_binding); /// \brief Constructs a ProxyEvent by querying the base proxie's ProxyBinding for the respective ProxyEventBinding. /// diff --git a/score/mw/com/impl/generic_proxy_event_test.cpp b/score/mw/com/impl/generic_proxy_event_test.cpp index c9e18dbe2..3e639a1c9 100644 --- a/score/mw/com/impl/generic_proxy_event_test.cpp +++ b/score/mw/com/impl/generic_proxy_event_test.cpp @@ -87,7 +87,7 @@ TEST(GenericProxyEventTest, SamplePtrsToSlotDataAreConst) ProxyBase empty_proxy(std::make_unique(), make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); GenericProxyEvent proxy_event{ - empty_proxy, std::unique_ptr{std::move(mock_proxy_event_ptr)}, kEventName}; + empty_proxy, kEventName, std::unique_ptr{std::move(mock_proxy_event_ptr)}}; EXPECT_CALL(mock_proxy_event, Subscribe(max_num_samples)); EXPECT_CALL(mock_proxy_event, GetNewSamples(_, _)); @@ -120,7 +120,7 @@ TEST(GenericProxyEventDeathTest, DieOnProxyDestructionWhileHoldingSamplePtrs) ProxyBase empty_proxy(std::make_unique(), make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); auto proxy_event = std::make_unique( - empty_proxy, std::unique_ptr{std::move(mock_proxy_event_ptr)}, kEventName); + empty_proxy, kEventName, std::unique_ptr{std::move(mock_proxy_event_ptr)}); EXPECT_CALL(mock_proxy_event, Subscribe(max_num_samples)); EXPECT_CALL(mock_proxy_event, GetNewSamples(_, _)); @@ -158,7 +158,7 @@ TEST(GenericProxyEventGetSampleSizeTest, GetSampleSizeDispatchesToBinding) ProxyBase empty_proxy(std::make_unique(), make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); GenericProxyEvent proxy_event{ - empty_proxy, std::unique_ptr{std::move(mock_proxy_event_ptr)}, kEventName}; + empty_proxy, kEventName, std::unique_ptr{std::move(mock_proxy_event_ptr)}}; // Expect that GetSampleSize is called once on the binding EXPECT_CALL(mock_proxy_event, GetSampleSize()).WillOnce(Return(expected_sample_size)); @@ -186,7 +186,7 @@ TEST(GenericProxyEventHasSerializedFormatTest, HasSerializedFormatDispatchesToBi ProxyBase empty_proxy(std::make_unique(), make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); GenericProxyEvent proxy_event{ - empty_proxy, std::unique_ptr{std::move(mock_proxy_event_ptr)}, kEventName}; + empty_proxy, kEventName, std::unique_ptr{std::move(mock_proxy_event_ptr)}}; // Expect that HasSerializedFormat is called once on the binding EXPECT_CALL(mock_proxy_event, HasSerializedFormat()).WillOnce(Return(expected_has_serialized_format)); @@ -217,7 +217,7 @@ TEST(GenericProxyEventGetNewSamplesTest, GetNewSamplesContainsCorrectReceiverSig ProxyBase empty_proxy(std::make_unique(), make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); GenericProxyEvent proxy_event{ - empty_proxy, std::unique_ptr{std::move(mock_proxy_event_ptr)}, kEventName}; + empty_proxy, kEventName, std::unique_ptr{std::move(mock_proxy_event_ptr)}}; // Expect that GetNewSamples is called once on the binding EXPECT_CALL(mock_proxy_event, GetNewSamples(_, _)) diff --git a/score/mw/com/impl/method_identifier_test.cpp b/score/mw/com/impl/method_identifier_test.cpp new file mode 100644 index 000000000..989a2fd12 --- /dev/null +++ b/score/mw/com/impl/method_identifier_test.cpp @@ -0,0 +1,95 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/method_identifier.h" + +#include + +namespace score::mw::com::impl +{ +namespace +{ + +TEST(UniqueMethodIdentifierTest, EqualIdentifiersAreEqual) +{ + // Given two identifiers with the same name and type + const UniqueMethodIdentifier lhs{"foo", MethodType::kMethod}; + const UniqueMethodIdentifier rhs{"foo", MethodType::kMethod}; + + // When comparing for equality + // Then they are equal + EXPECT_TRUE(lhs == rhs); + EXPECT_FALSE(lhs != rhs); +} + +TEST(UniqueMethodIdentifierTest, DifferentNamesAreNotEqual) +{ + // Given two identifiers with different names but same type + const UniqueMethodIdentifier lhs{"foo", MethodType::kMethod}; + const UniqueMethodIdentifier rhs{"bar", MethodType::kMethod}; + + // When comparing + // Then they are not equal + EXPECT_FALSE(lhs == rhs); + EXPECT_TRUE(lhs != rhs); +} + +TEST(UniqueMethodIdentifierTest, DifferentTypesAreNotEqual) +{ + // Given two identifiers with same name but different type + const UniqueMethodIdentifier lhs{"foo", MethodType::kGet}; + const UniqueMethodIdentifier rhs{"foo", MethodType::kSet}; + + // When comparing + // Then they are not equal + EXPECT_FALSE(lhs == rhs); + EXPECT_TRUE(lhs != rhs); +} + +TEST(UniqueMethodIdentifierTest, LessThanComparesNameFirst) +{ + // Given identifiers where lhs name < rhs name + const UniqueMethodIdentifier lhs{"aaa", MethodType::kSet}; + const UniqueMethodIdentifier rhs{"bbb", MethodType::kGet}; + + // When comparing with operator< + // Then lhs < rhs because name is the primary sort key + EXPECT_TRUE(lhs < rhs); + EXPECT_FALSE(rhs < lhs); +} + +TEST(UniqueMethodIdentifierTest, LessThanComparesTypeWhenNamesEqual) +{ + // Given identifiers with same name but different types + const UniqueMethodIdentifier lhs{"foo", MethodType::kGet}; + const UniqueMethodIdentifier rhs{"foo", MethodType::kSet}; + + // When comparing with operator< + // Then ordering falls back to the MethodType enum value, so kGet < kSet + EXPECT_TRUE(lhs < rhs); + EXPECT_FALSE(rhs < lhs); +} + +TEST(UniqueMethodIdentifierTest, LessThanReturnsFalseForEqualIdentifiers) +{ + // Given two equal identifiers + const UniqueMethodIdentifier lhs{"foo", MethodType::kMethod}; + const UniqueMethodIdentifier rhs{"foo", MethodType::kMethod}; + + // When comparing with operator< + // Then neither is less than the other + EXPECT_FALSE(lhs < rhs); + EXPECT_FALSE(rhs < lhs); +} + +} // namespace +} // namespace score::mw::com::impl diff --git a/score/mw/com/impl/method_type.h b/score/mw/com/impl/method_type.h index e880e6033..804a0d3d2 100644 --- a/score/mw/com/impl/method_type.h +++ b/score/mw/com/impl/method_type.h @@ -19,26 +19,6 @@ namespace score::mw::com::impl { -namespace detail -{ - -/// \brief Tag types used on ProxyField/SkeletonField level to accomplish overload-resolution for various signatures, -/// which depend on the availability of Get/Set/Notifier. -struct EnableBothTag -{ -}; -struct EnableGetOnlyTag -{ -}; -struct EnableSetOnlyTag -{ -}; -struct EnableNeitherTag -{ -}; - -} // namespace detail - /// \brief Enum used to differentiate between regular service methods and field Get/Set methods. enum class MethodType : std::uint8_t { diff --git a/score/mw/com/impl/method_type_test.cpp b/score/mw/com/impl/method_type_test.cpp new file mode 100644 index 000000000..9558fe360 --- /dev/null +++ b/score/mw/com/impl/method_type_test.cpp @@ -0,0 +1,65 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/method_type.h" + +#include + +namespace score::mw::com::impl +{ +namespace +{ + +TEST(MethodTypeToStringTest, ReturnsMethodForKMethod) +{ + // Given a MethodType of kMethod + // When converting to string + // Then the result is "Method" + EXPECT_EQ(to_string(MethodType::kMethod), "Method"); +} + +TEST(MethodTypeToStringTest, ReturnsGetForKGet) +{ + // Given a MethodType of kGet + // When converting to string + // Then the result is "Get" + EXPECT_EQ(to_string(MethodType::kGet), "Get"); +} + +TEST(MethodTypeToStringTest, ReturnsSetForKSet) +{ + // Given a MethodType of kSet + // When converting to string + // Then the result is "Set" + EXPECT_EQ(to_string(MethodType::kSet), "Set"); +} + +TEST(MethodTypeToStringTest, ReturnsUnknownForKUnknown) +{ + // Given a MethodType of kUnknown + // When converting to string + // Then the result is "Unknown" + EXPECT_EQ(to_string(MethodType::kUnknown), "Unknown"); +} + +TEST(MethodTypeToStringTest, ReturnsInvalidForOutOfRangeValue) +{ + // Given an out-of-range MethodType value + const auto invalid_value = static_cast(255); + + // When converting to string + // Then the result is "Invalid" + EXPECT_EQ(to_string(invalid_value), "Invalid"); +} + +} // namespace +} // namespace score::mw::com::impl diff --git a/score/mw/com/impl/methods/proxy_method_base.h b/score/mw/com/impl/methods/proxy_method_base.h index 5e194bf00..4e6b41665 100644 --- a/score/mw/com/impl/methods/proxy_method_base.h +++ b/score/mw/com/impl/methods/proxy_method_base.h @@ -32,8 +32,8 @@ class ProxyMethodBase { public: ProxyMethodBase(ProxyBase& proxy_base, - std::unique_ptr proxy_method_binding, std::string_view method_name, + std::unique_ptr proxy_method_binding, MethodType method_type = MethodType::kMethod) noexcept : proxy_base_{proxy_base}, method_name_{method_name}, diff --git a/score/mw/com/impl/methods/proxy_method_test.cpp b/score/mw/com/impl/methods/proxy_method_test.cpp index 3c017bde2..8e08e0239 100644 --- a/score/mw/com/impl/methods/proxy_method_test.cpp +++ b/score/mw/com/impl/methods/proxy_method_test.cpp @@ -88,7 +88,7 @@ class ProxyMethodTestFixture : public ::testing::Test ProxyMethodTestFixture& GivenAValidProxyMethod() { unit_ = std::make_unique( - proxy_base_, std::make_unique(proxy_method_binding_mock_), kMethodName); + proxy_base_, kMethodName, std::make_unique(proxy_method_binding_mock_)); return *this; } @@ -160,8 +160,8 @@ TYPED_TEST(ProxyMethodAllArgCombinationsTestFixture, Construction) // Then the ProxyMethod can be constructed using ProxyMethodType = ProxyMethod; ProxyMethodType{this->proxy_base_, - std::make_unique(this->proxy_method_binding_mock_), - kMethodName}; + kMethodName, + std::make_unique(this->proxy_method_binding_mock_)}; } TYPED_TEST(ProxyMethodAllArgCombinationsTestFixture, WhenMoveConstructingProxyMethodUpdateMethodIsCalled) @@ -171,8 +171,8 @@ TYPED_TEST(ProxyMethodAllArgCombinationsTestFixture, WhenMoveConstructingProxyMe // Given a proxy method auto proxy_method = ProxyMethodType{this->proxy_base_, - std::make_unique(this->proxy_method_binding_mock_), - kMethodName}; + kMethodName, + std::make_unique(this->proxy_method_binding_mock_)}; // When movie-constructing this method auto moved_method{std::move(proxy_method)}; @@ -190,15 +190,15 @@ TYPED_TEST(ProxyMethodAllArgCombinationsTestFixture, WhenMoveAssigningProxyMetho // Given a proxy method auto proxy_method = ProxyMethodType{this->proxy_base_, - std::make_unique(this->proxy_method_binding_mock_), - kMethodName}; + kMethodName, + std::make_unique(this->proxy_method_binding_mock_)}; ProxyBase other_proxy_base = {std::make_unique(), this->config_store_.GetHandle()}; auto other_proxy_method = ProxyMethodType{other_proxy_base, - std::make_unique(this->proxy_method_binding_mock_), - "this_method_will_be_overwritten_soon"}; + "this_method_will_be_overwritten_soon", + std::make_unique(this->proxy_method_binding_mock_)}; // When move-assigning this method other_proxy_method = std::move(proxy_method); @@ -218,8 +218,8 @@ TYPED_TEST(ProxyMethodAllArgCombinationsTestFixture, // Given a proxy method auto proxy_method = ProxyMethodType{this->proxy_base_, - std::make_unique(this->proxy_method_binding_mock_), - kMethodName}; + kMethodName, + std::make_unique(this->proxy_method_binding_mock_)}; auto same_method_ptr = &proxy_method; // When move-assigning this method to itself @@ -263,7 +263,7 @@ TYPED_TEST(ProxyMethodAllArgCombinationsTestFixture, InvalidBindingInConstructor using ProxyMethodType = ProxyMethod; // When a proxy method is created with an invalid binding - auto proxy_method = std::make_unique(this->proxy_base_, nullptr, kMethodName); + auto proxy_method = std::make_unique(this->proxy_base_, kMethodName, nullptr); // Then calling AreBindingsValid returns false EXPECT_FALSE(ProxyBaseView{this->proxy_base_}.AreBindingsValid()); diff --git a/score/mw/com/impl/methods/proxy_method_with_in_args.h b/score/mw/com/impl/methods/proxy_method_with_in_args.h index 113b7e8d9..9d1280f23 100644 --- a/score/mw/com/impl/methods/proxy_method_with_in_args.h +++ b/score/mw/com/impl/methods/proxy_method_with_in_args.h @@ -52,18 +52,18 @@ class ProxyMethod final : public ProxyMethodBase public: ProxyMethod(ProxyBase& proxy_base, std::string_view method_name) noexcept : ProxyMethod(proxy_base, + method_name, ProxyMethodBindingFactory::Create(proxy_base.GetHandle(), ProxyBaseView{proxy_base}.GetBinding(), method_name, - MethodType::kMethod), - method_name) + MethodType::kMethod)) { } ProxyMethod(ProxyBase& proxy_base, - std::unique_ptr proxy_method_binding, - std::string_view method_name) noexcept - : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kMethod), + std::string_view method_name, + std::unique_ptr proxy_method_binding) noexcept + : ProxyMethodBase(proxy_base, method_name, std::move(proxy_method_binding), MethodType::kMethod), are_in_arg_ptrs_active_(kCallQueueSize) { auto proxy_base_view = ProxyBaseView{proxy_base}; diff --git a/score/mw/com/impl/methods/proxy_method_with_in_args_and_return.h b/score/mw/com/impl/methods/proxy_method_with_in_args_and_return.h index fcfb3bcf5..b8163f75e 100644 --- a/score/mw/com/impl/methods/proxy_method_with_in_args_and_return.h +++ b/score/mw/com/impl/methods/proxy_method_with_in_args_and_return.h @@ -40,7 +40,7 @@ namespace score::mw::com::impl { -template +template class ProxyField; /// \brief Partial specialization of ProxyMethod for function signatures with arguments and non-void return @@ -55,10 +55,10 @@ class ProxyMethod final : public ProxyMethodBase // coverity[autosar_cpp14_a11_3_1_violation] friend class ProxyMethodView; - friend class ProxyField; - friend class ProxyField; - friend class ProxyField; - friend class ProxyField; + // ProxyField needs to instantiate ProxyMethod via the private FieldOnlyConstructorEnabler tag. + // coverity[autosar_cpp14_a11_3_1_violation] + template + friend class ProxyField; struct FieldOnlyConstructorEnabler { @@ -68,11 +68,11 @@ class ProxyMethod final : public ProxyMethodBase ProxyMethod(ProxyBase& proxy_base, std::string_view method_name) noexcept : ProxyMethodBase( proxy_base, + method_name, ProxyMethodBindingFactory::Create(proxy_base.GetHandle(), ProxyBaseView{proxy_base}.GetBinding(), method_name, MethodType::kMethod), - method_name, MethodType::kMethod), are_in_arg_ptrs_active_(kCallQueueSize) { @@ -86,9 +86,9 @@ class ProxyMethod final : public ProxyMethodBase } ProxyMethod(ProxyBase& proxy_base, - std::unique_ptr proxy_method_binding, - std::string_view method_name) noexcept - : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kMethod), + std::string_view method_name, + std::unique_ptr proxy_method_binding) noexcept + : ProxyMethodBase(proxy_base, method_name, std::move(proxy_method_binding), MethodType::kMethod), are_in_arg_ptrs_active_(kCallQueueSize) { auto proxy_base_view = ProxyBaseView{proxy_base}; @@ -101,10 +101,10 @@ class ProxyMethod final : public ProxyMethodBase } ProxyMethod(ProxyBase& proxy_base, - std::unique_ptr proxy_method_binding, std::string_view method_name, + std::unique_ptr proxy_method_binding, FieldOnlyConstructorEnabler) noexcept - : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kSet), + : ProxyMethodBase(proxy_base, method_name, std::move(proxy_method_binding), MethodType::kSet), are_in_arg_ptrs_active_(kCallQueueSize) { auto proxy_base_view = ProxyBaseView{proxy_base}; diff --git a/score/mw/com/impl/methods/proxy_method_with_return_type.h b/score/mw/com/impl/methods/proxy_method_with_return_type.h index edeea7b8c..f8bcad6a9 100644 --- a/score/mw/com/impl/methods/proxy_method_with_return_type.h +++ b/score/mw/com/impl/methods/proxy_method_with_return_type.h @@ -36,7 +36,7 @@ namespace score::mw::com::impl { -template +template class ProxyField; /// \brief Partial specialization of ProxyMethod for function signatures with no arguments and non-void return @@ -49,10 +49,11 @@ class ProxyMethod final : public ProxyMethodBase // This enables us to hide unnecessary internals from the end-user. // coverity[autosar_cpp14_a11_3_1_violation] friend class ProxyMethodView; - friend class ProxyField; - friend class ProxyField; - friend class ProxyField; - friend class ProxyField; + + // ProxyField needs to instantiate ProxyMethod via the private FieldOnlyConstructorEnabler tag. + // coverity[autosar_cpp14_a11_3_1_violation] + template + friend class ProxyField; struct FieldOnlyConstructorEnabler { @@ -61,11 +62,11 @@ class ProxyMethod final : public ProxyMethodBase public: ProxyMethod(ProxyBase& proxy_base, std::string_view method_name) noexcept : ProxyMethodBase(proxy_base, + method_name, ProxyMethodBindingFactory::Create(proxy_base.GetHandle(), ProxyBaseView{proxy_base}.GetBinding(), method_name, MethodType::kMethod), - method_name, MethodType::kMethod) { auto proxy_base_view = ProxyBaseView{proxy_base}; @@ -78,9 +79,9 @@ class ProxyMethod final : public ProxyMethodBase } ProxyMethod(ProxyBase& proxy_base, - std::unique_ptr proxy_method_binding, - std::string_view method_name) noexcept - : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kMethod) + std::string_view method_name, + std::unique_ptr proxy_method_binding) noexcept + : ProxyMethodBase(proxy_base, method_name, std::move(proxy_method_binding), MethodType::kMethod) { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); @@ -92,10 +93,10 @@ class ProxyMethod final : public ProxyMethodBase } ProxyMethod(ProxyBase& proxy_base, - std::unique_ptr proxy_method_binding, std::string_view method_name, + std::unique_ptr proxy_method_binding, FieldOnlyConstructorEnabler) noexcept - : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kGet) + : ProxyMethodBase(proxy_base, method_name, std::move(proxy_method_binding), MethodType::kGet) { auto proxy_base_view = ProxyBaseView{proxy_base}; if (binding_ == nullptr) diff --git a/score/mw/com/impl/methods/proxy_method_without_in_args_or_return.h b/score/mw/com/impl/methods/proxy_method_without_in_args_or_return.h index e9a36fd28..c45467fad 100644 --- a/score/mw/com/impl/methods/proxy_method_without_in_args_or_return.h +++ b/score/mw/com/impl/methods/proxy_method_without_in_args_or_return.h @@ -46,11 +46,11 @@ class ProxyMethod final : public ProxyMethodBase public: ProxyMethod(ProxyBase& proxy_base, std::string_view method_name) noexcept : ProxyMethodBase(proxy_base, + method_name, ProxyMethodBindingFactory::Create(proxy_base.GetHandle(), ProxyBaseView{proxy_base}.GetBinding(), method_name, MethodType::kMethod), - method_name, MethodType::kMethod) { auto proxy_base_view = ProxyBaseView{proxy_base}; @@ -63,9 +63,9 @@ class ProxyMethod final : public ProxyMethodBase } ProxyMethod(ProxyBase& proxy_base, - std::unique_ptr proxy_method_binding, - std::string_view method_name) noexcept - : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kMethod) + std::string_view method_name, + std::unique_ptr proxy_method_binding) noexcept + : ProxyMethodBase(proxy_base, method_name, std::move(proxy_method_binding), MethodType::kMethod) { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); diff --git a/score/mw/com/impl/methods/skeleton_method.h b/score/mw/com/impl/methods/skeleton_method.h index 8164f2761..63c7e9508 100644 --- a/score/mw/com/impl/methods/skeleton_method.h +++ b/score/mw/com/impl/methods/skeleton_method.h @@ -31,7 +31,7 @@ namespace score::mw::com::impl { -template +template class SkeletonField; template @@ -53,7 +53,7 @@ class SkeletonMethod final : public SkeletonMethodBase static_assert(return_value_is_not_a_pointer, "Return value can not be a pointer, since we can not put them in shared memory."); - template + template // coverity[autosar_cpp14_a11_3_1_violation] friend class SkeletonField; diff --git a/score/mw/com/impl/mocking/proxy_event_mock_test.cpp b/score/mw/com/impl/mocking/proxy_event_mock_test.cpp index 30966ed05..c6a55a345 100644 --- a/score/mw/com/impl/mocking/proxy_event_mock_test.cpp +++ b/score/mw/com/impl/mocking/proxy_event_mock_test.cpp @@ -45,15 +45,27 @@ class ProxyEventFieldMockFixture : public ::testing::Test using ProxyEventFieldMock = typename T::ProxyEventFieldMock; ProxyEventFieldMockFixture() + : unit_{[this] { + if constexpr (detail::is_proxy_field_v) + { + return ProxyEventField{proxy_base_, + kDummyEventFieldName, + detail::WithTestTag{}, + std::unique_ptr>{nullptr}}; + } + else + { + return ProxyEventField{ + proxy_base_, kDummyEventFieldName, std::unique_ptr>{nullptr}}; + } + }()} { unit_.InjectMock(proxy_service_element_mock_); } ProxyEventFieldMock proxy_service_element_mock_{}; ProxyBase proxy_base_{nullptr, MakeFakeHandle(1U)}; - ProxyEventField unit_{proxy_base_, - std::unique_ptr>{nullptr}, - kDummyEventFieldName}; + ProxyEventField unit_; }; struct ProxyEventStruct @@ -64,7 +76,7 @@ struct ProxyEventStruct struct ProxyFieldStruct { - using ProxyEventField = ProxyField; + using ProxyEventField = ProxyField; using ProxyEventFieldMock = ProxyEventMock; }; diff --git a/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp b/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp index 674527449..5d1f64587 100644 --- a/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp +++ b/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp @@ -54,7 +54,7 @@ class MyInterface : public InterfaceTrait::Base typename InterfaceTrait::template Event some_event{*this, kEventName}; typename InterfaceTrait::template Event some_event_2{*this, kEventName2}; - typename InterfaceTrait::template Field some_field{*this, kFieldName}; + typename InterfaceTrait::template Field some_field{*this, kFieldName}; }; using MyProxy = AsProxy; @@ -230,7 +230,7 @@ class FieldOnlyInterface : public InterfaceTrait::Base public: using InterfaceTrait::Base::Base; - typename InterfaceTrait::template Field some_field{*this, kFieldName}; + typename InterfaceTrait::template Field some_field{*this, kFieldName}; }; using FieldOnlyProxy = AsProxy; diff --git a/score/mw/com/impl/mocking/skeleton_field_mock_test.cpp b/score/mw/com/impl/mocking/skeleton_field_mock_test.cpp index a3bf86536..a1e7081b7 100644 --- a/score/mw/com/impl/mocking/skeleton_field_mock_test.cpp +++ b/score/mw/com/impl/mocking/skeleton_field_mock_test.cpp @@ -41,7 +41,7 @@ class SkeletonFieldMockFixture : public ::testing::Test SkeletonFieldMock skeleton_field_mock_{}; SkeletonBase skeleton_base_{nullptr, MakeFakeInstanceIdentifier(1U)}; - SkeletonField unit_{skeleton_base_, kDummyFieldName, nullptr}; + SkeletonField unit_{skeleton_base_, kDummyFieldName, nullptr}; }; TEST_F(SkeletonFieldMockFixture, AllocateDispatchesToMockAfterInjectingMock) diff --git a/score/mw/com/impl/plumbing/BUILD b/score/mw/com/impl/plumbing/BUILD index 862b878e0..93d73ab61 100644 --- a/score/mw/com/impl/plumbing/BUILD +++ b/score/mw/com/impl/plumbing/BUILD @@ -829,6 +829,20 @@ cc_gtest_unit_test( ], ) +cc_gtest_unit_test( + name = "lola_proxy_element_building_blocks_test", + srcs = [ + "lola_proxy_element_building_blocks_test.cpp", + ], + features = COMPILER_WARNING_FEATURES, + deps = [ + ":lola_proxy_element_building_blocks", + "//score/mw/com/impl/bindings/lola/test:proxy_event_test_resources", + "//score/mw/com/impl/configuration/test:configuration_store", + "//score/mw/com/impl/test:dummy_instance_identifier_builder", + ], +) + cc_gtest_unit_test( name = "skeleton_field_binding_factory_test", srcs = [ @@ -899,6 +913,7 @@ cc_unit_test_suites_for_host_and_qnx( ":skeleton_event_binding_factory_test", ":skeleton_field_binding_factory_test", ":skeleton_service_element_binding_factory_test", + ":lola_proxy_element_building_blocks_test", ":unit_test", ], visibility = [ diff --git a/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.cpp b/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.cpp index 805d87d18..7bd9ffd8c 100644 --- a/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.cpp +++ b/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.cpp @@ -71,7 +71,7 @@ std::optional LookupForLola(const HandleType& ha const lola::ElementFqId element_fq_id{ lola_type_deployment.service_id_, element_id, lola_instance_id->GetId(), element_type}; - return LoLaProxyElementBuildingBlocks{*lola_parent, lola_instance_id->GetId(), element_fq_id}; + return LoLaProxyElementBuildingBlocks{*lola_parent, element_fq_id}; } } // namespace diff --git a/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.h b/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.h index a5d73f654..fe59da959 100644 --- a/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.h +++ b/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.h @@ -16,7 +16,6 @@ #include "score/mw/com/impl/bindings/lola/element_fq_id.h" #include "score/mw/com/impl/bindings/lola/proxy.h" #include "score/mw/com/impl/configuration/lola_service_element_id.h" -#include "score/mw/com/impl/configuration/lola_service_instance_deployment.h" #include "score/mw/com/impl/handle_type.h" #include "score/mw/com/impl/proxy_base.h" #include "score/mw/com/impl/proxy_binding.h" @@ -35,7 +34,6 @@ namespace score::mw::com::impl struct LoLaProxyElementBuildingBlocks { lola::Proxy& parent; - LolaServiceInstanceId::InstanceId instance_id; lola::ElementFqId element_fq_id; }; diff --git a/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks_test.cpp b/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks_test.cpp new file mode 100644 index 000000000..fa1398ce7 --- /dev/null +++ b/score/mw/com/impl/plumbing/lola_proxy_element_building_blocks_test.cpp @@ -0,0 +1,168 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/plumbing/lola_proxy_element_building_blocks.h" + +#include "score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.h" +#include "score/mw/com/impl/configuration/lola_service_instance_deployment.h" +#include "score/mw/com/impl/configuration/lola_service_instance_id.h" +#include "score/mw/com/impl/configuration/quality_type.h" +#include "score/mw/com/impl/configuration/service_identifier_type.h" +#include "score/mw/com/impl/configuration/service_instance_id.h" +#include "score/mw/com/impl/configuration/test/configuration_store.h" +#include "score/mw/com/impl/handle_type.h" +#include "score/mw/com/impl/proxy_base.h" +#include "score/mw/com/impl/service_element_type.h" +#include "score/mw/com/impl/test/dummy_instance_identifier_builder.h" + +#include + +#include +#include +#include + +namespace score::mw::com::impl +{ +namespace +{ + +using namespace ::testing; + +constexpr auto kDummyEventName{"Event1"}; +constexpr auto kDummyFieldName{"Field1"}; +constexpr auto kDummyMethodName{"Method1"}; + +constexpr std::uint16_t kDummyEventId{5U}; +constexpr std::uint16_t kDummyFieldId{6U}; +constexpr std::uint16_t kDummyMethodId{7U}; + +constexpr uint16_t kInstanceId = 0x31U; +const LolaServiceId kServiceId{1U}; +const auto kInstanceSpecifier = InstanceSpecifier::Create(std::string{"/my_dummy_instance_specifier"}).value(); + +const LolaServiceInstanceDeployment kLolaServiceInstanceDeployment{ + LolaServiceInstanceId{kInstanceId}, + {{kDummyEventName, LolaEventInstanceDeployment{{1U}, {3U}, 1U, true, 0}}}, + {{kDummyFieldName, LolaFieldInstanceDeployment{{1U}, {3U}, 1U, true, 0}}}, + {{kDummyMethodName, LolaMethodInstanceDeployment{5U}}}}; + +const LolaServiceTypeDeployment kLolaServiceTypeDeployment{kServiceId, + {{kDummyEventName, kDummyEventId}}, + {{kDummyFieldName, kDummyFieldId}}, + {{kDummyMethodName, kDummyMethodId}}}; + +constexpr auto kQualityType = QualityType::kASIL_B; +ConfigurationStore kConfigStoreAsilB{kInstanceSpecifier, + make_ServiceIdentifierType("/a/service/somewhere/out/there", 13U, 37U), + kQualityType, + kLolaServiceTypeDeployment, + kLolaServiceInstanceDeployment}; + +class LookupLolaProxyElementFixture : public lola::ProxyMockedMemoryFixture +{ + public: + LookupLolaProxyElementFixture& WithAProxyBaseWithValidBinding(const HandleType& handle) + { + proxy_base_ = std::make_unique(std::move(proxy_), handle); + return *this; + } + + LookupLolaProxyElementFixture& WithAProxyBaseWithNullBinding(const HandleType& handle) + { + proxy_base_ = std::make_unique(nullptr, handle); + return *this; + } + + std::unique_ptr proxy_base_{nullptr}; + DummyInstanceIdentifierBuilder dummy_instance_identifier_builder_{}; +}; + +struct ElementLookupParam +{ + const char* element_name; + std::uint16_t expected_element_id; + ServiceElementType element_type; +}; + +class LookupLolaProxyElementParamFixture : public LookupLolaProxyElementFixture, + public ::testing::WithParamInterface +{ +}; + +TEST_P(LookupLolaProxyElementParamFixture, ReturnsValidResultForElementLookup) +{ + // Given a valid proxy with a lola binding + const auto& param = GetParam(); + const auto handle = kConfigStoreAsilB.GetHandle(); + InitialiseProxyWithConstructor(kConfigStoreAsilB.GetInstanceIdentifier()); + WithAProxyBaseWithValidBinding(handle); + + // When looking up the element + auto result = LookupLolaProxyElement( + handle, ProxyBaseView{*proxy_base_}.GetBinding(), param.element_name, param.element_type); + + // Then the result is valid and every field of the element FQ id matches the deployment + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->element_fq_id.element_id_, param.expected_element_id); + EXPECT_EQ(result->element_fq_id.service_id_, kServiceId); + EXPECT_EQ(result->element_fq_id.instance_id_, kInstanceId); + EXPECT_EQ(result->element_fq_id.element_type_, param.element_type); +} + +INSTANTIATE_TEST_SUITE_P( + AllElementTypes, + LookupLolaProxyElementParamFixture, + ::testing::Values(ElementLookupParam{kDummyEventName, kDummyEventId, ServiceElementType::EVENT}, + ElementLookupParam{kDummyFieldName, kDummyFieldId, ServiceElementType::FIELD}, + ElementLookupParam{kDummyMethodName, kDummyMethodId, ServiceElementType::METHOD})); + +TEST_F(LookupLolaProxyElementFixture, ReturnsNulloptWhenParentBindingIsNull) +{ + // Given a proxy base with a null binding + const auto handle = kConfigStoreAsilB.GetHandle(); + WithAProxyBaseWithNullBinding(handle); + + // When looking up an element + auto result = LookupLolaProxyElement(handle, nullptr, kDummyEventName, ServiceElementType::EVENT); + + // Then the result is nullopt + EXPECT_FALSE(result.has_value()); +} + +TEST_F(LookupLolaProxyElementFixture, ReturnsNulloptForSomeIpBinding) +{ + // Given a handle with a non-lola SomeIP deployment + const auto instance_identifier = dummy_instance_identifier_builder_.CreateSomeIpBindingInstanceIdentifier(); + const auto handle = make_HandleType(instance_identifier, ServiceInstanceId{LolaServiceInstanceId{kInstanceId}}); + + // When looking up an element + auto result = LookupLolaProxyElement(handle, nullptr, kDummyEventName, ServiceElementType::EVENT); + + // Then the result is nullopt + EXPECT_FALSE(result.has_value()); +} + +TEST_F(LookupLolaProxyElementFixture, ReturnsNulloptForBlankBinding) +{ + // Given a handle with a non-lola blank deployment + const auto instance_identifier = dummy_instance_identifier_builder_.CreateBlankBindingInstanceIdentifier(); + const auto handle = make_HandleType(instance_identifier, ServiceInstanceId{LolaServiceInstanceId{kInstanceId}}); + + // When looking up an element + auto result = LookupLolaProxyElement(handle, nullptr, kDummyEventName, ServiceElementType::EVENT); + + // Then the result is nullopt + EXPECT_FALSE(result.has_value()); +} + +} // namespace +} // namespace score::mw::com::impl diff --git a/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp b/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp index 5abbf7cec..266ea7009 100644 --- a/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp +++ b/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp @@ -11,6 +11,8 @@ * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ #include "score/mw/com/impl/plumbing/proxy_method_binding_factory.h" +#include "score/mw/com/impl/bindings/lola/methods/proxy_method_instance_identifier.h" +#include "score/mw/com/impl/bindings/lola/proxy_method.h" #include "score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.h" #include "score/mw/com/impl/configuration/lola_service_instance_deployment.h" #include "score/mw/com/impl/configuration/lola_service_instance_id.h" @@ -34,6 +36,23 @@ #include #include +namespace score::mw::com::impl::lola +{ +class ProxyMethodAttorney +{ + public: + explicit ProxyMethodAttorney(ProxyMethod& method) noexcept : method_{method} {} + + const ProxyMethodInstanceIdentifier& GetProxyMethodInstanceIdentifier() const noexcept + { + return method_.proxy_method_instance_identifier_; + } + + private: + ProxyMethod& method_; +}; +} // namespace score::mw::com::impl::lola + namespace score::mw::com::impl { @@ -65,6 +84,28 @@ ConfigurationStore kConfigStoreAsilB{kInstanceSpecifier, kLolaServiceTypeDeployment, kLolaServiceInstanceDeployment}; +// Deployment with a field alongside the method, for the kGet / kSet tests. +constexpr auto kDummyFieldName{"Field1"}; +constexpr std::uint16_t kDummyFieldId{6U}; +const auto kFieldInstanceSpecifier = InstanceSpecifier::Create(std::string{"/my_field_instance_specifier"}).value(); + +const LolaServiceInstanceDeployment kLolaServiceInstanceDeploymentWithField{ + LolaServiceInstanceId{kInstanceId}, + {}, + {{kDummyFieldName, LolaFieldInstanceDeployment{{1U}, {3U}, 1U, true, 0}}}, + {{kDummyMethodName, LolaMethodInstanceDeployment{kQueueSize}}}}; + +const LolaServiceTypeDeployment kLolaServiceTypeDeploymentWithField{kServiceId, + {}, + {{kDummyFieldName, kDummyFieldId}}, + {{kDummyMethodName, kDummyMethodId}}}; + +ConfigurationStore kConfigStoreWithFieldAsilB{kFieldInstanceSpecifier, + make_ServiceIdentifierType("/a/service/somewhere/out/there", 13U, 37U), + kQualityType, + kLolaServiceTypeDeploymentWithField, + kLolaServiceInstanceDeploymentWithField}; + const LolaServiceInstanceDeployment kLolaServiceInstanceDeploymentWithEmptyQueueSize{ LolaServiceInstanceId{kInstanceId}, {}, @@ -87,6 +128,11 @@ class ProxyMethodFactoryFixture : public lola::ProxyMockedMemoryFixture return kConfigStoreAsilB.GetHandle(); } + HandleType GetValidLoLaHandleWithField() + { + return kConfigStoreWithFieldAsilB.GetHandle(); + } + HandleType GetValidSomIpHandle() { const auto instance_identifier = dummy_instance_identifier_builder_.CreateSomeIpBindingInstanceIdentifier(); @@ -122,7 +168,6 @@ TYPED_TEST_SUITE(ProxyMethodFactoryTypedFixture, RegisteredFunctionTypes, ); TYPED_TEST(ProxyMethodFactoryTypedFixture, CanConstructProxyMethod) { - // Given a valid lola binding const auto handle = this->GetValidLoLaHandle(); @@ -157,7 +202,7 @@ TYPED_TEST(ProxyMethodFactoryTypedFixture, CannotCreateProxyServiceWhenProxyBind TYPED_TEST(ProxyMethodFactoryTypedFixture, CannotConstructEventFromSomeIpBinding) { - + // Given a handle carrying a SomeIp deployment const auto handle = this->GetValidSomIpHandle(); // Given a valid someip binding @@ -174,12 +219,11 @@ TYPED_TEST(ProxyMethodFactoryTypedFixture, CannotConstructEventFromSomeIpBinding TYPED_TEST(ProxyMethodFactoryTypedFixture, CannotConstructEventFromBlankBinding) { + // Given a handle carrying a blank deployment const auto handle = this->GetBlankBindingHandle(); - // Given a blank binding + // When Create is called auto proxy_binding = this->CreateBindingFromHandle(handle); - - // When creating a ProxyMethod using MethodBindingFactory using MethodSignature = TypeParam; auto proxy_method = ProxyMethodBindingFactory::Create( handle, proxy_binding, kDummyMethodName, MethodType::kMethod); @@ -188,6 +232,112 @@ TYPED_TEST(ProxyMethodFactoryTypedFixture, CannotConstructEventFromBlankBinding) EXPECT_EQ(proxy_method, nullptr); } +TYPED_TEST(ProxyMethodFactoryTypedFixture, CanConstructFieldGetMethod) +{ + // Given a valid lola binding with a field in the deployment + const auto handle = this->GetValidLoLaHandleWithField(); + this->InitialiseProxyWithConstructor(handle.GetInstanceIdentifier()); + + // When Create is called with MethodType::kGet + auto proxy_binding = this->CreateBindingFromHandle(handle); + using MethodSignature = TypeParam; + auto proxy_method = + ProxyMethodBindingFactory::Create(handle, proxy_binding, kDummyFieldName, MethodType::kGet); + + // Then the returned method's id is (field, kGet) + ASSERT_NE(proxy_method, nullptr); + auto* const lola_method = dynamic_cast(proxy_method.get()); + ASSERT_NE(lola_method, nullptr); + const auto& unique_id = + lola::ProxyMethodAttorney{*lola_method}.GetProxyMethodInstanceIdentifier().unique_method_identifier; + EXPECT_EQ(unique_id.method_or_field_id, kDummyFieldId); + EXPECT_EQ(unique_id.method_type, MethodType::kGet); +} + +TYPED_TEST(ProxyMethodFactoryTypedFixture, CanConstructFieldSetMethod) +{ + // Given a valid lola binding with a field in the deployment + const auto handle = this->GetValidLoLaHandleWithField(); + this->InitialiseProxyWithConstructor(handle.GetInstanceIdentifier()); + + // When Create is called with MethodType::kSet + auto proxy_binding = this->CreateBindingFromHandle(handle); + using MethodSignature = TypeParam; + auto proxy_method = + ProxyMethodBindingFactory::Create(handle, proxy_binding, kDummyFieldName, MethodType::kSet); + + // Then the returned method's id is (field, kSet) + ASSERT_NE(proxy_method, nullptr); + auto* const lola_method = dynamic_cast(proxy_method.get()); + ASSERT_NE(lola_method, nullptr); + const auto& unique_id = + lola::ProxyMethodAttorney{*lola_method}.GetProxyMethodInstanceIdentifier().unique_method_identifier; + EXPECT_EQ(unique_id.method_or_field_id, kDummyFieldId); + EXPECT_EQ(unique_id.method_type, MethodType::kSet); +} + +TYPED_TEST(ProxyMethodFactoryTypedFixture, GetAndSetForSameFieldProduceDistinctUniqueIdentifiers) +{ + // Given a valid lola binding with a field in the deployment + const auto handle = this->GetValidLoLaHandleWithField(); + this->InitialiseProxyWithConstructor(handle.GetInstanceIdentifier()); + auto proxy_binding = this->CreateBindingFromHandle(handle); + using MethodSignature = TypeParam; + + // When Create is called for the same field name with kGet and then kSet + auto get_method = + ProxyMethodBindingFactory::Create(handle, proxy_binding, kDummyFieldName, MethodType::kGet); + auto set_method = + ProxyMethodBindingFactory::Create(handle, proxy_binding, kDummyFieldName, MethodType::kSet); + + // Then both succeed, share the field id, and differ only on MethodType + ASSERT_NE(get_method, nullptr); + ASSERT_NE(set_method, nullptr); + auto* const lola_get = dynamic_cast(get_method.get()); + auto* const lola_set = dynamic_cast(set_method.get()); + ASSERT_NE(lola_get, nullptr); + ASSERT_NE(lola_set, nullptr); + + const auto& get_id = + lola::ProxyMethodAttorney{*lola_get}.GetProxyMethodInstanceIdentifier().unique_method_identifier; + const auto& set_id = + lola::ProxyMethodAttorney{*lola_set}.GetProxyMethodInstanceIdentifier().unique_method_identifier; + EXPECT_EQ(get_id.method_or_field_id, set_id.method_or_field_id); + EXPECT_NE(get_id.method_type, set_id.method_type); + EXPECT_EQ(get_id.method_type, MethodType::kGet); + EXPECT_EQ(set_id.method_type, MethodType::kSet); +} + +TYPED_TEST(ProxyMethodFactoryTypedFixture, CannotConstructFieldGetMethodWhenProxyBindingIsNullptr) +{ + // Given a valid lola handle with a field in the deployment + const auto handle = this->GetValidLoLaHandleWithField(); + + // When Create is called with MethodType::kGet and a null parent binding + ProxyBinding* proxy_binding = nullptr; + using MethodSignature = TypeParam; + auto proxy_method = + ProxyMethodBindingFactory::Create(handle, proxy_binding, kDummyFieldName, MethodType::kGet); + + // Then nullptr is returned + ASSERT_EQ(proxy_method, nullptr); +} + +TYPED_TEST(ProxyMethodFactoryTypedFixture, CannotConstructFieldSetMethodWhenProxyBindingIsNullptr) +{ + // Given a valid lola handle with a field in the deployment + const auto handle = this->GetValidLoLaHandleWithField(); + + // When Create is called with MethodType::kSet and a null parent binding + ProxyBinding* proxy_binding = nullptr; + using MethodSignature = TypeParam; + auto proxy_method = + ProxyMethodBindingFactory::Create(handle, proxy_binding, kDummyFieldName, MethodType::kSet); + + // Then nullptr is returned + ASSERT_EQ(proxy_method, nullptr); +} + TYPED_TEST(ProxyMethodFactoryTypedFixture, GetQueueSizeReturnsValueForMethodInLolaDeployment) { // Given a handle to a valid lola deployment which contains a method diff --git a/score/mw/com/impl/proxy_base_test.cpp b/score/mw/com/impl/proxy_base_test.cpp index 2ed375ad0..c696379af 100644 --- a/score/mw/com/impl/proxy_base_test.cpp +++ b/score/mw/com/impl/proxy_base_test.cpp @@ -593,27 +593,27 @@ class ProxyBaseServiceElementReferencesFixture : public ::testing::Test MyProxy proxy_{std::make_unique(proxy_binding_mock_), handle_}; ProxyEventBase event_0_{proxy_, + event_name_0_, &proxy_binding_mock_, - std::make_unique(), - event_name_0_}; + std::make_unique()}; ProxyEventBase event_1_{proxy_, + event_name_1_, &proxy_binding_mock_, - std::make_unique(), - event_name_1_}; + std::make_unique()}; ProxyEventBase field_event_dispatch_0_{proxy_, + field_name_0_, &proxy_binding_mock_, - std::make_unique(), - field_name_0_}; + std::make_unique()}; ProxyEventBase field_event_dispatch_1_{proxy_, + field_name_1_, &proxy_binding_mock_, - std::make_unique(), - field_name_1_}; - ProxyFieldBase field_0_{proxy_, &field_event_dispatch_0_, field_name_0_}; - ProxyFieldBase field_1_{proxy_, &field_event_dispatch_1_, field_name_1_}; + std::make_unique()}; + ProxyFieldBase field_0_{proxy_, field_name_0_, &field_event_dispatch_0_}; + ProxyFieldBase field_1_{proxy_, field_name_1_, &field_event_dispatch_1_}; - DummyProxyMethod method_0_{proxy_, std::make_unique(), method_name_0_}; - DummyProxyMethod method_1_{proxy_, std::make_unique(), method_name_1_}; + DummyProxyMethod method_0_{proxy_, method_name_0_, std::make_unique()}; + DummyProxyMethod method_1_{proxy_, method_name_1_, std::make_unique()}; }; TEST_F(ProxyBaseServiceElementReferencesFixture, RegisteringServiceElementStoresReferenceInMap) @@ -707,11 +707,11 @@ TEST_F(ProxyBaseServiceElementReferencesFixture, MoveAssigningUpdatesReferencesT // and given that an Event, Field and Method were registered on the second proxy ProxyEventBase event{ - proxy_2, &proxy_binding_mock, std::make_unique(), other_event_name}; + proxy_2, other_event_name, &proxy_binding_mock, std::make_unique()}; ProxyEventBase field_event_dispatch{ - proxy_2, &proxy_binding_mock, std::make_unique(), other_field_name}; - ProxyFieldBase field{proxy_2, &field_event_dispatch, other_field_name}; - DummyProxyMethod method{proxy_2, std::make_unique(), other_method_name}; + proxy_2, other_field_name, &proxy_binding_mock, std::make_unique()}; + ProxyFieldBase field{proxy_2, other_field_name, &field_event_dispatch}; + DummyProxyMethod method{proxy_2, other_method_name, std::make_unique()}; ProxyBaseView{proxy_2}.RegisterEvent(other_event_name, event); ProxyBaseView{proxy_2}.RegisterField(other_field_name, field); ProxyBaseView{proxy_2}.RegisterMethod(other_method_name, method); diff --git a/score/mw/com/impl/proxy_event.h b/score/mw/com/impl/proxy_event.h index cd699be9b..3ebcdae85 100644 --- a/score/mw/com/impl/proxy_event.h +++ b/score/mw/com/impl/proxy_event.h @@ -40,7 +40,7 @@ namespace score::mw::com::impl // False Positive: this is a normal forward declaration. // Which is used to avoid cyclic dependency with proxy_field.h // coverity[autosar_cpp14_m3_2_3_violation] -template +template class ProxyField; /// \brief This is the user-visible class of an event that is part of a proxy. It contains ProxyEvent functionality that @@ -59,10 +59,10 @@ class ProxyEvent final : public ProxyEventBase // coverity[autosar_cpp14_a11_3_1_violation] friend class ProxyEventView; - // Design decission: ProxyField uses composition pattern to reuse code from ProxyEvent. These two classes also have - // shared private APIs which necessitates the use of the friend keyword. + // ProxyField uses composition pattern to reuse code from ProxyEvent and needs access to + // FieldOnlyConstructorEnabler. // coverity[autosar_cpp14_a11_3_1_violation] - template + template friend class ProxyField; // Empty struct that is used to make the second constructor only accessible to ProxyField (as it is a friend). @@ -80,8 +80,8 @@ class ProxyEvent final : public ProxyEventBase /// /// \param proxy_binding The binding that shall be associated with this proxy. ProxyEvent(ProxyBase& base, - std::unique_ptr> proxy_event_binding, - const std::string_view event_name); + const std::string_view event_name, + std::unique_ptr> proxy_event_binding); /// \brief Constructor that allows to set the binding directly. /// @@ -89,8 +89,8 @@ class ProxyEvent final : public ProxyEventBase /// /// \param proxy_binding The binding that shall be associated with this proxy. ProxyEvent(ProxyBase& base, - std::unique_ptr> proxy_binding, const std::string_view event_name, + std::unique_ptr> proxy_binding, FieldOnlyConstructorEnabler); /// \brief Constructs a ProxyEvent by querying the base proxie's ProxyBinding for the respective ProxyEventBinding. @@ -143,9 +143,9 @@ class ProxyEvent final : public ProxyEventBase template ProxyEvent::ProxyEvent(ProxyBase& base, - std::unique_ptr> proxy_event_binding, - const std::string_view event_name) - : ProxyEventBase{base, ProxyBaseView{base}.GetBinding(), std::move(proxy_event_binding), event_name}, + const std::string_view event_name, + std::unique_ptr> proxy_event_binding) + : ProxyEventBase{base, event_name, ProxyBaseView{base}.GetBinding(), std::move(proxy_event_binding)}, proxy_event_mock_{nullptr} { ProxyBaseView proxy_base_view{base}; @@ -158,7 +158,7 @@ ProxyEvent::ProxyEvent(ProxyBase& base, template ProxyEvent::ProxyEvent(ProxyBase& base, const std::string_view event_name) - : ProxyEvent{base, ProxyEventBindingFactory::Create(base, event_name), event_name} + : ProxyEvent{base, event_name, ProxyEventBindingFactory::Create(base, event_name)} { if (GetTypedEventBinding() != nullptr) { @@ -171,10 +171,10 @@ ProxyEvent::ProxyEvent(ProxyBase& base, const std::string_view event template ProxyEvent::ProxyEvent(ProxyBase& base, - std::unique_ptr> proxy_binding, const std::string_view event_name, + std::unique_ptr> proxy_binding, FieldOnlyConstructorEnabler) - : ProxyEvent{base, std::move(proxy_binding), event_name} + : ProxyEvent{base, event_name, std::move(proxy_binding)} { // This is the specific ctor that is used by ProxyField for its "dispatch event" composite. Therefore, we do not // register the event in the ProxyBase's event map, since registration in the correct field map is done by diff --git a/score/mw/com/impl/proxy_event_base.cpp b/score/mw/com/impl/proxy_event_base.cpp index 4e748c0d8..c8e7c3fd1 100644 --- a/score/mw/com/impl/proxy_event_base.cpp +++ b/score/mw/com/impl/proxy_event_base.cpp @@ -77,9 +77,9 @@ class EventBindingRegistrationGuard final // This is false positive. Function is declared only once. // coverity[autosar_cpp14_a3_1_1_violation] ProxyEventBase::ProxyEventBase(ProxyBase& proxy_base, + std::string_view event_name, ProxyBinding* proxy_binding_ptr, - std::unique_ptr proxy_event_binding, - std::string_view event_name) noexcept + std::unique_ptr proxy_event_binding) noexcept : binding_base_{std::move(proxy_event_binding)}, proxy_base_(proxy_base), event_name_{event_name}, diff --git a/score/mw/com/impl/proxy_event_base.h b/score/mw/com/impl/proxy_event_base.h index 1e8ccf594..1092d3247 100644 --- a/score/mw/com/impl/proxy_event_base.h +++ b/score/mw/com/impl/proxy_event_base.h @@ -59,9 +59,9 @@ class ProxyEventBase /// \param proxy_event_binding The binding that shall be associated with this proxy event. /// \param event_name Event name of the event. ProxyEventBase(ProxyBase& proxy_base, + std::string_view event_name, ProxyBinding* proxy_binding_ptr, - std::unique_ptr proxy_event_binding, - std::string_view event_name) noexcept; + std::unique_ptr proxy_event_binding) noexcept; /// \brief A ProxyEventBase shall not be copyable ProxyEventBase(const ProxyEventBase&) = delete; diff --git a/score/mw/com/impl/proxy_event_base_test.cpp b/score/mw/com/impl/proxy_event_base_test.cpp index 6ec683ec8..2c755e875 100644 --- a/score/mw/com/impl/proxy_event_base_test.cpp +++ b/score/mw/com/impl/proxy_event_base_test.cpp @@ -40,6 +40,21 @@ using namespace ::testing; using TestEventOrFieldType = std::uint16_t; +/// Constructs a service element (ProxyEvent / GenericProxyEvent / ProxyField) for testing, inserting the +/// detail::WithTestTag disambiguator when the target type is a ProxyField. +template +auto MakeServiceElementForTest(ProxyT& proxy, const char* name, Binding binding) +{ + if constexpr (detail::is_proxy_field_v) + { + return std::make_unique(proxy, name, detail::WithTestTag{}, std::move(binding)); + } + else + { + return std::make_unique(proxy, name, std::move(binding)); + } +} + const ServiceTypeDeployment kEmptyTypeDeployment{score::cpp::blank{}}; const ServiceIdentifierType kFooservice{make_ServiceIdentifierType("foo")}; const auto kInstanceSpecifier = InstanceSpecifier::Create(std::string{"/dummy_instance_specifier"}).value(); @@ -73,8 +88,8 @@ class ProxyEventBaseFixture : public ::testing::Test auto mock_service_element_binding_ptr = std::make_unique(); mock_service_element_binding_ = mock_service_element_binding_ptr.get(); - service_element_ = - std::make_unique(empty_proxy_, std::move(mock_service_element_binding_ptr), kEventName); + service_element_ = MakeServiceElementForTest( + empty_proxy_, kEventName, std::move(mock_service_element_binding_ptr)); ON_CALL(*mock_service_element_binding_, SetReceiveHandler(_)).WillByDefault(Return(score::Result{})); } @@ -99,7 +114,7 @@ struct GenericProxyEventStruct struct ProxyFieldStruct { - using ServiceElementType = ProxyField; + using ServiceElementType = ProxyField; using MockServiceElementType = mock_binding::ProxyEvent; }; @@ -164,8 +179,9 @@ TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithValidEventBi make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); // When creating a ProxyEvent with a valid binding - auto service_element = std::make_unique::ServiceElementType>( - dummy_proxy, std::move(valid_proxy_event_binding), kEventName); + auto service_element = + MakeServiceElementForTest::ServiceElementType>( + dummy_proxy, kEventName, std::move(valid_proxy_event_binding)); // Then the proxy should report that all bindings are valid EXPECT_TRUE(dummy_proxy.AreBindingsValid()); @@ -178,10 +194,11 @@ TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithInvalidEvent make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); // When creating a ProxyEvent with an invalid binding - auto service_element = std::make_unique::ServiceElementType>( - dummy_proxy, - std::unique_ptr::MockServiceElementType>(nullptr), - kEventName); + auto service_element = + MakeServiceElementForTest::ServiceElementType>( + dummy_proxy, + kEventName, + std::unique_ptr::MockServiceElementType>(nullptr)); // Then the proxy should report that all bindings are not valid EXPECT_FALSE(dummy_proxy.AreBindingsValid()); @@ -197,8 +214,9 @@ TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithInvalidProxy make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); // When creating a ProxyEvent with a valid binding - auto service_element = std::make_unique::ServiceElementType>( - dummy_proxy, std::move(valid_proxy_event_binding), kEventName); + auto service_element = + MakeServiceElementForTest::ServiceElementType>( + dummy_proxy, kEventName, std::move(valid_proxy_event_binding)); // Then the proxy should report that all bindings are not valid EXPECT_FALSE(dummy_proxy.AreBindingsValid()); @@ -223,8 +241,9 @@ TYPED_TEST(ProxyEventBaseCreationFixture, make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); // When creating a ProxyEvent with a valid binding - auto service_element = std::make_unique::ServiceElementType>( - dummy_proxy, std::move(valid_proxy_event_binding), kEventName); + auto service_element = + MakeServiceElementForTest::ServiceElementType>( + dummy_proxy, kEventName, std::move(valid_proxy_event_binding)); } TYPED_TEST(ProxyEventBaseCreationFixture, @@ -243,10 +262,10 @@ TYPED_TEST(ProxyEventBaseCreationFixture, make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); // When creating a ProxyEvent with an invalid binding - auto service_element = std::make_unique::ServiceElementType>( - dummy_proxy, - std::unique_ptr::MockServiceElementType>(nullptr), - kEventName); + using ServiceElementType = typename ProxyEventBaseCreationFixture::ServiceElementType; + using MockServiceElementType = typename ProxyEventBaseCreationFixture::MockServiceElementType; + auto service_element = MakeServiceElementForTest( + dummy_proxy, kEventName, std::unique_ptr(nullptr)); } TYPED_TEST(ProxyEventBaseUnsubscribeFixture, CallingUnsubscribeWhileSubscribedCallsUnsubscribeOnBinding) @@ -357,8 +376,8 @@ class AServiceElement : public ::testing::Test auto mock_service_element_binding_ptr = std::make_unique(); mock_service_element_binding_ = mock_service_element_binding_ptr.get(); - service_element_ = - std::make_unique(empty_proxy_, std::move(mock_service_element_binding_ptr), kEventName); + service_element_ = MakeServiceElementForTest( + empty_proxy_, kEventName, std::move(mock_service_element_binding_ptr)); ASSERT_NE(service_element_, nullptr); ON_CALL(*mock_service_element_binding_, SetReceiveHandler(_)).WillByDefault(Return(score::Result{})); @@ -1040,7 +1059,7 @@ TEST(ProxyEventBaseTest, MoveConstructingProxyEventDoesNotCrash) // Given a Service Element, that is connected to a mock binding ProxyEventBase dummy_event{ - empty_proxy, ProxyBaseView{empty_proxy}.GetBinding(), std::move(mock_proxy_event_binding_ptr), kEventName}; + empty_proxy, kEventName, ProxyBaseView{empty_proxy}.GetBinding(), std::move(mock_proxy_event_binding_ptr)}; // And a new Service Element is created with the move constructor ProxyEventBase dummy_event_2{std::move(dummy_event)}; @@ -1066,13 +1085,13 @@ TEST(ProxyEventBaseTest, MoveAssigningProxyEventDoesNotCrash) // Given a Service Element, that is connected to a mock binding ProxyEventBase dummy_event{ - empty_proxy, ProxyBaseView{empty_proxy}.GetBinding(), std::move(mock_proxy_event_binding_ptr), kEventName}; + empty_proxy, kEventName, ProxyBaseView{empty_proxy}.GetBinding(), std::move(mock_proxy_event_binding_ptr)}; // And a new Service Element is created and move assigned ProxyEventBase dummy_event_2{empty_proxy, + kEventName2, ProxyBaseView{empty_proxy}.GetBinding(), - std::make_unique>(), - kEventName2}; + std::make_unique>()}; dummy_event_2 = std::move(dummy_event); // Expect that Subscribe is called on the binding only once diff --git a/score/mw/com/impl/proxy_event_test.cpp b/score/mw/com/impl/proxy_event_test.cpp index d6b24f899..f8eaa5dfe 100644 --- a/score/mw/com/impl/proxy_event_test.cpp +++ b/score/mw/com/impl/proxy_event_test.cpp @@ -83,7 +83,7 @@ struct GenericProxyEventStruct struct ProxyFieldStruct { using SampleType = TestSampleType; - using ProxyEventType = ProxyField; + using ProxyEventType = ProxyField; using MockProxyEventType = StrictMock>; }; @@ -106,7 +106,17 @@ class ProxyEventFixture : public ::testing::Test make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))}, mock_proxy_event_ptr_{std::make_unique()}, mock_proxy_event_{*mock_proxy_event_ptr_}, - proxy_event_{empty_proxy_, std::move(mock_proxy_event_ptr_), kEventName} + proxy_event_{[this] { + if constexpr (detail::is_proxy_field_v) + { + return ProxyEventType{ + empty_proxy_, kEventName, detail::WithTestTag{}, std::move(mock_proxy_event_ptr_)}; + } + else + { + return ProxyEventType{empty_proxy_, kEventName, std::move(mock_proxy_event_ptr_)}; + } + }()} { } @@ -303,7 +313,7 @@ TYPED_TEST(ProxyEventFixture, CanConstructUnboundProxy) // When creating an unbound proxy event (note we need a different event name here as the fixture already uses the // first) - ProxyEvent proxy_event{this->empty_proxy_, nullptr, kEventName2}; + ProxyEvent proxy_event{this->empty_proxy_, kEventName2, nullptr}; // Nothing bad happens } @@ -448,7 +458,7 @@ TEST(ProxyEventTest, SamplePtrsToSlotDataAreConst) ProxyBase empty_proxy(std::make_unique(), make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); ProxyEvent proxy{ - empty_proxy, std::unique_ptr>{std::move(mock_proxy_ptr)}, kEventName}; + empty_proxy, kEventName, std::unique_ptr>{std::move(mock_proxy_ptr)}}; EXPECT_CALL(mock_proxy, Subscribe(max_num_samples)); EXPECT_CALL(mock_proxy, GetNewSamples(_, _)); @@ -481,7 +491,7 @@ TEST(ProxyEventDeathTest, DieOnProxyDestructionWhileHoldingSamplePtrs) ProxyBase empty_proxy(std::make_unique(), make_HandleType(make_InstanceIdentifier(kEmptyInstanceDeployment, kEmptyTypeDeployment))); auto proxy = std::make_unique>( - empty_proxy, std::unique_ptr>{std::move(mock_proxy_ptr)}, kEventName); + empty_proxy, kEventName, std::unique_ptr>{std::move(mock_proxy_ptr)}); EXPECT_CALL(mock_proxy, Subscribe(max_num_samples)); EXPECT_CALL(mock_proxy, GetNewSamples(_, _)); diff --git a/score/mw/com/impl/proxy_field.h b/score/mw/com/impl/proxy_field.h index 6019f55b8..de8fcd0ea 100644 --- a/score/mw/com/impl/proxy_field.h +++ b/score/mw/com/impl/proxy_field.h @@ -13,6 +13,7 @@ #ifndef SCORE_MW_COM_IMPL_PROXY_FIELD_H #define SCORE_MW_COM_IMPL_PROXY_FIELD_H +#include "score/mw/com/impl/field_tags.h" #include "score/mw/com/impl/methods/proxy_method_with_in_args_and_return.h" #include "score/mw/com/impl/methods/proxy_method_with_return_type.h" #include "score/mw/com/impl/plumbing/proxy_field_binding_factory.h" @@ -39,187 +40,165 @@ namespace score::mw::com::impl // Suppress "AUTOSAR C++14 M3-2-3" rule finding. This rule states: "An identifier with external linkage shall have // exactly one definition". This a forward class declaration. // coverity[autosar_cpp14_m3_2_3_violation] -template +template class ProxyFieldAttorney; +template +class ProxyField; + +namespace detail +{ + +template +struct is_proxy_field : std::false_type +{ +}; +template +struct is_proxy_field> : std::true_type +{ +}; +template +constexpr bool is_proxy_field_v = is_proxy_field::value; + +} // namespace detail + /// \brief This is the user-visible class of a field that is part of a proxy. It delegates all functionality to /// ProxyEvent. /// /// \tparam SampleDataType Type of data that is transferred by the event. -/// \tparam EnableGet Whether the get method shall be enabled for this field. If true, a Get() method will be available. -/// \tparam EnableSet Whether the set method shall be enabled for this field. If true, a Set() method will be available. -/// \tparam EnableNotifier Whether the notifier functionality shall be enabled for this field. Whether this has an -/// effect, depends on the binding that is used. The LoLa/shm-binding ignores this template parameter. -template +/// \tparam Tags Optional tag pack; presence of WithGetter / WithSetter / WithNotifier enables Get() / Set() / +/// Subscribe() and other event related methods respectively. +template class ProxyField final : public ProxyFieldBase { + + static_assert( + detail::contains_type::value || detail::contains_type::value, + "A field must either have WithGetter or WithNotifier enabled otherwise, there is no way for the consumer to " + "receive the field values."); + // Suppress "AUTOSAR C++14 A11-3-1", The rule declares: "Friend declarations shall not be used". // Design decision: The "*Attorney" class is a helper, which sets the internal state of this class accessing // private members and used for testing purposes only. // coverity[autosar_cpp14_a11_3_1_violation] - friend class ProxyFieldAttorney; + friend class ProxyFieldAttorney; public: using FieldType = SampleDataType; - /// Constructor that allows to set the binding directly (both EnableGet and EnableSet are true). - /// - /// This is used for testing only. Allows for directly setting the bindings, and usually the mock binding is used - /// here. - template > + /// Testing ctor: bindings are passed in directly (used with mock bindings). Disambiguated from the production + /// ctors via the detail::WithTestTag marker, required as the 3rd argument. Bindings default to nullptr; ProxyField(ProxyBase& proxy_base, - std::unique_ptr> event_binding, - std::unique_ptr get_method_binding, - std::unique_ptr set_method_binding, const std::string_view field_name, - detail::EnableBothTag = {}) + detail::WithTestTag, + std::unique_ptr> event_binding = nullptr, + std::unique_ptr get_method_binding = nullptr, + std::unique_ptr set_method_binding = nullptr) : ProxyField{proxy_base, - std::make_unique>(proxy_base, std::move(event_binding), field_name), - std::make_unique>( - proxy_base, - std::move(get_method_binding), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - std::make_unique>( - proxy_base, - std::move(set_method_binding), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - field_name} + field_name, + std::make_unique>(proxy_base, field_name, std::move(event_binding)), + get_method_binding == nullptr + ? nullptr + : std::make_unique>( + proxy_base, + field_name, + std::move(get_method_binding), + typename ProxyMethod::FieldOnlyConstructorEnabler{}), + set_method_binding == nullptr + ? nullptr + : std::make_unique>( + proxy_base, + field_name, + std::move(set_method_binding), + typename ProxyMethod::FieldOnlyConstructorEnabler{})} { } - /// Constructor that allows to set the binding directly (only EnableGet is true). - /// - /// This is used for testing only. Allows for directly setting the bindings, and usually the mock binding is used - /// here. - template > - ProxyField(ProxyBase& proxy_base, - std::unique_ptr> event_binding, - std::unique_ptr get_method_binding, - const std::string_view field_name, - detail::EnableGetOnlyTag = {}) - : ProxyField{proxy_base, - std::make_unique>(proxy_base, std::move(event_binding), field_name), - std::make_unique>( - proxy_base, - std::move(get_method_binding), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - field_name} - { - } - - /// Constructor that allows to set the binding directly (only EnableSet is true). - /// - /// This is used for testing only. Allows for directly setting the bindings, and usually the mock binding is used - /// here. - template > - ProxyField(ProxyBase& proxy_base, - std::unique_ptr> event_binding, - std::unique_ptr set_method_binding, - const std::string_view field_name, - detail::EnableSetOnlyTag = {}) + /// \brief Production ctor for tag pack . + template ::value && + !detail::is_tag_enabled::value && + !detail::is_tag_enabled::value>> + ProxyField(ProxyBase& proxy_base, const std::string_view field_name, WithGetter = {}) : ProxyField{proxy_base, - std::make_unique>(proxy_base, std::move(event_binding), field_name), - std::make_unique>( - proxy_base, - std::move(set_method_binding), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - field_name} + field_name, + MakeEventDispatchFromFactory(proxy_base, field_name), + MakeGetMethodDispatchFromFactory(proxy_base, field_name), + MakeSetMethodDispatchFromFactory(proxy_base, field_name)} { } - /// Constructor that allows to set the binding directly (both EnableGet and EnableSet are false). - /// - /// This is used for testing only. Allows for directly setting the bindings, and usually the mock binding is used - /// here. - template > - ProxyField(ProxyBase& proxy_base, - std::unique_ptr> event_binding, - const std::string_view field_name, - detail::EnableNeitherTag = {}) + /// \brief Production ctor for tag pack . + template ::value && + detail::is_tag_enabled::value && + !detail::is_tag_enabled::value>> + ProxyField(ProxyBase& proxy_base, const std::string_view field_name, WithGetter = {}, WithSetter = {}) : ProxyField{proxy_base, - std::make_unique>(proxy_base, std::move(event_binding), field_name), - field_name} + field_name, + MakeEventDispatchFromFactory(proxy_base, field_name), + MakeGetMethodDispatchFromFactory(proxy_base, field_name), + MakeSetMethodDispatchFromFactory(proxy_base, field_name)} { } - /// \brief Constructs a ProxyField (both EnableGet and EnableSet are true). Normal ctor that is used in production - /// code. - template > - ProxyField(ProxyBase& proxy_base, const std::string_view field_name, detail::EnableBothTag = {}) + /// \brief Production ctor for tag pack . + template ::value && + !detail::is_tag_enabled::value && + detail::is_tag_enabled::value>> + ProxyField(ProxyBase& proxy_base, const std::string_view field_name, WithGetter = {}, WithNotifier = {}) : ProxyField{proxy_base, - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateEventBinding(proxy_base, field_name), - field_name, - typename ProxyEvent::FieldOnlyConstructorEnabler{}), - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateGetMethodBinding(proxy_base, field_name), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateSetMethodBinding(proxy_base, field_name), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - field_name} + field_name, + MakeEventDispatchFromFactory(proxy_base, field_name), + MakeGetMethodDispatchFromFactory(proxy_base, field_name), + MakeSetMethodDispatchFromFactory(proxy_base, field_name)} { } - /// \brief Constructs a ProxyField (only EnableGet is true). Normal ctor that is used in production code. - template > - ProxyField(ProxyBase& proxy_base, const std::string_view field_name, detail::EnableGetOnlyTag = {}) + /// \brief Production ctor for tag pack . + template ::value && + detail::is_tag_enabled::value && + detail::is_tag_enabled::value>> + ProxyField(ProxyBase& proxy_base, + const std::string_view field_name, + WithGetter = {}, + WithSetter = {}, + WithNotifier = {}) : ProxyField{proxy_base, - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateEventBinding(proxy_base, field_name), - field_name, - typename ProxyEvent::FieldOnlyConstructorEnabler{}), - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateGetMethodBinding(proxy_base, field_name), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - field_name} + field_name, + MakeEventDispatchFromFactory(proxy_base, field_name), + MakeGetMethodDispatchFromFactory(proxy_base, field_name), + MakeSetMethodDispatchFromFactory(proxy_base, field_name)} { } - /// \brief Constructs a ProxyField (only EnableSet is true). Normal ctor that is used in production code. - template > - ProxyField(ProxyBase& proxy_base, const std::string_view field_name, detail::EnableSetOnlyTag = {}) + /// \brief Production ctor for tag pack . + template ::value && + detail::is_tag_enabled::value && + detail::is_tag_enabled::value>> + ProxyField(ProxyBase& proxy_base, const std::string_view field_name, WithSetter = {}, WithNotifier = {}) : ProxyField{proxy_base, - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateEventBinding(proxy_base, field_name), - field_name, - typename ProxyEvent::FieldOnlyConstructorEnabler{}), - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateSetMethodBinding(proxy_base, field_name), - field_name, - typename ProxyMethod::FieldOnlyConstructorEnabler{}), - field_name} + field_name, + MakeEventDispatchFromFactory(proxy_base, field_name), + MakeGetMethodDispatchFromFactory(proxy_base, field_name), + MakeSetMethodDispatchFromFactory(proxy_base, field_name)} { } - /// \brief Constructs a ProxyField (both EnableGet and EnableSet are false). Normal ctor that is used in production - /// code. - template > - ProxyField(ProxyBase& proxy_base, const std::string_view field_name, detail::EnableNeitherTag = {}) + /// \brief Production ctor for tag pack . + template ::value && + !detail::is_tag_enabled::value && + detail::is_tag_enabled::value>> + ProxyField(ProxyBase& proxy_base, const std::string_view field_name, WithNotifier = {}) : ProxyField{proxy_base, - std::make_unique>( - proxy_base, - ProxyFieldBindingFactory::CreateEventBinding(proxy_base, field_name), - field_name, - typename ProxyEvent::FieldOnlyConstructorEnabler{}), - field_name} + field_name, + MakeEventDispatchFromFactory(proxy_base, field_name), + MakeGetMethodDispatchFromFactory(proxy_base, field_name), + MakeSetMethodDispatchFromFactory(proxy_base, field_name)} { } @@ -246,91 +225,161 @@ class ProxyField final : public ProxyFieldBase * \param max_num_samples Maximum number of samples to return via the given callable. * \return Number of samples that were handed over to the callable or an error. */ - template + template ::value>> Result GetNewSamples(F&& receiver, const std::size_t max_num_samples) noexcept { return proxy_event_dispatch_->GetNewSamples(std::forward(receiver), max_num_samples); } + template ::value>> + Result Subscribe(const std::size_t max_sample_count) noexcept + { + return ProxyFieldBase::Subscribe(max_sample_count); + } + + template ::value>> + void Unsubscribe() noexcept + { + ProxyFieldBase::Unsubscribe(); + } + + template ::value>> + SubscriptionState GetSubscriptionState() noexcept + { + return ProxyFieldBase::GetSubscriptionState(); + } + + template ::value>> + std::size_t GetFreeSampleCount() noexcept + { + return ProxyFieldBase::GetFreeSampleCount(); + } + + template ::value>> + Result GetNumNewSamplesAvailable() noexcept + { + return ProxyFieldBase::GetNumNewSamplesAvailable(); + } + + template ::value>> + Result SetReceiveHandler(EventReceiveHandler handler) noexcept + { + return ProxyFieldBase::SetReceiveHandler(std::move(handler)); + } + + template ::value>> + Result UnsetReceiveHandler() noexcept + { + return ProxyFieldBase::UnsetReceiveHandler(); + } + template ::value>> + typename = std::enable_if_t::value>> score::Result> Get() noexcept { return proxy_method_get_dispatch_->operator()(); } template ::value>> + typename = std::enable_if_t::value>> score::Result> Set(SampleDataType& new_field_value) noexcept { return proxy_method_set_dispatch_->operator()(new_field_value); } + template ::value>> void InjectMock(IProxyEvent& proxy_event_mock) { proxy_event_dispatch_->InjectMock(proxy_event_mock); } private: - /// \brief Private constructor overload for when both EnableGet and EnableSet are true. - template > - ProxyField(ProxyBase& proxy_base, - std::unique_ptr> proxy_event_dispatch, - std::unique_ptr> proxy_method_get_dispatch, - std::unique_ptr> proxy_method_set_dispatch, - const std::string_view field_name) - : ProxyFieldBase{proxy_base, proxy_event_dispatch.get(), field_name}, - proxy_event_dispatch_{std::move(proxy_event_dispatch)}, - proxy_method_get_dispatch_{std::move(proxy_method_get_dispatch)}, - proxy_method_set_dispatch_{std::move(proxy_method_set_dispatch)} + static constexpr bool kHasGetter = detail::contains_type::value; + static constexpr bool kHasSetter = detail::contains_type::value; + static constexpr bool kHasNotifier = detail::contains_type::value; + + /// Builds the proxy event dispatch via the binding factory when WithNotifier is enabled. Returns nullptr + /// without invoking the factory when WithNotifier is disabled. + static std::unique_ptr> MakeEventDispatchFromFactory(ProxyBase& proxy_base, + const std::string_view field_name) { - SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(proxy_event_dispatch_ != nullptr); - - ProxyBaseView proxy_base_view{proxy_base}; - proxy_base_view.RegisterField(field_name, *this); + if constexpr (kHasNotifier) + { + return std::make_unique>( + proxy_base, + field_name, + ProxyFieldBindingFactory::CreateEventBinding(proxy_base, field_name), + typename ProxyEvent::FieldOnlyConstructorEnabler{}); + } + else + { + return nullptr; + } } - /// \brief Private constructor overload for when only EnableGet is true. - template > - ProxyField(ProxyBase& proxy_base, - std::unique_ptr> proxy_event_dispatch, - std::unique_ptr> proxy_method_get_dispatch, - const std::string_view field_name) - : ProxyFieldBase{proxy_base, proxy_event_dispatch.get(), field_name}, - proxy_event_dispatch_{std::move(proxy_event_dispatch)}, - proxy_method_get_dispatch_{std::move(proxy_method_get_dispatch)} + /// Builds the Get-method dispatch via the binding factory when WithGetter is enabled. Returns nullptr + /// without invoking the factory when WithGetter is disabled. + static std::unique_ptr> MakeGetMethodDispatchFromFactory(ProxyBase& proxy_base, + const std::string_view field_name) { - SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(proxy_event_dispatch_ != nullptr); - - ProxyBaseView proxy_base_view{proxy_base}; - proxy_base_view.RegisterField(field_name, *this); + if constexpr (kHasGetter) + { + return std::make_unique>( + proxy_base, + field_name, + ProxyFieldBindingFactory::CreateGetMethodBinding(proxy_base, field_name), + typename ProxyMethod::FieldOnlyConstructorEnabler{}); + } + else + { + return nullptr; + } } - /// \brief Private constructor overload for when only EnableSet is true. - template > - ProxyField(ProxyBase& proxy_base, - std::unique_ptr> proxy_event_dispatch, - std::unique_ptr> proxy_method_set_dispatch, - const std::string_view field_name) - : ProxyFieldBase{proxy_base, proxy_event_dispatch.get(), field_name}, - proxy_event_dispatch_{std::move(proxy_event_dispatch)}, - proxy_method_set_dispatch_{std::move(proxy_method_set_dispatch)} + /// Builds the Set-method dispatch via the binding factory when WithSetter is enabled. Returns nullptr + /// without invoking the factory when WithSetter is disabled. + static std::unique_ptr> MakeSetMethodDispatchFromFactory( + ProxyBase& proxy_base, + const std::string_view field_name) { - SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(proxy_event_dispatch_ != nullptr); - - ProxyBaseView proxy_base_view{proxy_base}; - proxy_base_view.RegisterField(field_name, *this); + if constexpr (kHasSetter) + { + return std::make_unique>( + proxy_base, + field_name, + ProxyFieldBindingFactory::CreateSetMethodBinding(proxy_base, field_name), + typename ProxyMethod::FieldOnlyConstructorEnabler{}); + } + else + { + return nullptr; + } } - /// \brief Private constructor overload for when both EnableGet and EnableSet are false. - template > ProxyField(ProxyBase& proxy_base, + const std::string_view field_name, std::unique_ptr> proxy_event_dispatch, - const std::string_view field_name) - : ProxyFieldBase{proxy_base, proxy_event_dispatch.get(), field_name}, - proxy_event_dispatch_{std::move(proxy_event_dispatch)} + std::unique_ptr> proxy_method_get_dispatch, + std::unique_ptr> proxy_method_set_dispatch) + : ProxyFieldBase{proxy_base, field_name, proxy_event_dispatch.get()}, + proxy_event_dispatch_{std::move(proxy_event_dispatch)}, + proxy_method_get_dispatch_{std::move(proxy_method_get_dispatch)}, + proxy_method_set_dispatch_{std::move(proxy_method_set_dispatch)} { - SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(proxy_event_dispatch_ != nullptr); + if constexpr (kHasNotifier) + { + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(proxy_event_dispatch_ != nullptr); + } ProxyBaseView proxy_base_view{proxy_base}; proxy_base_view.RegisterField(field_name, *this); @@ -338,23 +387,16 @@ class ProxyField final : public ProxyFieldBase std::unique_ptr> proxy_event_dispatch_; - struct empty - { - }; - - using ProxyGetMethodType = std::conditional_t>, empty>; - using ProxySetMethodType = std::conditional_t>, empty>; - - ProxyGetMethodType proxy_method_get_dispatch_; - ProxySetMethodType proxy_method_set_dispatch_; + std::unique_ptr> proxy_method_get_dispatch_; + std::unique_ptr> proxy_method_set_dispatch_; static_assert(std::is_same>>::value, "proxy_event_dispatch_ needs to be a unique_ptr since we pass a pointer to it to ProxyFieldBase, so " "we must ensure that it doesn't move when the ProxyField is moved to avoid dangling references. "); }; -template -ProxyField::ProxyField(ProxyField&& other) noexcept +template +ProxyField::ProxyField(ProxyField&& other) noexcept : ProxyFieldBase(std::move(static_cast(other))), proxy_event_dispatch_(std::move(other.proxy_event_dispatch_)), proxy_method_get_dispatch_(std::move(other.proxy_method_get_dispatch_)), @@ -364,14 +406,14 @@ ProxyField::ProxyField(ProxyFie proxy_base_view.UpdateField(field_name_, *this); } -template +template // Suppress "AUTOSAR C++14 A6-2-1" rule violation. The rule states "Move and copy assignment operators shall either move // or respectively copy base classes and data members of a class, without any side effects." // Rationale: The parent proxy stores a reference to the ProxyEvent. The address that is pointed to must be // updated when the ProxyField is moved. Therefore, side effects are required. // coverity[autosar_cpp14_a6_2_1_violation] -auto ProxyField::operator=(ProxyField&& other) & noexcept - -> ProxyField& +auto ProxyField::operator=(ProxyField&& other) & noexcept + -> ProxyField& { if (this != &other) { diff --git a/score/mw/com/impl/proxy_field_base.h b/score/mw/com/impl/proxy_field_base.h index dc49bcaee..3ab47ccd0 100644 --- a/score/mw/com/impl/proxy_field_base.h +++ b/score/mw/com/impl/proxy_field_base.h @@ -30,10 +30,10 @@ namespace score::mw::com::impl class ProxyFieldBase { public: - ProxyFieldBase(ProxyBase& proxy_base, ProxyEventBase* proxy_event_base_dispatch, std::string_view field_name) + /// \param proxy_event_base_dispatch May be nullptr when the field has no notifier tag. + ProxyFieldBase(ProxyBase& proxy_base, std::string_view field_name, ProxyEventBase* proxy_event_base_dispatch) : proxy_base_{proxy_base}, proxy_event_base_dispatch_{proxy_event_base_dispatch}, field_name_{field_name} { - SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(proxy_event_base_dispatch != nullptr); } /// \brief A ProxyFieldBase shall not be copyable diff --git a/score/mw/com/impl/proxy_field_test.cpp b/score/mw/com/impl/proxy_field_test.cpp index e637d19fd..794316189 100644 --- a/score/mw/com/impl/proxy_field_test.cpp +++ b/score/mw/com/impl/proxy_field_test.cpp @@ -19,10 +19,20 @@ #include "score/mw/com/impl/proxy_field.h" +#include "score/mw/com/impl/bindings/mock_binding/proxy.h" +#include "score/mw/com/impl/bindings/mock_binding/proxy_event.h" +#include "score/mw/com/impl/bindings/mock_binding/proxy_method.h" +#include "score/mw/com/impl/com_error.h" +#include "score/mw/com/impl/configuration/lola_service_instance_deployment.h" +#include "score/mw/com/impl/configuration/lola_service_type_deployment.h" +#include "score/mw/com/impl/configuration/quality_type.h" +#include "score/mw/com/impl/configuration/service_identifier_type.h" +#include "score/mw/com/impl/configuration/test/configuration_store.h" #include "score/mw/com/impl/runtime.h" #include "score/mw/com/impl/runtime_mock.h" #include "score/mw/com/impl/test/binding_factory_resources.h" #include "score/mw/com/impl/test/proxy_resources.h" +#include "score/mw/com/impl/test/runtime_mock_guard.h" #include #include @@ -37,6 +47,125 @@ using namespace ::testing; using TestSampleType = std::uint8_t; +class TestProxyBase : public ProxyBase +{ + public: + using ProxyBase::ProxyBase; + const ProxyBase::ProxyMethods& GetMethods() + { + return methods_; + } + const ProxyBase::ProxyFields& GetFields() + { + return fields_; + } +}; + +namespace +{ +// SFINAE detection traits used to verify that the notifier surface is gated on the WithNotifier tag. +// Each trait checks whether the corresponding member call is well-formed on the field type. + +template +struct has_subscribe : std::false_type +{ +}; +template +struct has_subscribe().Subscribe(std::declval()))>> + : std::true_type +{ +}; + +template +struct has_unsubscribe : std::false_type +{ +}; +template +struct has_unsubscribe().Unsubscribe())>> : std::true_type +{ +}; + +template +struct has_get_subscription_state : std::false_type +{ +}; +template +struct has_get_subscription_state().GetSubscriptionState())>> : std::true_type +{ +}; + +template +struct has_get_free_sample_count : std::false_type +{ +}; +template +struct has_get_free_sample_count().GetFreeSampleCount())>> : std::true_type +{ +}; + +template +struct has_get_num_new_samples_available : std::false_type +{ +}; +template +struct has_get_num_new_samples_available().GetNumNewSamplesAvailable())>> + : std::true_type +{ +}; + +template +struct has_set_receive_handler : std::false_type +{ +}; +template +struct has_set_receive_handler< + T, + std::void_t().SetReceiveHandler(std::declval()))>> : std::true_type +{ +}; + +template +struct has_unset_receive_handler : std::false_type +{ +}; +template +struct has_unset_receive_handler().UnsetReceiveHandler())>> : std::true_type +{ +}; + +struct DummySampleReceiver +{ + template + void operator()(SamplePtrT) noexcept + { + } +}; + +template +struct has_get_new_samples : std::false_type +{ +}; +template +struct has_get_new_samples().GetNewSamples(std::declval(), + std::declval()))>> + : std::true_type +{ +}; + +template +struct has_inject_mock : std::false_type +{ +}; +template +struct has_inject_mock< + T, + std::void_t().InjectMock(std::declval&>()))>> + : std::true_type +{ +}; +} // namespace + TEST(ProxyFieldTest, NotCopyable) { RecordProperty("Verifies", "SCR-17397027"); @@ -45,14 +174,18 @@ TEST(ProxyFieldTest, NotCopyable) RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - static_assert(!std::is_copy_constructible>::value, "Is wrongly copyable"); - static_assert(!std::is_copy_assignable>::value, "Is wrongly copyable"); + static_assert(!std::is_copy_constructible>::value, + "Is wrongly copyable"); + static_assert(!std::is_copy_assignable>::value, + "Is wrongly copyable"); } TEST(ProxyFieldTest, IsMoveable) { - static_assert(std::is_move_constructible>::value, "Is not move constructible"); - static_assert(std::is_move_assignable>::value, "Is not move assignable"); + static_assert(std::is_move_constructible>::value, + "Is not move constructible"); + static_assert(std::is_move_assignable>::value, + "Is not move assignable"); } TEST(ProxyFieldTest, ClassTypeDependsOnFieldDataType) @@ -63,12 +196,20 @@ TEST(ProxyFieldTest, ClassTypeDependsOnFieldDataType) RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - using FirstProxyFieldType = ProxyField; - using SecondProxyFieldType = ProxyField; + using FirstProxyFieldType = ProxyField; + using SecondProxyFieldType = ProxyField; static_assert(!std::is_same_v, "Class type does not depend on field data type"); } +TEST(ProxyFieldTest, ClassTypeDependsOnFieldTagType) +{ + using FirstProxyFieldType = ProxyField; + using SecondProxyFieldType = ProxyField; + static_assert(!std::is_same_v, + "Class type does not depend on field tag type"); +} + TEST(ProxyFieldTest, ProxyFieldContainsPublicFieldType) { RecordProperty("Verifies", "SCR-17291997"); @@ -79,8 +220,311 @@ TEST(ProxyFieldTest, ProxyFieldContainsPublicFieldType) RecordProperty("DerivationTechnique", "Analysis of requirements"); using CustomFieldType = std::uint16_t; - static_assert(std::is_same::FieldType, CustomFieldType>::value, - "Incorrect FieldType."); + static_assert( + std::is_same::FieldType, CustomFieldType>::value, + "Incorrect FieldType."); +} + +TEST(ProxyFieldNotifierGatingTest, NotifierSurfaceExistsWhenWithNotifierTagIsPresent) +{ + + using NotifierField = ProxyField; + static_assert(has_subscribe::value); + static_assert(has_unsubscribe::value); + static_assert(has_get_subscription_state::value); + static_assert(has_get_free_sample_count::value); + static_assert(has_get_num_new_samples_available::value); + static_assert(has_set_receive_handler::value); + static_assert(has_unset_receive_handler::value); + static_assert(has_get_new_samples::value); + static_assert(has_inject_mock::value); +} + +TEST(ProxyFieldNotifierGatingTest, NotifierSurfaceIsAbsentWhenWithNotifierTagIsMissing) +{ + + using GetterOnlyField = ProxyField; + static_assert(!has_subscribe::value); + static_assert(!has_unsubscribe::value); + static_assert(!has_get_subscription_state::value); + static_assert(!has_get_free_sample_count::value); + static_assert(!has_get_num_new_samples_available::value); + static_assert(!has_set_receive_handler::value); + static_assert(!has_unset_receive_handler::value); + static_assert(!has_get_new_samples::value); + static_assert(!has_inject_mock::value); +} + +/// \brief Test fixture for ProxyField with Get and/or Set enabled. +class ProxyFieldGetSetFixture : public ::testing::Test +{ + public: + void SetUp() override + { + ON_CALL(runtime_mock_guard_.runtime_mock_, GetTracingFilterConfig()).WillByDefault(Return(nullptr)); + + ON_CALL(proxy_method_binding_mock_, GetReturnValueBuffer(0)) + .WillByDefault(Return(score::Result>{ + score::cpp::span{return_type_buffer_.data(), return_type_buffer_.size()}})); + ON_CALL(proxy_method_binding_mock_, GetInArgsBuffer(0)) + .WillByDefault(Return(score::Result>{ + score::cpp::span{in_args_buffer_.data(), in_args_buffer_.size()}})); + ON_CALL(proxy_method_binding_mock_, DoCall(0)).WillByDefault(Return(score::ResultBlank{})); + + ON_CALL(set_method_binding_mock_, GetReturnValueBuffer(0)) + .WillByDefault(Return(score::Result>{ + score::cpp::span{return_type_buffer_.data(), return_type_buffer_.size()}})); + ON_CALL(set_method_binding_mock_, GetInArgsBuffer(0)) + .WillByDefault(Return(score::Result>{ + score::cpp::span{in_args_buffer_.data(), in_args_buffer_.size()}})); + ON_CALL(set_method_binding_mock_, DoCall(0)).WillByDefault(Return(score::ResultBlank{})); + } + + ProxyField MakeFieldWithGetOnly(const std::string_view name = "TestField") + { + return ProxyField{ + proxy_base_, + name, + detail::WithTestTag{}, + std::make_unique>(proxy_event_mock_), + std::make_unique(proxy_method_binding_mock_)}; + } + + ProxyField MakeFieldWithSetAndNotifier( + const std::string_view name = "TestField") + { + return ProxyField{ + proxy_base_, + name, + detail::WithTestTag{}, + std::make_unique>(proxy_event_mock_), + nullptr, + std::make_unique(set_method_binding_mock_)}; + } + + ProxyField MakeFieldWithGetAndSet(const std::string_view name = "TestField") + { + return ProxyField{ + proxy_base_, + name, + detail::WithTestTag{}, + std::make_unique>(proxy_event_mock_), + std::make_unique(proxy_method_binding_mock_), + std::make_unique(set_method_binding_mock_)}; + } + + ProxyField MakeFieldWithNotifierOnly(const std::string_view name = "TestField") + { + return ProxyField{ + proxy_base_, + name, + detail::WithTestTag{}, + std::make_unique>(proxy_event_mock_)}; + } + + alignas(8) std::array return_type_buffer_{}; + alignas(8) std::array in_args_buffer_{}; + RuntimeMockGuard runtime_mock_guard_{}; + ConfigurationStore config_store_{InstanceSpecifier::Create(std::string{"/my_dummy_instance_specifier"}).value(), + make_ServiceIdentifierType("foo"), + QualityType::kASIL_QM, + LolaServiceTypeDeployment{42U}, + LolaServiceInstanceDeployment{1U}}; + TestProxyBase proxy_base_{std::make_unique(), config_store_.GetHandle()}; + mock_binding::ProxyEvent proxy_event_mock_{}; + mock_binding::ProxyMethod proxy_method_binding_mock_{}; + mock_binding::ProxyMethod set_method_binding_mock_{}; +}; + +TEST_F(ProxyFieldGetSetFixture, GetDelegatesToProxyMethodBinding) +{ + // Given a ProxyField with EnableGet=true and a mock method binding + auto field = MakeFieldWithGetOnly(); + + // When calling Get() + EXPECT_CALL(proxy_method_binding_mock_, GetReturnValueBuffer(0)); + EXPECT_CALL(proxy_method_binding_mock_, DoCall(0)); + auto result = field.Get(); + + // Then the call is delegated to the underlying method binding + EXPECT_TRUE(result.has_value()); +} + +TEST_F(ProxyFieldGetSetFixture, SetDelegatesToProxyMethodBindingAndCopiesValueIntoInArgsBuffer) +{ + // Given a Set-only ProxyField wired to a mock method binding + auto field = MakeFieldWithSetAndNotifier(); + TestSampleType value{42U}; + EXPECT_CALL(set_method_binding_mock_, GetInArgsBuffer(0)); + EXPECT_CALL(set_method_binding_mock_, GetReturnValueBuffer(0)); + EXPECT_CALL(set_method_binding_mock_, DoCall(0)); + + // When Set is called + auto result = field.Set(value); + + // Then the call delegates to the binding and the value lands in the in-args buffer + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(static_cast(in_args_buffer_[0]), value); +} + +TEST_F(ProxyFieldGetSetFixture, GetPropagatesBindingError) +{ + // Given a ProxyField with EnableGet=true where the binding returns an error + auto field = MakeFieldWithGetOnly(); + + // When calling Get() and the binding reports an error + EXPECT_CALL(proxy_method_binding_mock_, GetReturnValueBuffer(0)) + .WillOnce(Return(MakeUnexpected(ComErrc::kBindingFailure))); + auto result = field.Get(); + + // Then the error is propagated back to the caller + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), ComErrc::kBindingFailure); +} + +TEST_F(ProxyFieldGetSetFixture, SetPropagatesBindingError) +{ + // Given a ProxyField with EnableSet=true where the binding returns an error + auto field = MakeFieldWithSetAndNotifier(); + + // When calling Set() and the binding reports an error + TestSampleType value{42U}; + EXPECT_CALL(set_method_binding_mock_, GetInArgsBuffer(0)) + .WillOnce(Return(MakeUnexpected(ComErrc::kBindingFailure))); + auto result = field.Set(value); + + // Then the error is propagated back to the caller + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), ComErrc::kBindingFailure); +} + +TEST_F(ProxyFieldGetSetFixture, GetMethodIsNotRegisteredAsRegularMethod) +{ + // When constructing a Get-only ProxyField + auto field = MakeFieldWithGetOnly(); + + // Then the field shows up on the parent proxy, but the Get method does not leak into the method map + EXPECT_EQ(proxy_base_.GetFields().size(), 1U); + EXPECT_TRUE(proxy_base_.GetMethods().empty()); +} + +TEST_F(ProxyFieldGetSetFixture, SetMethodIsNotRegisteredAsRegularMethod) +{ + // When constructing a Set-only ProxyField + auto field = MakeFieldWithSetAndNotifier(); + + // Then the field shows up on the parent proxy, but the Set method does not leak into the method map + EXPECT_EQ(proxy_base_.GetFields().size(), 1U); + EXPECT_TRUE(proxy_base_.GetMethods().empty()); +} + +TEST_F(ProxyFieldGetSetFixture, NotifierOnlyFieldRegistersWithoutAnyMethods) +{ + // When constructing a ProxyField with WithNotifier only (no Get, no Set) + auto field = MakeFieldWithNotifierOnly(); + + // Then the field is still registered on the parent proxy, and no methods are tracked + EXPECT_EQ(proxy_base_.GetFields().size(), 1U); + EXPECT_TRUE(proxy_base_.GetMethods().empty()); +} + +TEST_F(ProxyFieldGetSetFixture, NullMethodBindingsDoNotRegisterMethodsAsRegularMethods) +{ + // Given a Get+Set ProxyField whose method bindings are null. + // Deliberately inlined (instead of using a helper) so the null binding is visible at the call site. + ProxyField field{ + proxy_base_, + "TestField", + detail::WithTestTag{}, + std::make_unique>(proxy_event_mock_), + nullptr, + nullptr}; + + // Then the field is registered and no methods land in the method map. Nullptr method bindings skip + // ProxyMethod construction entirely (see the test-only ctor in proxy_field.h), so they do not + // mark the service-element binding invalid. + EXPECT_EQ(proxy_base_.GetFields().size(), 1U); + EXPECT_TRUE(proxy_base_.GetMethods().empty()); +} + +TEST_F(ProxyFieldGetSetFixture, SubscribeOnNotifierFieldForwardsMaxSampleCountToBinding) +{ + // Given a WithNotifier ProxyField in the unsubscribed state + auto field = MakeFieldWithNotifierOnly(); + constexpr std::size_t kMaxSampleCount = 7U; + EXPECT_CALL(proxy_event_mock_, GetSubscriptionState()).WillOnce(Return(SubscriptionState::kNotSubscribed)); + + // When Subscribe(kMaxSampleCount) is called + EXPECT_CALL(proxy_event_mock_, Subscribe(kMaxSampleCount)).WillOnce(Return(score::ResultBlank{})); + const auto result = field.Subscribe(kMaxSampleCount); + + // Then the call delegates to the binding with the same max-sample-count and the success result is propagated + EXPECT_TRUE(result.has_value()); +} + +TEST_F(ProxyFieldGetSetFixture, SubscribeOnNotifierFieldPropagatesBindingError) +{ + // Given a WithNotifier ProxyField whose binding's Subscribe returns an error + auto field = MakeFieldWithNotifierOnly(); + EXPECT_CALL(proxy_event_mock_, GetSubscriptionState()).WillOnce(Return(SubscriptionState::kNotSubscribed)); + EXPECT_CALL(proxy_event_mock_, Subscribe(_)).WillOnce(Return(MakeUnexpected(ComErrc::kBindingFailure))); + + // When Subscribe is called + const auto result = field.Subscribe(3U); + + // Then the binding error is propagated to the caller + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), ComErrc::kBindingFailure); +} + +TEST_F(ProxyFieldGetSetFixture, GetSubscriptionStateOnNotifierFieldReturnsBindingValue) +{ + // Given a WithNotifier ProxyField + auto field = MakeFieldWithNotifierOnly(); + + // When GetSubscriptionState() is called and the binding reports kSubscribed + EXPECT_CALL(proxy_event_mock_, GetSubscriptionState()).WillOnce(Return(SubscriptionState::kSubscribed)); + const auto state = field.GetSubscriptionState(); + + // Then exactly the value the binding returned is reported back + EXPECT_EQ(state, SubscriptionState::kSubscribed); +} + +TEST_F(ProxyFieldGetSetFixture, GetNumNewSamplesAvailableOnNotifierFieldReturnsBindingValue) +{ + // Given a WithNotifier ProxyField in the subscribed state + auto field = MakeFieldWithNotifierOnly(); + EXPECT_CALL(proxy_event_mock_, GetSubscriptionState()).WillRepeatedly(Return(SubscriptionState::kSubscribed)); + + // When the binding reports 4 new samples + EXPECT_CALL(proxy_event_mock_, GetNumNewSamplesAvailable()).WillOnce(Return(4U)); + const auto result = field.GetNumNewSamplesAvailable(); + + // Then exactly that count is forwarded + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(*result, 4U); +} + +TEST_F(ProxyFieldGetSetFixture, NotifierSurfaceCoexistsWithSetterOnSameField) +{ + // Given a ProxyField tagged WithSetter + WithNotifier (no getter), both surfaces should be usable + auto field = MakeFieldWithSetAndNotifier(); + constexpr std::size_t kMaxSampleCount = 5U; + TestSampleType set_value{42U}; + + // Notifier surface: Subscribe forwards to binding + EXPECT_CALL(proxy_event_mock_, GetSubscriptionState()).WillOnce(Return(SubscriptionState::kNotSubscribed)); + EXPECT_CALL(proxy_event_mock_, Subscribe(kMaxSampleCount)).WillOnce(Return(score::ResultBlank{})); + const auto subscribe_result = field.Subscribe(kMaxSampleCount); + ASSERT_TRUE(subscribe_result.has_value()); + + // Setter surface: Set forwards to method binding and the value lands in the in-args buffer + EXPECT_CALL(set_method_binding_mock_, GetInArgsBuffer(0)); + EXPECT_CALL(set_method_binding_mock_, GetReturnValueBuffer(0)); + EXPECT_CALL(set_method_binding_mock_, DoCall(0)); + const auto set_result = field.Set(set_value); + ASSERT_TRUE(set_result.has_value()); + EXPECT_EQ(static_cast(in_args_buffer_[0]), set_value); } } // namespace diff --git a/score/mw/com/impl/skeleton_base_test.cpp b/score/mw/com/impl/skeleton_base_test.cpp index 5f2138b2b..efbefbbf9 100644 --- a/score/mw/com/impl/skeleton_base_test.cpp +++ b/score/mw/com/impl/skeleton_base_test.cpp @@ -63,7 +63,7 @@ class MyDummySkeleton final : public SkeletonBase SkeletonEvent dummy_event{*this, kDummyEventName}; SkeletonEvent dummy_event2{*this, kDummyEventName2}; - SkeletonField dummy_field{*this, kDummyFieldName}; + SkeletonField dummy_field{*this, kDummyFieldName}; }; mock_binding::Skeleton* GetMockBinding(MyDummySkeleton& skeleton) noexcept diff --git a/score/mw/com/impl/skeleton_event.h b/score/mw/com/impl/skeleton_event.h index ac50d016a..27c341639 100644 --- a/score/mw/com/impl/skeleton_event.h +++ b/score/mw/com/impl/skeleton_event.h @@ -36,7 +36,7 @@ namespace score::mw::com::impl // False Positive: this is a normal forward declaration. // coverity[autosar_cpp14_m3_2_3_violation] -template +template class SkeletonField; template @@ -51,7 +51,7 @@ class SkeletonEvent : public SkeletonEventBase // SkeletonField uses composition pattern to reuse code from SkeletonEvent. These two classes also have shared // private APIs which necessitates the use of the friend keyword. // coverity[autosar_cpp14_a11_3_1_violation] - template + template friend class SkeletonField; // Empty struct that is used to make the second constructor only accessible to SkeletonEvent and SkeletonField (as diff --git a/score/mw/com/impl/skeleton_field.h b/score/mw/com/impl/skeleton_field.h index 228ab650d..fe39e42aa 100644 --- a/score/mw/com/impl/skeleton_field.h +++ b/score/mw/com/impl/skeleton_field.h @@ -13,6 +13,7 @@ #ifndef SCORE_MW_COM_IMPL_SKELETON_FIELD_H #define SCORE_MW_COM_IMPL_SKELETON_FIELD_H +#include "score/mw/com/impl/field_tags.h" #include "score/mw/com/impl/method_type.h" #include "score/mw/com/impl/methods/skeleton_method.h" #include "score/mw/com/impl/plumbing/sample_allocatee_ptr.h" @@ -26,6 +27,7 @@ #include "score/result/result.h" #include +#include #include #include @@ -35,37 +37,37 @@ namespace score::mw::com::impl { -template +template class SkeletonField : public SkeletonFieldBase { public: using FieldType = SampleDataType; - /// \brief Constructs a SkeletonField with setter enabled. Normal ctor used in production code. - /// - /// \param parent Skeleton that contains this field. - /// \param field_name Field name of the field. - /// \param detail::EnableSetOnlyTag This parameter is only used for constructor overload disambiguation and - /// has no semantic meaning. The tag disambiguates the setter-enabled ctor from the no-setter ctor, as - /// otherwise both would have the same signature. - template > - SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableSetOnlyTag = {}); - - /// \brief Constructs a SkeletonField with no setter. Normal ctor used in production code. - /// - /// \param parent Skeleton that contains this field. - /// \param field_name Field name of the field. - /// \param detail::EnableNeitherTag This parameter is only used for constructor overload disambiguation and - /// has no semantic meaning. - template > - SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableNeitherTag = {}); - - /// Constructor that allows to set the binding directly. - /// - /// This is used only used for testing. + /// Normal ctor, setter enabled. + template ::value>> + SkeletonField(SkeletonBase& parent, const std::string_view field_name, WithSetter = {}) + : SkeletonField{parent, MakeSkeletonEvent(parent, field_name), field_name, WithSetter{}} + { + } + + /// Normal ctor, setter not enabled. + template ::value>> + SkeletonField(SkeletonBase& parent, const std::string_view field_name) + : SkeletonField{parent, MakeSkeletonEvent(parent, field_name), field_name} + { + } + + /// Testing ctor: binding is passed in directly (used with mock bindings). SkeletonField(SkeletonBase& skeleton_base, const std::string_view field_name, - std::unique_ptr> binding); + std::unique_ptr> binding) + : SkeletonField{skeleton_base, + std::make_unique>(skeleton_base, field_name, std::move(binding)), + field_name} + { + } ~SkeletonField() override = default; @@ -112,14 +114,12 @@ class SkeletonField : public SkeletonFieldBase skeleton_field_mock_ = &skeleton_field_mock; } - // SFINAE-gated: only exists when EnableSet == true - // - // \ brief Registers a handler that is invoked by the middleware whenever a proxy calls the field setter. - // - // \tparam CallableType Any callable (std::function, score::cpp::callback, lambda, ...) with the signature: - // void(FieldType& new_value) - // - new_value : the value requested by the proxy. - template ::type = 0, typename CallableType> + /// Registers a handler invoked when a proxy issues the field setter. + /// The handler receives the proxy-requested value by reference and may modify it in-place; + /// the (possibly modified) value is then stored as the "latest value" via call to SkeletonField::Update(). + template ::value>> Result RegisterSetHandler(CallableType&& handler) { static_assert(std::is_invocable_v, @@ -159,50 +159,45 @@ class SkeletonField : public SkeletonFieldBase SkeletonEvent* GetTypedEvent() const noexcept; - std::unique_ptr initial_field_value_; - ISkeletonField* skeleton_field_mock_; - - // Zero-cost conditional storage: unique_ptr when EnableSet=true, zero-size tag when false. - using SetMethodSignature = FieldType(FieldType); - using SetMethodType = - std::conditional_t>, detail::EnableSetOnlyTag>; - SetMethodType set_method_; - - // Stores the user-provided set handler. Kept as a member so that the wrapped - // callback can invoke it via this->set_handler_. The concrete storage type is - // score::cpp::callback with the expected signature so that any callable provided - // to RegisterSetHandler is type-erased here. - // Zero-cost when EnableSet=false. - using SetHandlerStorageType = - std::conditional_t, detail::EnableSetOnlyTag>; - SetHandlerStorageType set_handler_{}; - - // Tracks whether RegisterSetHandler() has been called. Zero-cost when EnableSet=false. - using IsSetHandlerRegisteredType = std::conditional_t; - IsSetHandlerRegisteredType is_set_handler_registered_{}; - - // EnableSet=true: checks the flag; EnableSet=false: no setter, no handler required. bool IsSetHandlerRegistered() const noexcept override { - if constexpr (EnableSet) + if constexpr (detail::contains_type::value) { return is_set_handler_registered_; } return true; } + static std::unique_ptr> MakeSkeletonEvent(SkeletonBase& parent, + const std::string_view field_name) + { + return std::make_unique>( + parent, + field_name, + SkeletonFieldBindingFactory::CreateEventBinding( + SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), parent, field_name), + typename SkeletonEvent::FieldOnlyConstructorEnabler{}); + } + /// \brief Private delegating constructor used by the setter-enabled public ctor. - template > SkeletonField(SkeletonBase& parent, std::unique_ptr> skeleton_event_dispatch, const std::string_view field_name, - detail::EnableSetOnlyTag); + WithSetter); /// \brief Private delegating constructor used by the no-setter public ctor and testing ctor. SkeletonField(SkeletonBase& parent, std::unique_ptr> skeleton_event_dispatch, const std::string_view field_name); + std::unique_ptr initial_field_value_; + ISkeletonField* skeleton_field_mock_; + + using SetMethodSignature = FieldType(FieldType); + std::unique_ptr> set_method_; + score::cpp::callback set_handler_; + bool is_set_handler_registered_{false}; + // TODO: Move get_method_ initialization into the delegating constructors (like set_method_) once the // Get handler is implemented. using GetMethodSignature = FieldType(); @@ -214,65 +209,11 @@ class SkeletonField : public SkeletonFieldBase typename SkeletonMethod::FieldOnlyConstructorEnabler{})}; }; -/// \brief Public ctor — EnableSet=true: delegates to the private ctor that also creates the set method. -template -template -SkeletonField::SkeletonField(SkeletonBase& parent, - const std::string_view field_name, - detail::EnableSetOnlyTag) - : SkeletonField{ - parent, - std::make_unique>(parent, - field_name, - SkeletonFieldBindingFactory::CreateEventBinding( - SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), - parent, - field_name), - typename SkeletonEvent::FieldOnlyConstructorEnabler{}), - field_name, - detail::EnableSetOnlyTag{}} -{ -} - -/// \brief Public ctor — EnableSet=false: delegates to the private ctor with no set method. -template -template -SkeletonField::SkeletonField(SkeletonBase& parent, - const std::string_view field_name, - detail::EnableNeitherTag) - : SkeletonField{ - parent, - std::make_unique>(parent, - field_name, - SkeletonFieldBindingFactory::CreateEventBinding( - SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), - parent, - field_name), - typename SkeletonEvent::FieldOnlyConstructorEnabler{}), - field_name} -{ -} - -/// \brief Testing ctor: binding is provided directly (used with mock bindings in tests). -template -SkeletonField::SkeletonField( - SkeletonBase& skeleton_base, - const std::string_view field_name, - std::unique_ptr> binding) - : SkeletonField{skeleton_base, - std::make_unique>(skeleton_base, field_name, std::move(binding)), - field_name} -{ -} - -/// \brief Private delegating ctor — setter enabled. -template -template -SkeletonField::SkeletonField( - SkeletonBase& parent, - std::unique_ptr> skeleton_event_dispatch, - const std::string_view field_name, - detail::EnableSetOnlyTag) +template +SkeletonField::SkeletonField(SkeletonBase& parent, + std::unique_ptr> skeleton_event_dispatch, + const std::string_view field_name, + WithSetter) : SkeletonFieldBase{parent, field_name, std::move(skeleton_event_dispatch)}, initial_field_value_{nullptr}, skeleton_field_mock_{nullptr} @@ -286,12 +227,10 @@ SkeletonField::SkeletonField( skeleton_base_view.RegisterField(field_name, *this); } -/// \brief Private delegating ctor — no setter. Receives the already-constructed event. -template -SkeletonField::SkeletonField( - SkeletonBase& parent, - std::unique_ptr> skeleton_event_dispatch, - const std::string_view field_name) +template +SkeletonField::SkeletonField(SkeletonBase& parent, + std::unique_ptr> skeleton_event_dispatch, + const std::string_view field_name) : SkeletonFieldBase{parent, field_name, std::move(skeleton_event_dispatch)}, initial_field_value_{nullptr}, skeleton_field_mock_{nullptr} @@ -300,8 +239,8 @@ SkeletonField::SkeletonField( skeleton_base_view.RegisterField(field_name, *this); } -template -SkeletonField::SkeletonField(SkeletonField&& other) noexcept +template +SkeletonField::SkeletonField(SkeletonField&& other) noexcept : SkeletonFieldBase(static_cast(other)), // known llvm bug (https://github.com/llvm/llvm-project/issues/63202) // This usage is safe because the previous line only moves the base class portion via static_cast. @@ -319,9 +258,9 @@ SkeletonField::SkeletonField(Skeleton skeleton_base_view.UpdateField(field_name_, *this); } -template -auto SkeletonField::operator=(SkeletonField&& other) & noexcept - -> SkeletonField& +template +auto SkeletonField::operator=(SkeletonField&& other) & noexcept + -> SkeletonField& { if (this != &other) { @@ -346,8 +285,8 @@ auto SkeletonField::operator=(Skeleto /// field cannot be set until the Skeleton has been set up via Skeleton::OfferService(). Therefore, we create a /// callback that will update the field value with sample_value which will be called in the first call to /// SkeletonFieldBase::PrepareOffer(). -template -Result SkeletonField::Update(const FieldType& sample_value) noexcept +template +Result SkeletonField::Update(const FieldType& sample_value) noexcept { if (skeleton_field_mock_ != nullptr) { @@ -364,9 +303,8 @@ Result SkeletonField::Update(co /// \brief FieldType is previously allocated by middleware and provided by the user to indicate that he is finished /// filling the provided pointer with live data. Dispatches to SkeletonEvent::Send() -template -Result SkeletonField::Update( - SampleAllocateePtr sample) noexcept +template +Result SkeletonField::Update(SampleAllocateePtr sample) noexcept { if (skeleton_field_mock_ != nullptr) { @@ -381,8 +319,8 @@ Result SkeletonField::Update( /// /// This function cannot be currently called to set the initial value of a field as the shared memory must be first /// set up in the Skeleton::PrepareOffer() before the user can obtain / use a SampleAllocateePtr. -template -Result> SkeletonField::Allocate() noexcept +template +Result> SkeletonField::Allocate() noexcept { if (skeleton_field_mock_ != nullptr) { @@ -400,12 +338,12 @@ Result> SkeletonFieldAllocate(); } -template +template // Suppress "AUTOSAR C++14 A0-1-3" rule finding. This rule states: "Every function defined in an anonymous // namespace, or static function with internal linkage, or private member function shall be used.". // False-positive, method is used in the base class in PrepareOffer(). // coverity[autosar_cpp14_a0_1_3_violation : FALSE] -Result SkeletonField::DoDeferredUpdate() noexcept +Result SkeletonField::DoDeferredUpdate() noexcept { SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( initial_field_value_ != nullptr, @@ -421,16 +359,14 @@ Result SkeletonField::DoDeferre return Result{}; } -template -Result SkeletonField::UpdateImpl( - const FieldType& sample_value) noexcept +template +Result SkeletonField::UpdateImpl(const FieldType& sample_value) noexcept { return GetTypedEvent()->Send(sample_value); } -template -auto SkeletonField::GetTypedEvent() const noexcept - -> SkeletonEvent* +template +auto SkeletonField::GetTypedEvent() const noexcept -> SkeletonEvent* { auto* const typed_event = dynamic_cast*>(skeleton_event_dispatch_.get()); SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(typed_event != nullptr, "Downcast to SkeletonEvent failed!"); diff --git a/score/mw/com/impl/skeleton_field_test.cpp b/score/mw/com/impl/skeleton_field_test.cpp index 1967b65c0..9d90031d8 100644 --- a/score/mw/com/impl/skeleton_field_test.cpp +++ b/score/mw/com/impl/skeleton_field_test.cpp @@ -65,7 +65,7 @@ class MyDummySkeleton : public SkeletonBase public: using SkeletonBase::SkeletonBase; - SkeletonField my_dummy_field_{*this, kFieldName}; + SkeletonField my_dummy_field_{*this, kFieldName}; }; TEST(SkeletonFieldTest, NotCopyable) @@ -76,14 +76,18 @@ TEST(SkeletonFieldTest, NotCopyable) RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - static_assert(!std::is_copy_constructible>::value, "Is wrongly copyable"); - static_assert(!std::is_copy_assignable>::value, "Is wrongly copyable"); + static_assert(!std::is_copy_constructible>::value, + "Is wrongly copyable"); + static_assert(!std::is_copy_assignable>::value, + "Is wrongly copyable"); } TEST(SkeletonFieldTest, IsMoveable) { - static_assert(std::is_move_constructible>::value, "Is not move constructible"); - static_assert(std::is_move_assignable>::value, "Is not move assignable"); + static_assert(std::is_move_constructible>::value, + "Is not move constructible"); + static_assert(std::is_move_assignable>::value, + "Is not move assignable"); } TEST(SkeletonFieldTest, ClassTypeDependsOnFieldDataType) @@ -94,12 +98,20 @@ TEST(SkeletonFieldTest, ClassTypeDependsOnFieldDataType) RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - using FirstSkeletonFieldType = SkeletonField; - using SecondSkeletonFieldType = SkeletonField; + using FirstSkeletonFieldType = SkeletonField; + using SecondSkeletonFieldType = SkeletonField; static_assert(!std::is_same_v, "Class type does not depend on field data type"); } +TEST(SkeletonFieldTest, ClassTypeDependsOnFieldTagType) +{ + using FirstSkeletonFieldType = SkeletonField; + using SecondSkeletonFieldType = SkeletonField; + static_assert(!std::is_same_v, + "Class type does not depend on field tag type"); +} + TEST(SkeletonFieldTest, SkeletonFieldContainsPublicSampleType) { RecordProperty("Verifies", "SCR-17433130"); @@ -109,7 +121,8 @@ TEST(SkeletonFieldTest, SkeletonFieldContainsPublicSampleType) RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - static_assert(std::is_same::FieldType, TestSampleType>::value, + static_assert(std::is_same::FieldType, + TestSampleType>::value, "Incorrect FieldType."); } @@ -959,35 +972,33 @@ TEST(SkeletonFieldDeathTest, UpdateWithInvalidFieldNameTriggersTermination) EXPECT_DEATH(SkeletonBaseView{unit}.UpdateField("non_existing_test_field_name", field);, ".*"); } -// Helper skeleton that holds an EnableSet=true field (setter-capable field) +// Helper skeleton that holds a setter-capable field (tagged WithSetter). class MySetterSkeleton : public SkeletonBase { public: using SkeletonBase::SkeletonBase; - SkeletonField my_setter_field_{*this, kFieldName}; + SkeletonField my_setter_field_{*this, kFieldName}; }; // Static type-trait tests for RegisterSetHandler availability -TEST(SkeletonFieldSetHandlerTest, RegisterSetHandlerOnlyExistsWhenEnableSetIsTrue) +TEST(SkeletonFieldSetHandlerTest, RegisterSetHandlerOnlyExistsWhenWithSetterTagPresent) { RecordProperty("Description", - "RegisterSetHandler() shall only exist on SkeletonField. " - "Verify via has_member detection that it is absent when EnableSet=false."); + "RegisterSetHandler() shall only exist on SkeletonField tagged WithSetter. " + "Verify that the tag distinguishes the two types."); RecordProperty("TestType", "Requirements-based test"); RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - // RegisterSetHandler should be callable on EnableSet=true - static_assert(std::is_same_v::FieldType, TestSampleType>, + static_assert(std::is_same_v::FieldType, TestSampleType>, "Setter-capable field must expose FieldType"); - // EnableSet=false field must not expose SetHandlerType at the member function level. We verify - // indirectly that the EnableSet template parameter distinguishes the types. - static_assert(!std::is_same_v, SkeletonField>, - "EnableSet=false and EnableSet=true fields must be different types"); + static_assert(!std::is_same_v, + SkeletonField>, + "Distinct tag packs must produce distinct types"); } // RegisterSetHandler – happy-path: handler forwarded to method binding @@ -995,7 +1006,7 @@ TEST(SkeletonFieldSetHandlerTest, RegisterSetHandlerOnlyExistsWhenEnableSetIsTru TEST(SkeletonFieldSetHandlerTest, RegisterSetHandlerForwardsToMethodBinding) { RecordProperty("Description", - "Calling RegisterSetHandler() on an EnableSet=true SkeletonField shall forward " + "Calling RegisterSetHandler() on a WithSetter-tagged SkeletonField shall forward " "the handler registration to the underlying SkeletonMethod binding and return " "success."); RecordProperty("TestType", "Requirements-based test"); @@ -1071,13 +1082,13 @@ TEST(SkeletonFieldSetHandlerTest, RegisterSetHandlerPropagatesBindingError) EXPECT_EQ(result.error(), ComErrc::kCommunicationLinkError); } -// PrepareOffer fails when EnableSet=true but no handler has been registered +// PrepareOffer fails when WithSetter is present but no handler has been registered TEST(SkeletonFieldSetHandlerTest, PrepareOfferFailsWhenSetHandlerNotRegistered) { RecordProperty("Verifies", "SCR-17563743"); RecordProperty("Description", - "When a SkeletonField is defined with EnableSet=true and no set handler has been " + "When a SkeletonField is tagged WithSetter and no set handler has been " "registered, PrepareOffer() shall return kSetHandlerNotSet."); RecordProperty("TestType", "Requirements-based test"); RecordProperty("Priority", "1"); @@ -1109,12 +1120,12 @@ TEST(SkeletonFieldSetHandlerTest, PrepareOfferFailsWhenSetHandlerNotRegistered) EXPECT_EQ(result.error(), ComErrc::kSetHandlerNotSet); } -// PrepareOffer succeeds when EnableSet=true and handler IS registered +// PrepareOffer succeeds when WithSetter is present and handler IS registered TEST(SkeletonFieldSetHandlerTest, PrepareOfferSucceedsAfterRegisterSetHandler) { RecordProperty("Description", - "When an EnableSet=true SkeletonField has a set handler registered and an initial " + "When a WithSetter-tagged SkeletonField has a set handler registered and an initial " "value set, PrepareOffer() shall succeed."); RecordProperty("TestType", "Requirements-based test"); RecordProperty("Priority", "1"); @@ -1161,12 +1172,12 @@ TEST(SkeletonFieldSetHandlerTest, PrepareOfferSucceedsAfterRegisterSetHandler) EXPECT_TRUE(result.has_value()); } -// EnableSet=false: PrepareOffer does NOT require a registered set handler +// No WithSetter tag: PrepareOffer does NOT require a registered set handler -TEST(SkeletonFieldSetHandlerTest, PrepareOfferSucceedsWithoutHandlerWhenEnableSetIsFalse) +TEST(SkeletonFieldSetHandlerTest, PrepareOfferSucceedsWithoutHandlerWhenWithSetterTagIsAbsent) { RecordProperty("Description", - "When a SkeletonField has EnableSet=false (no setter), PrepareOffer() shall " + "When a SkeletonField has no WithSetter tag (no setter), PrepareOffer() shall " "succeed without registering a set handler."); RecordProperty("TestType", "Requirements-based test"); RecordProperty("Priority", "1"); @@ -1187,7 +1198,7 @@ TEST(SkeletonFieldSetHandlerTest, PrepareOfferSucceedsWithoutHandlerWhenEnableSe EXPECT_CALL(event_binding, PrepareOffer()).WillOnce(Return(Result{})); EXPECT_CALL(event_binding, Send(initial_value, _)).WillOnce(Return(Result{})); - // A non-setter skeleton (EnableSet=false) + // A skeleton field without WithSetter tag MyDummySkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; ASSERT_TRUE(unit.my_dummy_field_.Update(initial_value).has_value()); @@ -1555,5 +1566,30 @@ TEST(SkeletonFieldSetHandlerTest, SecondRegisterSetHandlerReplacesHandler) EXPECT_TRUE(second_callback_called); } +TEST(SkeletonFieldGetSetMemberTest, SkeletonFieldDoesNotRegisterGetSetMethodsAsRegularMethods) +{ + // Given the field and method binding factories are mocked + RuntimeMockGuard runtime_mock_guard{}; + ON_CALL(runtime_mock_guard.runtime_mock_, GetTracingFilterConfig()).WillByDefault(Return(nullptr)); + SkeletonMethodBindingFactoryMockGuard skeleton_method_binding_factory_mock_guard{}; + SkeletonFieldBindingFactoryMockGuard skeleton_field_binding_factory_mock_guard{}; + + EXPECT_CALL(skeleton_field_binding_factory_mock_guard.factory_mock_, CreateEventBinding(_, _, _)) + .WillOnce(Return(ByMove(std::make_unique>()))); + EXPECT_CALL(skeleton_method_binding_factory_mock_guard.factory_mock_, Create(_, _, _, MethodType::kGet)) + .WillOnce(Return(ByMove(nullptr))); + EXPECT_CALL(skeleton_method_binding_factory_mock_guard.factory_mock_, Create(_, _, _, MethodType::kSet)) + .WillOnce(Return(ByMove(nullptr))); + + // When constructing a SkeletonField which internally creates get_method_ and set_method_ + MySetterSkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; + + // Then the field is registered, but its Get/Set SkeletonMethods do not appear in the skeleton's + // regular method map,they are field-only and tracked by the field itself + SkeletonBaseView skeleton_view{unit}; + EXPECT_EQ(skeleton_view.GetFields().size(), 1U); + EXPECT_TRUE(skeleton_view.GetMethods().empty()); +} + } // namespace } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/test/proxy_resources.h b/score/mw/com/impl/test/proxy_resources.h index f33ff549d..fe75c5110 100644 --- a/score/mw/com/impl/test/proxy_resources.h +++ b/score/mw/com/impl/test/proxy_resources.h @@ -24,11 +24,11 @@ namespace score::mw::com::impl { -template +template class ProxyFieldAttorney { public: - ProxyFieldAttorney(ProxyField& proxy_field) noexcept : proxy_field_{proxy_field} {} + ProxyFieldAttorney(ProxyField& proxy_field) noexcept : proxy_field_{proxy_field} {} ProxyEvent& GetProxyEvent() noexcept { @@ -37,7 +37,7 @@ class ProxyFieldAttorney } private: - ProxyField& proxy_field_; + ProxyField& proxy_field_; }; class ProxyEventBaseAttorney @@ -45,9 +45,9 @@ class ProxyEventBaseAttorney public: ProxyEventBaseAttorney(ProxyEventBase& proxy_event_base) noexcept : proxy_event_base_{proxy_event_base} {} - template - ProxyEventBaseAttorney(ProxyField& proxy_field) noexcept - : proxy_event_base_{ProxyFieldAttorney{proxy_field}.GetProxyEvent()} + template + ProxyEventBaseAttorney(ProxyField& proxy_field) noexcept + : proxy_event_base_{ProxyFieldAttorney{proxy_field}.GetProxyEvent()} { } diff --git a/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp b/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp index 6549d0700..1a098abc6 100644 --- a/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp +++ b/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp @@ -69,7 +69,7 @@ class MyDummyProxyWithField : public ProxyBase public: using ProxyBase::ProxyBase; - ProxyField my_service_element_{*this, kServiceElementName}; + ProxyField my_service_element_{*this, kServiceElementName}; }; /// \brief Structs containing types for templated gtests. diff --git a/score/mw/com/impl/tracing/test/skeleton_field_tracing_test.cpp b/score/mw/com/impl/tracing/test/skeleton_field_tracing_test.cpp index 0c5afa436..3d0b5fb36 100644 --- a/score/mw/com/impl/tracing/test/skeleton_field_tracing_test.cpp +++ b/score/mw/com/impl/tracing/test/skeleton_field_tracing_test.cpp @@ -73,7 +73,7 @@ class MyDummySkeleton : public SkeletonBase public: using SkeletonBase::SkeletonBase; - SkeletonField my_dummy_field_{*this, kFieldName}; + SkeletonField my_dummy_field_{*this, kFieldName}; }; TEST(SkeletonFieldTracingTest, TracePointsAreDisabledIfConfigNotReturnedByRuntime) diff --git a/score/mw/com/impl/traits.h b/score/mw/com/impl/traits.h index 1c51a0587..8bfd35816 100644 --- a/score/mw/com/impl/traits.h +++ b/score/mw/com/impl/traits.h @@ -14,6 +14,7 @@ #define SCORE_MW_COM_IMPL_TRAITS_H #include "score/mw/com/impl/com_error.h" +#include "score/mw/com/impl/field_tags.h" #include "score/mw/com/impl/flag_owner.h" #include "score/mw/com/impl/handle_type.h" #include "score/mw/com/impl/instance_identifier.h" @@ -100,18 +101,18 @@ class ProxyWrapperClassTestView; /// typename Trait::template Event struct_event_1_{*this, event_name_0}; /// typename Trait::template Event struct_event_2_{*this, event_name_1}; /// -/// typename Trait::template Field struct_field_1_{*this, -/// field_name_0}; -/// typename Trait::template Field -/// struct_field_2_{*this, field_name_1}; +/// typename Trait::template Field struct_field_1_{*this, +/// field_name_0}; +/// typename Trait::template Field struct_field_2_{*this, field_name_1}; /// /// typename Trait::template Method struct_method_1_{*this, method_name_0}; /// typename Trait::template Method struct_method_2_{*this, method_name_1}; /// /// }; -/// Notes regarding template args: A field has (besides its data type arg) three bool template args (enable_getter, -/// enable_setter and enable_notifier). The enable_notifier template parameter is only relevant for certain bindings, -/// e.g. the LoLa binding does not distinguish between true/false of this template parameter. +/// Notes regarding template args: a field takes its data type plus a pack of tags from {WithGetter, WithSetter, +/// WithNotifier}. WithGetter / WithSetter enable Get() / Set() on the proxy. WithNotifier enables the proxy notifier +/// surface (Subscribe, GetNewSamples, ...). At least one of WithGetter or WithNotifier must be present, otherwise the +/// value would be invisible to consumers. The skeleton-side Update/Allocate are part of every field. /// A method has a template arg describing the method signature in the form ReturnType(InArgType1, InArgType2, ...). /// InArgs and ReturnType are optional. Therefore, these are valid signatures: /// - void() @@ -139,10 +140,8 @@ class ProxyTrait template using Event = ProxyEvent; - // Note : at the moment the SkeletonField::Get implementation is not in the branch which means the skeleton and - // proxy side does not have same template parameters. - template - using Field = ProxyField; + template + using Field = ProxyField; template using Method = ProxyMethod; @@ -164,8 +163,8 @@ class SkeletonTrait template using Event = SkeletonEvent; - template - using Field = SkeletonField; + template + using Field = SkeletonField; template using Method = SkeletonMethod; diff --git a/score/mw/com/impl/traits_test.cpp b/score/mw/com/impl/traits_test.cpp index 7074a6829..f735c18ec 100644 --- a/score/mw/com/impl/traits_test.cpp +++ b/score/mw/com/impl/traits_test.cpp @@ -67,7 +67,7 @@ class MyInterface : public InterfaceTrait::Base using InterfaceTrait::Base::Base; typename InterfaceTrait::template Event some_event{*this, kEventName}; - typename InterfaceTrait::template Field some_field{*this, kFieldName}; + typename InterfaceTrait::template Field some_field{*this, kFieldName}; typename InterfaceTrait::template Method some_method{*this, kMethodName}; }; using MyProxy = AsProxy; @@ -549,13 +549,14 @@ TEST_F(GeneratedSkeletonCreationInstanceSpecifierTestFixture, EXPECT_CALL(skeleton_field_binding_factory_mock_guard_.factory_mock_, CreateEventBinding(identifier_with_valid_binding_, _, kFieldName)) .WillOnce(Return(ByMove(std::move(skeleton_field_binding_mock_ptr)))); - // Field get method binding (not yet fully implemented; set method only exists when EnableSet=true) - EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(_, _, _, _)) + // Field get method binding always created for every SkeletonField regardless of tags, because get_method_ is + // unconditionally initialized (see TODO in skeleton_field.h). Once get_method_ becomes tag-conditional this + // expectation will need to be removed or adjusted accordingly. + EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, + Create(identifier_with_valid_binding_, _, kFieldName, MethodType::kGet)) .Times(1) - .WillOnce(Invoke([this](const InstanceIdentifier&, SkeletonBinding*, const std::string_view, MethodType) - -> std::unique_ptr { - return std::make_unique(skeleton_field_method_binding_mock_); - })); + .WillOnce( + Return(ByMove(std::make_unique(skeleton_field_method_binding_mock_)))); EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(identifier_with_valid_binding_, _, kMethodName, _)) .WillOnce(Return(ByMove(std::move(skeleton_method_binding_mock_ptr)))); @@ -644,8 +645,11 @@ TEST_F(GeneratedSkeletonCreationInstanceSpecifierTestFixture, RecordProperty("DerivationTechnique", "Analysis of requirements"); // Expecting that the Create call on the SkeletonMethodBindingFactory returns an invalid binding for the method. - // Field get method binding (not yet fully implemented; set method only exists when EnableSet=true) - EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(_, _, _, _)).Times(1); + // Field get method is always created at the moment (see TODO in skeleton_field.h). Once get_method_ becomes + // tag-conditional this expectation can be adjusted accordingly. + EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, + Create(identifier_with_valid_binding_, _, kFieldName, MethodType::kGet)) + .Times(1); EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(identifier_with_valid_binding_, _, kMethodName, _)) .WillOnce(Return(ByMove(nullptr))); @@ -779,8 +783,12 @@ TEST_F(GeneratedSkeletonCreationInstanceIdentifierTestFixture, ConstructingFromI { // Expecting that the Create call on the SkeletonMethodBindingFactory returns an invalid binding for the method. - // Field get method binding (not yet fully implemented; set method only exists when EnableSet=true) - EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(_, _, _, _)).Times(1); + // Field get method binding always created for every SkeletonField regardless of tags, because get_method_ is + // unconditionally initialized (see TODO in skeleton_field.h). Once get_method_ becomes tag-conditional this + // expectation will need to be removed or adjusted accordingly. + EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, + Create(identifier_with_valid_binding_, _, kFieldName, MethodType::kGet)) + .Times(1); EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(identifier_with_valid_binding_, _, kMethodName, _)) .WillOnce(Return(ByMove(nullptr))); diff --git a/score/mw/com/test/common_test_resources/test_interface.h b/score/mw/com/test/common_test_resources/test_interface.h index 64cf4997e..0f2a63250 100644 --- a/score/mw/com/test/common_test_resources/test_interface.h +++ b/score/mw/com/test/common_test_resources/test_interface.h @@ -14,6 +14,8 @@ #ifndef SCORE_MW_COM_TEST_COMMON_TEST_RESOURCES_TEST_INTERFACE_H #define SCORE_MW_COM_TEST_COMMON_TEST_RESOURCES_TEST_INTERFACE_H +#include "score/mw/com/types.h" + #include #include @@ -26,7 +28,7 @@ class TestInterface : public T::Base public: using T::Base::Base; - typename T::template Field test_field{*this, "test_field"}; + typename T::template Field test_field{*this, "test_field"}; }; } // namespace score::mw::com::test diff --git a/score/mw/com/test/field_initial_value/test_datatype.h b/score/mw/com/test/field_initial_value/test_datatype.h index 3f7348692..174ccff15 100644 --- a/score/mw/com/test/field_initial_value/test_datatype.h +++ b/score/mw/com/test/field_initial_value/test_datatype.h @@ -31,7 +31,7 @@ class TestInterface : public T::Base public: using T::Base::Base; - typename T::template Field test_field{*this, "test_field"}; + typename T::template Field test_field{*this, "test_field"}; }; using TestDataProxy = score::mw::com::AsProxy; diff --git a/score/mw/com/test/find_any_semantics/test_datatype.h b/score/mw/com/test/find_any_semantics/test_datatype.h index b6aa58428..5daddd695 100644 --- a/score/mw/com/test/find_any_semantics/test_datatype.h +++ b/score/mw/com/test/find_any_semantics/test_datatype.h @@ -34,7 +34,7 @@ class TestInterface : public T::Base public: using T::Base::Base; - typename T::template Field test_field{*this, "test_field"}; + typename T::template Field test_field{*this, "test_field"}; }; using TestDataProxy = score::mw::com::AsProxy; diff --git a/score/mw/com/types.h b/score/mw/com/types.h index c29f16ab9..7c9e5bca4 100644 --- a/score/mw/com/types.h +++ b/score/mw/com/types.h @@ -105,6 +105,12 @@ using MethodInArgTypePtr = impl::MethodInArgPtr; /// \requirement SWS_CM_00309 using EventReceiveHandler = impl::EventReceiveHandler; +/// \api +/// \brief Field tag types used in service-interface definitions to enable Get/Set/Notifier on a field. +using WithGetter = impl::WithGetter; +using WithSetter = impl::WithSetter; +using WithNotifier = impl::WithNotifier; + /// \api /// \brief Interpret an interface that follows our traits as proxy (cf. impl/traits.h) template