Page MenuHomePhabricator

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

Authored by kamil on Jan 22 2024, 2:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 27, 11:57 PM
Unknown Object (File)
Wed, Nov 27, 10:34 AM
Unknown Object (File)
Wed, Nov 27, 8:55 AM
Unknown Object (File)
Wed, Nov 27, 6:35 AM
Unknown Object (File)
Wed, Nov 6, 8:05 PM
Unknown Object (File)
Oct 15 2024, 10:47 AM
Unknown Object (File)
Oct 15 2024, 10:47 AM
Unknown Object (File)
Oct 15 2024, 10:47 AM
Subscribers

Details

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
Branch
publish-ks-sql-2
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Jan 22 2024, 6:32 AM
lib/shared/transforms/keyserver-store-transform.js
37 ↗(On Diff #35909)

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 ↗(On Diff #35909)

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 ↗(On Diff #35909)

We can just remove this line

42–70 ↗(On Diff #35909)

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

lib/shared/transforms/keyserver-store-transform.js
36

It looks like this code is just getting moved... but I think the spread can be skipped for defaultConnectionInfo

This revision is now accepted and ready to land.Jan 29 2024, 1:39 AM