feat: Use async functions from c2pa-rs #26
Conversation
Settings need to be applied at the beginning of the tokio::spawn, since a thread may be reused and not have settings applied.
js-src/Reader.spec.ts
Outdated
| }); | ||
|
|
||
| // Call postValidateCawg to decode CAWG assertions | ||
| // this is now a NO-OP |
There was a problem hiding this comment.
Why leave it in if it's a no-op? Backwards compatibility?
There was a problem hiding this comment.
Yes. I just told people they had to use that method.
js-src/Reader.ts
Outdated
|
|
||
| async postValidateCawg(): Promise<void> { | ||
| return getNeonBinary().readerPostValidateCawg.call(this.reader); | ||
| // No-op: CAWG validation is now handled automatically by async reader constructors |
| } from "./Settings.js"; | ||
|
|
||
| describe("Settings", () => { | ||
| afterAll(() => { |
There was a problem hiding this comment.
Suggestion: Do it in beforeAll too? So if something else would run on the same thread before, only the settings from the tests would be set, no doubts?
There was a problem hiding this comment.
I could do it beforeAll in all of the test suites or none of them. It's been a real headache in c2pa-rs finding all of the tests that modify and don't reset the settings. I like the idea of detecting when tests don't put things back as they should.
| pub fn reset_settings(mut cx: FunctionContext) -> JsResult<JsUndefined> { | ||
| // Create default settings and get their TOML representation | ||
| let default_settings = Settings::default(); | ||
| let default_toml = toml::to_string(&default_settings) |
There was a problem hiding this comment.
contentauth/c2pa-rs#1533 may be of interest once it lands
emensch
left a comment
emensch
left a comment
There was a problem hiding this comment.
LGTM but I think we might be better off removing postValidateCawg, lest people get confused down the line just looking at the public API surface
Settings need to be applied at the beginning of the tokio::spawn, since a thread may be reused and not have settings applied.