From c469b48e52d6899f0604ca8843b684f89a60f4e3 Mon Sep 17 00:00:00 2001 From: manuelfehlhammer Date: Wed, 27 May 2026 10:55:47 +0200 Subject: [PATCH 1/2] Introduce ServiceElementMapView Up to now GenericProxy and GenericSkeleton provided inconsistent views for their contained events. This commit now introduces a common solution with ServiceElementMapView, which solves both issues: - map itself is immutable (no addition/removal of events) - events within the maps are mutable Issue: #467 --- score/mw/com/impl/BUILD | 31 +++- score/mw/com/impl/generic_proxy.cpp | 18 +-- score/mw/com/impl/generic_proxy.h | 24 ++-- score/mw/com/impl/generic_proxy_test.cpp | 53 +++++-- score/mw/com/impl/generic_skeleton.cpp | 12 +- score/mw/com/impl/generic_skeleton.h | 39 ++--- score/mw/com/impl/service_element_map.h | 98 ------------- .../mw/com/impl/service_element_map_test.cpp | 126 ----------------- ...t_map.cpp => service_element_map_view.cpp} | 7 +- score/mw/com/impl/service_element_map_view.h | 93 ++++++++++++ .../impl/service_element_map_view_factory.cpp | 14 ++ .../impl/service_element_map_view_factory.h | 45 ++++++ .../impl/service_element_map_view_test.cpp | 133 ++++++++++++++++++ 13 files changed, 401 insertions(+), 292 deletions(-) delete mode 100644 score/mw/com/impl/service_element_map.h delete mode 100644 score/mw/com/impl/service_element_map_test.cpp rename score/mw/com/impl/{service_element_map.cpp => service_element_map_view.cpp} (78%) create mode 100644 score/mw/com/impl/service_element_map_view.h create mode 100644 score/mw/com/impl/service_element_map_view_factory.cpp create mode 100644 score/mw/com/impl/service_element_map_view_factory.h create mode 100644 score/mw/com/impl/service_element_map_view_test.cpp diff --git a/score/mw/com/impl/BUILD b/score/mw/com/impl/BUILD index 29037f276..bdeacb902 100644 --- a/score/mw/com/impl/BUILD +++ b/score/mw/com/impl/BUILD @@ -98,7 +98,8 @@ cc_library( ":handle_type", ":proxy_base", ":proxy_binding", - ":service_element_map", + ":service_element_map_view", + ":service_element_map_view_factory", "@score_baselibs//score/result", ], ) @@ -170,7 +171,8 @@ cc_library( ":instance_identifier", ":instance_specifier", ":runtime", - ":service_element_map", + ":service_element_map_view", + ":service_element_map_view_factory", ":skeleton_base", ":skeleton_binding", "@score_baselibs//score/result", @@ -218,6 +220,7 @@ cc_library( cc_library( name = "data_type_meta_info", + srcs = ["data_type_meta_info.cpp"], hdrs = ["data_type_meta_info.h"], features = COMPILER_WARNING_FEATURES, tags = ["FFI"], @@ -970,9 +973,23 @@ cc_library( ) cc_library( - name = "service_element_map", - srcs = ["service_element_map.cpp"], - hdrs = ["service_element_map.h"], + name = "service_element_map_view_factory", + srcs = ["service_element_map_view_factory.cpp"], + hdrs = ["service_element_map_view_factory.h"], + features = COMPILER_WARNING_FEATURES, + tags = ["FFI"], + visibility = [ + "//score/mw/com/impl:__subpackages__", + ], + deps = [ + ":service_element_map_view", + ], +) + +cc_library( + name = "service_element_map_view", + srcs = ["service_element_map_view.cpp"], + hdrs = ["service_element_map_view.h"], features = COMPILER_WARNING_FEATURES, tags = ["FFI"], visibility = [ @@ -1287,7 +1304,7 @@ cc_unit_test( "proxy_event_base_test.cpp", "proxy_event_binding_base_test.cpp", "proxy_event_test.cpp", - "service_element_map_test.cpp", + "service_element_map_view_test.cpp", "skeleton_binding_test.cpp", "skeleton_event_base_test.cpp", "skeleton_event_binding_test.cpp", @@ -1303,7 +1320,7 @@ cc_unit_test( ":impl", ":runtime_mock", ":service_discovery_mock", - ":service_element_map", + ":service_element_map_view", "//score/mw/com:types", "//score/mw/com/impl/bindings/lola:runtime_mock", "//score/mw/com/impl/bindings/lola/test_doubles", diff --git a/score/mw/com/impl/generic_proxy.cpp b/score/mw/com/impl/generic_proxy.cpp index 3e2091287..659d764b5 100644 --- a/score/mw/com/impl/generic_proxy.cpp +++ b/score/mw/com/impl/generic_proxy.cpp @@ -10,14 +10,12 @@ * * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ -/// -/// @file -/// @copyright Copyright (C) 2023, Bayerische Motoren Werke Aktiengesellschaft (BMW AG) -/// + #include "score/mw/com/impl/generic_proxy.h" #include "score/mw/com/impl/com_error.h" #include "score/mw/com/impl/plumbing/proxy_binding_factory.h" +#include "score/mw/com/impl/service_element_map_view_factory.h" #include "score/mw/log/logging.h" @@ -86,7 +84,8 @@ Result GenericProxy::Create(HandleType instance_handle) noexcept } GenericProxy::GenericProxy(std::unique_ptr proxy_binding, HandleType instance_handle) - : ProxyBase{std::move(proxy_binding), std::move(instance_handle)} + : ProxyBase{std::move(proxy_binding), std::move(instance_handle)}, + events_(std::make_unique>()) { } @@ -96,7 +95,7 @@ void GenericProxy::FillEventMap(const std::vector& event_names { if (proxy_binding_->IsEventProvided(event_name)) { - const auto emplace_result = events_.emplace( + const auto emplace_result = events_->emplace( std::piecewise_construct, std::forward_as_tuple(event_name), std::forward_as_tuple(*this, event_name)); SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(emplace_result.second, "Could not emplace GenericProxyEvent in map."); @@ -110,12 +109,9 @@ void GenericProxy::FillEventMap(const std::vector& event_names } } -GenericProxy::EventMap& GenericProxy::GetEvents() noexcept +GenericProxy::EventMapView GenericProxy::GetEvents() const noexcept { - // The signature of this function is part of the public API of the GenericProxy, specified in this requirement: - // broken_link_c/issue/14006006 - // coverity[autosar_cpp14_a9_3_1_violation] - return events_; + return ServiceElementMapViewFactory::Create(*events_); } } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/generic_proxy.h b/score/mw/com/impl/generic_proxy.h index afd71027c..3c0f87f78 100644 --- a/score/mw/com/impl/generic_proxy.h +++ b/score/mw/com/impl/generic_proxy.h @@ -10,10 +10,6 @@ * * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ -/// -/// @file -/// @copyright Copyright (C) 2023, Bayerische Motoren Werke Aktiengesellschaft (BMW AG) -/// #ifndef SCORE_MW_COM_IMPL_GENERIC_PROXY_H #define SCORE_MW_COM_IMPL_GENERIC_PROXY_H @@ -22,7 +18,8 @@ #include "score/mw/com/impl/handle_type.h" #include "score/mw/com/impl/proxy_base.h" #include "score/mw/com/impl/proxy_binding.h" -#include "score/mw/com/impl/service_element_map.h" +#include "score/mw/com/impl/service_element_map_view.h" +#include "score/mw/com/impl/service_element_map_view_factory.h" #include "score/result/result.h" @@ -53,13 +50,13 @@ class GenericProxyAttorney; class GenericProxy : public ProxyBase { // Suppress "AUTOSAR C++14 A11-3-1", The rule declares: "Friend declarations shall not be used". - // Design dessision: The "*Attorney" class is a helper, which sets the internal state of this class accessing + // 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 test::GenericProxyAttorney; public: - using EventMap = ServiceElementMap; + using EventMapView = ServiceElementMapView; /** * \api @@ -71,17 +68,22 @@ class GenericProxy : public ProxyBase /** * \api - * \brief Returns a reference to the event map. - * \return Reference to the event map. + * \brief Returns a read-only view to the name-keyed map of events. + * \note The returned view is valid as long as the GenericProxy lives. + * \return View to the event map. */ - EventMap& GetEvents() noexcept; + EventMapView GetEvents() const noexcept; private: GenericProxy(std::unique_ptr proxy_binding, HandleType instance_handle); void FillEventMap(const std::vector& event_names) noexcept; - EventMap events_; + /// \brief This map owns all GenericProxyEvent instances. + /// \details This map needs to be covered in a unique_ptr as it shall not be relocated by a move of the + /// GenericProxy. This is required as we hand out views to this map (see GetEvents()), which need to be valid + /// even after the GenericProxy instance has been moved. + std::unique_ptr::map_type> events_; }; } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/generic_proxy_test.cpp b/score/mw/com/impl/generic_proxy_test.cpp index c13ef770b..b79ca7e90 100644 --- a/score/mw/com/impl/generic_proxy_test.cpp +++ b/score/mw/com/impl/generic_proxy_test.cpp @@ -23,6 +23,7 @@ #include "score/mw/com/impl/configuration/lola_service_type_deployment.h" #include "score/mw/com/impl/configuration/service_instance_id.h" #include "score/mw/com/impl/generic_proxy.h" + #include "score/mw/com/impl/generic_proxy_event.h" #include "score/mw/com/impl/handle_type.h" #include "score/mw/com/impl/instance_specifier.h" @@ -33,6 +34,7 @@ #include "score/mw/com/impl/test/binding_factory_resources.h" #include "score/mw/com/impl/test/dummy_instance_identifier_builder.h" #include "score/mw/com/impl/test/runtime_mock_guard.h" +#include "service_element_map_view_factory.h" #include @@ -145,9 +147,10 @@ TEST(GenericProxyTest, ServiceElementsAreIndexedUsingElementFqId) RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - using ActualEventMapType = GenericProxy::EventMap::map_type; - using ExpectedEventMapType = std::map; - static_assert(std::is_same_v, "GenericProxy Event map is not a std::map"); + using ActualEventMapViewType = ServiceElementMapViewFactory::map_type; + using ExpectedEventMapViewType = std::map; + static_assert(std::is_same_v, + "GenericProxy Event map view is not a std::map"); } class GenericProxyFixture : public ::testing::Test @@ -434,6 +437,33 @@ TEST_F(GenericProxyFixture, GenericProxyWillLogErrorMessageForEventsProvidedInCo EXPECT_TRUE(log_output.find(text_snippet, text_location) != log_output.npos); } +TEST_F(GenericProxyFixture, MovingGenericProxyLeavesEventMapIntact) +{ + std::optional moved_generic_proxy; + std::optional events; + + { + // Given a valid GenericProxy with some GenericProxyEvents + CreateAHandle({kEventName1, kEventName2, kEventName3}); + auto generic_proxy_result = GenericProxy::Create(*handle_); + EXPECT_TRUE(generic_proxy_result.has_value()); + + // and given a ServiceElementMapView of the event map + auto& generic_proxy = generic_proxy_result.value(); + events = generic_proxy.GetEvents(); + // when moving the GenericProxy + moved_generic_proxy = std::move(generic_proxy); + // and destroying the moved-from GenericProxy + } + + // Then the ServiceElementMapView acquired previously from the moved-from GenericProxy still points to the events + // now owned by the moved-to GenericProxy. + EXPECT_TRUE(events.value().find(kEventName1) != events.value().cend()); + // and that the moved-to GenericProxy also contains the events. + auto events_from_moved_proxy = moved_generic_proxy.value().GetEvents(); + EXPECT_TRUE(events_from_moved_proxy.find(kEventName1) != events_from_moved_proxy.cend()); +} + using GenericProxyDeathTest = GenericProxyFixture; TEST_F(GenericProxyDeathTest, FillingEventMapWithDuplicateEventNamesWillTerminate) { @@ -573,29 +603,32 @@ TEST(GenericProxyEventMapTest, GenericProxyContainsEventMapClass) RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - static_assert(std::is_class::value, "GenericProxy does not contain public EventMap class"); + static_assert(std::is_class::value, + "GenericProxy does not contain public EventMapView class"); } TEST(GenericProxyEventMapTest, CheckEventMapClassInterface) { RecordProperty("Verifies", "SCR-14031544"); - RecordProperty("Description", - "Checks that the EventMap class adheres to the required interface and that EventMap is a " - "ServiceElementMap. ServiceElementMap unit tests check that EventMap behaves like std::map."); + RecordProperty( + "Description", + "Checks that the EventMapView class adheres to the required interface and that EventMapView is a " + "ServiceElementMapView. ServiceElementMap unit tests check that EventMapView behaves like std::map."); RecordProperty("TestType", "Requirements-based test"); RecordProperty("Priority", "1"); RecordProperty("DerivationTechnique", "Analysis of requirements"); - static_assert(std::is_same>::value, + static_assert(std::is_same>::value, "EventMap type is incorrect"); - using EventMapValueType = GenericProxy::EventMap::value_type; + using EventMapValueType = GenericProxy::EventMapView::value_type; static_assert(std::is_same::value, "EventMap key type is incorrect"); static_assert(std::is_same::value, "EventMap value type is incorrect"); // Check that GenericProxy::EventMap contains the required functions - GenericProxy::EventMap event_map{}; + ServiceElementMapViewFactory::map_type initial_map; + auto event_map = ServiceElementMapViewFactory::Create(initial_map); score::cpp::ignore = event_map.cbegin(); score::cpp::ignore = event_map.cend(); score::cpp::ignore = event_map.find(std::string_view{""}); diff --git a/score/mw/com/impl/generic_skeleton.cpp b/score/mw/com/impl/generic_skeleton.cpp index 4ca883a97..c82013770 100644 --- a/score/mw/com/impl/generic_skeleton.cpp +++ b/score/mw/com/impl/generic_skeleton.cpp @@ -17,6 +17,7 @@ #include "score/mw/com/impl/plumbing/generic_skeleton_event_binding_factory.h" #include "score/mw/com/impl/plumbing/skeleton_binding_factory.h" #include "score/mw/com/impl/runtime.h" +#include "score/mw/com/impl/service_element_map_view_factory.h" #include "score/mw/com/impl/skeleton_binding.h" #include @@ -84,7 +85,7 @@ Result GenericSkeleton::Create(const InstanceIdentifier& identi for (const auto& info : in.events) { // Check for duplicates - if (skeleton.events_.find(info.name) != skeleton.events_.cend()) + if (skeleton.events_->find(info.name) != skeleton.events_->cend()) { score::mw::log::LogError("GenericSkeleton") << "Duplicate event name provided: " << info.name; return MakeUnexpected(ComErrc::kServiceElementAlreadyExists); @@ -107,7 +108,7 @@ Result GenericSkeleton::Create(const InstanceIdentifier& identi return MakeUnexpected(ComErrc::kBindingFailure); } - const auto emplace_result = skeleton.events_.emplace( + const auto emplace_result = skeleton.events_->emplace( std::piecewise_construct, std::forward_as_tuple(stable_name), std::forward_as_tuple(skeleton, stable_name, std::move(event_binding_result).value())); @@ -122,9 +123,9 @@ Result GenericSkeleton::Create(const InstanceIdentifier& identi return skeleton; } -const GenericSkeleton::EventMap& GenericSkeleton::GetEvents() const noexcept +ServiceElementMapView GenericSkeleton::GetEvents() const noexcept { - return events_; + return ServiceElementMapViewFactory::Create(*events_); } Result GenericSkeleton::OfferService() noexcept @@ -138,7 +139,8 @@ void GenericSkeleton::StopOfferService() noexcept } GenericSkeleton::GenericSkeleton(const InstanceIdentifier& identifier, std::unique_ptr binding) - : SkeletonBase(std::move(binding), identifier) + : SkeletonBase(std::move(binding), identifier), + events_(std::make_unique>()) { } diff --git a/score/mw/com/impl/generic_skeleton.h b/score/mw/com/impl/generic_skeleton.h index 3424889ce..1c57441f5 100644 --- a/score/mw/com/impl/generic_skeleton.h +++ b/score/mw/com/impl/generic_skeleton.h @@ -17,15 +17,14 @@ #include "score/mw/com/impl/generic_skeleton_event.h" #include "score/mw/com/impl/instance_identifier.h" #include "score/mw/com/impl/instance_specifier.h" -#include "score/mw/com/impl/service_element_map.h" +#include "score/mw/com/impl/service_element_map_view.h" +#include "score/mw/com/impl/service_element_map_view_factory.h" #include "score/mw/com/impl/skeleton_base.h" #include "score/result/result.h" #include -#include #include #include -#include namespace score::mw::com::impl { @@ -49,10 +48,10 @@ struct GenericSkeletonServiceElementInfo class GenericSkeleton : public SkeletonBase { public: - using EventMap = ServiceElementMap; - /// @brief Creates a GenericSkeleton and all its service elements (events + fields) atomically. + using EventMapView = ServiceElementMapView; + /// \brief Creates a GenericSkeleton and all its service elements (events + fields) atomically. /// - /// @contract + /// \contract /// - Empty spans are allowed for `in.events` and/or `in.fields` /// - Each provided name must exist in the binding deployment for this instance (events/fields respectively). /// - All element names must be unique across all element kinds within this skeleton. @@ -62,31 +61,33 @@ class GenericSkeleton : public SkeletonBase [[nodiscard]] static Result Create(const InstanceIdentifier& identifier, const GenericSkeletonServiceElementInfo& in) noexcept; - /// @brief Same as Create(InstanceIdentifier, ...) but resolves the specifier first. - /// @param specifier The instance specifier. - /// @param in Input parameters for creation. - /// @param mode The method call processing mode. - /// @return A GenericSkeleton or an error. + /// \brief Same as Create(InstanceIdentifier, ...) but resolves the specifier first. + /// \param specifier The instance specifier. + /// \param in Input parameters for creation. + /// \return A GenericSkeleton or an error. [[nodiscard]] static Result Create(const InstanceSpecifier& specifier, const GenericSkeletonServiceElementInfo& in) noexcept; - /// @brief Returns a const reference to the name-keyed map of events. - /// @note The returned reference is valid as long as the GenericSkeleton lives. - [[nodiscard]] const EventMap& GetEvents() const noexcept; + /// \brief Returns a read-only view to the name-keyed map of events. + /// \note The returned view is valid as long as the GenericSkeleton lives. + [[nodiscard]] EventMapView GetEvents() const noexcept; - /// @brief Offers the service instance. - /// @return A blank result, or an error if offering fails. + /// \brief Offers the service instance. + /// \return A blank result, or an error if offering fails. [[nodiscard]] Result OfferService() noexcept; - /// @brief Stops offering the service instance. + /// \brief Stops offering the service instance. void StopOfferService() noexcept; private: // Private constructor, only callable by static Create methods. GenericSkeleton(const InstanceIdentifier& identifier, std::unique_ptr binding); - /// @brief This map owns all GenericSkeletonEvent instances. - EventMap events_; + /// \brief This map owns all GenericSkeletonEvent instances. + /// \details This map needs to be covered in a unique_ptr as it shall not be relocated by a move of the + /// GenericSkeleton. This is required as we hand out views to this map (see GetEvents()), which need to be valid + /// even after the GenericSkeleton instance has been moved. + std::unique_ptr::map_type> events_; }; } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/service_element_map.h b/score/mw/com/impl/service_element_map.h deleted file mode 100644 index d4524f144..000000000 --- a/score/mw/com/impl/service_element_map.h +++ /dev/null @@ -1,98 +0,0 @@ -/******************************************************************************** - * Copyright (c) 2025 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 - ********************************************************************************/ -/// -/// @file -/// @copyright Copyright (C) 2023, Bayerische Motoren Werke Aktiengesellschaft (BMW AG) -/// - -#ifndef SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_H -#define SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_H - -#include -#include -#include - -namespace score::mw::com::impl -{ - -/// \brief Map that will be used in GenericProxies to store GenericProxyEvents and possibly GenericProxyFields and -/// GenericProxyMethods once they're supported by LoLa. -template -class ServiceElementMap -{ - public: - using Key = std::string_view; - using map_type = std::map; - using value_type = std::pair; - using iterator = typename std::map::iterator; - using const_iterator = typename std::map::const_iterator; - using size_type = typename std::map::size_type; - - std::pair insert(const value_type& value) - { - return map_.insert(value); - } - - template - std::pair emplace(Args&&... args) - { - return map_.emplace(std::forward(args)...); - } - - iterator erase(const_iterator pos) - { - return map_.erase(pos); - } - - size_type erase(const Key& key) - { - return map_.erase(key); - } - - const_iterator cbegin() const noexcept - { - return map_.cbegin(); - } - - const_iterator cend() const noexcept - { - return map_.cend(); - } - - iterator find(const Key& search_key) noexcept - { - return map_.find(search_key); - } - - const_iterator find(const Key& search_key) const noexcept - { - return map_.find(search_key); - } - - size_type size() const noexcept - { - return map_.size(); - } - - bool empty() const noexcept - { - return map_.empty(); - } - - private: - map_type map_; -}; - -} // namespace score::mw::com::impl - -#endif // SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_H diff --git a/score/mw/com/impl/service_element_map_test.cpp b/score/mw/com/impl/service_element_map_test.cpp deleted file mode 100644 index 6fb563a5a..000000000 --- a/score/mw/com/impl/service_element_map_test.cpp +++ /dev/null @@ -1,126 +0,0 @@ -/******************************************************************************** - * Copyright (c) 2025 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 - ********************************************************************************/ -/// -/// @file -/// @copyright Copyright (C) 2023, Bayerische Motoren Werke Aktiengesellschaft (BMW AG) -/// -#include "score/mw/com/impl/service_element_map.h" - -#include - -namespace score::mw::com::impl -{ -namespace -{ - -TEST(ServiceElementMapTest, MapSizeChangesWithInsertionOfElements) -{ - RecordProperty("Verifies", "SCR-14031544"); - RecordProperty("Description", "Checks that the GenericProxy EventMap class behaves identically to a std::map"); - RecordProperty("TestType", "Requirements-based test"); - RecordProperty("Priority", "1"); - RecordProperty("DerivationTechnique", "Analysis of requirements"); - - ServiceElementMap map; - - EXPECT_EQ(map.size(), 0); - map.insert({"0", 0}); - EXPECT_EQ(map.size(), 1); - map.emplace("1", 1); - EXPECT_EQ(map.size(), 2); -} - -TEST(ServiceElementMapTest, MapSizeChangesWithRemovalOfElements) -{ - RecordProperty("Verifies", "SCR-14031544"); - RecordProperty("Description", "Checks that the GenericProxy EventMap class behaves identically to a std::map"); - RecordProperty("TestType", "Requirements-based test"); - RecordProperty("Priority", "1"); - RecordProperty("DerivationTechnique", "Analysis of requirements"); - - ServiceElementMap map; - map.insert({"0", 0}); - map.emplace("1", 1); - map.emplace("2", 2); - - EXPECT_EQ(map.size(), 3); - map.erase("0"); - EXPECT_EQ(map.size(), 2); - - map.erase(map.cbegin()); - EXPECT_EQ(map.size(), 1); - - map.erase(map.cbegin()); - EXPECT_EQ(map.size(), 0); -} - -TEST(ServiceElementMapTest, MapWithElementsIsNotEmpty) -{ - RecordProperty("Verifies", "SCR-14031544"); - RecordProperty("Description", "Checks that the GenericProxy EventMap class behaves identically to a std::map"); - RecordProperty("TestType", "Requirements-based test"); - RecordProperty("Priority", "1"); - RecordProperty("DerivationTechnique", "Analysis of requirements"); - - ServiceElementMap map; - EXPECT_TRUE(map.empty()); - map.insert({"0", 0}); - EXPECT_FALSE(map.empty()); - map.erase(map.cbegin()); - EXPECT_TRUE(map.empty()); -} - -TEST(ServiceElementMapTest, CanFindElementsInMap) -{ - RecordProperty("Verifies", "SCR-14031544"); - RecordProperty("Description", "Checks that the GenericProxy EventMap class behaves identically to a std::map"); - RecordProperty("TestType", "Requirements-based test"); - RecordProperty("Priority", "1"); - RecordProperty("DerivationTechnique", "Analysis of requirements"); - - ServiceElementMap map; - map.insert({"0", 0}); - map.emplace("1", 1); - map.emplace("2", 2); - - auto first_it = map.find("0"); - ASSERT_NE(first_it, map.cend()); - EXPECT_FALSE(first_it->first.compare(std::string_view{"0"})); - EXPECT_EQ(first_it->second, 0); - - auto second_it = map.find("1"); - ASSERT_NE(second_it, map.cend()); - EXPECT_FALSE(second_it->first.compare("1")); - EXPECT_EQ(second_it->second, 1); - - auto third_it = map.find("2"); - ASSERT_NE(third_it, map.cend()); - EXPECT_FALSE(third_it->first.compare("2")); - EXPECT_EQ(third_it->second, 2); - - auto invalid_it = map.find("3"); - ASSERT_EQ(invalid_it, map.cend()); -} - -TEST(ServiceElementMapTest, MapCanBeCopied) -{ - ServiceElementMap map; - map.insert({"0", 0}); - map.insert({"1", 1}); - - const ServiceElementMap new_map(map); - EXPECT_EQ(new_map.size(), 2); -} - -} // namespace -} // namespace score::mw::com::impl diff --git a/score/mw/com/impl/service_element_map.cpp b/score/mw/com/impl/service_element_map_view.cpp similarity index 78% rename from score/mw/com/impl/service_element_map.cpp rename to score/mw/com/impl/service_element_map_view.cpp index 69f8f4fea..c9fe98335 100644 --- a/score/mw/com/impl/service_element_map.cpp +++ b/score/mw/com/impl/service_element_map_view.cpp @@ -10,8 +10,5 @@ * * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ -/// -/// @file -/// @copyright Copyright (C) 2023, Bayerische Motoren Werke Aktiengesellschaft (BMW AG) -/// -#include "score/mw/com/impl/service_element_map.h" + +#include "score/mw/com/impl/service_element_map_view.h" diff --git a/score/mw/com/impl/service_element_map_view.h b/score/mw/com/impl/service_element_map_view.h new file mode 100644 index 000000000..ea145d773 --- /dev/null +++ b/score/mw/com/impl/service_element_map_view.h @@ -0,0 +1,93 @@ +/******************************************************************************** + * Copyright (c) 2025 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 SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_VIEW_H +#define SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_VIEW_H + +#include +#include +#include +#include + +namespace score::mw::com::impl +{ + +template +class ServiceElementMapViewFactory; + +/// \brief A map view that will be provided to the user in case of GenericProxy and GenericSkeleton to allow access to +/// the GenericProxyEvents/GenericSkeletonEvents and possibly fields/methods in a later stage. The view is NOT +/// owning. I.e. the view is set up by the owner of the underlying map and provides a read-only view to this map. +template +class ServiceElementMapView +{ + // Suppress "AUTOSAR C++14 A11-3-1", The rule declares: "Friend declarations shall not be used". + // The "ServiceElementMapViewFactory" class is a helper, which allows to hide the ctor of this class from the user, + // which shall not be able to construct instances on their own. + // coverity[autosar_cpp14_a11_3_1_violation] + friend class ServiceElementMapViewFactory; + + public: + using key = std::string_view; + using value_type = std::pair; + using iterator = typename std::map::iterator; + using const_iterator = typename std::map::const_iterator; + using size_type = typename std::map::size_type; + + const_iterator cbegin() const noexcept + { + return elements_.get().cbegin(); + } + + const_iterator cend() const noexcept + { + return elements_.get().cend(); + } + + iterator find(const key& search_key) noexcept + { + return elements_.get().find(search_key); + } + + const_iterator find(const key& search_key) const noexcept + { + return elements_.get().find(search_key); + } + + size_type size() const noexcept + { + return elements_.get().size(); + } + + bool empty() const noexcept + { + return elements_.get().empty(); + } + + private: + using map_type = std::map; + + /// \brief Creates a ServiceElementMapView that provides "read-only" access to the provided map. That is no elements + /// can be added or removed from the map via the ServiceElementMapView, but changes to the map elements is + /// possible. + /// \details ctor is private. Creation shall be done via ServiceElementMapView<>::Create(). + /// @param service_element_map underlying map on which the view is created. It must not be moved or copied after + /// being passed to this ctor. + ServiceElementMapView(map_type& service_element_map) noexcept : elements_{service_element_map} {} + + std::reference_wrapper elements_; +}; + +} // namespace score::mw::com::impl + +#endif // SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_VIEW_H diff --git a/score/mw/com/impl/service_element_map_view_factory.cpp b/score/mw/com/impl/service_element_map_view_factory.cpp new file mode 100644 index 000000000..9d576f5a8 --- /dev/null +++ b/score/mw/com/impl/service_element_map_view_factory.cpp @@ -0,0 +1,14 @@ +/******************************************************************************** + * 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/service_element_map_view_factory.h" diff --git a/score/mw/com/impl/service_element_map_view_factory.h b/score/mw/com/impl/service_element_map_view_factory.h new file mode 100644 index 000000000..9ce002e39 --- /dev/null +++ b/score/mw/com/impl/service_element_map_view_factory.h @@ -0,0 +1,45 @@ +/******************************************************************************** + * 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 SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_VIEW_FACTORY_H +#define SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_VIEW_FACTORY_H + +#include "score/mw/com/impl/service_element_map_view.h" + +namespace score::mw::com::impl +{ + +/// \brief Factory clas template for creation of ServiceElementMapView instances. +/// \details ServiceElementMapView instances are used/returned from GenericProxy/GenericSkeleton to the +/// user. We don't want the user to be able to create ServiceElementMapView on its own, thus the ctor is +/// private. +/// Therefore, we have this factory, which is a friend to ServiceElementMapView and not accessible by the +/// users. +template +class ServiceElementMapViewFactory +{ + public: + using map_type = typename ServiceElementMapView::map_type; + + /// \brief Create factory method to create a ServiceElementMapView instance + /// @param element_map underlying map for which the view gets created + /// @return read-only view to the map. + static ServiceElementMapView Create(map_type& element_map) + { + return ServiceElementMapView{element_map}; + } +}; + +} // namespace score::mw::com::impl + +#endif // SCORE_MW_COM_IMPL_SERVICE_ELEMENT_MAP_VIEW_FACTORY_H diff --git a/score/mw/com/impl/service_element_map_view_test.cpp b/score/mw/com/impl/service_element_map_view_test.cpp new file mode 100644 index 000000000..d8274fc3e --- /dev/null +++ b/score/mw/com/impl/service_element_map_view_test.cpp @@ -0,0 +1,133 @@ +/******************************************************************************** + * Copyright (c) 2025 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/service_element_map_view.h" +#include "score/mw/com/impl/service_element_map_view_factory.h" + +#include + +namespace score::mw::com::impl +{ +namespace +{ + +TEST(ServiceElementMapViewTest, MapSizeChangesWithInsertionOfElements) +{ + RecordProperty("Verifies", "SCR-14031544"); + RecordProperty("Description", + "Checks that the ServiceElementMapView class behaves identically to a " + "std::map regarding non-mutable methods"); + RecordProperty("TestType", "Requirements-based test"); + RecordProperty("Priority", "1"); + RecordProperty("DerivationTechnique", "Analysis of requirements"); + + std::map initial_map; + ServiceElementMapView map_view = ServiceElementMapViewFactory::Create(initial_map); + + EXPECT_EQ(map_view.size(), 0); + initial_map.insert({"0", 0}); + EXPECT_EQ(map_view.size(), 1); + initial_map.insert({"1", 1}); + EXPECT_EQ(map_view.size(), 2); +} + +TEST(ServiceElementMapViewTest, MapSizeChangesWithRemovalOfElements) +{ + RecordProperty("Verifies", "SCR-14031544"); + RecordProperty("Description", "Checks that the GenericProxy EventMap class behaves identically to a std::map"); + RecordProperty("TestType", "Requirements-based test"); + RecordProperty("Priority", "1"); + RecordProperty("DerivationTechnique", "Analysis of requirements"); + + std::map initial_map; + ServiceElementMapView map_view = ServiceElementMapViewFactory::Create(initial_map); + + initial_map.insert({"0", 0}); + initial_map.emplace("1", 1); + initial_map.emplace("2", 2); + + EXPECT_EQ(map_view.size(), 3); + initial_map.erase("0"); + EXPECT_EQ(map_view.size(), 2); + + initial_map.erase(map_view.cbegin()); + EXPECT_EQ(map_view.size(), 1); + + initial_map.erase(map_view.cbegin()); + EXPECT_EQ(map_view.size(), 0); +} + +TEST(ServiceElementMapViewTest, MapWithElementsIsNotEmpty) +{ + RecordProperty("Verifies", "SCR-14031544"); + RecordProperty("Description", "Checks that the GenericProxy EventMap class behaves identically to a std::map"); + RecordProperty("TestType", "Requirements-based test"); + RecordProperty("Priority", "1"); + RecordProperty("DerivationTechnique", "Analysis of requirements"); + + std::map initial_map; + ServiceElementMapView map_view = ServiceElementMapViewFactory::Create(initial_map); + EXPECT_TRUE(map_view.empty()); + initial_map.insert({"0", 0}); + EXPECT_FALSE(map_view.empty()); + initial_map.erase(initial_map.cbegin()); + EXPECT_TRUE(map_view.empty()); +} + +TEST(ServiceElementMapViewTest, CanFindElementsInMap) +{ + RecordProperty("Verifies", "SCR-14031544"); + RecordProperty("Description", "Checks that the GenericProxy EventMap class behaves identically to a std::map"); + RecordProperty("TestType", "Requirements-based test"); + RecordProperty("Priority", "1"); + RecordProperty("DerivationTechnique", "Analysis of requirements"); + + std::map initial_map; + ServiceElementMapView map_view = ServiceElementMapViewFactory::Create(initial_map); + initial_map.insert({"0", 0}); + initial_map.emplace("1", 1); + initial_map.emplace("2", 2); + + auto first_it = map_view.find("0"); + ASSERT_NE(first_it, map_view.cend()); + EXPECT_FALSE(first_it->first.compare(std::string_view{"0"})); + EXPECT_EQ(first_it->second, 0); + + auto second_it = map_view.find("1"); + ASSERT_NE(second_it, map_view.cend()); + EXPECT_FALSE(second_it->first.compare("1")); + EXPECT_EQ(second_it->second, 1); + + auto third_it = map_view.find("2"); + ASSERT_NE(third_it, map_view.cend()); + EXPECT_FALSE(third_it->first.compare("2")); + EXPECT_EQ(third_it->second, 2); + + auto invalid_it = map_view.find("3"); + ASSERT_EQ(invalid_it, map_view.cend()); +} + +TEST(ServiceElementMapViewTest, MapCanBeCopied) +{ + std::map initial_map; + ServiceElementMapView map_view = ServiceElementMapViewFactory::Create(initial_map); + initial_map.insert({"0", 0}); + initial_map.emplace("1", 1); + initial_map.emplace("2", 2); + + const ServiceElementMapView new_map(map_view); + EXPECT_EQ(new_map.size(), 3); +} + +} // namespace +} // namespace score::mw::com::impl From 1088562fe8c4c538ea8dcb49b2ef36d789b8299e Mon Sep 17 00:00:00 2001 From: manuelfehlhammer Date: Fri, 29 May 2026 11:10:44 +0200 Subject: [PATCH 2/2] Update GenericProxy/Skeleton design Introduction of ServiceElementMapView required a design doc update. Issue: #467 --- .../skeleton_proxy/generic_proxy/README.md | 2 +- .../generic_proxy/generic_proxy_model.puml | 32 +++++++++++------ .../generic_skeleton_model.puml | 34 +++++++++++++------ score/mw/com/impl/service_element_map_view.h | 2 +- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/score/mw/com/design/skeleton_proxy/generic_proxy/README.md b/score/mw/com/design/skeleton_proxy/generic_proxy/README.md index 22005b0eb..59767614b 100644 --- a/score/mw/com/design/skeleton_proxy/generic_proxy/README.md +++ b/score/mw/com/design/skeleton_proxy/generic_proxy/README.md @@ -25,7 +25,7 @@ This comprises the following items: * `GenericProxy` class: We will provide a class `GenericProxy`, but opposed to the concept (linked above) **not** in the `ara::com`, but in the `mw::com` namespace. * This `GenericProxy` class will (in relation to the concept) **only** provide a public method `GetEvents()`, which - returns a `GenericProxy::EventMap` (which is a stripped down `std::map`) + returns a `GenericProxy::EventMapView` (which is a read-only view to a `std::map`) * this map returned by `GetEvents()` will only contain all the events mentioned in the service deployment. Later (when our fields get extended with get/set method functionality), this will change (broken_link_j/Ticket-156027) diff --git a/score/mw/com/design/skeleton_proxy/generic_proxy/generic_proxy_model.puml b/score/mw/com/design/skeleton_proxy/generic_proxy/generic_proxy_model.puml index 5377754af..bef0b7391 100644 --- a/score/mw/com/design/skeleton_proxy/generic_proxy/generic_proxy_model.puml +++ b/score/mw/com/design/skeleton_proxy/generic_proxy/generic_proxy_model.puml @@ -32,11 +32,13 @@ abstract class "ProxyBinding" { class "mw::com::impl::GenericProxy" #yellow { using HandleType = ProxyBase::HandleType - using EventMap = ServiceElementMap + using EventMapView = ServiceElementMapView .. - +GenericProxy(ConstructionToken&&) - +GenericProxy(HandleType) - +events_ : EventMap + +{static} Create(HandleType instance_handle) : Result + -GenericProxy(std::unique_ptr proxy_binding, HandleType instance_handle) + +GetEvents() : EventMapView + + -events_ : std::unique_ptr::map_type> events_ } class "lola::Proxy" { @@ -179,11 +181,19 @@ class "lola::GenericProxyEvent" #yellow { -common_dispatch_ : lola::ProxyEventCommon } -class "ServiceElementMap" { - using key_type = score::cpp::stringview - using mapped_type = GenericProxyEvent - using value_type = std::pair - using const_iterator = LegacyBidirectionalIterator to const value_type +class "ServiceElementMapViewFactory" { + using map_type = typename ServiceElementMapView::map_type + .. + +Create(map_type&) : ServiceElementMapView +} + +class "ServiceElementMapView" { + using key = std::string_view; + using value_type = std::pair + using iterator = typename std::map::iterator + using const_iterator = typename std::map::const_iterator + using size_type = typename std::map::size_type + using map_type = std::map .. +cbegin() : const_iterator +cend() : const_iterator @@ -198,7 +208,6 @@ class "ServiceElementMap" { "score::mw::com::impl::ProxyBase" *--> "ProxyBinding" "score::mw::com::impl::ProxyBase" <|-- "DummyProxy" "score::mw::com::impl::ProxyBase" <|-- "mw::com::impl::GenericProxy" -"mw::com::impl::GenericProxy" *-- "ServiceElementMap" "ProxyBindingFactory" ..> "ProxyBinding" : creates "ProxyBinding" <|-- "lola::Proxy" "lola::ProxyEventCommon" o-- "1" "lola::Proxy" @@ -206,6 +215,9 @@ class "ServiceElementMap" { "ProxyBindingFactory" <.. "DummyProxy" : uses "DummyProxy" *--> "mw::com::impl::ProxyEvent" : "0..n" "mw::com::impl::GenericProxy" *--> "mw::com::impl::GenericProxyEvent" : "0..n" +"mw::com::impl::GenericProxy" ..> "ServiceElementMapViewFactory" : uses +"ServiceElementMapViewFactory" ..> "ServiceElementMapView" : creates +"ServiceElementMapView" o-- "mw::com::impl::GenericProxyEvent" : "0..n" "mw::com::impl::ProxyEventBase" *-- "SampleReferenceTracker" "mw::com::impl::ProxyEventBase" <|-- "mw::com::impl::ProxyEvent" "mw::com::impl::ProxyEventBase" <|-- "mw::com::impl::GenericProxyEvent" diff --git a/score/mw/com/design/skeleton_proxy/generic_skeleton/generic_skeleton_model.puml b/score/mw/com/design/skeleton_proxy/generic_skeleton/generic_skeleton_model.puml index 86d5ca4a3..502e809cf 100644 --- a/score/mw/com/design/skeleton_proxy/generic_skeleton/generic_skeleton_model.puml +++ b/score/mw/com/design/skeleton_proxy/generic_skeleton/generic_skeleton_model.puml @@ -31,14 +31,16 @@ abstract class "SkeletonBinding" { } class "mw::com::impl::GenericSkeleton" #yellow { - using EventMap = ServiceElementMap + using EventMapView = ServiceElementMapView .. - +GenericSkeleton(const InstanceIdentifier&, std::unique_ptr, MethodCallProcessingMode) - +Create(const InstanceIdentifier&, const GenericSkeletonServiceElementInfo&, MethodCallProcessingMode): Result - +Create(const InstanceSpecifier&, const GenericSkeletonServiceElementInfo&, MethodCallProcessingMode): Result - +events_ : EventMap + +Create(const InstanceIdentifier&, const GenericSkeletonServiceElementInfo&): Result + +Create(const InstanceSpecifier&, const GenericSkeletonServiceElementInfo&): Result + +GetEvents() : EventMapView +OfferService(): Result +StopOfferService(): void + .. + -GenericSkeleton(const InstanceIdentifier&, std::unique_ptr) + -events_ : std::unique_ptr::map_type> events_ } class "lola::Skeleton" { @@ -87,11 +89,19 @@ class "GenericSkeletonEventBindingFactory" { +{static} Create(SkeletonBase& parent, std::string_view event_name, const DataTypeMetaInfo& size_info): Result> } -class "ServiceElementMap" { - using key_type = score::cpp::stringview - using mapped_type = GenericSkeletonEvent - using value_type = std::pair - using const_iterator = LegacyBidirectionalIterator to const value_type +class "ServiceElementMapViewFactory" { + using map_type = typename ServiceElementMapView::map_type + .. + +Create(map_type&) : ServiceElementMapView +} + +class "ServiceElementMapView" { + using key = std::string_view; + using value_type = std::pair + using iterator = typename std::map::iterator + using const_iterator = typename std::map::const_iterator + using size_type = typename std::map::size_type + using map_type = std::map .. +cbegin() : const_iterator +cend() : const_iterator @@ -156,7 +166,6 @@ class "DummySkeleton" <> { "score::mw::com::impl::SkeletonBase" *--> "SkeletonBinding" "score::mw::com::impl::SkeletonBase" <|-- "DummySkeleton" "score::mw::com::impl::SkeletonBase" <|-- "mw::com::impl::GenericSkeleton" -"mw::com::impl::GenericSkeleton" *-- "ServiceElementMap" "SkeletonBindingFactory" ..> "SkeletonBinding" : creates "SkeletonBinding" <|-- "lola::Skeleton" @@ -179,5 +188,8 @@ class "DummySkeleton" <> { "DummySkeleton" *--> "mw::com::impl::SkeletonEvent" : "0..n" "mw::com::impl::GenericSkeleton" *--> "mw::com::impl::GenericSkeletonEvent" : "0..n" +"mw::com::impl::GenericSkeleton" ..> "ServiceElementMapViewFactory" : uses +"ServiceElementMapViewFactory" ..> "ServiceElementMapView" : creates +"ServiceElementMapView" o-- "mw::com::impl::GenericSkeletonEvent" : "0..n" @enduml diff --git a/score/mw/com/impl/service_element_map_view.h b/score/mw/com/impl/service_element_map_view.h index ea145d773..12d2001bf 100644 --- a/score/mw/com/impl/service_element_map_view.h +++ b/score/mw/com/impl/service_element_map_view.h @@ -75,7 +75,7 @@ class ServiceElementMapView } private: - using map_type = std::map; + using map_type = std::map; /// \brief Creates a ServiceElementMapView that provides "read-only" access to the provided map. That is no elements /// can be added or removed from the map via the ServiceElementMapView, but changes to the map elements is