feat: Updates API and adds new examples #1298
Conversation
BREAKING CHANGE: Builder.add_ingredient now returns an Result that must be handled. This should have always been the case. It now allows more types that can be converted into Ingredients as a parameter. Updates more example code.
| /// | ||
| /// The Manifest must not have have a parent ingredient. | ||
| /// A `c2pa.created` action will be added if not provided. | ||
| #[serde(rename = "create")] |
There was a problem hiding this comment.
You can also do #[serde(rename_all = "lowercase")] on the struct definition.
| x509-parser = "0.16.0" | ||
| zeroize = { version = "1.8", features = ["zeroize_derive"] } | ||
| zip = { version = "3.0.0", default-features = false } | ||
| clap = { version = "4.0", features = ["derive"] } |
There was a problem hiding this comment.
Did you mean to add this to the SDK dependencies?
| #[allow(deprecated)] | ||
| builder | ||
| .set_claim_generator_info(generator) | ||
| .add_ingredient(parent) |
There was a problem hiding this comment.
Do we add the parent ingredient automatically now?
| dest: Option<String>, | ||
| /// Settings file path | ||
| #[arg(long)] | ||
| settings: Option<String>, |
There was a problem hiding this comment.
You may be able to use a PathBuf here.
| builder.add_action(json!({ | ||
| "action": "c2pa.edited", | ||
| "description": "my special edits", | ||
| "parameters": { | ||
| "name": "any value" | ||
| }, | ||
| }))?; |
There was a problem hiding this comment.
Do we want the examples to show passing in json or building the Actions struct directly? I'm leaning a bit more towards the latter to keep everything more Rusty.
There was a problem hiding this comment.
This is a little of both, to show that it works both ways.
| impl TryFrom<&str> for Ingredient { | ||
| type Error = crate::Error; | ||
|
|
||
| fn try_from(json: &str) -> std::result::Result<Self, Self::Error> { | ||
| Self::from_json(json) | ||
| } | ||
| } | ||
|
|
||
| impl TryFrom<String> for Ingredient { | ||
| type Error = crate::Error; | ||
|
|
||
| fn try_from(json: String) -> std::result::Result<Self, Self::Error> { | ||
| Self::from_json(&json) | ||
| } | ||
| } | ||
|
|
||
| impl TryFrom<&String> for Ingredient { | ||
| type Error = crate::Error; | ||
|
|
||
| fn try_from(json: &String) -> std::result::Result<Self, Self::Error> { | ||
| Self::from_json(json) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
You may be able to do impl TryFrom<AsRef<str>> rather than 3 implementations. I'm not too sure how I feel about this either, I think it makes more sense to have an explicit function to convert from json to an Ingredient.
| pub fn add_ingredient<I>(&mut self, ingredient: I) -> Result<&mut Self> | ||
| where | ||
| I: Into<Ingredient>, | ||
| I: TryInto<Ingredient>, | ||
| I::Error: std::fmt::Display, |
There was a problem hiding this comment.
I'm leaning a bit more towards having Builder.add_ingredient(Ingredient::from_json(json)) or something of the sort. I think this makes the most sense for Rust, but in our C bindings it would be fine if we had something like c2pa_builder_add_ingredient(json)
2 participants
Builder::open() renamed to Builder::Edit()
Breaking change: Builder.add_ingredient() now returns a Result that must be checked.
It also allows passing anything that can be serialized into an Ingredient.
Updated Builder.add_ingredient_with_stream to accept a serializable Ingredient parameter.
Updates example and test code to use the new methods.