From 695c93459b767182f58c2de46dbac7d6fa0df675 Mon Sep 17 00:00:00 2001 From: Gavin Peacock Date: Tue, 8 Oct 2024 15:37:06 -0700 Subject: [PATCH] fix: Actions, support ingredient_ids parameter. This allows associating multiple ingredients with an action. It replaces the now deprecated use of instanceId. --- sdk/src/assertions/actions.rs | 82 ++++++++++++++++++--------------- sdk/tests/v2_api_integration.rs | 36 ++++++++++++++- 2 files changed, 79 insertions(+), 39 deletions(-) diff --git a/sdk/src/assertions/actions.rs b/sdk/src/assertions/actions.rs index f08fbee60..65c14f2e9 100644 --- a/sdk/src/assertions/actions.rs +++ b/sdk/src/assertions/actions.rs @@ -20,7 +20,6 @@ use crate::{ assertion::{Assertion, AssertionBase, AssertionCbor}, assertions::{labels, region_of_interest::RegionOfInterest, Actor, Metadata}, error::Result, - hashed_uri::HashedUri, resource_store::UriOrResource, utils::cbor_types::DateT, ClaimGeneratorInfo, @@ -84,20 +83,6 @@ impl From for SoftwareAgent { } } -#[derive(Deserialize, Serialize, Clone, Debug, PartialEq, Eq)] -#[serde()] -pub struct ActionParameters { - #[serde(skip_serializing_if = "Option::is_none")] - ingredient: Option, // v1 - #[serde(skip_serializing_if = "Option::is_none")] - description: Option, //v1 - #[serde(skip_serializing_if = "Option::is_none")] - ingredients: Option>, // v2 - - #[serde(flatten)] - other: HashMap, -} - /// Defines a single action taken on an asset. /// /// An [`Action`] describes what took place on the asset, when it took place, @@ -154,11 +139,6 @@ pub struct Action { // The reason why this action was performed, required when the action is `c2pa.redacted` #[serde(skip_serializing_if = "Option::is_none")] reason: Option, - - /// This is only used for pairing actions with an Ingredient it is not in the spec and not written to CBOR. - /// Set this to match the instance_id field of the ingredient you are pairing with. - #[serde(skip_serializing)] - ingredient_ids: Option>, } impl Action { @@ -295,10 +275,20 @@ impl Action { // internal function to return any ingredients referenced by this action pub(crate) fn ingredient_ids(&self) -> Option> { - if self.ingredient_ids.is_none() && self.instance_id.is_some() { - return self.instance_id().map(|id| vec![id.to_string()]); + match self.get_parameter("ingredient_ids") { + Some(Value::Array(ids)) => { + let mut result = Vec::new(); + for id in ids { + if let Value::Text(s) = id { + result.push(s.clone()); + } + } + Some(result) + } + Some(_) => None, // Invalid format, so ignore it. + // If there is no ingredient_ids parameter, check for the deprecated instance_id + None => self.instance_id().map(|id| vec![id.to_string()]), } - self.ingredient_ids.clone() } /// Sets the additional parameters for this action. @@ -326,6 +316,27 @@ impl Action { Ok(self) } + pub(crate) fn set_parameter_ref, T: Serialize>( + &mut self, + key: S, + value: T, + ) -> Result<&mut Self> { + let value_bytes = serde_cbor::ser::to_vec(&value)?; + let value = serde_cbor::from_slice(&value_bytes)?; + self.parameters = Some(match self.parameters.take() { + Some(mut parameters) => { + parameters.insert(key.into(), value); + parameters + } + None => { + let mut p = HashMap::new(); + p.insert(key.into(), value); + p + } + }); + Ok(self) + } + /// Sets the array of [`Actor`]s that undertook this action. pub fn set_actors(mut self, actors: Option<&Vec>) -> Self { self.actors = actors.cloned(); @@ -370,16 +381,14 @@ impl Action { } /// Adds an ingredient id to the action. - pub fn add_ingredient_id(mut self, ingredient_id: &str) -> Self { - match self.ingredient_ids { - Some(ref mut ingredient_ids) => { - ingredient_ids.push(ingredient_id.to_owned()); - } - None => { - self.ingredient_ids = Some(vec![ingredient_id.to_owned()]); - } + pub fn add_ingredient_id(&mut self, ingredient_id: &str) -> Result<&mut Self> { + if let Some(Value::Array(ids)) = self.get_parameter_mut("ingredient_ids") { + ids.push(Value::Text(ingredient_id.to_string())); + return Ok(self); } - self + let ids = vec![Value::Text(ingredient_id.to_string())]; + self.set_parameter_ref("ingredient_ids", ids)?; + Ok(self) } } @@ -592,7 +601,7 @@ pub mod tests { .set_parameter("ingredient".to_owned(), make_hashed_uri1()) .unwrap() .set_changed(Some(&["this", "that"].to_vec())) - .add_ingredient_id("xmp.iid:cb9f5498-bb58-4572-8043-8c369e6bfb9b") + // .add_ingredient_id("xmp.iid:cb9f5498-bb58-4572-8043-8c369e6bfb9b").unwrap() .set_actors(Some( &[Actor::new( Some("Somebody"), @@ -653,8 +662,8 @@ pub mod tests { ); assert_eq!(result.actions[1].action(), original.actions[1].action()); assert_eq!( - result.actions[1].parameters.as_ref().unwrap().get("name"), - original.actions[1].parameters.as_ref().unwrap().get("name") + result.actions[1].parameters().unwrap().get("name"), + original.actions[1].parameters().unwrap().get("name") ); assert_eq!(result.actions[1].when(), original.actions[1].when()); assert_eq!( @@ -782,12 +791,11 @@ pub mod tests { "mytag": "myvalue" } }); - let mut original = Actions::from_json_value(&json).expect("from json"); + let original = Actions::from_json_value(&json).expect("from json"); let assertion = original.to_assertion().expect("build_assertion"); let result = Actions::from_assertion(&assertion).expect("extract_assertion"); assert_eq!(result.label(), labels::ACTIONS); println!("{}", serde_json::to_string_pretty(&result).unwrap()); - original.actions[1].ingredient_ids = None; // remove this since it won't be in the result assert_eq!(original.actions, result.actions); assert_eq!( result.actions[0].software_agent().unwrap(), diff --git a/sdk/tests/v2_api_integration.rs b/sdk/tests/v2_api_integration.rs index 0e2b51b1f..c2b33e599 100644 --- a/sdk/tests/v2_api_integration.rs +++ b/sdk/tests/v2_api_integration.rs @@ -59,7 +59,11 @@ mod integration_v2 { "title": "Test", "format": "image/jpeg", "instance_id": "12345", - "relationship": "inputTo" + "relationship": "inputTo", + "metadata": { + "dateTime": "1985-04-12T23:20:50.52Z", + "my_custom_metadata": "my custom metatdata value" + } } ], "assertions": [ @@ -70,10 +74,33 @@ mod integration_v2 { { "action": "c2pa.edited", "digitalSourceType": "http://cv.iptc.org/newscodes/digitalsourcetype/trainedAlgorithmicMedia", - "softwareAgent": "Adobe Firefly 0.1.0" + "softwareAgent": "Adobe Firefly 0.1.0", + "parameters": { + "description": "This image was edited by Adobe Firefly", + } } ] } + }, + { + "label": "c2pa.metadata", + "data": { + "@context" : { + "exif": "http://ns.adobe.com/exif/1.0/", + "exifEX": "http://cipa.jp/exif/2.32/", + "tiff": "http://ns.adobe.com/tiff/1.0/", + "Iptc4xmpExt": "http://iptc.org/std/Iptc4xmpExt/2008-02-29/", + "photoshop" : "http://ns.adobe.com/photoshop/1.0/" + }, + "photoshop:DateCreated": "Aug 31, 2022", + "Iptc4xmpExt:DigitalSourceType": "https://cv.iptc.org/newscodes/digitalsourcetype/digitalCapture", + "exif:GPSVersionID": "2.2.0.0", + "exif:GPSLatitude": "39,21.102N", + "exif:GPSLongitude": "74,26.5737W", + "exif:GPSAltitudeRef": 0, + "exif:GPSAltitude": "100963/29890", + "exifEX:LensSpecification": { "@list": [ 1.55, 4.2, 1.6, 2.4 ] } + } } ] }).to_string() @@ -120,6 +147,11 @@ mod integration_v2 { dest }; + // write dest to file for debugging + let debug_path = format!("{}/../target/v2_test.jpg", env!("CARGO_MANIFEST_DIR")); + std::fs::write(debug_path, dest.get_ref())?; + dest.rewind()?; + let reader = Reader::from_stream(format, &mut dest)?; // extract a thumbnail image from the ManifestStore