[native][web][lib] unify keyserver transform
ClosedPublic
Actions

Authored by kamil on Jan 22 2024, 2:38 AM.

Details

Reviewers

inka

Commits

rCOMMdac395421aed: [native][web][lib] unify keyserver transform

Summary

The only difference was that sessionID does not exist on native, so excluding this undefined field or setting it as null is not an issue.
We need to store this in the same version in SQLite, and for backup and schema consistency unify the transform, which later in the stack is used for converting data to SQLite format.

It also reduces code duplication and makes things more consistent.

Note: after migrating everything to SQLite those transforms are not needed anymore, and then those will be needed only for transforming data to SQLite.

Test Plan

Test if persistence works the same (mostly on native, for web it's just moving code to /lib)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Jan 22 2024, 2:38 AM

kamil held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJan 22 2024, 2:38 AM

kamil added a child revision: D10755: [lib] implement `KeyserverStore` spec.Jan 22 2024, 2:41 AM

kamil edited the summary of this revision. (Show Details)Jan 22 2024, 2:43 AM

Harbormaster failed remote builds in B25977: Diff 35909!Jan 22 2024, 3:06 AM

kamil published this revision for review.Jan 22 2024, 6:32 AM

inka added inline comments.Jan 22 2024, 8:07 AM

lib/shared/transforms/keyserver-store-transform.js
37	This makes `sessionID` field be present on native. It didn't use to ever be present on native. Actually the types used to prohibit this. I'm not sure this is a good idea, to have a field that is always `null`.
42–70	We would reduce this using generics and passing `transformPersistedKeyserverInfoToKeyserverInfo`/`transformKeyserverInfoToPersistedKeyserverInfo` as an argument, but if that's too much work/you feel like it would be less readable I won't insist

remove sessionID: null

lib/shared/transforms/keyserver-store-transform.js
37	We can just remove this line
42–70	as soon as we fully switch the keyserver store to SQLite those won't be needed so we can leave it as it is for now