-
Notifications
You must be signed in to change notification settings - Fork 23
refactor: Initial Settings, retry, and fetch refactor #168
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
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
d5687da
Initial Settings, retry, and fetch refactor
ale-adobe a5e9713
Add changeset
ale-adobe f7ef329
Remove unused function; add more docs
ale-adobe f7c0cfd
Update tsconfig; reorganize code
ale-adobe 778b3cb
Remove unused package
ale-adobe 2ed8974
Add missing copyright header
ale-adobe a82b928
Update tsconfig.lib.json for c2pa-utilities
ale-adobe b42ef59
Update lockfile
ale-adobe d916826
Initial fetch with retry configurations
ale-adobe 2b320a3
Merge branch 'main' of github.com:contentauth/c2pa-js into feat/unifi…
ale-adobe 521a954
Update deprecation notice
ale-adobe db27006
Remove redundant test case
ale-adobe 90be07a
Move snakeCaseify logic into new caseConversion module
ale-adobe e52c868
Update docs
ale-adobe 5fdf70d
Feedback: update changeset
ale-adobe 92f1b95
Merge CawgTrustSettings into TrustSettings
ale-adobe c3a170b
Add retry to loadSettingsFromUrl
ale-adobe b6ab593
Add URL validation before fetching
ale-adobe 6f4035d
Check Content-Length header when fetching
ale-adobe 82de3ba
Merge branch 'main' of github.com:contentauth/c2pa-js into feat/unifi…
ale-adobe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@contentauth/c2pa-utilities': minor | ||
| '@contentauth/c2pa-node': minor | ||
| '@contentauth/c2pa-web': minor | ||
| --- | ||
|
|
||
| Introduce new c2pa-utilities package, and update c2pa-web and c2pa-node to use it. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,230 +4,13 @@ | |
| // or the MIT license (http://opensource.org/licenses/MIT), | ||
| // at your option. | ||
|
|
||
| import { | ||
| createTrustSettings, | ||
| createCawgTrustSettings, | ||
| createVerifySettings, | ||
| mergeSettings, | ||
| settingsToJson, | ||
| loadSettingsFromFile, | ||
| loadSettingsFromUrl, | ||
| } from "./Settings.js"; | ||
| import type { TrustConfig, VerifyConfig, SettingsContext } from "./types.d.ts"; | ||
| import * as fs from "fs-extra"; | ||
| import * as path from "path"; | ||
| import * as os from "os"; | ||
| import { vi } from "vitest"; | ||
|
|
||
| // Mock node-fetch | ||
| vi.mock("node-fetch", () => ({ | ||
| default: vi.fn(), | ||
| })); | ||
| import { loadSettingsFromFile } from "./Settings.js"; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests are consolidated and moved into the |
||
|
|
||
| describe("Settings", () => { | ||
| it("creates trust settings", () => { | ||
| const trustConfig: TrustConfig = { | ||
| verifyTrustList: true, | ||
| userAnchors: "test", | ||
| allowedList: "allowed", | ||
| }; | ||
|
|
||
| const settings = createTrustSettings(trustConfig); | ||
| expect(settings.trust).toBeDefined(); | ||
| expect(settings.trust?.verifyTrustList).toBe(true); | ||
| expect(settings.trust?.userAnchors).toBe("test"); | ||
| expect(settings.trust?.allowedList).toBe("allowed"); | ||
| }); | ||
|
|
||
| it("creates CAWG trust settings", () => { | ||
| const trustConfig: TrustConfig = { | ||
| verifyTrustList: false, | ||
| trustAnchors: "anchors", | ||
| }; | ||
|
|
||
| const settings = createCawgTrustSettings(trustConfig); | ||
| expect(settings.cawgTrust).toBeDefined(); | ||
| expect(settings.cawgTrust?.verifyTrustList).toBe(false); | ||
| expect(settings.cawgTrust?.trustAnchors).toBe("anchors"); | ||
| }); | ||
|
|
||
| it("creates verify settings", () => { | ||
| const verifyConfig: VerifyConfig = { | ||
| verifyAfterReading: true, | ||
| verifyAfterSign: false, | ||
| verifyTrust: true, | ||
| verifyTimestampTrust: false, | ||
| ocspFetch: true, | ||
| remoteManifestFetch: false, | ||
| skipIngredientConflictResolution: true, | ||
| strictV1Validation: false, | ||
| }; | ||
|
|
||
| const settings = createVerifySettings(verifyConfig); | ||
| expect(settings.verify).toBeDefined(); | ||
| expect(settings.verify?.verifyAfterReading).toBe(true); | ||
| expect(settings.verify?.verifyAfterSign).toBe(false); | ||
| expect(settings.verify?.verifyTrust).toBe(true); | ||
| expect(settings.verify?.ocspFetch).toBe(true); | ||
| }); | ||
|
|
||
| it("creates verify settings with partial config", () => { | ||
| const settings = createVerifySettings({ | ||
| verifyAfterReading: false, | ||
| }); | ||
|
|
||
| expect(settings.verify).toBeDefined(); | ||
| expect(settings.verify?.verifyAfterReading).toBe(false); | ||
| expect(settings.verify?.verifyAfterSign).toBeUndefined(); | ||
| expect(settings.verify?.verifyTrust).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("merges multiple settings", () => { | ||
| const trustSettings = createTrustSettings({ | ||
| verifyTrustList: true, | ||
| userAnchors: "test", | ||
| }); | ||
|
|
||
| const verifySettings = createVerifySettings({ | ||
| verifyAfterReading: false, | ||
| verifyAfterSign: true, | ||
| verifyTrust: true, | ||
| verifyTimestampTrust: true, | ||
| ocspFetch: false, | ||
| remoteManifestFetch: true, | ||
| skipIngredientConflictResolution: false, | ||
| strictV1Validation: false, | ||
| }); | ||
|
|
||
| const merged = mergeSettings(trustSettings, verifySettings); | ||
| expect(merged.trust).toBeDefined(); | ||
| expect(merged.verify).toBeDefined(); | ||
| expect(merged.trust?.verifyTrustList).toBe(true); | ||
| expect(merged.verify?.verifyAfterReading).toBe(false); | ||
| }); | ||
|
|
||
| it("converts settings to JSON with snake_case keys", () => { | ||
| const settings = createVerifySettings({ | ||
| verifyAfterReading: true, | ||
| verifyAfterSign: true, | ||
| verifyTrust: false, | ||
| verifyTimestampTrust: true, | ||
| ocspFetch: false, | ||
| remoteManifestFetch: true, | ||
| skipIngredientConflictResolution: false, | ||
| strictV1Validation: false, | ||
| }); | ||
|
|
||
| const json = settingsToJson(settings); | ||
| expect(json).toContain("verify"); | ||
| expect(json).toContain("verify_after_reading"); | ||
|
|
||
| // Should be parseable with snake_case keys | ||
| const parsed = JSON.parse(json); | ||
| expect(parsed.verify.verify_after_reading).toBe(true); | ||
| }); | ||
|
|
||
| it("does not include undefined values in trust settings JSON", () => { | ||
| const trustConfig: TrustConfig = { | ||
| verifyTrustList: true, | ||
| }; | ||
|
|
||
| const settings = createTrustSettings(trustConfig); | ||
| const json = settingsToJson(settings); | ||
| const parsed = JSON.parse(json); | ||
|
|
||
| expect(parsed.trust.verify_trust_list).toBe(true); | ||
| expect("user_anchors" in parsed.trust).toBe(false); | ||
| expect("trust_anchors" in parsed.trust).toBe(false); | ||
| expect("trust_config" in parsed.trust).toBe(false); | ||
| expect("allowed_list" in parsed.trust).toBe(false); | ||
| }); | ||
|
|
||
| it("does not include undefined values in CAWG trust settings JSON", () => { | ||
| const trustConfig: TrustConfig = { | ||
| verifyTrustList: false, | ||
| }; | ||
|
|
||
| const settings = createCawgTrustSettings(trustConfig); | ||
| const json = settingsToJson(settings); | ||
| const parsed = JSON.parse(json); | ||
|
|
||
| expect(parsed.cawg_trust.verify_trust_list).toBe(false); | ||
| expect("user_anchors" in parsed.cawg_trust).toBe(false); | ||
| expect("trust_anchors" in parsed.cawg_trust).toBe(false); | ||
| }); | ||
|
|
||
| it("does not include undefined values in verify settings JSON", () => { | ||
| const verifyConfig: VerifyConfig = { | ||
| verifyAfterReading: true, | ||
| verifyAfterSign: false, | ||
| }; | ||
|
|
||
| const settings = createVerifySettings(verifyConfig); | ||
| const json = settingsToJson(settings); | ||
| const parsed = JSON.parse(json); | ||
|
|
||
| expect(parsed.verify.verify_after_reading).toBe(true); | ||
| expect(parsed.verify.verify_after_sign).toBe(false); | ||
| expect("verify_trust" in parsed.verify).toBe(false); | ||
| expect("verify_timestamp_trust" in parsed.verify).toBe(false); | ||
| expect("ocsp_fetch" in parsed.verify).toBe(false); | ||
| expect("remote_manifest_fetch" in parsed.verify).toBe(false); | ||
| }); | ||
|
|
||
| it("does not include undefined values when merging settings", () => { | ||
| const settings1: SettingsContext = { | ||
| trust: { | ||
| verifyTrustList: true, | ||
| userAnchors: "test", | ||
| }, | ||
| }; | ||
|
|
||
| const settings2: SettingsContext = { | ||
| trust: { | ||
| verifyTrustList: true, | ||
| allowedList: undefined, | ||
| }, | ||
| verify: { | ||
| verifyAfterReading: false, | ||
| }, | ||
| }; | ||
|
|
||
| const merged = mergeSettings(settings1, settings2); | ||
| const json = settingsToJson(merged); | ||
| const parsed = JSON.parse(json); | ||
|
|
||
| expect(parsed.trust.verify_trust_list).toBe(true); | ||
| expect(parsed.trust.user_anchors).toBe("test"); | ||
| expect("allowed_list" in parsed.trust).toBe(false); | ||
| expect(parsed.verify.verify_after_reading).toBe(false); | ||
| }); | ||
|
|
||
| it("merges settings with later values overriding earlier ones", () => { | ||
| const settings1 = createVerifySettings({ | ||
| verifyAfterReading: true, | ||
| verifyAfterSign: true, | ||
| verifyTrust: false, | ||
| verifyTimestampTrust: true, | ||
| ocspFetch: false, | ||
| remoteManifestFetch: true, | ||
| skipIngredientConflictResolution: false, | ||
| strictV1Validation: false, | ||
| }); | ||
|
|
||
| const settings2: SettingsContext = { | ||
| verify: { | ||
| verifyTrust: true, | ||
| ocspFetch: true, | ||
| }, | ||
| }; | ||
|
|
||
| const merged = mergeSettings(settings1, settings2); | ||
| expect(merged.verify?.verifyAfterReading).toBe(true); // from settings1 | ||
| expect(merged.verify?.verifyTrust).toBe(true); // overridden by settings2 | ||
| expect(merged.verify?.ocspFetch).toBe(true); // overridden by settings2 | ||
| }); | ||
|
|
||
| describe("loadSettingsFromFile", () => { | ||
| let tempDir: string; | ||
|
|
||
|
|
@@ -274,52 +57,4 @@ verify_after_sign = false`; | |
| await expect(loadSettingsFromFile(filePath)).rejects.toThrow(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("loadSettingsFromUrl", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("loads settings from a URL", async () => { | ||
| const mockSettings = JSON.stringify({ | ||
| verify: { | ||
| verify_after_reading: true, | ||
| }, | ||
| }); | ||
|
|
||
| const fetch = (await import("node-fetch")).default; | ||
| vi.mocked(fetch).mockResolvedValue({ | ||
| ok: true, | ||
| text: async () => mockSettings, | ||
| } as any); | ||
|
|
||
| const loaded = await loadSettingsFromUrl( | ||
| "https://example.com/settings.json", | ||
| ); | ||
| expect(loaded).toBe(mockSettings); | ||
| expect(fetch).toHaveBeenCalledWith("https://example.com/settings.json"); | ||
| }); | ||
|
|
||
| it("throws error for failed fetch", async () => { | ||
| const fetch = (await import("node-fetch")).default; | ||
| vi.mocked(fetch).mockResolvedValue({ | ||
| ok: false, | ||
| status: 404, | ||
| statusText: "Not Found", | ||
| } as any); | ||
|
|
||
| await expect( | ||
| loadSettingsFromUrl("https://example.com/missing.json"), | ||
| ).rejects.toThrow("Failed to fetch settings from URL: 404 Not Found"); | ||
| }); | ||
|
|
||
| it("throws error for network failure", async () => { | ||
| const fetch = (await import("node-fetch")).default; | ||
| vi.mocked(fetch).mockRejectedValue(new Error("Network error")); | ||
|
|
||
| await expect( | ||
| loadSettingsFromUrl("https://example.com/settings.json"), | ||
| ).rejects.toThrow("Network error"); | ||
| }); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.