[1] Feat: Add context, settings, and JSON APIs#47
[1] Feat: Add context, settings, and JSON APIs#47redaranj wants to merge 1 commit intocontentauth:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #47 +/- ##
============================================
+ Coverage 59.82% 68.33% +8.51%
Complexity 9 9
============================================
Files 52 57 +5
Lines 1558 1936 +378
Branches 162 267 +105
============================================
+ Hits 932 1323 +391
+ Misses 526 496 -30
- Partials 100 117 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
redaranj
force-pushed
the
feature/context-settings-api
branch
from
f43629c to
3e2acab
Compare
Introduce C2PAContext and C2PASettings for shared configuration across readers and builders. Add C2PAJson for centralized JSON serialization. Add SettingsValidator for validating C2PA settings. Rewrite Builder to use context-based creation flow. Add Reader context support with withStream and withFragment. Update native library to v0.75.19.
redaranj
force-pushed
the
feature/context-settings-api
branch
from
3e2acab to
22a7f35
Compare
| import org.contentauth.c2pa.SigningAlgorithm | ||
|
|
||
| /** | ||
| * Validates C2PA settings JSON/TOML for schema compliance and provides warnings for common issues. |
There was a problem hiding this comment.
This is very cool, but … it's at risk of getting out of date as we evolve the set of settings. Is there a straightforward way to track such changes?
There was a problem hiding this comment.
@scouten-adobe Perhaps I could extend the current CI c2pa-rs version check (or maybe better, add another scheduled job), that checks for changes in specific files (FFI, settings, etc) and alerts or opens a PR when something new is detected.
Overall, this brings up an issue that I'm working through: where is the line where I defer to c2pa-rs? My general goal has been that devs can look just at the mobile library (not c2pa-rs or the spec) and understand what's possible. The options I considered here are:
- accept any settings string and pass it straight through, refer users to c2pa-rs docs for details (no maintenance, but not great developer experience)
- same thing, but include docs on the currently available settings (light maintenance)
- create a struct that mirrors the shape of the settings and require that as an argument everywhere (very nice, informative developer experience, but creates a hard requirement to update the struct when the allowable settings change)
- middle path 1: create the struct, but accept string settings everywhere, users can either pass a raw string or a serialized struct (requires maintenance, but doesn't block people from adopting new settings if the struct hasn't been updated)
- middle path 2: accept raw settings string, but give users a way to validate their structure against the current schema (similar tradeoffs as previous one)
Let me know if you have a preference here. This issue will come up again in a subsequent PR where we have created a similar manifest validator.
There was a problem hiding this comment.
I think my preference is for your "middle path 1" but I'll ask a couple other people on the team for their opinions as well.
There was a problem hiding this comment.
For our JS APIs we added a tool that generates JSON schema and we generated the client side apis using htat. You could leverage the schema to validate and update it whenever it changes.
| * @see ManifestValidator | ||
| * @see SettingsValidator | ||
| */ | ||
| data class ValidationResult( |
There was a problem hiding this comment.
I think this class should have a different name since "validation result" has a specific meaning in C2PA: https://spec.c2pa.org/specifications/specifications/2.3/specs/C2PA_Specification.html#_returning_validation_results.
There was a problem hiding this comment.
Makes sense, I will rename it.
library/src/main/kotlin/org/contentauth/c2pa/manifest/ValidationResult.kt
Show resolved
Hide resolved
|
@redaranj I finished reading the PR and have no further comments; ping me when you're ready for re-review. |
3 participants
Changes in this pull request
Types of changes
Checklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment