Cleanup NullStr a bit, add basic tests

Author: jamesmunns

gateway-messages/src/lib.rs

@@ -24,8 +24,6 @@ pub use hubpack::{SerializedSize, deserialize, serialize};
 pub use mgs_to_sp::*;
 pub use sp_to_mgs::*;
 
-use crate::nullstr::NullStr;
-
 /// The SP should detach an attached serial console client if it has not heard
 /// from it in this long (based on the assumption that it has gone away without
 /// sending an explicit detach).
@@ -284,16 +282,23 @@ impl From<UpdateId> for uuid::Uuid {
 mod nullstr {
     use super::*;
 
+    /// A reusable type that implements a fixed-max-capacity string where the
+    /// unused capacity is always guaranteed to be zero-filled. This is used
+    /// for strings sent over the wire using [`hubpack`], which requires fixed-size
+    /// items.
+    ///
+    /// It is not intended for this type to be part of the public API, see
+    /// [`SpComponent`] for usage guidance.
     #[derive(Clone, Copy, PartialEq, Eq, Hash, SerializedSize)]
     pub struct NullStr<const N: usize> {
         contents: [u8; N],
     }
 
     impl<const N: usize> NullStr<N> {
-        /// Interpret the component name as a human-readable string.
+        /// Interpret the content as a human-readable string.
         ///
-        /// Our current expectation of component names is that this should never
-        /// fail (i.e., we're always storing component names as human-readable
+        /// Our current expectation of contents is that this should never
+        /// fail (i.e., we're always storing contents as human-readable
         /// strings), but because we reconstitute components from network messages
         /// we still need to check.
         pub fn as_str(&self) -> Option<&str> {
@@ -470,7 +475,10 @@ mod nullstr {
 }
 
 /// Identifier for a single component managed by an SP.
-#[derive(Clone, Copy, PartialEq, Eq, Hash, SerializedSize)]
+#[derive(
+    Clone, Copy, PartialEq, Eq, Hash, SerializedSize, Serialize, Deserialize,
+)]
+#[serde(transparent)]
 pub struct SpComponent {
     /// The ID of the component.
     ///
@@ -483,34 +491,6 @@ pub struct SpComponent {
     id: nullstr::NullStr<{ Self::MAX_ID_LENGTH }>,
 }
 
-impl Serialize for SpComponent {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        // TODO(AJM): I don't think we can just derive deserialize here because I think the old impl
-        // served as an implicit "flatten", and we don't want to introduce another level of nesting.
-        nullstr::NullStr::<{ Self::MAX_ID_LENGTH }>::serialize(
-            &self.id, serializer,
-        )
-    }
-}
-
-impl<'de> Deserialize<'de> for SpComponent {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        // TODO(AJM): I don't think we can just derive deserialize here because I think the old impl
-        // served as an implicit "flatten", and we don't want to introduce another level of nesting.
-        Ok(Self {
-            id: NullStr::<{ Self::MAX_ID_LENGTH }>::deserialize::<D>(
-                deserializer,
-            )?,
-        })
-    }
-}
-
 impl core::fmt::Display for SpComponent {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         self.id.fmt(f)
@@ -638,42 +618,15 @@ impl TryFrom<&str> for SpComponent {
     }
 }
 
-// -----
-
 /// Identifier for a single power rail.
-#[derive(Clone, Copy, PartialEq, Eq, Hash, SerializedSize)]
+#[derive(
+    Clone, Copy, PartialEq, Eq, Hash, SerializedSize, Serialize, Deserialize,
+)]
+#[serde(transparent)]
 pub struct PowerRailName {
     name: nullstr::NullStr<{ Self::MAX_NAME_LENGTH }>,
 }
 
-impl Serialize for PowerRailName {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        // TODO(AJM): I don't think we can just derive deserialize here because I think the old impl
-        // served as an implicit "flatten", and we don't want to introduce another level of nesting.
-        nullstr::NullStr::<{ Self::MAX_NAME_LENGTH }>::serialize(
-            &self.name, serializer,
-        )
-    }
-}
-
-impl<'de> Deserialize<'de> for PowerRailName {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        // TODO(AJM): I don't think we can just derive deserialize here because I think the old impl
-        // served as an implicit "flatten", and we don't want to introduce another level of nesting.
-        Ok(Self {
-            name: NullStr::<{ Self::MAX_NAME_LENGTH }>::deserialize::<D>(
-                deserializer,
-            )?,
-        })
-    }
-}
-
 impl core::fmt::Display for PowerRailName {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         self.name.fmt(f)
@@ -701,7 +654,7 @@ impl PowerRailName {
     /// This function should only be used in const contexts when the caller
     /// knows the component is valid (e.g., one of this type's associated
     /// constants); for component names parsed or constructed at runtime, prefer
-    /// [`SpComponent::as_str()`] which performs runtime validation.
+    /// [`PowerRailName::as_str()`] which performs runtime validation.
     #[inline]
     pub const fn const_as_str(&self) -> &str {
         self.name.const_as_str()
@@ -730,8 +683,6 @@ impl fmt::Debug for PowerRailName {
     }
 }
 
-// -----
-
 /// Minimum guaranteed space for trailing data in a single packet.
 ///
 /// Depending on the [`Message`] payload, there may be more space for trailing
@@ -850,4 +801,33 @@ mod tests {
             (component, &[] as &[u8])
         );
     }
+
+    #[test]
+    fn test_human_readable_power_rail() {
+        let rail = const { PowerRailName::from_const("V0P96_NIC_VDD_A0HP") };
+        let expected_value =
+            serde_json::Value::String("V0P96_NIC_VDD_A0HP".to_string());
+
+        assert_eq!(serde_json::to_value(rail).unwrap(), expected_value);
+        assert_eq!(
+            serde_json::from_value::<PowerRailName>(expected_value).unwrap(),
+            rail
+        );
+    }
+
+    #[test]
+    fn test_non_human_readable_power_rail() {
+        let rail = const { PowerRailName::from_const("V0P96_NIC_VDD_A0HP") };
+        let expected_value = rail.name;
+
+        let mut out = [0; PowerRailName::MAX_SIZE];
+        let n = hubpack::serialize(&mut out, &rail).unwrap();
+        assert_eq!(&out[..n], expected_value.contents());
+
+        assert_eq!(
+            hubpack::deserialize::<PowerRailName>(expected_value.contents())
+                .unwrap(),
+            (rail, &[] as &[u8])
+        );
+    }
 }