Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adds validation_state to the json reports from the Reader #930

Merged
merged 6 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cawg_identity/src/identity_assertion/assertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@
///
/// [`ManifestStore`]: c2pa::ManifestStore
#[cfg(feature = "v1_api")]
pub async fn summarize_manifest_store<SV: SignatureVerifier>(
store: &c2pa::ManifestStore,
pub async fn summarize_reader<SV: SignatureVerifier>(
reader: &c2pa::Reader,

Check warning on line 165 in cawg_identity/src/identity_assertion/assertion.rs

View check run for this annotation

Codecov / codecov/patch

cawg_identity/src/identity_assertion/assertion.rs#L164-L165

Added lines #L164 - L165 were not covered by tests
verifier: &SV,
) -> impl Serialize {
// NOTE: We can't write this using .map(...).collect() because there are async
Expand All @@ -174,7 +174,7 @@
>,
> = BTreeMap::new();

for (id, manifest) in store.manifests() {
for (id, manifest) in reader.manifests() {

Check warning on line 177 in cawg_identity/src/identity_assertion/assertion.rs

View check run for this annotation

Codecov / codecov/patch

cawg_identity/src/identity_assertion/assertion.rs#L177

Added line #L177 was not covered by tests
let report = Self::summarize_all_impl(manifest, verifier).await;
reports.insert(id.clone(), report);
}
Expand Down
23 changes: 9 additions & 14 deletions make_test_images/src/compare_manifests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use std::collections::HashMap;
use std::{fs, path::Path};

use c2pa::{Error, Reader as ManifestStore, Result};
use c2pa::{Error, Reader, Result};

/// Compares all the files in two directories and returns a list of issues
pub fn compare_folders<P: AsRef<Path>, Q: AsRef<Path>>(folder1: P, folder2: Q) -> Result<()> {
Expand Down Expand Up @@ -79,17 +79,16 @@
) -> Result<Vec<String>> {
let manifest_store1 = match m1.as_ref().extension() {
Some(ext) if ext == "json" => {
ManifestStore::from_json(&fs::read_to_string(m1)?)
Reader::from_json(&fs::read_to_string(m1)?)

Check warning on line 82 in make_test_images/src/compare_manifests.rs

View check run for this annotation

Codecov / codecov/patch

make_test_images/src/compare_manifests.rs#L82

Added line #L82 was not covered by tests
//serde_json::from_str(&fs::read_to_string(m1)?).map_err(Error::JsonError)
}
_ => ManifestStore::from_file(m1.as_ref()),
_ => Reader::from_file(m1.as_ref()),

Check warning on line 85 in make_test_images/src/compare_manifests.rs

View check run for this annotation

Codecov / codecov/patch

make_test_images/src/compare_manifests.rs#L85

Added line #L85 was not covered by tests
};
let manifest_store2 = match m2.as_ref().extension() {
Some(ext) if ext == "json" => ManifestStore::from_json(&fs::read_to_string(m2)?),
_ => ManifestStore::from_file(m2.as_ref()),
Some(ext) if ext == "json" => Reader::from_json(&fs::read_to_string(m2)?),
_ => Reader::from_file(m2.as_ref()),

Check warning on line 89 in make_test_images/src/compare_manifests.rs

View check run for this annotation

Codecov / codecov/patch

make_test_images/src/compare_manifests.rs#L88-L89

Added lines #L88 - L89 were not covered by tests
};
// let manifest_store1 = ManifestStore::from_file(m1);
// let manifest_store2 = ManifestStore::from_file(m2);

match (manifest_store1, manifest_store2) {
(Ok(manifest_store1), Ok(manifest_store2)) => {
compare_manifests(&manifest_store1, &manifest_store2)
Expand All @@ -102,8 +101,8 @@

/// Compares two manifest stores and returns a list of issues.
pub fn compare_manifests(
manifest_store1: &ManifestStore,
manifest_store2: &ManifestStore,
manifest_store1: &Reader,
manifest_store2: &Reader,

Check warning on line 105 in make_test_images/src/compare_manifests.rs

View check run for this annotation

Codecov / codecov/patch

make_test_images/src/compare_manifests.rs#L104-L105

Added lines #L104 - L105 were not covered by tests
) -> Result<Vec<String>> {
// first we need to gather all the manifests in the order they are first seen recursively
let mut labels1 = Vec::new();
Expand Down Expand Up @@ -136,11 +135,7 @@
}

// creates list of manifests in the order they are first seen from the active manifest
fn gather_manifests(
manifest_store: &ManifestStore,
manifest_label: &str,
labels: &mut Vec<String>,
) {
fn gather_manifests(manifest_store: &Reader, manifest_label: &str, labels: &mut Vec<String>) {

Check warning on line 138 in make_test_images/src/compare_manifests.rs

View check run for this annotation

Codecov / codecov/patch

make_test_images/src/compare_manifests.rs#L138

Added line #L138 was not covered by tests
if !labels.contains(&manifest_label.to_string()) {
labels.push(manifest_label.to_string());
}
Expand Down
7 changes: 4 additions & 3 deletions sdk/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1469,7 +1469,7 @@ mod tests {
.add("thumbnail.jpg", TEST_THUMBNAIL.to_vec())
.unwrap();

// sign the ManifestStoreBuilder and write it to the output stream
// sign the Builder and write it to the output stream
let signer = crate::utils::test::temp_async_remote_signer();
builder
.sign_async(signer.as_ref(), format, &mut source, &mut dest)
Expand Down Expand Up @@ -1504,7 +1504,7 @@ mod tests {
.add_resource("thumbnail.jpg", Cursor::new(TEST_THUMBNAIL))
.unwrap();

// sign the ManifestStoreBuilder and write it to the output stream
// sign the Builder and write it to the output stream
let signer = test_signer(SigningAlg::Ps256);
let manifest_data = builder
.sign(signer.as_ref(), "image/jpeg", &mut source, &mut dest)
Expand Down Expand Up @@ -1669,7 +1669,7 @@ mod tests {
zipped.rewind().unwrap();
let mut builder = Builder::from_archive(&mut zipped).unwrap();

// sign the ManifestStoreBuilder and write it to the output stream
// sign the Builder and write it to the output stream
let signer = test_signer(SigningAlg::Ps256);
let _manifest_data = builder
.sign(signer.as_ref(), "image/jpeg", &mut source, &mut dest)
Expand Down Expand Up @@ -1855,6 +1855,7 @@ mod tests {
.expect("builder sign");

output.set_position(0);
println!("output len: {}", output.get_ref().len());
let reader = Reader::from_stream("jpeg", &mut output).expect("from_bytes");
println!("reader = {reader}");
let m = reader.active_manifest().unwrap();
Expand Down
1 change: 1 addition & 0 deletions sdk/src/claim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ static _V2_SPEC_DEPRECATED_ASSERTIONS: [&str; 4] = [
// Enum to encapsulate the data type of the source asset. This simplifies
// having different implementations for functions as a single entry point can be
// used to handle different data types.
#[allow(dead_code)] // Bytes and third param in StreamFragment not used without v1_api feature
pub enum ClaimAssetData<'a> {
#[cfg(feature = "file_io")]
Path(&'a Path),
Expand Down
4 changes: 2 additions & 2 deletions sdk/src/ingredient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::{
store::Store,
utils::xmp_inmemory_utils::XmpInfo,
validation_results::ValidationResults,
validation_status::{self, validation_results_for_store, ValidationStatus},
validation_status::{self, ValidationStatus},
};

#[derive(Debug, Default, Deserialize, Serialize)]
Expand Down Expand Up @@ -589,7 +589,7 @@ impl Ingredient {
match result {
Ok(store) => {
// generate validation results from the store
let validation_results = validation_results_for_store(&store, validation_log);
let validation_results = ValidationResults::from_store(&store, validation_log);

if let Some(claim) = store.provenance_claim() {
// if the parent claim is valid and has a thumbnail, use it
Expand Down
5 changes: 2 additions & 3 deletions sdk/src/jumbf_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ pub fn load_jumbf_from_file<P: AsRef<Path>>(in_path: P) -> Result<Vec<u8>> {
}
}

#[cfg(feature = "file_io")]
#[cfg(all(feature = "v1_api", feature = "file_io"))]
pub(crate) fn object_locations(in_path: &Path) -> Result<Vec<HashObjectPositions>> {
let ext = get_file_extension(in_path).ok_or(Error::UnsupportedType)?;

Expand Down Expand Up @@ -329,14 +329,13 @@ where
}
}

#[cfg(feature = "file_io")]
/// removes the C2PA JUMBF from an asset
/// Note: Use with caution since this deletes C2PA data
/// It is useful when creating remote manifests from embedded manifests
///
/// path - path to file to be updated
/// returns Unsupported type or errors from remove_cai_store
#[allow(dead_code)]
#[cfg(feature = "file_io")]
pub fn remove_jumbf_from_file<P: AsRef<Path>>(path: P) -> Result<()> {
let ext = get_file_extension(path.as_ref()).ok_or(Error::UnsupportedType)?;
match get_assetio_handler(&ext) {
Expand Down
2 changes: 2 additions & 0 deletions sdk/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ pub use manifest::{Manifest, SignatureInfo};
pub use manifest_assertion::{ManifestAssertion, ManifestAssertionKind};
#[cfg(feature = "v1_api")]
pub use manifest_store::ManifestStore;
#[cfg(feature = "v1_api")]
pub use manifest_store_report::ManifestStoreReport;
pub use reader::Reader;
pub use resource_store::{ResourceRef, ResourceStore};
Expand Down Expand Up @@ -148,6 +149,7 @@ pub(crate) mod jumbf;

pub(crate) mod manifest;
pub(crate) mod manifest_assertion;
#[cfg(feature = "v1_api")]
pub(crate) mod manifest_store;
pub(crate) mod manifest_store_report;
#[allow(dead_code)]
Expand Down
11 changes: 3 additions & 8 deletions sdk/src/manifest_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::{
jumbf::labels::{manifest_label_from_uri, to_absolute_uri, to_relative_uri},
store::Store,
validation_results::ValidationResults,
validation_status::{validation_results_for_store, ValidationStatus},
validation_status::ValidationStatus,
Error, Manifest, Result,
};

Expand Down Expand Up @@ -183,7 +183,7 @@ impl ManifestStore {
validation_log: &impl StatusTracker,
#[cfg(feature = "file_io")] resource_path: Option<&Path>,
) -> ManifestStore {
let mut validation_results = validation_results_for_store(&store, validation_log);
let mut validation_results = ValidationResults::from_store(&store, validation_log);

let mut manifest_store = ManifestStore::new();
manifest_store.active_manifest = store.provenance_label();
Expand Down Expand Up @@ -220,7 +220,7 @@ impl ManifestStore {
validation_log: &impl StatusTracker,
#[cfg(feature = "file_io")] resource_path: Option<&Path>,
) -> ManifestStore {
let mut validation_results = validation_results_for_store(&store, validation_log);
let mut validation_results = ValidationResults::from_store(&store, validation_log);

let mut manifest_store = ManifestStore::new();
manifest_store.active_manifest = store.provenance_label();
Expand Down Expand Up @@ -252,10 +252,6 @@ impl ManifestStore {
manifest_store
}

pub(crate) fn store(&self) -> &Store {
&self.store
}

/// Creates a new Manifest Store from a Manifest
#[allow(dead_code)]
#[deprecated(since = "0.38.0", note = "Please use Reader::from_json() instead")]
Expand All @@ -272,7 +268,6 @@ impl ManifestStore {
}

/// Generate a Store from a format string and bytes.
#[cfg(feature = "v1_api")]
#[deprecated(since = "0.38.0", note = "Please use Reader::from_stream() instead")]
#[cfg(feature = "v1_api")]
#[async_generic]
Expand Down
14 changes: 13 additions & 1 deletion sdk/src/manifest_store_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

use crate::{
assertion::AssertionData, claim::Claim, store::Store, validation_results::ValidationResults,
validation_status::ValidationStatus, Result,
validation_status::ValidationStatus, Result, ValidationState,
};

/// Low level JSON based representation of Manifest Store - used for debugging
Expand All @@ -39,6 +39,7 @@
#[serde(skip_serializing_if = "Option::is_none")]
validation_status: Option<Vec<ValidationStatus>>,
pub(crate) validation_results: Option<ValidationResults>,
pub(crate) validation_state: Option<ValidationState>,
}

impl ManifestStoreReport {
Expand All @@ -54,9 +55,20 @@
manifests,
validation_status: None,
validation_results: None,
validation_state: None,
})
}

pub(crate) fn from_store_with_results(
store: &Store,
validation_results: &ValidationResults,
) -> Result<Self> {
let mut report = Self::from_store(store)?;
report.validation_results = Some(validation_results.clone());
report.validation_state = Some(validation_results.validation_state());
Ok(report)
}

Check warning on line 70 in sdk/src/manifest_store_report.rs

View check run for this annotation

Codecov / codecov/patch

sdk/src/manifest_store_report.rs#L62-L70

Added lines #L62 - L70 were not covered by tests

/// Prints tree view of manifest store
#[cfg(feature = "file_io")]
#[cfg(feature = "v1_api")]
Expand Down
Loading