Page MenuHomePhabricator
Differential D10718

[lib] Remove branching from `threadInfoFromRawThreadInfo(...)`
ClosedPublic
Actions

Authored by atul on Jan 18 2024, 11:26 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 23, 7:14 AM
Unknown Object (File)
Wed, Apr 23, 7:14 AM
Unknown Object (File)
Wed, Apr 23, 7:14 AM
Unknown Object (File)
Wed, Apr 23, 7:14 AM
Unknown Object (File)
Wed, Apr 23, 7:14 AM
Unknown Object (File)
Wed, Apr 23, 7:14 AM
Unknown Object (File)
Tue, Apr 22, 6:37 AM
Unknown Object (File)
Sat, Apr 19, 6:10 PM
View All Files
Subscribers
None

Details

Reviewers
ashoat
ginsu
tomek
Commits
rCOMM0173927531a1: [lib] Remove branching from `threadInfoFromRawThreadInfo(...)`
Summary

Part of threadInfoFromRawThreadInfo(...).

Required changes to createPendingThread and sendPushNotifs.

Depends on D10715

Test Plan

flow, will make sure push notifs continue to work as expected.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Jan 18 2024, 11:26 AM
atul updated this revision to Diff 35839.Jan 18 2024, 11:34 AM

fix massive issue that flow didn't catch..

atul published this revision for review.Jan 18 2024, 11:40 AM
atul added inline comments.
keyserver/src/push/send.js
139–143 ↗(On Diff #35839)

By passing { minimallyEncodePermissions: true } to rawThreadInfoFromServerThreadInfo we're "guaranteed" to get minimally encoded RawThreadInfos back.

If we don't, that would indicate something has gone very wrong... so I think it's acceptable to use an invariant here.

We could explore parameterizing rawThreadInfoFromServerThreadInfo based on options.minimallyEncodePermissions, but I'm not sure how to do that.

CC @ashoat

lib/shared/thread-utils.js
366 ↗(On Diff #35839)

Wow... glad I caught that role wasn't minimallyEncoded in the first revision of this diff (I guess first diff of this revision). flow didn't report any issues and we would've had malformed RawThreadInfos.

Will take a sec, but going to write/generate some unit tests for this function.

Harbormaster completed remote builds in B25918: Diff 35838.Jan 18 2024, 11:45 AM
atul added a child revision: D10720: [lib] Remove `LegacyThreadInfo`-related type utilities.Jan 18 2024, 11:52 AM
Harbormaster completed remote builds in B25919: Diff 35839.Jan 18 2024, 11:57 AM
ashoat requested changes to this revision.Jan 21 2024, 8:43 PM

Passing back for those unit tests and a question inline

keyserver/src/push/send.js
139–143 ↗(On Diff #35839)

Is there a reason we have to use such a peculiar API that is difficult to type?

As an alternative, what if we just had two functions, one that returned a minimally-encoded type and one that didn't?

lib/shared/thread-utils.js
366 ↗(On Diff #35839)

Would be good to investigate how Flow missed this. Unit tests sound like a good idea too

This revision now requires changes to proceed.Jan 21 2024, 8:43 PM
atul updated this revision to Diff 36070.Jan 23 2024, 2:30 PM

rebase

atul planned changes to this revision.Jan 23 2024, 2:30 PM
Harbormaster completed remote builds in B26099: Diff 36070.Jan 23 2024, 2:49 PM
atul mentioned this in D10818: [lib] Add `threadInfoFromRawThreadInfo` test to `thread-utils.test`.Jan 25 2024, 11:18 AM
atul requested review of this revision.Jan 25 2024, 11:28 AM
atul added inline comments.
keyserver/src/push/send.js
139–143 ↗(On Diff #35839)

Is there a reason we have to use such a peculiar API that is difficult to type?

It's because rawThreadInfoFromServerThreadInfo accepts RawThreadInfoOptions:

6b5333.png (640×1 px, 142 KB)

which are passed in based on version checks at rawThreadInfoFromServerThreadInfo callsite:

d37ed4.png (1×1 px, 398 KB)

I think it makes sense to maintain the symmetry w/ the other options that are passed in via codeVersion. Would the alternative be adding a separate argument to the function to toggle between encodings?

As an alternative, what if we just had two functions, one that returned a minimally-encoded type and one that didn't?

Would we want to duplicate functionality between the two, or would we want one function to be a wrapper around the other that either encodes/decodes the thread infos?

FWIW will be updating rawThreadInfoFromServerThreadInfo as part of future "encoded permissions in memberships table" project, so it might make sense to sequence then?

lib/shared/thread-utils.js
366 ↗(On Diff #35839)

Added unit test in D10818. Will create a task to document the flow issue and jot down any notes from investigating.

atul attached a referenced file: F1088547: d37ed4.png. (Show Details)Jan 25 2024, 11:29 AM
atul attached a referenced file: F1088546: 6b5333.png. (Show Details)
ashoat accepted this revision.Jan 25 2024, 5:45 PM

Thanks for explaining

This revision is now accepted and ready to land.Jan 25 2024, 5:45 PM
atul added inline comments.Jan 26 2024, 1:02 PM
lib/shared/thread-utils.js
366 ↗(On Diff #35839)

Documentation of the flow issue: https://linear.app/comm/issue/ENG-6596/investigate-why-flow-didnt-detect-issue-with-creatependingthread

atul updated this revision to Diff 36199.Jan 26 2024, 2:32 PM

land

Harbormaster completed remote builds in B26210: Diff 36199.Jan 26 2024, 2:51 PM
Closed by commit rCOMM0173927531a1: [lib] Remove branching from `threadInfoFromRawThreadInfo(...)` (authored by atul). · Explain WhyJan 26 2024, 3:32 PM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM0173927531a1: [lib] Remove branching from `threadInfoFromRawThreadInfo(...)`.
atul mentioned this in rCOMM3147b6e753f0: [lib] Add `threadInfoFromRawThreadInfo` test to `thread-utils.test`.Jan 29 2024, 7:16 PM

Revision Contents
Changeset List

PathSize
keyserver/
src/
push/
6 lines
lib/
shared/
90 lines

Diff 35838

View Options

keyserver/src/push/send.js

// @flow // @flow
import invariant from 'invariant';
import { import {
setThreadUnreadStatusActionTypes, setThreadUnreadStatusActionTypes,
updateActivityActionTypes, updateActivityActionTypes,
} from '../actions/activity-actions.js'; } from '../actions/activity-actions.js';
import { setClientDBStoreActionType } from '../actions/client-db-store-actions.js'; import { setClientDBStoreActionType } from '../actions/client-db-store-actions.js';
import { saveMessagesActionType } from '../actions/message-actions.js'; import { saveMessagesActionType } from '../actions/message-actions.js';
import { siweAuthActionTypes } from '../actions/siwe-actions.js'; import { siweAuthActionTypes } from '../actions/siwe-actions.js';
import { import {
Show All 15 Linesimport {
updateSubscriptionActionTypes, updateSubscriptionActionTypes,
} from '../actions/user-actions.js'; } from '../actions/user-actions.js';
import { import {
type ThreadStoreOperation, type ThreadStoreOperation,
threadStoreOpsHandlers, threadStoreOpsHandlers,
} from '../ops/thread-store-ops.js'; } from '../ops/thread-store-ops.js';
import { stateSyncSpecs } from '../shared/state-sync/state-sync-specs.js'; import { stateSyncSpecs } from '../shared/state-sync/state-sync-specs.js';
import { updateSpecs } from '../shared/updates/update-specs.js'; import { updateSpecs } from '../shared/updates/update-specs.js';
import type { MinimallyEncodedRawThreadInfo } from '../types/minimally-encoded-thread-permissions-types.js';
import type { BaseAction } from '../types/redux-types.js'; import type { BaseAction } from '../types/redux-types.js';
import { type ClientThreadInconsistencyReportCreationRequest } from '../types/report-types.js'; import { type ClientThreadInconsistencyReportCreationRequest } from '../types/report-types.js';
import { import {
serverRequestTypes, serverRequestTypes,
processServerRequestsActionType, processServerRequestsActionType,
} from '../types/request-types.js'; } from '../types/request-types.js';
import { import {
fullStateSyncActionType, fullStateSyncActionType,
incrementalStateSyncActionType, incrementalStateSyncActionType,
} from '../types/socket-types.js'; } from '../types/socket-types.js';
import type { import type {
RawThreadInfo, MinimallyEncodedRawThreadInfos,
RawThreadInfos,
ThreadStore, ThreadStore,
} from '../types/thread-types.js'; } from '../types/thread-types.js';
import { import {
type ClientUpdateInfo, type ClientUpdateInfo,
processUpdatesActionType, processUpdatesActionType,
} from '../types/update-types.js'; } from '../types/update-types.js';
import { setNewSessionActionType } from '../utils/action-utils.js'; import { setNewSessionActionType } from '../utils/action-utils.js';
const { processStoreOperations: processThreadStoreOperations } = const { processStoreOperations: processThreadStoreOperations } =
threadStoreOpsHandlers; threadStoreOpsHandlers;
function generateOpsForThreadUpdates( function generateOpsForThreadUpdates(
threadInfos: RawThreadInfos, threadInfos: MinimallyEncodedRawThreadInfos,
payload: { payload: {
+updatesResult: { +newUpdates: $ReadOnlyArray<ClientUpdateInfo>, ... }, +updatesResult: { +newUpdates: $ReadOnlyArray<ClientUpdateInfo>, ... },
... ...
}, },
): $ReadOnlyArray<ThreadStoreOperation> { ): $ReadOnlyArray<ThreadStoreOperation> {
return payload.updatesResult.newUpdates return payload.updatesResult.newUpdates
.map(update => .map(update =>
updateSpecs[update.type].generateOpsForThreadUpdates?.( updateSpecs[update.type].generateOpsForThreadUpdates?.(
Show All 19 Lines if (
action.type === registerActionTypes.success || action.type === registerActionTypes.success ||
action.type === fullStateSyncActionType action.type === fullStateSyncActionType
) { ) {
const newThreadInfos = action.payload.threadInfos; const newThreadInfos = action.payload.threadInfos;
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'remove_all', type: 'remove_all',
}, },
...Object.keys(newThreadInfos).map((id: string) => ({ ...Object.keys(newThreadInfos).map((id: string) => {
invariant(
newThreadInfos[id].minimallyEncoded,
'newThreadInfos must be minimallyEncoded for current clients',
);
atulAuthorUnsubmitted
Done
Inline Actions

I thought putting an invariant here was a reasonable trade-off vs. refactoring the types in the fullStateSyncActionType payload (specifically StateSyncFullActionPayload which is derived from ClientFullStateSync which is derived from BaseFullStateSync) which are used across clients and keyserver.

Open to re-exploring if that would be preferred.

atul: I thought putting an `invariant` here was a reasonable trade-off vs. refactoring the types in…
return {
type: 'replace', type: 'replace',
payload: { id, threadInfo: newThreadInfos[id] }, payload: { id, threadInfo: newThreadInfos[id] },
})), };
}),
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
▲ Show 20 Lines • Show All 58 Lines • ▼ Show 20 Lines ) {
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
} else if (action.type === updateSubscriptionActionTypes.success) { } else if (action.type === updateSubscriptionActionTypes.success) {
const { threadID, subscription } = action.payload; const { threadID, subscription } = action.payload;
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
const newThreadInfo = {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
subscription,
},
};
const threadStoreOperations = [
{
type: 'replace',
payload: {
id: threadID,
threadInfo: newThreadInfo,
},
},
];
const updatedThreadStore = processThreadStoreOperations(
state,
threadStoreOperations,
);
return {
threadStore: updatedThreadStore,
newThreadInconsistencies: [],
threadStoreOperations,
};
} else {
const newThreadInfo = { const newThreadInfo = {
...threadInfo, ...threadInfo,
currentUser: { currentUser: {
...threadInfo.currentUser, ...threadInfo.currentUser,
subscription, subscription,
}, },
}; };
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'replace', type: 'replace',
payload: { payload: {
id: threadID, id: threadID,
threadInfo: newThreadInfo, threadInfo: newThreadInfo,
}, },
}, },
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
}
} else if (action.type === saveMessagesActionType) { } else if (action.type === saveMessagesActionType) {
const threadIDToMostRecentTime = new Map<string, number>(); const threadIDToMostRecentTime = new Map<string, number>();
for (const messageInfo of action.payload.rawMessageInfos) { for (const messageInfo of action.payload.rawMessageInfos) {
const current = threadIDToMostRecentTime.get(messageInfo.threadID); const current = threadIDToMostRecentTime.get(messageInfo.threadID);
if (!current || current < messageInfo.time) { if (!current || current < messageInfo.time) {
threadIDToMostRecentTime.set(messageInfo.threadID, messageInfo.time); threadIDToMostRecentTime.set(messageInfo.threadID, messageInfo.time);
} }
} }
const changedThreadInfos: { [string]: RawThreadInfo } = {}; const changedThreadInfos: { [string]: MinimallyEncodedRawThreadInfo } = {};
for (const [threadID, mostRecentTime] of threadIDToMostRecentTime) { for (const [threadID, mostRecentTime] of threadIDToMostRecentTime) {
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
if ( if (
!threadInfo || !threadInfo ||
threadInfo.currentUser.unread || threadInfo.currentUser.unread ||
action.payload.updatesCurrentAsOf > mostRecentTime action.payload.updatesCurrentAsOf > mostRecentTime
) { ) {
continue; continue;
} }
const changedThreadInfo = state.threadInfos[threadID]; const changedThreadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (changedThreadInfo.minimallyEncoded) {
changedThreadInfos[threadID] = {
...changedThreadInfo,
currentUser: {
...changedThreadInfo.currentUser,
unread: true,
},
};
} else {
changedThreadInfos[threadID] = { changedThreadInfos[threadID] = {
...changedThreadInfo, ...changedThreadInfo,
currentUser: { currentUser: {
...changedThreadInfo.currentUser, ...changedThreadInfo.currentUser,
unread: true, unread: true,
}, },
}; };
} }
}
if (Object.keys(changedThreadInfos).length !== 0) { if (Object.keys(changedThreadInfos).length !== 0) {
const threadStoreOperations = Object.keys(changedThreadInfos).map(id => ({ const threadStoreOperations = Object.keys(changedThreadInfos).map(id => ({
type: 'replace', type: 'replace',
payload: { payload: {
id, id,
threadInfo: changedThreadInfos[id], threadInfo: changedThreadInfos[id],
}, },
})); }));
Show All 25 Lines if (!rawThreadInfos && !deleteThreadIDs) {
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
const threadStoreOperations: ThreadStoreOperation[] = []; const threadStoreOperations: ThreadStoreOperation[] = [];
if (rawThreadInfos) { if (rawThreadInfos) {
for (const rawThreadInfo of rawThreadInfos) { for (const rawThreadInfo of rawThreadInfos) {
invariant(
rawThreadInfo.minimallyEncoded,
'rawThreadInfo must be minimallyEncoded on new clients',
);
atulAuthorUnsubmitted
Done
Inline Actions

Similarly thought this was a reasonable trade-off to avoid having to refactor types related to state check mechanism.

Open to re-exploring if that would be preferred.

atul: Similarly thought this was a reasonable trade-off to avoid having to refactor types related to…
threadStoreOperations.push({ threadStoreOperations.push({
type: 'replace', type: 'replace',
payload: { payload: {
id: rawThreadInfo.id, id: rawThreadInfo.id,
threadInfo: rawThreadInfo, threadInfo: rawThreadInfo,
}, },
}); });
} }
Show All 20 Lines const newThreadInconsistencies =
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies, newThreadInconsistencies,
threadStoreOperations, threadStoreOperations,
}; };
} else if (action.type === updateActivityActionTypes.success) { } else if (action.type === updateActivityActionTypes.success) {
const updatedThreadInfos: { [string]: RawThreadInfo } = {}; const updatedThreadInfos: { [string]: MinimallyEncodedRawThreadInfo } = {};
for (const setToUnread of action.payload.result.unfocusedToUnread) { for (const setToUnread of action.payload.result.unfocusedToUnread) {
const threadInfo = state.threadInfos[setToUnread]; const threadInfo = state.threadInfos[setToUnread];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
if (threadInfo && !threadInfo.currentUser.unread) {
updatedThreadInfos[setToUnread] = {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
unread: true,
},
};
}
} else {
if (threadInfo && !threadInfo.currentUser.unread) { if (threadInfo && !threadInfo.currentUser.unread) {
updatedThreadInfos[setToUnread] = { updatedThreadInfos[setToUnread] = {
...threadInfo, ...threadInfo,
currentUser: { currentUser: {
...threadInfo.currentUser, ...threadInfo.currentUser,
unread: true, unread: true,
}, },
}; };
} }
} }
}
if (Object.keys(updatedThreadInfos).length === 0) { if (Object.keys(updatedThreadInfos).length === 0) {
return { return {
threadStore: state, threadStore: state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
const threadStoreOperations = Object.keys(updatedThreadInfos).map(id => ({ const threadStoreOperations = Object.keys(updatedThreadInfos).map(id => ({
Show All 10 Lines ) {
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
} else if (action.type === setThreadUnreadStatusActionTypes.started) { } else if (action.type === setThreadUnreadStatusActionTypes.started) {
const { threadID, unread } = action.payload; const { threadID, unread } = action.payload;
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
const updatedThreadInfo = {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
unread,
},
};
const threadStoreOperations = [
{
type: 'replace',
payload: {
id: threadID,
threadInfo: updatedThreadInfo,
},
},
];
const updatedThreadStore = processThreadStoreOperations(
state,
threadStoreOperations,
);
return {
threadStore: updatedThreadStore,
newThreadInconsistencies: [],
threadStoreOperations,
};
} else {
const updatedThreadInfo = { const updatedThreadInfo = {
...threadInfo, ...threadInfo,
currentUser: { currentUser: {
...threadInfo.currentUser, ...threadInfo.currentUser,
unread, unread,
}, },
}; };
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'replace', type: 'replace',
payload: { payload: {
id: threadID, id: threadID,
threadInfo: updatedThreadInfo, threadInfo: updatedThreadInfo,
}, },
}, },
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
}
} else if (action.type === setThreadUnreadStatusActionTypes.success) { } else if (action.type === setThreadUnreadStatusActionTypes.success) {
const { threadID, resetToUnread } = action.payload; const { threadID, resetToUnread } = action.payload;
const currentUser = state.threadInfos[threadID].currentUser; const currentUser = state.threadInfos[threadID].currentUser;
if (!resetToUnread || currentUser.unread) { if (!resetToUnread || currentUser.unread) {
return { return {
threadStore: state, threadStore: state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
const updatedThread = { const updatedThread = {
...threadInfo, ...threadInfo,
currentUser: { ...threadInfo.currentUser, unread: true }, currentUser: { ...threadInfo.currentUser, unread: true },
}; };
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'replace', type: 'replace',
payload: { payload: {
id: threadID, id: threadID,
threadInfo: updatedThread, threadInfo: updatedThread,
}, },
}, },
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
} else {
const updatedThread = {
...threadInfo,
currentUser: { ...threadInfo.currentUser, unread: true },
};
const threadStoreOperations = [
{
type: 'replace',
payload: {
id: threadID,
threadInfo: updatedThread,
},
},
];
const updatedThreadStore = processThreadStoreOperations(
state,
threadStoreOperations,
);
return {
threadStore: updatedThreadStore,
newThreadInconsistencies: [],
threadStoreOperations,
};
}
} else if (action.type === setClientDBStoreActionType) { } else if (action.type === setClientDBStoreActionType) {
return { return {
threadStore: action.payload.threadStore ?? state, threadStore: action.payload.threadStore ?? state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
return { return {
threadStore: state, threadStore: state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
export { reduceThreadInfos }; export { reduceThreadInfos };
View Options

lib/shared/thread-utils.js

// @flow // @flow
import invariant from 'invariant';
import { import {
setThreadUnreadStatusActionTypes, setThreadUnreadStatusActionTypes,
updateActivityActionTypes, updateActivityActionTypes,
} from '../actions/activity-actions.js'; } from '../actions/activity-actions.js';
import { setClientDBStoreActionType } from '../actions/client-db-store-actions.js'; import { setClientDBStoreActionType } from '../actions/client-db-store-actions.js';
import { saveMessagesActionType } from '../actions/message-actions.js'; import { saveMessagesActionType } from '../actions/message-actions.js';
import { siweAuthActionTypes } from '../actions/siwe-actions.js'; import { siweAuthActionTypes } from '../actions/siwe-actions.js';
import { import {
Show All 15 Linesimport {
updateSubscriptionActionTypes, updateSubscriptionActionTypes,
} from '../actions/user-actions.js'; } from '../actions/user-actions.js';
import { import {
type ThreadStoreOperation, type ThreadStoreOperation,
threadStoreOpsHandlers, threadStoreOpsHandlers,
} from '../ops/thread-store-ops.js'; } from '../ops/thread-store-ops.js';
import { stateSyncSpecs } from '../shared/state-sync/state-sync-specs.js'; import { stateSyncSpecs } from '../shared/state-sync/state-sync-specs.js';
import { updateSpecs } from '../shared/updates/update-specs.js'; import { updateSpecs } from '../shared/updates/update-specs.js';
import type { MinimallyEncodedRawThreadInfo } from '../types/minimally-encoded-thread-permissions-types.js';
import type { BaseAction } from '../types/redux-types.js'; import type { BaseAction } from '../types/redux-types.js';
import { type ClientThreadInconsistencyReportCreationRequest } from '../types/report-types.js'; import { type ClientThreadInconsistencyReportCreationRequest } from '../types/report-types.js';
import { import {
serverRequestTypes, serverRequestTypes,
processServerRequestsActionType, processServerRequestsActionType,
} from '../types/request-types.js'; } from '../types/request-types.js';
import { import {
fullStateSyncActionType, fullStateSyncActionType,
incrementalStateSyncActionType, incrementalStateSyncActionType,
} from '../types/socket-types.js'; } from '../types/socket-types.js';
import type { import type {
RawThreadInfo, MinimallyEncodedRawThreadInfos,
RawThreadInfos,
ThreadStore, ThreadStore,
} from '../types/thread-types.js'; } from '../types/thread-types.js';
import { import {
type ClientUpdateInfo, type ClientUpdateInfo,
processUpdatesActionType, processUpdatesActionType,
} from '../types/update-types.js'; } from '../types/update-types.js';
import { setNewSessionActionType } from '../utils/action-utils.js'; import { setNewSessionActionType } from '../utils/action-utils.js';
const { processStoreOperations: processThreadStoreOperations } = const { processStoreOperations: processThreadStoreOperations } =
threadStoreOpsHandlers; threadStoreOpsHandlers;
function generateOpsForThreadUpdates( function generateOpsForThreadUpdates(
threadInfos: RawThreadInfos, threadInfos: MinimallyEncodedRawThreadInfos,
payload: { payload: {
+updatesResult: { +newUpdates: $ReadOnlyArray<ClientUpdateInfo>, ... }, +updatesResult: { +newUpdates: $ReadOnlyArray<ClientUpdateInfo>, ... },
... ...
}, },
): $ReadOnlyArray<ThreadStoreOperation> { ): $ReadOnlyArray<ThreadStoreOperation> {
return payload.updatesResult.newUpdates return payload.updatesResult.newUpdates
.map(update => .map(update =>
updateSpecs[update.type].generateOpsForThreadUpdates?.( updateSpecs[update.type].generateOpsForThreadUpdates?.(
Show All 19 Lines if (
action.type === registerActionTypes.success || action.type === registerActionTypes.success ||
action.type === fullStateSyncActionType action.type === fullStateSyncActionType
) { ) {
const newThreadInfos = action.payload.threadInfos; const newThreadInfos = action.payload.threadInfos;
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'remove_all', type: 'remove_all',
}, },
...Object.keys(newThreadInfos).map((id: string) => ({ ...Object.keys(newThreadInfos).map((id: string) => {
invariant(
newThreadInfos[id].minimallyEncoded,
'newThreadInfos must be minimallyEncoded for current clients',
);
atulAuthorUnsubmitted
Done
Inline Actions

I thought putting an invariant here was a reasonable trade-off vs. refactoring the types in the fullStateSyncActionType payload (specifically StateSyncFullActionPayload which is derived from ClientFullStateSync which is derived from BaseFullStateSync) which are used across clients and keyserver.

Open to re-exploring if that would be preferred.

atul: I thought putting an `invariant` here was a reasonable trade-off vs. refactoring the types in…
return {
type: 'replace', type: 'replace',
payload: { id, threadInfo: newThreadInfos[id] }, payload: { id, threadInfo: newThreadInfos[id] },
})), };
}),
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
▲ Show 20 Lines • Show All 58 Lines • ▼ Show 20 Lines ) {
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
} else if (action.type === updateSubscriptionActionTypes.success) { } else if (action.type === updateSubscriptionActionTypes.success) {
const { threadID, subscription } = action.payload; const { threadID, subscription } = action.payload;
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
const newThreadInfo = {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
subscription,
},
};
const threadStoreOperations = [
{
type: 'replace',
payload: {
id: threadID,
threadInfo: newThreadInfo,
},
},
];
const updatedThreadStore = processThreadStoreOperations(
state,
threadStoreOperations,
);
return {
threadStore: updatedThreadStore,
newThreadInconsistencies: [],
threadStoreOperations,
};
} else {
const newThreadInfo = { const newThreadInfo = {
...threadInfo, ...threadInfo,
currentUser: { currentUser: {
...threadInfo.currentUser, ...threadInfo.currentUser,
subscription, subscription,
}, },
}; };
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'replace', type: 'replace',
payload: { payload: {
id: threadID, id: threadID,
threadInfo: newThreadInfo, threadInfo: newThreadInfo,
}, },
}, },
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
}
} else if (action.type === saveMessagesActionType) { } else if (action.type === saveMessagesActionType) {
const threadIDToMostRecentTime = new Map<string, number>(); const threadIDToMostRecentTime = new Map<string, number>();
for (const messageInfo of action.payload.rawMessageInfos) { for (const messageInfo of action.payload.rawMessageInfos) {
const current = threadIDToMostRecentTime.get(messageInfo.threadID); const current = threadIDToMostRecentTime.get(messageInfo.threadID);
if (!current || current < messageInfo.time) { if (!current || current < messageInfo.time) {
threadIDToMostRecentTime.set(messageInfo.threadID, messageInfo.time); threadIDToMostRecentTime.set(messageInfo.threadID, messageInfo.time);
} }
} }
const changedThreadInfos: { [string]: RawThreadInfo } = {}; const changedThreadInfos: { [string]: MinimallyEncodedRawThreadInfo } = {};
for (const [threadID, mostRecentTime] of threadIDToMostRecentTime) { for (const [threadID, mostRecentTime] of threadIDToMostRecentTime) {
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
if ( if (
!threadInfo || !threadInfo ||
threadInfo.currentUser.unread || threadInfo.currentUser.unread ||
action.payload.updatesCurrentAsOf > mostRecentTime action.payload.updatesCurrentAsOf > mostRecentTime
) { ) {
continue; continue;
} }
const changedThreadInfo = state.threadInfos[threadID]; const changedThreadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (changedThreadInfo.minimallyEncoded) {
changedThreadInfos[threadID] = {
...changedThreadInfo,
currentUser: {
...changedThreadInfo.currentUser,
unread: true,
},
};
} else {
changedThreadInfos[threadID] = { changedThreadInfos[threadID] = {
...changedThreadInfo, ...changedThreadInfo,
currentUser: { currentUser: {
...changedThreadInfo.currentUser, ...changedThreadInfo.currentUser,
unread: true, unread: true,
}, },
}; };
} }
}
if (Object.keys(changedThreadInfos).length !== 0) { if (Object.keys(changedThreadInfos).length !== 0) {
const threadStoreOperations = Object.keys(changedThreadInfos).map(id => ({ const threadStoreOperations = Object.keys(changedThreadInfos).map(id => ({
type: 'replace', type: 'replace',
payload: { payload: {
id, id,
threadInfo: changedThreadInfos[id], threadInfo: changedThreadInfos[id],
}, },
})); }));
Show All 25 Lines if (!rawThreadInfos && !deleteThreadIDs) {
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
const threadStoreOperations: ThreadStoreOperation[] = []; const threadStoreOperations: ThreadStoreOperation[] = [];
if (rawThreadInfos) { if (rawThreadInfos) {
for (const rawThreadInfo of rawThreadInfos) { for (const rawThreadInfo of rawThreadInfos) {
invariant(
rawThreadInfo.minimallyEncoded,
'rawThreadInfo must be minimallyEncoded on new clients',
);
atulAuthorUnsubmitted
Done
Inline Actions

Similarly thought this was a reasonable trade-off to avoid having to refactor types related to state check mechanism.

Open to re-exploring if that would be preferred.

atul: Similarly thought this was a reasonable trade-off to avoid having to refactor types related to…
threadStoreOperations.push({ threadStoreOperations.push({
type: 'replace', type: 'replace',
payload: { payload: {
id: rawThreadInfo.id, id: rawThreadInfo.id,
threadInfo: rawThreadInfo, threadInfo: rawThreadInfo,
}, },
}); });
} }
Show All 20 Lines const newThreadInconsistencies =
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies, newThreadInconsistencies,
threadStoreOperations, threadStoreOperations,
}; };
} else if (action.type === updateActivityActionTypes.success) { } else if (action.type === updateActivityActionTypes.success) {
const updatedThreadInfos: { [string]: RawThreadInfo } = {}; const updatedThreadInfos: { [string]: MinimallyEncodedRawThreadInfo } = {};
for (const setToUnread of action.payload.result.unfocusedToUnread) { for (const setToUnread of action.payload.result.unfocusedToUnread) {
const threadInfo = state.threadInfos[setToUnread]; const threadInfo = state.threadInfos[setToUnread];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
if (threadInfo && !threadInfo.currentUser.unread) {
updatedThreadInfos[setToUnread] = {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
unread: true,
},
};
}
} else {
if (threadInfo && !threadInfo.currentUser.unread) { if (threadInfo && !threadInfo.currentUser.unread) {
updatedThreadInfos[setToUnread] = { updatedThreadInfos[setToUnread] = {
...threadInfo, ...threadInfo,
currentUser: { currentUser: {
...threadInfo.currentUser, ...threadInfo.currentUser,
unread: true, unread: true,
}, },
}; };
} }
} }
}
if (Object.keys(updatedThreadInfos).length === 0) { if (Object.keys(updatedThreadInfos).length === 0) {
return { return {
threadStore: state, threadStore: state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
const threadStoreOperations = Object.keys(updatedThreadInfos).map(id => ({ const threadStoreOperations = Object.keys(updatedThreadInfos).map(id => ({
Show All 10 Lines ) {
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
} else if (action.type === setThreadUnreadStatusActionTypes.started) { } else if (action.type === setThreadUnreadStatusActionTypes.started) {
const { threadID, unread } = action.payload; const { threadID, unread } = action.payload;
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
const updatedThreadInfo = {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
unread,
},
};
const threadStoreOperations = [
{
type: 'replace',
payload: {
id: threadID,
threadInfo: updatedThreadInfo,
},
},
];
const updatedThreadStore = processThreadStoreOperations(
state,
threadStoreOperations,
);
return {
threadStore: updatedThreadStore,
newThreadInconsistencies: [],
threadStoreOperations,
};
} else {
const updatedThreadInfo = { const updatedThreadInfo = {
...threadInfo, ...threadInfo,
currentUser: { currentUser: {
...threadInfo.currentUser, ...threadInfo.currentUser,
unread, unread,
}, },
}; };
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'replace', type: 'replace',
payload: { payload: {
id: threadID, id: threadID,
threadInfo: updatedThreadInfo, threadInfo: updatedThreadInfo,
}, },
}, },
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
}
} else if (action.type === setThreadUnreadStatusActionTypes.success) { } else if (action.type === setThreadUnreadStatusActionTypes.success) {
const { threadID, resetToUnread } = action.payload; const { threadID, resetToUnread } = action.payload;
const currentUser = state.threadInfos[threadID].currentUser; const currentUser = state.threadInfos[threadID].currentUser;
if (!resetToUnread || currentUser.unread) { if (!resetToUnread || currentUser.unread) {
return { return {
threadStore: state, threadStore: state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
const threadInfo = state.threadInfos[threadID]; const threadInfo = state.threadInfos[threadID];
// TODO (atul): Try to get rid of this ridiculous branching.
if (threadInfo.minimallyEncoded) {
const updatedThread = { const updatedThread = {
...threadInfo, ...threadInfo,
currentUser: { ...threadInfo.currentUser, unread: true }, currentUser: { ...threadInfo.currentUser, unread: true },
}; };
const threadStoreOperations = [ const threadStoreOperations = [
{ {
type: 'replace', type: 'replace',
payload: { payload: {
id: threadID, id: threadID,
threadInfo: updatedThread, threadInfo: updatedThread,
}, },
}, },
]; ];
const updatedThreadStore = processThreadStoreOperations( const updatedThreadStore = processThreadStoreOperations(
state, state,
threadStoreOperations, threadStoreOperations,
); );
return { return {
threadStore: updatedThreadStore, threadStore: updatedThreadStore,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations, threadStoreOperations,
}; };
} else {
const updatedThread = {
...threadInfo,
currentUser: { ...threadInfo.currentUser, unread: true },
};
const threadStoreOperations = [
{
type: 'replace',
payload: {
id: threadID,
threadInfo: updatedThread,
},
},
];
const updatedThreadStore = processThreadStoreOperations(
state,
threadStoreOperations,
);
return {
threadStore: updatedThreadStore,
newThreadInconsistencies: [],
threadStoreOperations,
};
}
} else if (action.type === setClientDBStoreActionType) { } else if (action.type === setClientDBStoreActionType) {
return { return {
threadStore: action.payload.threadStore ?? state, threadStore: action.payload.threadStore ?? state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
return { return {
threadStore: state, threadStore: state,
newThreadInconsistencies: [], newThreadInconsistencies: [],
threadStoreOperations: [], threadStoreOperations: [],
}; };
} }
export { reduceThreadInfos }; export { reduceThreadInfos };