Issue-294: return Down if peer has no state associated and add unit t…

…ests, remove inserting peer state while storing peer descriptor

Co-authored-by: Anna Völker <anna.voelker@mercedes-benz.com>
Co-authored-by: Matthias Twardawski <matthias.twardawski@mercedes-benz.com>

opendut-carl/src/actions/peers/get_peer_state.rs

@@ -1,7 +1,7 @@
-use crate::resources::manager::ResourcesManagerRef;
-use opendut_carl_api::carl::peer::GetPeerStateError;
+use crate::resources::manager::{ResourcesManagerRef};
+use opendut_carl_api::carl::peer::{GetPeerStateError};
 use opendut_types::peer::state::PeerState;
-use opendut_types::peer::PeerId;
+use opendut_types::peer::{PeerDescriptor, PeerId};
 use tracing::{debug, error, info};
 
 pub struct GetPeerStateParams {
@@ -18,13 +18,21 @@ pub async fn get_peer_state(params: GetPeerStateParams) -> Result<PeerState, Get
         let resources_manager = params.resources_manager;
 
         debug!("Querying state of peer with peer_id <{}>.", peer_id);
-
+        
         let peer_state = resources_manager.resources_mut(|resources| {
-            resources.get::<PeerState>(peer_id)
-        }).await
-            .map_err(|cause| GetPeerStateError::Internal {  peer_id ,cause: cause.to_string() })?
-            .ok_or(GetPeerStateError::PeerNotFound { peer_id })?;
-
+            let peer_state = resources.get::<PeerState>(peer_id)
+                .map_err(|cause| GetPeerStateError::Internal {  peer_id ,cause: cause.to_string() })?;
+            match peer_state {
+                Some(peer_state) => { Ok(peer_state) }
+                None => {
+                    match resources.get::<PeerDescriptor>(peer_id)
+                        .map_err(|cause| GetPeerStateError::Internal {  peer_id ,cause: cause.to_string() })? {
+                        Some(_) => { Ok(PeerState::Down)  }
+                        None => { Err(GetPeerStateError::PeerNotFound { peer_id }) }
+                    }
+                }
+            }
+        }).await?;
 
         info!("Successfully queried state of peer with peer_id <{}>.", peer_id);
 
@@ -34,3 +42,63 @@ pub async fn get_peer_state(params: GetPeerStateParams) -> Result<PeerState, Get
     inner(params).await
         .inspect_err(|err| error!("{err}"))
 }
+
+#[cfg(test)]
+mod tests {
+    use std::sync::Arc;
+    use googletest::assert_that;
+    use googletest::matchers::{eq};
+    use rstest::rstest;
+    use opendut_types::peer::PeerId;
+    use opendut_types::peer::state::PeerState;
+    use crate::actions;
+    use crate::actions::{GetPeerStateParams, StorePeerDescriptorOptions, StorePeerDescriptorParams};
+    use crate::actions::peers::testing::{fixture, store_peer_descriptor_options, Fixture};
+
+    #[rstest]
+    #[tokio::test]
+    async fn should_get_peer_state(fixture: Fixture, store_peer_descriptor_options: StorePeerDescriptorOptions) -> anyhow::Result<()> {
+
+        let resources_manager = fixture.resources_manager;
+
+        actions::store_peer_descriptor(StorePeerDescriptorParams {
+            resources_manager: Arc::clone(&resources_manager),
+            vpn: fixture.vpn,
+            peer_descriptor: fixture.peer_a_descriptor,
+            options: store_peer_descriptor_options,
+        }).await?;
+
+        let peer_state = actions::get_peer_state(GetPeerStateParams {
+            peer: fixture.peer_a_id,
+            resources_manager: Clone::clone(&resources_manager),
+        }).await?;
+
+        assert_that!(peer_state, eq(PeerState::Down));
+        Ok(())
+    }
+
+    #[rstest]
+    #[tokio::test]
+    async fn should_not_find_peer_for_id(fixture: Fixture, store_peer_descriptor_options: StorePeerDescriptorOptions) -> anyhow::Result<()> {
+
+        let resources_manager = fixture.resources_manager;
+
+        actions::store_peer_descriptor(StorePeerDescriptorParams {
+            resources_manager: Arc::clone(&resources_manager),
+            vpn: fixture.vpn,
+            peer_descriptor: fixture.peer_a_descriptor,
+            options: store_peer_descriptor_options,
+        }).await?;
+
+        let peer_id = PeerId::random();
+        let peer_state_result = actions::get_peer_state(GetPeerStateParams {
+            peer: peer_id,
+            resources_manager: Clone::clone(&resources_manager),
+        }).await;
+
+        assert!(peer_state_result.is_err());
+        let expected_error_message = format!("A peer with id <{peer_id}> could not be found!");
+        assert_that!(peer_state_result.unwrap_err().to_string(), eq(expected_error_message));
+        Ok(())
+    }
+}

opendut-carl/src/actions/peers/store_peer_descriptor.rs

@@ -4,7 +4,6 @@ use crate::vpn::Vpn;
 use opendut_carl_api::carl::peer::StorePeerDescriptorError;
 use opendut_types::peer;
 use opendut_types::peer::configuration::{PeerConfiguration, PeerConfiguration2, PeerNetworkConfiguration};
-use opendut_types::peer::state::PeerState;
 use opendut_types::peer::{PeerDescriptor, PeerId};
 use opendut_types::topology::DeviceDescriptor;
 use opendut_types::util::net::NetworkInterfaceName;
@@ -92,11 +91,6 @@ pub async fn store_peer_descriptor(params: StorePeerDescriptorParams) -> Result<
                 peer_configuration2
             };
             resources.insert(peer_id, peer_configuration2)?; //FIXME don't just insert, but rather update existing values via ID with intelligent logic (in a separate action)
-
-
-            let peer_state = resources.get::<PeerState>(peer_id)?
-                .unwrap_or(PeerState::Down); // If peer is new or no peer was found in the database, we consider it as PeerState::Down.
-            resources.insert(peer_id, peer_state)?;
             resources.insert(peer_id, peer_descriptor)?;
 
             Ok(is_new_peer)
@@ -147,6 +141,7 @@ mod tests {
     use opendut_types::util::net::NetworkInterfaceId;
     use rstest::rstest;
     use std::sync::Arc;
+    use opendut_types::peer::state::PeerState;
 
     #[rstest]
     #[tokio::test]
@@ -176,7 +171,7 @@ mod tests {
         assert_that!(resources_manager.get::<PeerDescriptor>(fixture.peer_a_id).await?.as_ref(), some(eq(&fixture.peer_a_descriptor)));
         assert_that!(resources_manager.get::<DeviceDescriptor>(fixture.peer_a_device_1).await?.as_ref(), some(eq(&fixture.peer_a_descriptor.topology.devices[0])));
         assert_that!(resources_manager.get::<DeviceDescriptor>(fixture.peer_a_device_2).await?.as_ref(), some(eq(&fixture.peer_a_descriptor.topology.devices[1])));
-        assert_that!(resources_manager.get::<PeerState>(fixture.peer_a_id).await?.as_ref(), some(eq(&PeerState::Down)));
+        assert_that!(resources_manager.get::<PeerState>(fixture.peer_a_id).await?.as_ref(), none());
 
         let additional_device_id = DeviceId::random();
         let additional_device = DeviceDescriptor {
@@ -208,7 +203,7 @@ mod tests {
         assert_that!(resources_manager.get::<DeviceDescriptor>(fixture.peer_a_device_1).await?.as_ref(), some(eq(&fixture.peer_a_descriptor.topology.devices[0])));
         assert_that!(resources_manager.get::<DeviceDescriptor>(additional_device_id).await?.as_ref(), some(eq(&additional_device)));
         assert_that!(resources_manager.get::<DeviceDescriptor>(fixture.peer_a_device_2).await?.as_ref(), none());
-        assert_that!(resources_manager.get::<PeerState>(fixture.peer_a_id).await?.as_ref(), some(eq(&PeerState::Down)));
+        assert_that!(resources_manager.get::<PeerState>(fixture.peer_a_id).await?.as_ref(), none());
 
         Ok(())
     }