Changes Layer's annotation field into a annotations, a list of annota…

…tions

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>

crates/spfs-cli/main/src/cmd_info.rs

@@ -139,11 +139,13 @@ impl CmdInfo {
                     }
                 );
 
-                if let Some(data) = obj.annotation() {
-                    let annotation: Annotation = data.into();
-                    println!(" {}", "annotation:".bright_blue());
-                    println!("  {} {}", "key:".bright_blue(), annotation.key());
-                    println!("  {} {}", "value:".bright_blue(), annotation.value());
+                if let Some(annotations) = obj.annotations() {
+                    for data in annotations {
+                        let annotation: Annotation = data.into();
+                        println!(" {}", "annotation:".bright_blue());
+                        println!("  {} {}", "key:".bright_blue(), annotation.key());
+                        println!("  {} {}", "value:".bright_blue(), annotation.value());
+                    }
                 } else {
                     println!(" {} none", "annotation:".bright_blue());
                 }

crates/spfs-proto/schema/spfs.fbs

@@ -21,7 +21,7 @@ table Platform {
 
 table Layer {
     manifest:Digest;
-    annotation:Annotation;
+    annotations:[Annotation];
 }
 
 table Manifest {

crates/spfs/src/check.rs

@@ -340,17 +340,32 @@ where
             // This layer has no manifest, don't worry about it
             CheckObjectResult::Ignorable
         };
-        let annotation_result = match layer.annotation() {
-            None => CheckAnnotationResult::Skipped,
-            Some(annotation) => self.check_annotation(annotation.into()).await?,
+        let annotation_results = match layer.annotations() {
+            None => vec![CheckObjectResult::Annotation(
+                CheckAnnotationResult::Skipped,
+            )],
+            Some(annotations) => {
+                let mut results = Vec::new();
+                for entry in annotations {
+                    results.push(CheckObjectResult::Annotation(
+                        self.check_annotation(entry.into()).await?,
+                    ));
+                }
+                if results.is_empty() {
+                    results.push(CheckObjectResult::Annotation(
+                        CheckAnnotationResult::Skipped,
+                    ));
+                }
+                results
+            }
         };
 
+        let mut results = vec![manifest_result];
+        results.extend(annotation_results);
+
         let res = CheckLayerResult {
             layer,
-            results: vec![
-                manifest_result,
-                CheckObjectResult::Annotation(annotation_result),
-            ],
+            results,
             repaired: false,
         };
         Ok(res)

crates/spfs/src/graph/annotation.rs

@@ -4,8 +4,6 @@
 
 use std::fmt::Display;
 
-use spfs_proto::AnnotationArgs;
-
 use crate::{encoding, Result};
 
 #[cfg(test)]
@@ -34,21 +32,43 @@ pub enum AnnotationValue {
     Blob(encoding::Digest),
 }
 
+impl Default for AnnotationValue {
+    fn default() -> Self {
+        AnnotationValue::String(Default::default())
+    }
+}
+
 impl AnnotationValue {
-    /// The underlying spfs_proto enum entry for this kind of value.
-    pub fn to_proto(&self) -> spfs_proto::AnnotationValue {
+    pub fn build(
+        &self,
+        builder: &mut flatbuffers::FlatBufferBuilder<'_>,
+    ) -> flatbuffers::WIPOffset<flatbuffers::UnionWIPOffset> {
         match self {
-            AnnotationValue::String(_) => spfs_proto::AnnotationValue::AnnotationString,
-            AnnotationValue::Blob(_) => spfs_proto::AnnotationValue::AnnotationDigest,
+            AnnotationValue::String(data_string) => {
+                let string_data = builder.create_string(data_string.as_ref());
+                spfs_proto::AnnotationString::create(
+                    builder,
+                    &spfs_proto::AnnotationStringArgs {
+                        data: Some(string_data),
+                    },
+                )
+                .as_union_value()
+            }
+            AnnotationValue::Blob(data_digest) => spfs_proto::AnnotationDigest::create(
+                builder,
+                &spfs_proto::AnnotationDigestArgs {
+                    digest: Some(data_digest),
+                },
+            )
+            .as_union_value(),
         }
     }
 
-    /// Return the data value as a string whether it is a string or a
-    /// digest for a blob object.
-    pub fn data_as_string(&self) -> String {
+    /// The underlying spfs_proto enum entry for this kind of value.
+    pub fn to_proto(&self) -> spfs_proto::AnnotationValue {
         match self {
-            AnnotationValue::String(data) => data.clone(),
-            AnnotationValue::Blob(digest) => digest.to_string(),
+            AnnotationValue::String(_) => spfs_proto::AnnotationValue::AnnotationString,
+            AnnotationValue::Blob(_) => spfs_proto::AnnotationValue::AnnotationDigest,
         }
     }
 
@@ -112,22 +132,6 @@ impl<'buf> From<spfs_proto::Annotation<'buf>> for Annotation<'buf> {
 }
 
 impl<'buf> Annotation<'buf> {
-    pub fn build<'fbb>(
-        &self,
-        builder: &mut flatbuffers::FlatBufferBuilder<'fbb>,
-    ) -> flatbuffers::WIPOffset<spfs_proto::Annotation<'fbb>> {
-        let fb_key = builder.create_string(self.key().as_ref());
-        let fb_data = builder.create_string(self.value().data_as_string().as_ref());
-        spfs_proto::Annotation::create(
-            builder,
-            &AnnotationArgs {
-                key: Some(fb_key),
-                data_type: self.0.data_type(),
-                data: Some(fb_data.as_union_value()),
-            },
-        )
-    }
-
     #[inline]
     pub fn key(&self) -> &'buf str {
         self.0.key()

crates/spfs/src/graph/layer.rs

@@ -28,7 +28,7 @@ impl std::fmt::Debug for Layer {
                     .manifest()
                     .map_or(String::from("None"), |d| d.to_string()),
             )
-            .field("annotation", &self.annotation())
+            .field("annotation", &self.annotations())
             .finish()
     }
 }
@@ -50,6 +50,14 @@ impl Layer {
         Self::builder().with_annotation(key, value).build()
     }
 
+    /// Build a layer with the default header that has the provided
+    /// annotation data but does not point at any manifest, for more
+    /// configuration use [`Self::builder`]
+    #[inline]
+    pub fn new_with_annotations(annotations: Vec<KeyAnnotationValuePair>) -> Self {
+        Self::builder().with_annotations(annotations).build()
+    }
+
     /// Build a layer with the default header that points at the
     /// provided manifest digest and the provided annotation, for
     /// more configuration use [`Self::builder`]
@@ -65,6 +73,20 @@ impl Layer {
             .build()
     }
 
+    /// Build a layer with the default header that points at the
+    /// provided manifest digest and the provided annotation, for
+    /// more configuration use [`Self::builder`]
+    #[inline]
+    pub fn new_with_manifest_and_annotations(
+        manifest: encoding::Digest,
+        annotations: Vec<KeyAnnotationValuePair>,
+    ) -> Self {
+        Self::builder()
+            .with_manifest(manifest)
+            .with_annotations(annotations)
+            .build()
+    }
+
     #[inline]
     pub fn builder() -> LayerBuilder {
         LayerBuilder::default()
@@ -76,8 +98,10 @@ impl Layer {
     }
 
     #[inline]
-    pub fn annotation(&self) -> Option<spfs_proto::Annotation> {
-        self.proto().annotation()
+    pub fn annotations(&self) -> Option<Vec<spfs_proto::Annotation>> {
+        self.proto()
+            .annotations()
+            .map(|annotations| annotations.iter().collect::<Vec<spfs_proto::Annotation>>())
     }
 
     /// Return the child object of this one in the object DG.
@@ -87,9 +111,11 @@ impl Layer {
         if let Some(manifest_digest) = self.manifest() {
             children.push(*manifest_digest)
         }
-        if let Some(data) = self.annotation() {
-            let annotation: Annotation = data.into();
-            children.extend(annotation.child_objects());
+        if let Some(annotations) = self.annotations() {
+            for entry in annotations {
+                let annotation: Annotation = entry.into();
+                children.extend(annotation.child_objects());
+            }
         }
         children
     }
@@ -99,18 +125,24 @@ impl Layer {
         let result = if let Some(manifest_digest) = self.manifest() {
             let manifest_result =
                 encoding::write_digest(&mut writer, manifest_digest).map_err(Error::Encoding);
-            if let Some(data) = self.annotation() {
+            if let Some(annotations) = self.annotations() {
                 tracing::trace!("layer with both manifest and annotation data");
-                let annotation: Annotation = data.into();
-                annotation.legacy_encode(&mut writer)
+                for entry in annotations {
+                    let annotation: Annotation = entry.into();
+                    annotation.legacy_encode(&mut writer)?;
+                }
+                Ok(())
             } else {
                 tracing::trace!("layer with manifest only: {manifest_result:?}");
                 manifest_result
             }
-        } else if let Some(data) = self.annotation() {
+        } else if let Some(annotations) = self.annotations() {
             tracing::trace!("layer with annotation only");
-            let annotation: Annotation = data.into();
-            annotation.legacy_encode(&mut writer)
+            for entry in annotations {
+                let annotation: Annotation = entry.into();
+                annotation.legacy_encode(&mut writer)?;
+            }
+            Ok(())
         } else {
             tracing::trace!("layer without either kind of data - an invalid layer");
             Err(Error::String(
@@ -137,20 +169,22 @@ impl std::cmp::PartialEq for Layer {
 
 impl std::cmp::Eq for Layer {}
 
+/// Data type for pairs of keys and annotation values used during
+/// construction of a layer's annotations.
+pub type KeyAnnotationValuePair = (String, AnnotationValue);
+
 pub struct LayerBuilder {
     header: super::object::HeaderBuilder,
     manifest: Option<encoding::Digest>,
-    key: Option<String>,
-    value: Option<AnnotationValue>,
+    annotations: Vec<KeyAnnotationValuePair>,
 }
 
 impl Default for LayerBuilder {
     fn default() -> Self {
         Self {
             header: super::object::HeaderBuilder::new(ObjectKind::Layer),
             manifest: None,
-            key: None,
-            value: None,
+            annotations: Vec::new(),
         }
     }
 }
@@ -170,57 +204,44 @@ impl LayerBuilder {
     }
 
     pub fn with_annotation(mut self, key: String, value: AnnotationValue) -> Self {
-        self.key = Some(key);
-        self.value = Some(value);
+        self.annotations.push((key, value));
+        self
+    }
+
+    pub fn with_annotations(mut self, annotations: Vec<KeyAnnotationValuePair>) -> Self {
+        self.annotations.extend(annotations);
         self
     }
 
     pub fn build(&self) -> Layer {
         super::BUILDER.with_borrow_mut(|builder| {
-            let annotation = match &self.key {
-                None => None,
-                Some(key) => {
-                    let fb_key = builder.create_string(key.as_ref());
-                    let value = match &self.value {
-                        None => AnnotationValue::String(Default::default()),
-                        Some(value) => value.clone(),
-                    };
-                    let data_type = value.to_proto();
-                    let fb_data = match &value {
-                        AnnotationValue::String(data_string) => {
-                            let string_data = builder.create_string(data_string.as_ref());
-                            spfs_proto::AnnotationString::create(
-                                builder,
-                                &spfs_proto::AnnotationStringArgs {
-                                    data: Some(string_data),
-                                },
-                            )
-                            .as_union_value()
-                        }
-                        AnnotationValue::Blob(data_digest) => spfs_proto::AnnotationDigest::create(
+            let annotations = if self.annotations.is_empty() {
+                None
+            } else {
+                let ffb_annotations: Vec<_> = self
+                    .annotations
+                    .iter()
+                    .map(|(k, v)| {
+                        let key = builder.create_string(k);
+                        let value = v.build(builder);
+                        spfs_proto::Annotation::create(
                             builder,
-                            &spfs_proto::AnnotationDigestArgs {
-                                digest: Some(data_digest),
+                            &spfs_proto::AnnotationArgs {
+                                key: Some(key),
+                                data_type: v.to_proto(),
+                                data: Some(value),
                             },
                         )
-                        .as_union_value(),
-                    };
-                    Some(spfs_proto::Annotation::create(
-                        builder,
-                        &spfs_proto::AnnotationArgs {
-                            key: Some(fb_key),
-                            data_type,
-                            data: Some(fb_data),
-                        },
-                    ))
-                }
+                    })
+                    .collect();
+                Some(builder.create_vector(&ffb_annotations))
             };
 
             let layer = spfs_proto::Layer::create(
                 builder,
                 &LayerArgs {
                     manifest: self.manifest.as_ref(),
-                    annotation,
+                    annotations,
                 },
             );
             let any = spfs_proto::AnyObject::create(

crates/spfs/src/graph/mod.rs

@@ -34,7 +34,7 @@ pub use database::{
 };
 pub use entry::Entry;
 pub use kind::{HasKind, Kind, ObjectKind};
-pub use layer::Layer;
+pub use layer::{KeyAnnotationValuePair, Layer};
 pub use manifest::Manifest;
 pub use object::{FlatObject, Object, ObjectProto};
 pub use platform::Platform;

crates/spfs/src/graph/object.rs

@@ -161,9 +161,11 @@ impl Object {
                             let item = repo.read_object(*manifest_digest).await?;
                             next_iter_objects.push(item);
                         }
-                        if let Some(data) = object.annotation() {
-                            let annotation: Annotation = data.into();
-                            total_size += annotation.size();
+                        if let Some(annotations) = object.annotations() {
+                            for entry in annotations {
+                                let annotation: Annotation = entry.into();
+                                total_size += annotation.size();
+                            }
                         }
                     }
                     Enum::Manifest(object) => {