From 63a3cd4ac50ec51f612f9fb17dffdcbe542cfc7a Mon Sep 17 00:00:00 2001 From: Brendan Emery Date: Tue, 12 May 2026 16:46:56 +0200 Subject: [PATCH 1/2] mw/com: Make ProxyBase::AreBindingsValid symmetrical with Skeleton side Previously, ProxyBase::AreBindingsValid was implemented differently to avoid have to store a map of service elements (which also requires updating the map when moving a Proxy) in ProxyBase. However, we have since added these maps so there's no longer any reason to have the asymmetry in the implementations of AreBindingsValid. --- score/mw/com/design/skeleton_proxy/README.md | 50 ++++++------------- score/mw/com/impl/generic_proxy.cpp | 7 ++- score/mw/com/impl/generic_proxy_event.cpp | 12 ----- score/mw/com/impl/methods/proxy_method_base.h | 20 ++++++++ .../impl/methods/proxy_method_with_in_args.h | 5 -- .../proxy_method_with_in_args_and_return.h | 16 ------ .../methods/proxy_method_with_return_type.h | 16 ------ .../proxy_method_without_in_args_or_return.h | 10 ---- score/mw/com/impl/proxy_base.cpp | 31 +++++++----- score/mw/com/impl/proxy_base.h | 10 +--- score/mw/com/impl/proxy_event.h | 13 ++--- score/mw/com/impl/proxy_event_base.h | 20 ++++++++ score/mw/com/impl/proxy_event_base_test.cpp | 33 ------------ score/mw/com/impl/proxy_field_base.h | 26 ++++++++++ 14 files changed, 110 insertions(+), 159 deletions(-) diff --git a/score/mw/com/design/skeleton_proxy/README.md b/score/mw/com/design/skeleton_proxy/README.md index d8958e391..81d96241b 100644 --- a/score/mw/com/design/skeleton_proxy/README.md +++ b/score/mw/com/design/skeleton_proxy/README.md @@ -92,6 +92,12 @@ The following sequence shows the instantiation of a service class up to its serv SKELETON_CREATE_OFFER_SEQ +On construction, the generated skeleton (`DummySkeleton`) checks that all the bindings of its contained service elements +(events/fields/methods) are valid. This is done by calling the `AreBindingsValid()` on `SkeletonBase`. This will then +iterate over the maps of references to its service elements described [below](#binding-independent-level-registration-of-skeleton-eventsfields-at-their-parent-skeleton) +and check if each binding was successfully created. If this is not the case, the construction of the skeleton instance +returns an error. + #### Binding independent level Registration of skeleton events/fields at their parent skeleton Due to our architectural constraints, the `impl::SkeletonBase` (the base class of the generated skeleton/`DummySkeleton`) @@ -168,23 +174,10 @@ On `ProxyBase` level we have two types of methods: a generated proxy instance, to detect, whether the instance can be successfully returned from `static Result ::Create()` or not. -The implementation of the latter one contains some indirection – again due to our architectural constraints, which -are the same at proxy and skeleton side and have been laid out -[here on the skeleton side](#binding-independent-level-registration-of-skeleton-eventsfields-at-their-parent-skeleton). - -The indirection is that for `ProxyBase::AreBindingsValid()` to work, the binding independent `impl::ProxyEvent`s set -the member of its semantic parent `ProxyBase::are_service_element_bindings_valid_` to `false` during their construction, -if they detect, that they don't have a valid underlying `pImpl` member of type `ProxyEventBindingBase` to dispatch to. -This is an indication, that the binding specific implementation of the `ProxyEvent` couldn't be successfully created by -the proxy side factories. - -So, the binding independent `impl::ProxyBase` doesn't "know" its semantically aggregated `ProxyEvent`s. -Although the `ProxyEvent`s get a reference to their parent `ProxyBase` during construction (and therefore "know" their -parent in the context of their `ctor`), they don't store this reference (as it isn't really needed currently and -would require additional logic to update this reference if the parent `impl::ProxyBase` gets moved). -Instead, they only set once (see above) the `ProxyBase::are_service_element_bindings_valid_` member variable of their -parent during construction as this is currently the only feedback needed between proxy and its proxy events on binding -independent level. +Similarly to the skeleton side, the `impl::SkeletonBase` doesn't "know" its event/field/method children. Therefore, +the `impl::ProxyEventBase` registers itself with the `impl::ProxyBase` in the same way as `impl::SkeletonEventBase` +(as explained +[here on the skeleton side](#binding-independent-level-registration-of-skeleton-eventsfields-at-their-parent-skeleton). #### Binding level Registration of proxy events/fields at their parent proxy @@ -192,30 +185,19 @@ On the binding **specific** level things look different! I.e. the binding specific proxy needs to interact with its dependent/child proxy events of type `ProxyEventBindingBase` (at least the `LoLa`/shared-memory specific does, so we introduced it on the `ProxyBinding` interface level) . -Therefore, `impl::ProxyBinding` has a method `RegisterEventBinding` (and `UnregisterEventBinding`) and our `LoLa` -binding specific proxy `lola::Proxy`, which implements `impl::ProxyBinding`, has a member `event_bindings_` (a map -containing its child proxy events), which it populates in its implementation of `RegisterEventBinding` with its -dependent proxy events. - -The call to `RegisterEventBinding` happens during creation of a binding independent `impl::ProxyEvent` via an `RAII` -style member `event_binding_registration_guard_` of type `EventBindingRegistrationGuard`, which the `impl::ProxyEvent` -owns. -This `EventBindingRegistrationGuard` takes over two functionalities: - -- setting `ProxyBase::are_service_element_bindings_valid_` member of its parent ProxyBase instance (see previous chapter) -- calling `RegisterEventBinding` on the underlying `ProxyBinding` of the given `ProxyBase` on construction of the - `EventBindingRegistrationGuard` and calling `UnregisterEventBinding` on destruction. Since - `event_binding_registration_guard_` is owned by the `impl::ProxyEvent`, the registration / unregistration is done on - construction / destruction of an `impl::ProxyEvent`. +Therefore, when each `lola::ProxyEvent` gets created (as a member of the generated proxy class), it registers itself +at its parent `lola::Proxy` via `lola::Proxy::RegisterEvent()`. The `lola::Proxy` stores the reference to each +child `lola::ProxyEvent`s in a map which it can later use. So **after** construction of user facing generated proxy class instance (`DummyProxy`), we have the following structure in place: 1. Only an instance of the generated proxy class gets returned from the call to `::Create()` in case its binding on proxy level (its `pImpl` target) implementing `ProxyBinding` could be constructed and also for **all** its - aggregated events/fields their related `ProxyEventBinding`s (`pImpl` targets) could be constructed. + aggregated events/fields their related `ProxyEventBinding`s (`pImpl` targets) could be constructed. If this is not + the case, then an error will be retrurned from `::Create()`. 2. The `ProxyBinding` (`ProxyBase::proxy_binding_`) has complete access to all its child events/fields as - `ProxyBinding::RegisterEventBinding` has been called for all contained events/fields. + `lola::Proxy::RegisterEvent()` has been called for all contained events/fields. #### Extract type agnostic code diff --git a/score/mw/com/impl/generic_proxy.cpp b/score/mw/com/impl/generic_proxy.cpp index 3e2091287..a76e5610c 100644 --- a/score/mw/com/impl/generic_proxy.cpp +++ b/score/mw/com/impl/generic_proxy.cpp @@ -77,7 +77,12 @@ Result GenericProxy::Create(HandleType instance_handle) noexcept const auto& instance_identifier = generic_proxy.handle_.GetInstanceIdentifier(); const auto event_names = GetEventNameList(instance_identifier); generic_proxy.FillEventMap(event_names); - if (!generic_proxy.AreBindingsValid()) + auto& generic_proxy_events = generic_proxy.GetEvents(); + const bool are_event_bindings_valid = + std::all_of(generic_proxy_events.cbegin(), generic_proxy_events.cend(), [](const auto& element) { + return ProxyEventBaseView{element.second}.GetBinding() != nullptr; + }); + if (!are_event_bindings_valid) { ::score::mw::log::LogError("lola") << "Could not create GenericProxy as binding is invalid."; return MakeUnexpected(ComErrc::kBindingFailure); diff --git a/score/mw/com/impl/generic_proxy_event.cpp b/score/mw/com/impl/generic_proxy_event.cpp index da5499356..68ad3318b 100644 --- a/score/mw/com/impl/generic_proxy_event.cpp +++ b/score/mw/com/impl/generic_proxy_event.cpp @@ -25,12 +25,6 @@ GenericProxyEvent::GenericProxyEvent(ProxyBase& base, const std::string_view eve GenericProxyEventBindingFactory::Create(base, event_name), event_name} { - ProxyBaseView proxy_base_view{base}; - if (!binding_base_) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } GenericProxyEvent::GenericProxyEvent(ProxyBase& base, @@ -38,12 +32,6 @@ GenericProxyEvent::GenericProxyEvent(ProxyBase& base, const std::string_view event_name) : ProxyEventBase{base, ProxyBaseView{base}.GetBinding(), std::move(proxy_binding), event_name} { - ProxyBaseView proxy_base_view{base}; - if (!binding_base_) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } std::size_t GenericProxyEvent::GetSampleSize() const noexcept diff --git a/score/mw/com/impl/methods/proxy_method_base.h b/score/mw/com/impl/methods/proxy_method_base.h index 5e194bf00..86091ada6 100644 --- a/score/mw/com/impl/methods/proxy_method_base.h +++ b/score/mw/com/impl/methods/proxy_method_base.h @@ -27,9 +27,15 @@ namespace score::mw::com::impl { class ProxyBase; +class ProxyMethodBaseView; class ProxyMethodBase { + // Suppress "AUTOSAR C++14 A11-3-1", The rule states: "Friend declarations shall not be used". + // Design decision. This class provides a view to the private members of this class. + // coverity[autosar_cpp14_a11_3_1_violation] + friend class ProxyMethodBaseView; + public: ProxyMethodBase(ProxyBase& proxy_base, std::unique_ptr proxy_method_binding, @@ -97,6 +103,20 @@ class ProxyMethodBase std::unique_ptr binding_; }; +class ProxyMethodBaseView +{ + public: + explicit ProxyMethodBaseView(const ProxyMethodBase& base) : base_{base} {} + + const ProxyMethodBinding* GetMethodBinding() const noexcept + { + return base_.binding_.get(); + } + + private: + const ProxyMethodBase& base_; +}; + } // namespace score::mw::com::impl #endif // SCORE_MW_COM_IMPL_METHODS_PROXY_METHOD_BASE_H 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..da608454d 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 @@ -68,11 +68,6 @@ class ProxyMethod final : public ProxyMethodBase { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ~ProxyMethod() final = default; 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..78ef90eef 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 @@ -78,11 +78,6 @@ class ProxyMethod final : public ProxyMethodBase { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ProxyMethod(ProxyBase& proxy_base, @@ -93,11 +88,6 @@ class ProxyMethod final : public ProxyMethodBase { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ProxyMethod(ProxyBase& proxy_base, @@ -107,12 +97,6 @@ class ProxyMethod final : public ProxyMethodBase : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kSet), are_in_arg_ptrs_active_(kCallQueueSize) { - auto proxy_base_view = ProxyBaseView{proxy_base}; - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ~ProxyMethod() final = default; 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..c39fcfca7 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 @@ -70,11 +70,6 @@ class ProxyMethod final : public ProxyMethodBase { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ProxyMethod(ProxyBase& proxy_base, @@ -84,11 +79,6 @@ class ProxyMethod final : public ProxyMethodBase { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ProxyMethod(ProxyBase& proxy_base, @@ -97,12 +87,6 @@ class ProxyMethod final : public ProxyMethodBase FieldOnlyConstructorEnabler) noexcept : ProxyMethodBase(proxy_base, std::move(proxy_method_binding), method_name, MethodType::kGet) { - auto proxy_base_view = ProxyBaseView{proxy_base}; - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ~ProxyMethod() final = default; 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..7f82a0e5c 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 @@ -55,11 +55,6 @@ class ProxyMethod final : public ProxyMethodBase { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ProxyMethod(ProxyBase& proxy_base, @@ -69,11 +64,6 @@ class ProxyMethod final : public ProxyMethodBase { auto proxy_base_view = ProxyBaseView{proxy_base}; proxy_base_view.RegisterMethod(method_name_, *this); - if (binding_ == nullptr) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } ~ProxyMethod() final = default; diff --git a/score/mw/com/impl/proxy_base.cpp b/score/mw/com/impl/proxy_base.cpp index 26209a6b3..159d4819c 100644 --- a/score/mw/com/impl/proxy_base.cpp +++ b/score/mw/com/impl/proxy_base.cpp @@ -28,19 +28,13 @@ namespace score::mw::com::impl { ProxyBase::ProxyBase(std::unique_ptr proxy_binding, HandleType handle) - : proxy_binding_{std::move(proxy_binding)}, - handle_{std::move(handle)}, - are_service_element_bindings_valid_{true}, - events_{}, - fields_{}, - methods_{} + : proxy_binding_{std::move(proxy_binding)}, handle_{std::move(handle)}, events_{}, fields_{}, methods_{} { } ProxyBase::ProxyBase(ProxyBase&& other) noexcept : proxy_binding_(std::move(other.proxy_binding_)), handle_(std::move(other.handle_)), - are_service_element_bindings_valid_(other.are_service_element_bindings_valid_), events_(std::move(other.events_)), fields_(std::move(other.fields_)), methods_(std::move(other.methods_)) @@ -72,7 +66,6 @@ ProxyBase& ProxyBase::operator=(ProxyBase&& other) noexcept proxy_binding_ = std::move(other.proxy_binding_); handle_ = std::move(other.handle_); - are_service_element_bindings_valid_ = other.are_service_element_bindings_valid_; events_ = std::move(other.events_); fields_ = std::move(other.fields_); methods_ = std::move(other.methods_); @@ -158,6 +151,23 @@ score::Result ProxyBase::StopFindService(const FindServiceHandle handle) n return stop_find_service_result; } +bool ProxyBase::AreBindingsValid() const noexcept +{ + const bool is_proxy_binding_valid{proxy_binding_ != nullptr}; + + const bool are_event_bindings_valid = std::all_of(events_.begin(), events_.end(), [](const auto& element) { + return ProxyEventBaseView{element.second.get()}.GetBinding() != nullptr; + }); + const bool are_field_bindings_valid = std::all_of(fields_.begin(), fields_.end(), [](const auto& element) { + return ProxyFieldBaseView{element.second.get()}.GetEventBinding() != nullptr; + }); + const bool are_method_bindings_valid = std::all_of(methods_.begin(), methods_.end(), [](const auto& element) { + return ProxyMethodBaseView{element.second.get()}.GetMethodBinding() != nullptr; + }); + + return is_proxy_binding_valid && are_event_bindings_valid && are_field_bindings_valid && are_method_bindings_valid; +} + Result ProxyBase::SetupMethods() { const auto result = proxy_binding_->SetupMethods(); @@ -186,11 +196,6 @@ const HandleType& ProxyBaseView::GetAssociatedHandleType() const noexcept return proxy_base_.handle_; } -void ProxyBaseView::MarkServiceElementBindingInvalid() noexcept -{ - proxy_base_.are_service_element_bindings_valid_ = false; -} - void ProxyBaseView::RegisterEvent(const std::string_view event_name, ProxyEventBase& event) { const auto result = proxy_base_.events_.emplace(event_name, event); diff --git a/score/mw/com/impl/proxy_base.h b/score/mw/com/impl/proxy_base.h index 60e608832..db1c95d99 100644 --- a/score/mw/com/impl/proxy_base.h +++ b/score/mw/com/impl/proxy_base.h @@ -131,11 +131,7 @@ class ProxyBase ProxyBase(ProxyBase&& other) noexcept; ProxyBase& operator=(ProxyBase&& other) noexcept; - bool AreBindingsValid() const noexcept - { - const bool is_proxy_binding_valid{proxy_binding_ != nullptr}; - return is_proxy_binding_valid && are_service_element_bindings_valid_; - } + bool AreBindingsValid() const noexcept; Result SetupMethods(); @@ -147,8 +143,6 @@ class ProxyBase std::unique_ptr proxy_binding_; // coverity[autosar_cpp14_m11_0_1_violation] HandleType handle_; - // coverity[autosar_cpp14_m11_0_1_violation] - bool are_service_element_bindings_valid_; ProxyEvents events_; ProxyFields fields_; @@ -170,8 +164,6 @@ class ProxyBaseView final const HandleType& GetAssociatedHandleType() const noexcept; - void MarkServiceElementBindingInvalid() noexcept; - void RegisterEvent(const std::string_view event_name, ProxyEventBase& event); void RegisterField(const std::string_view field_name, ProxyFieldBase& field); diff --git a/score/mw/com/impl/proxy_event.h b/score/mw/com/impl/proxy_event.h index 38160790d..83b68a8fe 100644 --- a/score/mw/com/impl/proxy_event.h +++ b/score/mw/com/impl/proxy_event.h @@ -75,8 +75,7 @@ class ProxyEvent final : public ProxyEventBase /// Constructor which is dispatched to by the other public constructors. /// - /// Instantiates the base class and members of ProxyEvent and MarkServiceElementBindingInvalid() if the binding is - /// invalid. Should only be called directly in tests. + /// Instantiates the base class and members of ProxyEvent. Should only be called directly in tests. /// /// \param proxy_binding The binding that shall be associated with this proxy. ProxyEvent(ProxyBase& base, @@ -148,22 +147,16 @@ ProxyEvent::ProxyEvent(ProxyBase& base, : ProxyEventBase{base, ProxyBaseView{base}.GetBinding(), std::move(proxy_event_binding), event_name}, proxy_event_mock_{nullptr} { - ProxyBaseView proxy_base_view{base}; - if (!binding_base_) - { - proxy_base_view.MarkServiceElementBindingInvalid(); - return; - } } template ProxyEvent::ProxyEvent(ProxyBase& base, const std::string_view event_name) : ProxyEvent{base, ProxyEventBindingFactory::Create(base, event_name), event_name} { + ProxyBaseView proxy_base_view{base}; + proxy_base_view.RegisterEvent(event_name, *this); if (GetTypedEventBinding() != nullptr) { - ProxyBaseView proxy_base_view{base}; - proxy_base_view.RegisterEvent(event_name, *this); const auto& instance_identifier = proxy_base_view.GetAssociatedHandleType().GetInstanceIdentifier(); tracing_data_ = tracing::GenerateProxyTracingStructFromEventConfig(instance_identifier, event_name); } diff --git a/score/mw/com/impl/proxy_event_base.h b/score/mw/com/impl/proxy_event_base.h index 55ebcb83a..e59a546ad 100644 --- a/score/mw/com/impl/proxy_event_base.h +++ b/score/mw/com/impl/proxy_event_base.h @@ -35,6 +35,7 @@ namespace score::mw::com::impl class EventBindingRegistrationGuard; class ProxyBase; +class ProxyEventBaseView; /// \brief This is the user-visible class of an event that is part of a proxy. It contains ProxyEvent functionality that /// is agnostic of the data type that is transferred by the event. @@ -49,6 +50,11 @@ class ProxyEventBase // coverity[autosar_cpp14_a11_3_1_violation] friend class ProxyEventBaseAttorney; + // Suppress "AUTOSAR C++14 A11-3-1", The rule states: "Friend declarations shall not be used". + // Design decision. This class provides a view to the private members of this class. + // coverity[autosar_cpp14_a11_3_1_violation] + friend ProxyEventBaseView; + public: /// \brief Constructs a ProxyEventBase with the given proxy event binding. /// \param proxy_base The parent proxy of this event. ProxyEventBase just stores a reference to it and allows the @@ -238,6 +244,20 @@ class ProxyEventBase static thread_local bool is_in_receive_handler_context; }; +class ProxyEventBaseView +{ + public: + explicit ProxyEventBaseView(const ProxyEventBase& proxy_event_base) : proxy_event_base_{proxy_event_base} {} + + const ProxyEventBindingBase* GetBinding() const + { + return proxy_event_base_.binding_base_.get(); + } + + private: + const ProxyEventBase& proxy_event_base_; +}; + } // namespace score::mw::com::impl #endif // SCORE_MW_COM_IMPL_PROXY_EVENT_BASE_H diff --git a/score/mw/com/impl/proxy_event_base_test.cpp b/score/mw/com/impl/proxy_event_base_test.cpp index 6ec683ec8..bba7ef762 100644 --- a/score/mw/com/impl/proxy_event_base_test.cpp +++ b/score/mw/com/impl/proxy_event_base_test.cpp @@ -171,39 +171,6 @@ TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithValidEventBi EXPECT_TRUE(dummy_proxy.AreBindingsValid()); } -TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithInvalidEventBindingMarksProxyBindingInvalid) -{ - // Given a proxy with a valid binding - DummyProxy dummy_proxy(std::make_unique(), - 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); - - // Then the proxy should report that all bindings are not valid - EXPECT_FALSE(dummy_proxy.AreBindingsValid()); -} - -TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithInvalidProxyBindingMarksProxyBindingInvalid) -{ - auto valid_proxy_event_binding = - std::make_unique::MockServiceElementType>(); - - // Given a proxy with a valid binding - DummyProxy dummy_proxy(nullptr, - 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); - - // Then the proxy should report that all bindings are not valid - EXPECT_FALSE(dummy_proxy.AreBindingsValid()); -} - TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithValidProxyEventBindingWillRegisterAndUnregisterEventBinding) { diff --git a/score/mw/com/impl/proxy_field_base.h b/score/mw/com/impl/proxy_field_base.h index 88068b96e..c4473a12d 100644 --- a/score/mw/com/impl/proxy_field_base.h +++ b/score/mw/com/impl/proxy_field_base.h @@ -15,6 +15,7 @@ #include "score/mw/com/impl/proxy_event_base.h" +#include "score/mw/com/impl/proxy_event_binding_base.h" #include "score/result/result.h" #include @@ -27,8 +28,15 @@ namespace score::mw::com::impl { +class ProxyFieldBaseView; + class ProxyFieldBase { + // Suppress "AUTOSAR C++14 A11-3-1", The rule states: "Friend declarations shall not be used". + // Design decision. This class provides a view to the private members of this class. + // coverity[autosar_cpp14_a11_3_1_violation] + friend ProxyFieldBaseView; + public: ProxyFieldBase(ProxyBase& proxy_base, ProxyEventBase* proxy_event_base_dispatch, std::string_view field_name) : proxy_base_{proxy_base}, proxy_event_base_dispatch_{proxy_event_base_dispatch}, field_name_{field_name} @@ -169,6 +177,24 @@ class ProxyFieldBase std::string_view field_name_; }; +class ProxyFieldBaseView +{ + public: + explicit ProxyFieldBaseView(const ProxyFieldBase& proxy_field_base) : proxy_field_base_{proxy_field_base} {} + + const ProxyEventBindingBase* GetEventBinding() const noexcept + { + if (proxy_field_base_.proxy_event_base_dispatch_ == nullptr) + { + return nullptr; + } + return ProxyEventBaseView{*proxy_field_base_.proxy_event_base_dispatch_}.GetBinding(); + } + + private: + const ProxyFieldBase& proxy_field_base_; +}; + } // namespace score::mw::com::impl #endif // SCORE_MW_COM_IMPL_PROXY_FIELD_BASE_H From d496d88a60e57412d0dcd7af62036c8580d64625 Mon Sep 17 00:00:00 2001 From: Brendan Emery Date: Wed, 13 May 2026 13:02:30 +0200 Subject: [PATCH 2/2] mw/com: Register Event binding on binding level Previously, the event binding was registered from the binding independent level via a function in the bindings interface. We change this so that the registration is done completely on the binding level to simplify the interface and also to allow each binding to decide whether it needs to register events with the parent proxy. This is now inline with how we handle registration for methods. This also fixes an issue that the EventRegistrationGuard was accessing the Proxy binding from the context of a ProxyEvent's destructor. When move assigning a skeleton, the ProxyBase move assignment operator will be called before that of the ProxyEvent, so the pointer to the Proxy binding would be invalid when calling UnregisterEventBinding. --- .../bindings/lola/generic_proxy_event.cpp | 1 + score/mw/com/impl/bindings/lola/proxy.cpp | 36 ++++------- score/mw/com/impl/bindings/lola/proxy.h | 22 +------ score/mw/com/impl/bindings/lola/proxy_event.h | 1 + .../impl/bindings/lola/proxy_event_test.cpp | 35 +++++++++++ .../mw/com/impl/bindings/lola/proxy_test.cpp | 59 ++++--------------- .../lola/test/proxy_event_test_resources.cpp | 14 ++++- .../mw/com/impl/bindings/mock_binding/proxy.h | 14 +---- score/mw/com/impl/proxy_binding.h | 7 --- score/mw/com/impl/proxy_binding_test.cpp | 2 - score/mw/com/impl/proxy_event_base.cpp | 41 ------------- score/mw/com/impl/proxy_event_base.h | 3 - score/mw/com/impl/proxy_event_base_test.cpp | 45 -------------- 13 files changed, 77 insertions(+), 203 deletions(-) diff --git a/score/mw/com/impl/bindings/lola/generic_proxy_event.cpp b/score/mw/com/impl/bindings/lola/generic_proxy_event.cpp index ca5a39903..46a7c2751 100644 --- a/score/mw/com/impl/bindings/lola/generic_proxy_event.cpp +++ b/score/mw/com/impl/bindings/lola/generic_proxy_event.cpp @@ -31,6 +31,7 @@ GenericProxyEvent::GenericProxyEvent(Proxy& parent, const ElementFqId element_fq proxy_event_common_{parent, element_fq_id, event_name}, meta_info_{parent.GetEventMetaInfo(element_fq_id)} { + parent.RegisterEvent(event_name, *this); } Result GenericProxyEvent::Subscribe(const std::size_t max_sample_count) noexcept diff --git a/score/mw/com/impl/bindings/lola/proxy.cpp b/score/mw/com/impl/bindings/lola/proxy.cpp index e8cb56091..6fee41ca7 100644 --- a/score/mw/com/impl/bindings/lola/proxy.cpp +++ b/score/mw/com/impl/bindings/lola/proxy.cpp @@ -639,32 +639,6 @@ bool Proxy::IsEventProvided(const std::string_view event_name) const noexcept return event_exists; } -void Proxy::RegisterEventBinding(const std::string_view service_element_name, - ProxyEventBindingBase& proxy_event_binding) noexcept -{ - // Suppress Autosar C++14 A8-5-3 states that auto variables shall not be initialized using braced initialization. - // This is a false positive, we don't use auto here - // coverity[autosar_cpp14_a8_5_3_violation : FALSE] - std::lock_guard lock{proxy_event_registration_mutex_}; - const auto insert_result = event_bindings_.emplace(service_element_name, proxy_event_binding); - SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(insert_result.second, - "Failed to insert proxy event binding into event binding map."); - proxy_event_binding.NotifyServiceInstanceChangedAvailability(is_service_instance_available_, GetSourcePid()); -} - -void Proxy::UnregisterEventBinding(const std::string_view service_element_name) noexcept -{ - // Suppress Autosar C++14 A8-5-3 states that auto variables shall not be initialized using braced initialization. - // This is a false positive, we don't use auto here - // coverity[autosar_cpp14_a8_5_3_violation : FALSE] - std::lock_guard lock{proxy_event_registration_mutex_}; - const auto number_of_elements_removed = event_bindings_.erase(service_element_name); - if (number_of_elements_removed == 0U) - { - score::mw::log::LogWarn("lola") << "UnregisterEventBinding that was never registered. Ignoring."; - } -} - score::Result Proxy::SetupMethods() { auto enabled_method_data = GetMethodIdAndQueueSizeForEnabledMethods(); @@ -921,6 +895,16 @@ pid_t Proxy::GetSourcePid() const noexcept return service_data_storage.skeleton_pid_; } +void Proxy::RegisterEvent(const std::string_view service_element_name, + ProxyEventBindingBase& proxy_event_binding) noexcept +{ + std::lock_guard lock{proxy_event_registration_mutex_}; + const auto insert_result = event_bindings_.emplace(service_element_name, proxy_event_binding); + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(insert_result.second, + "Failed to insert proxy event binding into event binding map."); + proxy_event_binding.NotifyServiceInstanceChangedAvailability(is_service_instance_available_, GetSourcePid()); +} + void Proxy::RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& proxy_method) noexcept { std::lock_guard lock{proxy_method_registration_mutex_}; diff --git a/score/mw/com/impl/bindings/lola/proxy.h b/score/mw/com/impl/bindings/lola/proxy.h index c4cca4320..5cc7b26d1 100644 --- a/score/mw/com/impl/bindings/lola/proxy.h +++ b/score/mw/com/impl/bindings/lola/proxy.h @@ -167,24 +167,6 @@ class Proxy : public ProxyBinding /// \return True if the event name exists, otherwise, false bool IsEventProvided(const std::string_view event_name) const noexcept override; - /// \brief Adds a reference to a Proxy service element binding to an internal map - /// - /// Will insert the provided ProxyEventBindingBase& into a map stored within the class which will be used to call - /// NotifyServiceInstanceChangedAvailability on all saved Proxy service elements by the FindServiceHandler of - /// find_service_guard_. It will then call NotifyServiceInstanceChangedAvailability on the provided - /// ProxyEventBindingBase. Since this function first locks proxy_event_registration_mutex_, it is ensured that the - /// provided Proxy service element will be notified synchronously about the availability of the provider and will - /// then be notified of any future changes via the callback, without missing any notifications. - void RegisterEventBinding(const std::string_view service_element_name, - ProxyEventBindingBase& proxy_event_binding) noexcept override; - - /// \brief Removes the reference to a Proxy service element binding from an internal map - /// - /// This must be called by a Proxy service element before destructing to ensure that the FindService handler in - /// find_service_guard_ does not call NotifyServiceInstanceChangedAvailability on a Proxy service element after it's - /// been destructed. - void UnregisterEventBinding(const std::string_view service_element_name) noexcept override; - score::Result SetupMethods() override; QualityType GetQualityType() const noexcept; @@ -198,6 +180,8 @@ class Proxy : public ProxyBinding return proxy_instance_identifier_; } + void RegisterEvent(const std::string_view service_element_name, + ProxyEventBindingBase& proxy_event_binding) noexcept; void RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& proxy_method) noexcept; private: @@ -231,7 +215,7 @@ class Proxy : public ProxyBinding HandleType handle_; std::unordered_map> event_bindings_; - /// Mutex which synchronises registration of Proxy service elements via Proxy::RegisterEventBinding with the + /// Mutex which synchronises registration of Proxy service elements via Proxy::RegisterEvent with the /// FindServiceHandler in find_service_guard_ which will call NotifyServiceInstanceChangedAvailability on all /// currently registered Proxy service elements. std::mutex proxy_event_registration_mutex_; diff --git a/score/mw/com/impl/bindings/lola/proxy_event.h b/score/mw/com/impl/bindings/lola/proxy_event.h index 73c3e4bc2..be37fc505 100644 --- a/score/mw/com/impl/bindings/lola/proxy_event.h +++ b/score/mw/com/impl/bindings/lola/proxy_event.h @@ -66,6 +66,7 @@ class ProxyEvent final : public ProxyEventBinding proxy_event_common_{parent, element_fq_id, event_name}, samples_{parent.GetEventDataStorage(element_fq_id)} { + parent.RegisterEvent(event_name, *this); } ProxyEvent(const ProxyEvent&) = delete; diff --git a/score/mw/com/impl/bindings/lola/proxy_event_test.cpp b/score/mw/com/impl/bindings/lola/proxy_event_test.cpp index 06aea0d11..9f9921134 100644 --- a/score/mw/com/impl/bindings/lola/proxy_event_test.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_event_test.cpp @@ -32,6 +32,21 @@ namespace score::mw::com::impl::lola { + +class ProxyTestAttorney +{ + public: + explicit ProxyTestAttorney(Proxy& proxy) : proxy_{proxy} {} + + const std::unordered_map>& GetEvents() const + { + return proxy_.event_bindings_; + } + + private: + Proxy& proxy_; +}; + namespace { @@ -152,6 +167,10 @@ class LolaProxyEventFixture : public LolaProxyEventResources using MyTypes = ::testing::Types; TYPED_TEST_SUITE(LolaProxyEventFixture, MyTypes, ); +template +using LolaProxyEventConstructionFixture = LolaProxyEventFixture; +TYPED_TEST_SUITE(LolaProxyEventConstructionFixture, MyTypes, ); + template using LolaProxyEventGetNewSamplesFixture = LolaProxyEventFixture; TYPED_TEST_SUITE(LolaProxyEventGetNewSamplesFixture, MyTypes, ); @@ -164,6 +183,22 @@ template using LolaProxyEventDeathTest = LolaProxyEventFixture; TYPED_TEST_SUITE(LolaProxyEventDeathTest, MyTypes, ); +TYPED_TEST(LolaProxyEventConstructionFixture, ConstructingRegistersEventWithParent) +{ + // Given a proxy with an events map which is initially empty + auto& events_map = ProxyTestAttorney(*this->proxy_).GetEvents(); + ASSERT_EQ(events_map.size(), 0U); + + // When constructing a ProxyEvent + this->GivenAProxyEvent(this->element_fq_id_, this->event_name_); + + // Then the event should have been registered in the parent Proxy's map of events + EXPECT_EQ(events_map.size(), 1U); + auto registered_event_it = events_map.find(this->event_name_); + EXPECT_NE(registered_event_it, events_map.end()); + EXPECT_EQ(®istered_event_it->second.get(), &(*this->test_proxy_event_)); +} + TYPED_TEST(LolaProxyEventGetNewSamplesFixture, CallsReceiverForEachAccessibleSample) { this->RecordProperty("Verifies", "SCR-14035773, SCR-21350367, SCR-6225206"); diff --git a/score/mw/com/impl/bindings/lola/proxy_test.cpp b/score/mw/com/impl/bindings/lola/proxy_test.cpp index aa48abea2..48e05cfbe 100644 --- a/score/mw/com/impl/bindings/lola/proxy_test.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_test.cpp @@ -427,14 +427,14 @@ TEST_F(ProxyAutoReconnectFixture, WhenStopFindServiceReturnsErrorOnProxyDestruct // Then the program does not terminate } -TEST_F(ProxyAutoReconnectFixture, RegisterEventBindingCallsNotifyOnEventWithFalseWhenProviderInitiallyDoesNotExist) +TEST_F(ProxyAutoReconnectFixture, RegisterEventCallsNotifyOnEventWithFalseWhenProviderInitiallyDoesNotExist) { const auto valid_find_service_handle = make_FindServiceHandle(10U); mock_binding::ProxyEvent proxy_event{}; // Expecting that StartFindService is called but the handler is not called since the provider does not exist - ON_CALL(service_discovery_mock_, StartFindService(_, EnrichedInstanceIdentifier{identifier_})) - .WillByDefault(Return(valid_find_service_handle)); + EXPECT_CALL(service_discovery_mock_, StartFindService(_, EnrichedInstanceIdentifier{identifier_})) + .WillOnce(Return(valid_find_service_handle)); // Then expecting that NotifyServiceInstanceChangedAvailability is called on the event with is_available false const bool is_available{false}; @@ -445,10 +445,10 @@ TEST_F(ProxyAutoReconnectFixture, RegisterEventBindingCallsNotifyOnEventWithFals EXPECT_NE(proxy_, nullptr); // and the ProxyEvent registers itself with the Proxy - proxy_->RegisterEventBinding("Event0", proxy_event); + proxy_->RegisterEvent("Event0", proxy_event); } -TEST_F(ProxyAutoReconnectFixture, RegisterEventBindingCallsNotifyOnEventWithTrueWhenProviderInitiallyExists) +TEST_F(ProxyAutoReconnectFixture, RegisterEventCallsNotifyOnEventWithTrueWhenProviderInitiallyExists) { const auto valid_find_service_handle = make_FindServiceHandle(10U); mock_binding::ProxyEvent proxy_event{}; @@ -470,10 +470,10 @@ TEST_F(ProxyAutoReconnectFixture, RegisterEventBindingCallsNotifyOnEventWithTrue EXPECT_NE(proxy_, nullptr); // and the ProxyEvent registers itself with the Proxy - proxy_->RegisterEventBinding("Event0", proxy_event); + proxy_->RegisterEvent("Event0", proxy_event); } -TEST_F(ProxyAutoReconnectFixture, RegisterEventBindingCallsNotifyOnEventWithLatestValueFromFindServiceHandler) +TEST_F(ProxyAutoReconnectFixture, RegisterEventCallsNotifyOnEventWithLatestValueFromFindServiceHandler) { const auto valid_find_service_handle = make_FindServiceHandle(10U); mock_binding::ProxyEvent proxy_event_0{}; @@ -523,14 +523,14 @@ TEST_F(ProxyAutoReconnectFixture, RegisterEventBindingCallsNotifyOnEventWithLate EXPECT_NE(proxy_, nullptr); // and the first ProxyEvent registers itself with the Proxy - proxy_->RegisterEventBinding("Event0", proxy_event_0); + proxy_->RegisterEvent("Event0", proxy_event_0); // And then the FindService handler is called with an empty service handle container ServiceHandleContainer empty_service_handle_container{}; saved_find_service_handler(empty_service_handle_container, valid_find_service_handle); // and the second ProxyEvent registers itself with the Proxy - proxy_->RegisterEventBinding("Event1", proxy_event_1); + proxy_->RegisterEvent("Event1", proxy_event_1); // And then the FindService handler is called again with a non-empty service handle container ServiceHandleContainer filled_service_handle_container{make_HandleType(identifier_)}; @@ -562,8 +562,8 @@ TEST_F(ProxyEventBindingFixture, RegisteringEventBindingWillCallNotifyServiceIns // Expecting that NotifyServiceInstanceChangedAvailability will be called on the binding EXPECT_CALL(mock_proxy_event_base_binding, NotifyServiceInstanceChangedAvailability(_, _)); - // When calling RegisterEventBinding - proxy_->RegisterEventBinding(kDummyEventName, mock_proxy_event_base_binding); + // When calling RegisterEvent + proxy_->RegisterEvent(kDummyEventName, mock_proxy_event_base_binding); } TEST_F(ProxyEventBindingFixture, @@ -582,47 +582,14 @@ TEST_F(ProxyEventBindingFixture, InitialiseProxyWithCreate(identifier_); // and that two bindings are registered - proxy_->RegisterEventBinding(kDummyEventName, mock_proxy_event_base_binding); - proxy_->RegisterEventBinding("some_other_event", mock_proxy_event_base_binding_2); - - // When the find service handler is called - const auto find_service_handler = find_service_handler_promise_.get_future().get(); - find_service_handler(ServiceHandleContainer{}, kDummyFindServiceHandle); -} - -TEST_F(ProxyEventBindingFixture, - UnregisteringEventBindingAfterRegisteringWillMakeBindingUnavailableToServiceAvailabilityChangeHandler) -{ - mock_binding::ProxyEventBase mock_proxy_event_base_binding{}; - - // Expecting that NotifyServiceInstanceChangedAvailability will only be called on the binding once when it's - // registered and not again when the find service handler is called - EXPECT_CALL(mock_proxy_event_base_binding, NotifyServiceInstanceChangedAvailability(_, _)); - - // Given a constructed Proxy which registered an event binding - WhichCapturesFindServiceHandler(identifier_); - InitialiseProxyWithCreate(identifier_); - proxy_->RegisterEventBinding(kDummyEventName, mock_proxy_event_base_binding); - - // and then unregistered the event binding - proxy_->UnregisterEventBinding(kDummyEventName); + proxy_->RegisterEvent(kDummyEventName, mock_proxy_event_base_binding); + proxy_->RegisterEvent("some_other_event", mock_proxy_event_base_binding_2); // When the find service handler is called const auto find_service_handler = find_service_handler_promise_.get_future().get(); find_service_handler(ServiceHandleContainer{}, kDummyFindServiceHandle); } -TEST_F(ProxyEventBindingFixture, UnregisteringEventBindingBeforeRegisteringWillNotTerminate) -{ - // Given a constructed Proxy - InitialiseProxyWithCreate(identifier_); - - // When calling UnregisterEventBinding when RegisterEventBinding was never called - proxy_->UnregisterEventBinding(kDummyEventName); - - // Then we don't terminate -} - using ProxyGetEventMetaInfoFixture = ProxyMockedMemoryFixture; TEST_F(ProxyGetEventMetaInfoFixture, GetEventMetaInfoWillReturnDataForEventThatWasCreatedBySkeleton) { diff --git a/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp b/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp index 884d739c8..795c46311 100644 --- a/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp +++ b/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp @@ -13,6 +13,7 @@ #include "score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.h" #include "score/memory/shared/memory_resource_registry.h" +#include "score/mw/com/impl/find_service_handle.h" #include #include @@ -95,7 +96,18 @@ void ProxyMockedMemoryFixture::InitialiseProxyWithConstructor(const InstanceIden { EnrichedInstanceIdentifier enriched_instance_identifier{instance_identifier}; ON_CALL(service_discovery_mock_, StartFindService(_, enriched_instance_identifier)) - .WillByDefault(Return(make_FindServiceHandle(10U))); + .WillByDefault( + testing::WithArg<0>(testing::Invoke([this](auto find_service_handler) -> Result { + // In practice, if a service is available at the point in time when the Proxy is created then the find + // service handler will be called synchronously in the StartFindService call. We simulate this here by + // calling the handler with a handle. + auto find_service_handle = make_FindServiceHandle(10U); + auto handle = make_HandleType(identifier_); + auto found_service_handle_container = ServiceHandleContainer{handle}; + + std::invoke(find_service_handler, found_service_handle_container, find_service_handle); + return find_service_handle; + }))); Proxy::EventNameToElementFqIdConverter event_name_to_element_fq_id_converter{lola_service_deployment_, lola_service_instance_id_.GetId()}; diff --git a/score/mw/com/impl/bindings/mock_binding/proxy.h b/score/mw/com/impl/bindings/mock_binding/proxy.h index 812581d18..69a358950 100644 --- a/score/mw/com/impl/bindings/mock_binding/proxy.h +++ b/score/mw/com/impl/bindings/mock_binding/proxy.h @@ -29,8 +29,6 @@ class Proxy : public ProxyBinding ~Proxy() override = default; MOCK_METHOD(bool, IsEventProvided, (const std::string_view), (const, noexcept, override)); - MOCK_METHOD(void, RegisterEventBinding, (std::string_view, ProxyEventBindingBase&), (noexcept, override)); - MOCK_METHOD(void, UnregisterEventBinding, (std::string_view), (noexcept, override)); MOCK_METHOD(Result, SetupMethods, (), (override)); }; @@ -45,17 +43,6 @@ class ProxyFacade : public ProxyBinding return proxy_.IsEventProvided(event_name); } - void RegisterEventBinding(const std::string_view service_element_name, - ProxyEventBindingBase& proxy_event_binding) noexcept override - { - proxy_.RegisterEventBinding(service_element_name, proxy_event_binding); - } - - void UnregisterEventBinding(const std::string_view service_element_name) noexcept override - { - proxy_.UnregisterEventBinding(service_element_name); - } - Result SetupMethods() override { return proxy_.SetupMethods(); @@ -64,6 +51,7 @@ class ProxyFacade : public ProxyBinding private: Proxy& proxy_; }; + } // namespace score::mw::com::impl::mock_binding #endif // SCORE_MW_COM_IMPL_BINDINGS_MOCK_BINDING_PROXY_H diff --git a/score/mw/com/impl/proxy_binding.h b/score/mw/com/impl/proxy_binding.h index c24524d36..cd5d50977 100644 --- a/score/mw/com/impl/proxy_binding.h +++ b/score/mw/com/impl/proxy_binding.h @@ -54,13 +54,6 @@ class ProxyBinding /// \return True if the event name exists, otherwise, false virtual bool IsEventProvided(const std::string_view event_name) const noexcept = 0; - /// Registers a ProxyEvent binding with its parent proxy - virtual void RegisterEventBinding(const std::string_view service_element_name, - ProxyEventBindingBase& proxy_event_binding) noexcept = 0; - - /// Unregisters a ProxyEvent binding with its parent proxy - virtual void UnregisterEventBinding(const std::string_view service_element_name) noexcept = 0; - virtual Result SetupMethods() = 0; }; diff --git a/score/mw/com/impl/proxy_binding_test.cpp b/score/mw/com/impl/proxy_binding_test.cpp index fd7ba4dd2..b5cd4dc2c 100644 --- a/score/mw/com/impl/proxy_binding_test.cpp +++ b/score/mw/com/impl/proxy_binding_test.cpp @@ -29,8 +29,6 @@ class MyProxy final : public ProxyBinding { return true; } - void RegisterEventBinding(std::string_view, ProxyEventBindingBase&) noexcept override {} - void UnregisterEventBinding(std::string_view) noexcept override {} Result SetupMethods() override { return {}; diff --git a/score/mw/com/impl/proxy_event_base.cpp b/score/mw/com/impl/proxy_event_base.cpp index faa9f8650..e32ce61dd 100644 --- a/score/mw/com/impl/proxy_event_base.cpp +++ b/score/mw/com/impl/proxy_event_base.cpp @@ -33,45 +33,6 @@ namespace score::mw::com::impl // Initialization of static thread_local variables! thread_local bool ProxyEventBase::is_in_receive_handler_context = false; -/// \brief Helper class which registers the ProxyEventBindingBase with its parent proxy (ProxyBinding) and unregisters -/// on destruction -/// -/// Since ProxyBase is moveable, we must ensure that this class does not store a reference or pointer to it. As if the -/// Proxy is moved, then the pointer or reference would be invalidated. However, the ProxyBinding is not moveable, so -/// storing a pointer to the ProxyBinding is safe. -class EventBindingRegistrationGuard final -{ - public: - EventBindingRegistrationGuard(ProxyBinding* proxy_binding, - ProxyEventBindingBase* proxy_event_binding_base, - std::string_view event_name) noexcept - : proxy_binding_{proxy_binding}, proxy_event_binding_base_{proxy_event_binding_base}, event_name_{event_name} - { - if ((proxy_binding_ != nullptr) && (proxy_event_binding_base_ != nullptr)) - { - proxy_binding_->RegisterEventBinding(event_name, *proxy_event_binding_base); - } - } - - ~EventBindingRegistrationGuard() noexcept - { - if ((proxy_binding_ != nullptr) && (proxy_event_binding_base_ != nullptr)) - { - proxy_binding_->UnregisterEventBinding(event_name_); - } - } - - EventBindingRegistrationGuard(const EventBindingRegistrationGuard&) = delete; - EventBindingRegistrationGuard& operator=(const EventBindingRegistrationGuard&) = delete; - EventBindingRegistrationGuard(EventBindingRegistrationGuard&& other) = delete; - EventBindingRegistrationGuard& operator=(EventBindingRegistrationGuard&& other) = delete; - - private: - ProxyBinding* proxy_binding_; - ProxyEventBindingBase* proxy_event_binding_base_; - std::string_view event_name_; -}; - // Suppress "AUTOSAR C++14 A3-1-1", The rule states: "It shall be possible to include any header file // in multiple translation units without violating the One Definition Rule." // This is false positive. Function is declared only once. @@ -85,8 +46,6 @@ ProxyEventBase::ProxyEventBase(ProxyBase& proxy_base, event_name_{event_name}, tracker_{std::make_unique()}, tracing_data_{}, - event_binding_registration_guard_{ - std::make_unique(proxy_binding_ptr, binding_base_.get(), event_name)}, proxy_event_base_mock_{nullptr}, receive_handler_scope_{} { diff --git a/score/mw/com/impl/proxy_event_base.h b/score/mw/com/impl/proxy_event_base.h index e59a546ad..07483faf9 100644 --- a/score/mw/com/impl/proxy_event_base.h +++ b/score/mw/com/impl/proxy_event_base.h @@ -33,7 +33,6 @@ namespace score::mw::com::impl { -class EventBindingRegistrationGuard; class ProxyBase; class ProxyEventBaseView; @@ -216,8 +215,6 @@ class ProxyEventBase std::unique_ptr tracker_; // coverity[autosar_cpp14_m11_0_1_violation] tracing::ProxyEventTracingData tracing_data_; - // coverity[autosar_cpp14_m11_0_1_violation] - std::unique_ptr event_binding_registration_guard_; IProxyEventBase* proxy_event_base_mock_; diff --git a/score/mw/com/impl/proxy_event_base_test.cpp b/score/mw/com/impl/proxy_event_base_test.cpp index bba7ef762..9e80e2d7c 100644 --- a/score/mw/com/impl/proxy_event_base_test.cpp +++ b/score/mw/com/impl/proxy_event_base_test.cpp @@ -171,51 +171,6 @@ TYPED_TEST(ProxyEventBaseCreationFixture, CreatingServiceElementWithValidEventBi EXPECT_TRUE(dummy_proxy.AreBindingsValid()); } -TYPED_TEST(ProxyEventBaseCreationFixture, - CreatingServiceElementWithValidProxyEventBindingWillRegisterAndUnregisterEventBinding) -{ - auto valid_proxy_binding = std::make_unique(); - auto valid_proxy_event_binding = - std::make_unique::MockServiceElementType>(); - - auto* const mock_proxy_binding = valid_proxy_binding.get(); - ProxyEventBindingBase* const mock_proxy_event_binding = valid_proxy_event_binding.get(); - - // Expecting that the ProxyEvent will register and unregister itself with the parent Proxy - EXPECT_CALL(*mock_proxy_binding, RegisterEventBinding(kEventName, Ref(*mock_proxy_event_binding))); - EXPECT_CALL(*mock_proxy_binding, UnregisterEventBinding(kEventName)); - - // Given a proxy with a valid binding - DummyProxy dummy_proxy(std::move(valid_proxy_binding), - 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); -} - -TYPED_TEST(ProxyEventBaseCreationFixture, - CreatingServiceElementWithInvalidProxyEventBindingWillNotRegisterOrUnregisterEventBinding) -{ - auto valid_proxy_binding = std::make_unique(); - - auto* const mock_proxy_binding = valid_proxy_binding.get(); - - // Expecting that the ProxyEvent will not register itself with the parent Proxy - EXPECT_CALL(*mock_proxy_binding, RegisterEventBinding(_, _)).Times(0); - EXPECT_CALL(*mock_proxy_binding, UnregisterEventBinding(_)).Times(0); - - // Given a proxy with a valid binding - DummyProxy dummy_proxy(std::move(valid_proxy_binding), - 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); -} - TYPED_TEST(ProxyEventBaseUnsubscribeFixture, CallingUnsubscribeWhileSubscribedCallsUnsubscribeOnBinding) { this->RecordProperty("Verifies", "SCR-14033377, SCR-17292399, SCR-14137271, SCR-21286218");