Page MenuHomePhabricator

[lib] Replace `ConnectionInfo.showDisconnectedBar` with `ConnectionInfo.unreachable`
ClosedPublic

Authored by atul on Jan 30 2024, 10:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 29, 1:44 PM
Unknown Object (File)
Thu, Jun 27, 8:21 AM
Unknown Object (File)
Wed, Jun 26, 4:36 AM
Unknown Object (File)
Wed, Jun 26, 4:35 AM
Unknown Object (File)
Mon, Jun 24, 11:32 PM
Unknown Object (File)
Fri, Jun 21, 3:02 AM
Unknown Object (File)
Thu, Jun 20, 7:01 PM
Unknown Object (File)
Jun 2 2024, 10:49 PM
Subscribers
None

Details

Summary

We want to rename showDisconnectedBar to be more general so it can be used for eg the keyserver status page.

We actually shouldn't need any migrations here since KeyserverInfo.connection is stripped out from KeyserverInfo before being persisted, and KeyserverInfo.connection is set to defaultConnectionInfo when being rehydrated. This is handled in keyserverStoreTransform on both web and native.

Do we need to send "legacy" ConnectionInfos to old clients?

My understanding is that ConnectionInfo is a client-only type and no inital value is sent from the keyserver. I was a bit confused because of connectionInfoValidator, but it turns out connectionInfoValidator is only consumed by keyserverInfoValidator which is only consumed by keyserverStoreValidator... which doesn't seem to be used anywhere? It looks like the only two KeyserverInfo fields we send as part of initialReduxState are sessionID and updatesCurrentAsOf.


Depends on D10879

Test Plan
  1. Open app
  2. Ensure that KeyserverStore is as expected in Redux DevTools with showDisconnectedBar replaced by unreachable.
  3. Flip on and off keyserver (killing and restarting) and ensure state updates as expected

2c6453.png (732×756 px, 82 KB)

671920.png (740×772 px, 92 KB)


Make sure old native clients continue to work as expected without any validation issues (not sure what specific flow to test since connectionInfoValidator appears unused?):

126d89.png (2×3 px, 1 MB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.

stage all

atul requested review of this revision.Jan 30 2024, 10:44 PM

My understanding is that ConnectionInfo is a client-only type and no inital value is sent from the keyserver. I was a bit confused because of connectionInfoValidator, but it turns out connectionInfoValidator is only consumed by keyserverInfoValidator which is only consumed by keyserverStoreValidator... which doesn't seem to be used anywhere? It looks like the only two KeyserverInfo fields we send as part of initialReduxState are sessionID and updatesCurrentAsOf.

This seems to be correct, but we should extend the test plan by checking if old clients can work correctly after this change.

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

we should extend the test plan by checking if old clients can work correctly after this change.

Specifically wrt possible validation issues?

Not sure how I'd test web client and keyserver that are out of sync (electron app?). But will do this for native before landing.

atul edited the test plan for this revision. (Show Details)

rebase and land after additional testing

This revision was landed with ongoing or failed builds.Jan 31 2024, 12:07 PM
This revision was automatically updated to reflect the committed changes.