Apply review changes

Author: lucaato

src/transport/grpc/convert.rs

@@ -85,17 +85,21 @@ pub(crate) fn map_property_to_astarte_type(
     Ok(Some(individual.try_into()?))
 }
 
-/// Map a list of properties that need to be set, unset value will result in an error of the conversion
+/// Map a list of properties, unset properties will be ignored
 pub(crate) fn map_set_stored_properties(
     mut message_hub_properties: astarte_message_hub_proto::StoredProperties,
 ) -> Result<Vec<StoredProp>, MessageHubProtoError> {
     message_hub_properties
         .interface_properties
         .iter_mut()
         .flat_map(|(name, prop_data)| {
-            prop_data.properties.iter().map(|p| {
+            prop_data.properties.iter().filter_map(|p| {
                 let path = p.path.clone();
-                let value: AstarteType = p.clone().try_into()?;
+                let value: AstarteType =
+                    match map_property_to_astarte_type(p.clone()).transpose()? {
+                        Ok(s) => s,
+                        Err(e) => return Some(Err(e)),
+                    };
 
                 let res = StoredProp {
                     interface: name.clone(),
@@ -105,7 +109,7 @@ pub(crate) fn map_set_stored_properties(
                     ownership: prop_data.ownership().into(),
                 };
 
-                Ok(res)
+                Some(Ok(res))
             })
         })
         .try_collect()
@@ -120,16 +124,6 @@ impl From<astarte_message_hub_proto::Ownership> for Ownership {
     }
 }
 
-/// Construct an sdk astarte type from a property that is required to be set
-impl TryFrom<astarte_message_hub_proto::Property> for AstarteType {
-    type Error = MessageHubProtoError;
-
-    fn try_from(property: astarte_message_hub_proto::Property) -> Result<Self, Self::Error> {
-        map_property_to_astarte_type(property)
-            .and_then(|value| value.ok_or(MessageHubProtoError::ExpectedSetProperty))
-    }
-}
-
 impl TryFrom<ProtoIndividualData> for AstarteType {
     type Error = MessageHubProtoError;
 
@@ -472,6 +466,8 @@ mod test {
     };
     use chrono::{DateTime, Utc};
 
+    use crate::transport::grpc::convert::map_property_to_astarte_type;
+
     use super::*;
 
     #[test]
@@ -1356,26 +1352,23 @@ mod test {
     }
 
     #[test]
-    fn map_property_to_astarte_type() {
+    fn map_property_to_astarte_type_ok() {
         let value: AstarteType = AstarteType::String("test".to_owned());
 
         let prop = make_messagehub_property("/path11", Some(value.clone()));
 
-        let astarte_type = AstarteType::try_from(prop).unwrap();
+        let astarte_type = map_property_to_astarte_type(prop).unwrap().unwrap();
 
         assert_eq!(value, astarte_type);
     }
 
     #[test]
-    fn map_property_to_astarte_type_error() {
+    fn map_property_to_astarte_type_none() {
         let prop = make_messagehub_property("/path11", None);
 
-        let astarte_type_err = AstarteType::try_from(prop);
+        let astarte_type_err = map_property_to_astarte_type(prop);
 
-        assert!(matches!(
-            astarte_type_err,
-            Err(MessageHubProtoError::ExpectedSetProperty)
-        ));
+        assert!(matches!(astarte_type_err, Ok(None)));
     }
 
     #[test]

src/transport/grpc/store.rs

@@ -69,10 +69,8 @@ impl GrpcStore {
             })
             .await
             .map(tonic::Response::into_inner)
-            .map_err(GrpcStoreError::from_status)
-            .and_then(|p| {
-                convert::map_set_stored_properties(p).map_err(GrpcStoreError::from_conversion)
-            })
+            .map_err(GrpcStoreError::from)
+            .and_then(|p| Ok(convert::map_set_stored_properties(p)?))
     }
 }
 
@@ -87,16 +85,12 @@ pub enum GrpcStoreError {
     Status(tonic::Status),
     /// Error while converting a proto received value to an internal type
     #[error("Error while converting a proto received value to an internal type: {0}")]
-    Conversion(MessageHubProtoError),
+    Conversion(#[from] MessageHubProtoError),
 }
 
-impl GrpcStoreError {
-    fn from_status(status: tonic::Status) -> Self {
-        Self::Status(status)
-    }
-
-    fn from_conversion(conversion: MessageHubProtoError) -> Self {
-        Self::Conversion(conversion)
+impl From<tonic::Status> for GrpcStoreError {
+    fn from(value: tonic::Status) -> Self {
+        Self::Status(value)
     }
 }
 
@@ -133,10 +127,10 @@ impl PropertyStore for GrpcStore {
                 path: property.path().to_owned(),
             })
             .await
-            .map_err(GrpcStoreError::from_status)
+            .map_err(GrpcStoreError::from)
             .map(tonic::Response::into_inner)?;
 
-        convert::map_property_to_astarte_type(property).map_err(GrpcStoreError::from_conversion)
+        Ok(convert::map_property_to_astarte_type(property)?)
     }
 
     async fn unset_prop(&self, _property: &PropertyMapping<'_>) -> Result<(), Self::Err> {
@@ -180,10 +174,8 @@ impl PropertyStore for GrpcStore {
             })
             .await
             .map(tonic::Response::into_inner)
-            .map_err(GrpcStoreError::from_status)
-            .and_then(|p| {
-                convert::map_set_stored_properties(p).map_err(GrpcStoreError::from_conversion)
-            })
+            .map_err(GrpcStoreError::from)
+            .and_then(|p| Ok(convert::map_set_stored_properties(p)?))
     }
 
     async fn delete_interface(&self, _interface: &PropertyInterface<'_>) -> Result<(), Self::Err> {

src/transport/mqtt/config/mod.rs

@@ -278,7 +278,7 @@ impl MqttConfig {
             Credential::ParingToken { pairing_token } => {
                 debug!("pairing token provided, retrieving credentials secret");
 
-                let Some(dir) = config.writable_dir.clone() else {
+                let Some(dir) = &config.writable_dir else {
                     return Err(MqttError::NoStorePairingToken);
                 };
 
@@ -294,7 +294,7 @@ impl MqttConfig {
     /// Register the device and stores the credentials secret in the given directory
     async fn read_secret_or_register(
         &self,
-        store_dir: PathBuf,
+        store_dir: &Path,
         pairing_token: &str,
     ) -> Result<String, PairingError> {
         let credential_file = store_dir.join(CREDENTIAL_FILE);