[eclipse-iceoryx#231] Introduce PortFactory trait; fix bug in builder…

… where the configured and not the actual static config is used; add test for property
elfenpiff · Jun 10, 2024 · 2087eb1 · 2087eb1
1 parent 3f0fa98
commit 2087eb1
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 62 deletions.
diff --git a/iceoryx2/src/prelude.rs b/iceoryx2/src/prelude.rs
@@ -14,8 +14,8 @@ pub use crate::iox2::Iox2;
 pub use crate::iox2::Iox2Event;
 pub use crate::port::event_id::EventId;
 pub use crate::service::{
-    port_factory::publisher::UnableToDeliverStrategy, process_local, service_name::ServiceName,
-    zero_copy, Service,
+    port_factory::publisher::UnableToDeliverStrategy, port_factory::PortFactory, process_local,
+    service_name::ServiceName, zero_copy, Service,
 };
 pub use iceoryx2_bb_derive_macros::PlacementDefault;
 pub use iceoryx2_bb_elementary::alignment::Alignment;

diff --git a/iceoryx2/src/service/builder/event.rs b/iceoryx2/src/service/builder/event.rs
@@ -197,7 +197,7 @@ impl<ServiceType: service::Service> Builder<ServiceType> {
                         "{} since the event does not exist.", msg);
                 }
                 Ok(Some((static_config, static_storage))) => {
-                    let static_config = self.verify_service_properties(&static_config)?;
+                    let event_static_config = self.verify_service_properties(&static_config)?;
 
                     let dynamic_config = Arc::new(
                         fail!(from self, when self.base.open_dynamic_config_storage(),
@@ -206,11 +206,11 @@ impl<ServiceType: service::Service> Builder<ServiceType> {
                     );
 
                     self.base.service_config.messaging_pattern =
-                        MessagingPattern::Event(static_config);
+                        MessagingPattern::Event(event_static_config);
 
                     return Ok(event::PortFactory::new(ServiceType::from_state(
                         service::ServiceState::new(
-                            self.base.service_config,
+                            static_config,
                             self.base.global_config,
                             dynamic_config,
                             static_storage,

diff --git a/iceoryx2/src/service/builder/publish_subscribe.rs b/iceoryx2/src/service/builder/publish_subscribe.rs
@@ -472,7 +472,7 @@ impl<PayloadType: Debug + ?Sized, ServiceType: service::Service> Builder<Payload
                         "{} since the service does not exist.", msg);
                 }
                 Ok(Some((static_config, static_storage))) => {
-                    let static_config = self.verify_service_properties(&static_config)?;
+                    let pub_sub_static_config = self.verify_service_properties(&static_config)?;
 
                     let dynamic_config = Arc::new(
                         fail!(from self, when self.base.open_dynamic_config_storage(),
@@ -481,11 +481,11 @@ impl<PayloadType: Debug + ?Sized, ServiceType: service::Service> Builder<Payload
                     );
 
                     self.base.service_config.messaging_pattern =
-                        MessagingPattern::PublishSubscribe(static_config.clone());
+                        MessagingPattern::PublishSubscribe(pub_sub_static_config.clone());
 
                     return Ok(publish_subscribe::PortFactory::new(
                         ServiceType::from_state(service::ServiceState::new(
-                            self.base.service_config.clone(),
+                            static_config,
                             self.base.global_config.clone(),
                             dynamic_config,
                             static_storage,

diff --git a/iceoryx2/src/service/port_factory/event.rs b/iceoryx2/src/service/port_factory/event.rs
@@ -53,37 +53,35 @@ pub struct PortFactory<Service: service::Service> {
 unsafe impl<Service: service::Service> Send for PortFactory<Service> {}
 unsafe impl<Service: service::Service> Sync for PortFactory<Service> {}
 
-impl<Service: service::Service> PortFactory<Service> {
-    pub(crate) fn new(service: Service) -> Self {
-        Self { service }
-    }
+impl<Service: service::Service> crate::service::port_factory::PortFactory for PortFactory<Service> {
+    type StaticConfig = static_config::event::StaticConfig;
+    type DynamicConfig = dynamic_config::event::DynamicConfig;
 
-    /// Returns the [`ServiceName`] of the [`crate::service::Service`]
-    pub fn name(&self) -> &ServiceName {
+    fn name(&self) -> &ServiceName {
         self.service.state().static_config.name()
     }
 
-    /// Returns the uuid of the [`crate::service::Service`]
-    pub fn uuid(&self) -> &str {
+    fn uuid(&self) -> &str {
         self.service.state().static_config.uuid()
     }
 
-    /// Returns the value of a property
-    pub fn property(&self, key: &str) -> Vec<&str> {
+    fn property(&self, key: &str) -> Vec<&str> {
         self.service.state().static_config.property(key)
     }
 
-    /// Returns the [`static_config::event::StaticConfig`] of the [`crate::service::Service`].
-    /// Contains all settings that never change during the lifetime of the service.
-    pub fn static_config(&self) -> &static_config::event::StaticConfig {
+    fn static_config(&self) -> &static_config::event::StaticConfig {
         self.service.state().static_config.event()
     }
 
-    /// Returns the [`dynamic_config::event::DynamicConfig`] of the [`crate::service::Service`].
-    /// Contains all dynamic settings, like the current participants etc..
-    pub fn dynamic_config(&self) -> &dynamic_config::event::DynamicConfig {
+    fn dynamic_config(&self) -> &dynamic_config::event::DynamicConfig {
         self.service.state().dynamic_storage.get().event()
     }
+}
+
+impl<Service: service::Service> PortFactory<Service> {
+    pub(crate) fn new(service: Service) -> Self {
+        Self { service }
+    }
 
     /// Returns a [`PortFactoryNotifier`] to create a new [`crate::port::notifier::Notifier`] port
     ///

diff --git a/iceoryx2/src/service/port_factory/mod.rs b/iceoryx2/src/service/port_factory/mod.rs
@@ -10,6 +10,8 @@
 //
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
+use super::service_name::ServiceName;
+
 /// Factory to create the endpoints of
 /// [`MessagingPattern::Event`](crate::service::messaging_pattern::MessagingPattern::Event) based
 /// communication and to acquire static and dynamic service information
@@ -31,3 +33,27 @@ pub mod publisher;
 
 /// Factory to create a [`Subscriber`](crate::port::subscriber::Subscriber)
 pub mod subscriber;
+
+/// The trait that contains the interface of all port factories for any kind of
+/// [`crate::service::messaging_pattern::MessagingPattern`].
+pub trait PortFactory {
+    type StaticConfig;
+    type DynamicConfig;
+
+    /// Returns the [`ServiceName`] of the service
+    fn name(&self) -> &ServiceName;
+
+    /// Returns the uuid of the [`crate::service::Service`]
+    fn uuid(&self) -> &str;
+
+    /// Returns the value of a property
+    fn property(&self, key: &str) -> Vec<&str>;
+
+    /// Returns the StaticConfig of the [`crate::service::Service`].
+    /// Contains all settings that never change during the lifetime of the service.
+    fn static_config(&self) -> &Self::StaticConfig;
+
+    /// Returns the DynamicConfig of the [`crate::service::Service`].
+    /// Contains all dynamic settings, like the current participants etc..
+    fn dynamic_config(&self) -> &Self::DynamicConfig;
+}
diff --git a/iceoryx2/src/service/port_factory/publish_subscribe.rs b/iceoryx2/src/service/port_factory/publish_subscribe.rs
@@ -69,44 +69,44 @@ unsafe impl<Service: service::Service, PayloadType: Debug + ?Sized> Sync
 {
 }
 
-impl<Service: service::Service, PayloadType: Debug + ?Sized> PortFactory<Service, PayloadType> {
-    pub(crate) fn new(service: Service) -> Self {
-        Self {
-            service,
-            _phantom_payload_type: PhantomData,
-        }
-    }
+impl<Service: service::Service, PayloadType: Debug + ?Sized>
+    crate::service::port_factory::PortFactory for PortFactory<Service, PayloadType>
+{
+    type StaticConfig = static_config::publish_subscribe::StaticConfig;
+    type DynamicConfig = dynamic_config::publish_subscribe::DynamicConfig;
 
-    /// Returns the [`ServiceName`] of the service
-    pub fn name(&self) -> &ServiceName {
+    fn name(&self) -> &ServiceName {
         self.service.state().static_config.name()
     }
 
-    /// Returns the uuid of the [`crate::service::Service`]
-    pub fn uuid(&self) -> &str {
+    fn uuid(&self) -> &str {
         self.service.state().static_config.uuid()
     }
 
-    /// Returns the value of a property
-    pub fn property(&self, key: &str) -> Vec<&str> {
+    fn property(&self, key: &str) -> Vec<&str> {
         self.service.state().static_config.property(key)
     }
 
-    /// Returns the [`static_config::event::StaticConfig`] of the [`crate::service::Service`].
-    /// Contains all settings that never change during the lifetime of the service.
-    pub fn static_config(&self) -> &static_config::publish_subscribe::StaticConfig {
+    fn static_config(&self) -> &static_config::publish_subscribe::StaticConfig {
         self.service.state().static_config.publish_subscribe()
     }
 
-    /// Returns the [`dynamic_config::event::DynamicConfig`] of the [`crate::service::Service`].
-    /// Contains all dynamic settings, like the current participants etc..
-    pub fn dynamic_config(&self) -> &dynamic_config::publish_subscribe::DynamicConfig {
+    fn dynamic_config(&self) -> &dynamic_config::publish_subscribe::DynamicConfig {
         self.service
             .state()
             .dynamic_storage
             .get()
             .publish_subscribe()
     }
+}
+
+impl<Service: service::Service, PayloadType: Debug + ?Sized> PortFactory<Service, PayloadType> {
+    pub(crate) fn new(service: Service) -> Self {
+        Self {
+            service,
+            _phantom_payload_type: PhantomData,
+        }
+    }
 
     /// Returns a [`PortFactorySubscriber`] to create a new
     /// [`crate::port::subscriber::Subscriber`] port.

diff --git a/iceoryx2/src/service/static_config/mod.rs b/iceoryx2/src/service/static_config/mod.rs
@@ -109,17 +109,17 @@ impl StaticConfig {
     }
 
     /// Returns the value of a property
-    pub(crate) fn property(&self, key: &str) -> Vec<&str> {
+    pub fn property(&self, key: &str) -> Vec<&str> {
         self.properties.get(key)
     }
 
     /// Returns the uuid of the [`crate::service::Service`]
-    pub(crate) fn uuid(&self) -> &str {
+    pub fn uuid(&self) -> &str {
         &self.uuid
     }
 
     /// Returns the [`ServiceName`] of the [`crate::service::Service`]
-    pub(crate) fn name(&self) -> &ServiceName {
+    pub fn name(&self) -> &ServiceName {
         &self.service_name
     }
 

diff --git a/iceoryx2/tests/service_publish_subscribe_tests.rs b/iceoryx2/tests/service_publish_subscribe_tests.rs
@@ -1677,7 +1677,7 @@ mod service_publish_subscribe {
             for n in names {
                 let mut name_found = false;
                 for s in &state {
-                    if *s.service_name() == n {
+                    if *s.name() == n {
                         name_found = true;
                         break;
                     }

diff --git a/iceoryx2/tests/service_tests.rs b/iceoryx2/tests/service_tests.rs
@@ -35,12 +35,18 @@ mod service {
     }
 
     trait SutFactory {
-        type Factory;
+        type Factory: PortFactory;
         type CreateError: std::fmt::Debug;
         type OpenError: std::fmt::Debug;
 
-        fn create(service_name: &ServiceName) -> Result<Self::Factory, Self::CreateError>;
-        fn open(service_name: &ServiceName) -> Result<Self::Factory, Self::OpenError>;
+        fn create(
+            service_name: &ServiceName,
+            properties: Vec<(String, String)>,
+        ) -> Result<Self::Factory, Self::CreateError>;
+        fn open(
+            service_name: &ServiceName,
+            properties: Vec<(String, String)>,
+        ) -> Result<Self::Factory, Self::OpenError>;
 
         fn assert_create_error(error: Self::CreateError);
         fn assert_open_error(error: Self::OpenError);
@@ -51,12 +57,26 @@ mod service {
         type CreateError = PublishSubscribeCreateError;
         type OpenError = PublishSubscribeOpenError;
 
-        fn create(service_name: &ServiceName) -> Result<Self::Factory, Self::CreateError> {
-            Sut::new(&service_name).publish_subscribe::<u64>().create()
+        fn create(
+            service_name: &ServiceName,
+            properties: Vec<(String, String)>,
+        ) -> Result<Self::Factory, Self::CreateError> {
+            let mut sut_builder = Sut::new(&service_name);
+            for property in properties {
+                sut_builder = sut_builder.add_property(&property.0, &property.1);
+            }
+            sut_builder.publish_subscribe::<u64>().create()
         }
 
-        fn open(service_name: &ServiceName) -> Result<Self::Factory, Self::OpenError> {
-            Sut::new(&service_name).publish_subscribe::<u64>().open()
+        fn open(
+            service_name: &ServiceName,
+            properties: Vec<(String, String)>,
+        ) -> Result<Self::Factory, Self::OpenError> {
+            let mut sut_builder = Sut::new(&service_name);
+            for property in properties {
+                sut_builder = sut_builder.add_property(&property.0, &property.1);
+            }
+            sut_builder.publish_subscribe::<u64>().open()
         }
 
         fn assert_create_error(error: Self::CreateError) {
@@ -86,12 +106,26 @@ mod service {
         type CreateError = EventCreateError;
         type OpenError = EventOpenError;
 
-        fn create(service_name: &ServiceName) -> Result<Self::Factory, Self::CreateError> {
-            Sut::new(&service_name).event().create()
+        fn create(
+            service_name: &ServiceName,
+            properties: Vec<(String, String)>,
+        ) -> Result<Self::Factory, Self::CreateError> {
+            let mut sut_builder = Sut::new(&service_name);
+            for property in properties {
+                sut_builder = sut_builder.add_property(&property.0, &property.1);
+            }
+            sut_builder.event().create()
         }
 
-        fn open(service_name: &ServiceName) -> Result<Self::Factory, Self::OpenError> {
-            Sut::new(&service_name).event().open()
+        fn open(
+            service_name: &ServiceName,
+            properties: Vec<(String, String)>,
+        ) -> Result<Self::Factory, Self::OpenError> {
+            let mut sut_builder = Sut::new(&service_name);
+            for property in properties {
+                sut_builder = sut_builder.add_property(&property.0, &property.1);
+            }
+            sut_builder.event().open()
         }
 
         fn assert_create_error(error: Self::CreateError) {
@@ -164,7 +198,7 @@ mod service {
                         let service_name = generate_name();
                         barrier_enter.wait();
 
-                        let _sut = Factory::create(&service_name).unwrap();
+                        let _sut = Factory::create(&service_name, Vec::new()).unwrap();
 
                         barrier_exit.wait();
                     }
@@ -198,7 +232,7 @@ mod service {
                     for _ in 0..NUMBER_OF_ITERATIONS {
                         barrier_enter.wait();
 
-                        let sut = Factory::create(&service_name);
+                        let sut = Factory::create(&service_name, Vec::new());
                         match sut {
                             Ok(_) => {
                                 success_counter.fetch_add(1, Ordering::Relaxed);
@@ -245,7 +279,7 @@ mod service {
             let mut threads = vec![];
             threads.push(s.spawn(|| {
                 for service_name in service_names {
-                    let sut = Factory::create(&service_name).unwrap();
+                    let sut = Factory::create(&service_name, Vec::new()).unwrap();
                     barrier_enter.wait();
 
                     drop(sut);
@@ -259,7 +293,7 @@ mod service {
                     for service_name in service_names {
                         barrier_enter.wait();
 
-                        let sut = Factory::open(&service_name);
+                        let sut = Factory::open(&service_name, Vec::new());
                         match sut {
                             Ok(_) => (),
                             Err(e) => {
@@ -278,6 +312,25 @@ mod service {
         });
     }
 
+    #[test]
+    fn setting_properties_in_creator_works<Sut: Service, Factory: SutFactory>() {
+        let service_name = generate_name();
+        let properties = vec![
+            ("1. Hello".to_string(), "Hypnotoad".to_string()),
+            ("2. No more".to_string(), "Coffee".to_string()),
+            ("3. Just have a".to_string(), "lick on the toad".to_string()),
+        ];
+        let sut_create = Factory::create(&service_name, properties.clone()).unwrap();
+        for property in &properties {
+            assert_that!(sut_create.property(&property.0), eq vec![&property.1]);
+        }
+
+        let sut_open = Factory::open(&service_name, Vec::new()).unwrap();
+        for property in &properties {
+            assert_that!(sut_open.property(&property.0), eq vec![&property.1]);
+        }
+    }
+
     mod zero_copy {
         use iceoryx2::service::port_factory::event::PortFactory as EventPortFactory;
         use iceoryx2::service::port_factory::publish_subscribe::PortFactory as PubSubPortFactory;