Page MenuHomePhabricator
Differential D9657

[keyserver] Fetch user infos together with other data
ClosedPublic
Actions

Authored by tomek on Oct 31 2023, 8:48 AM.
Tags
None
Referenced Files
F5111767: D9657.id32776.diff
Wed, Apr 2, 4:43 AM
Unknown Object (File)
Wed, Mar 19, 6:34 PM
Unknown Object (File)
Fri, Mar 14, 6:13 PM
Unknown Object (File)
Tue, Mar 4, 1:42 AM
Unknown Object (File)
Mar 1 2025, 11:13 AM
Unknown Object (File)
Mar 1 2025, 10:56 AM
Unknown Object (File)
Mar 1 2025, 10:56 AM
Unknown Object (File)
Mar 1 2025, 9:35 AM
View All 81 Files
Subscribers
ashoat
will

Details

Reviewers
kamil
inka
bartek
Commits
rCOMMab2cfbce0f09: [keyserver] Fetch user infos together with other data
Summary

Instead of fetching user infos separately, we can fetch them with the rest of the infos.

Depends on D9656

Test Plan

Generate some updates, reintroduce old code, and compare new with old result using _.isEqual.
For one update, generated during sending a friend request to an unknown user, the contents don't match. That seems to expose a race condition which was already present in our code (we're fetching the users a little earlier now) - going to investigate it further.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek created this revision.Oct 31 2023, 8:48 AM
Herald added subscribers: will, ashoat. · View Herald TranscriptOct 31 2023, 8:48 AM
tomek added a child revision: D9658: [keyserver] Move update delete condition to a spec.Oct 31 2023, 8:56 AM
Harbormaster completed remote builds in B23613: Diff 32556.Oct 31 2023, 9:46 AM
tomek requested review of this revision.Oct 31 2023, 9:46 AM
kamil requested changes to this revision.Nov 2 2023, 7:17 AM

For one update, generated during sending a friend request to an unknown user, the contents don't match. That seems to expose a race condition which was already present in our code (we're fetching the users a little earlier now) - going to investigate it further.

I think we first need to resolve this issue before shipping this code

This revision now requires changes to proceed.Nov 2 2023, 7:17 AM
ashoat added a comment.Nov 2 2023, 7:26 AM

Might be good to create (and link) a task here to investigate that issue

tomek added a comment.Nov 3 2023, 7:47 AM

Created a task describing the issue https://linear.app/comm/issue/ENG-5657/race-condition-when-sending-a-friend-request

tomek requested review of this revision.Nov 6 2023, 4:27 AM
kamil accepted this revision.Nov 6 2023, 5:10 AM
This revision is now accepted and ready to land.Nov 6 2023, 5:10 AM
Closed by commit rCOMMab2cfbce0f09: [keyserver] Fetch user infos together with other data (authored by tomek). · Explain WhyNov 6 2023, 6:59 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMab2cfbce0f09: [keyserver] Fetch user infos together with other data.

Revision Contents
Changeset List

PathSize
keyserver/
src/
creators/
25 lines
lib/
shared/
updates/
2 lines
5 lines

Diff 32776

View Options

keyserver/src/creators/update-creator.js

// @flow // @flow
import type { PossiblyStaleNavigationState } from '@react-navigation/native'; import type { PossiblyStaleNavigationState } from '@react-navigation/native';
import { useRoute } from '@react-navigation/native';
import _memoize from 'lodash/memoize.js'; import _memoize from 'lodash/memoize.js';
import * as React from 'react'; import * as React from 'react';
import { createSelector } from 'reselect'; import { createSelector } from 'reselect';
import { nonThreadCalendarFiltersSelector } from 'lib/selectors/calendar-filter-selectors.js'; import { nonThreadCalendarFiltersSelector } from 'lib/selectors/calendar-filter-selectors.js';
import { currentCalendarQuery } from 'lib/selectors/nav-selectors.js'; import { currentCalendarQuery } from 'lib/selectors/nav-selectors.js';
import { useCanEditMessage } from 'lib/shared/edit-messages-utils.js';
import type { CalendarQuery } from 'lib/types/entry-types.js'; import type { CalendarQuery } from 'lib/types/entry-types.js';
import type { CalendarFilter } from 'lib/types/filter-types.js'; import type { CalendarFilter } from 'lib/types/filter-types.js';
import type {
ComposableMessageInfo,
RobotextMessageInfo,
} from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js';
import type { NavContextType } from './navigation-context.js'; import type { NavContextType } from './navigation-context.js';
import { NavContext } from './navigation-context.js'; import { NavContext } from './navigation-context.js';
import { import {
getStateFromNavigatorRoute, getStateFromNavigatorRoute,
getThreadIDFromRoute, getThreadIDFromRoute,
currentLeafRoute, currentLeafRoute,
} from './navigation-utils.js'; } from './navigation-utils.js';
import { import {
AppRouteName, AppRouteName,
TabNavigatorRouteName, TabNavigatorRouteName,
MessageListRouteName, MessageListRouteName,
ChatRouteName, ChatRouteName,
CalendarRouteName, CalendarRouteName,
ThreadPickerModalRouteName, ThreadPickerModalRouteName,
ActionResultModalRouteName, ActionResultModalRouteName,
accountModals, accountModals,
scrollBlockingModals, scrollBlockingModals,
chatRootModals, chatRootModals,
threadRoutes, threadRoutes,
CommunityDrawerNavigatorRouteName, CommunityDrawerNavigatorRouteName,
MessageResultsScreenRouteName,
MessageSearchRouteName,
} from './route-names.js'; } from './route-names.js';
import type { RemoveEditMode } from '../chat/message-list-types'; import type { RemoveEditMode } from '../chat/message-list-types';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
import type { NavPlusRedux } from '../types/selector-types.js'; import type { NavPlusRedux } from '../types/selector-types.js';
import type { GlobalTheme } from '../types/themes.js'; import type { GlobalTheme } from '../types/themes.js';
const baseCreateIsForegroundSelector = (routeName: string) => const baseCreateIsForegroundSelector = (routeName: string) =>
createSelector( createSelector(
▲ Show 20 Lines • Show All 343 Lines • ▼ Show 20 Linesfunction useCurrentLeafRouteName(): ?string {
return React.useMemo(() => { return React.useMemo(() => {
if (!navContext) { if (!navContext) {
return undefined; return undefined;
} }
return currentLeafRoute(navContext.state).name; return currentLeafRoute(navContext.state).name;
}, [navContext]); }, [navContext]);
} }
function useCanEditMessageNative(
threadInfo: ThreadInfo,
targetMessageInfo: ComposableMessageInfo | RobotextMessageInfo,
): boolean {
const route = useRoute();
const screenKey = route.key;
const threadCreationTime = threadInfo.creationTime;
const messageCreationTime = targetMessageInfo.time;
const canEditInThisScreen =
!screenKey.startsWith(MessageSearchRouteName) &&
!screenKey.startsWith(MessageResultsScreenRouteName) &&
messageCreationTime >= threadCreationTime;
return (
useCanEditMessage(threadInfo, targetMessageInfo) && canEditInThisScreen
);
}
export { export {
createIsForegroundSelector, createIsForegroundSelector,
useIsAppLoggedIn, useIsAppLoggedIn,
createActiveTabSelector, createActiveTabSelector,
scrollBlockingModalsClosedSelector, scrollBlockingModalsClosedSelector,
selectBackgroundIsDark, selectBackgroundIsDark,
activeThreadSelector, activeThreadSelector,
activeMessageListSelector, activeMessageListSelector,
useActiveThread, useActiveThread,
useActiveMessageList, useActiveMessageList,
calendarActiveSelector, calendarActiveSelector,
nativeCalendarQuery, nativeCalendarQuery,
nonThreadCalendarQuery, nonThreadCalendarQuery,
useCalendarQuery, useCalendarQuery,
drawerSwipeEnabledSelector, drawerSwipeEnabledSelector,
useCurrentLeafRouteName, useCurrentLeafRouteName,
getRemoveEditMode, getRemoveEditMode,
getTabNavState, getTabNavState,
getChatNavStateFromTabNavState, getChatNavStateFromTabNavState,
useCanEditMessageNative,
}; };
View Options

lib/shared/updates/update-spec.js

// @flow // @flow
import type { PossiblyStaleNavigationState } from '@react-navigation/native'; import type { PossiblyStaleNavigationState } from '@react-navigation/native';
import { useRoute } from '@react-navigation/native';
import _memoize from 'lodash/memoize.js'; import _memoize from 'lodash/memoize.js';
import * as React from 'react'; import * as React from 'react';
import { createSelector } from 'reselect'; import { createSelector } from 'reselect';
import { nonThreadCalendarFiltersSelector } from 'lib/selectors/calendar-filter-selectors.js'; import { nonThreadCalendarFiltersSelector } from 'lib/selectors/calendar-filter-selectors.js';
import { currentCalendarQuery } from 'lib/selectors/nav-selectors.js'; import { currentCalendarQuery } from 'lib/selectors/nav-selectors.js';
import { useCanEditMessage } from 'lib/shared/edit-messages-utils.js';
import type { CalendarQuery } from 'lib/types/entry-types.js'; import type { CalendarQuery } from 'lib/types/entry-types.js';
import type { CalendarFilter } from 'lib/types/filter-types.js'; import type { CalendarFilter } from 'lib/types/filter-types.js';
import type {
ComposableMessageInfo,
RobotextMessageInfo,
} from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js';
import type { NavContextType } from './navigation-context.js'; import type { NavContextType } from './navigation-context.js';
import { NavContext } from './navigation-context.js'; import { NavContext } from './navigation-context.js';
import { import {
getStateFromNavigatorRoute, getStateFromNavigatorRoute,
getThreadIDFromRoute, getThreadIDFromRoute,
currentLeafRoute, currentLeafRoute,
} from './navigation-utils.js'; } from './navigation-utils.js';
import { import {
AppRouteName, AppRouteName,
TabNavigatorRouteName, TabNavigatorRouteName,
MessageListRouteName, MessageListRouteName,
ChatRouteName, ChatRouteName,
CalendarRouteName, CalendarRouteName,
ThreadPickerModalRouteName, ThreadPickerModalRouteName,
ActionResultModalRouteName, ActionResultModalRouteName,
accountModals, accountModals,
scrollBlockingModals, scrollBlockingModals,
chatRootModals, chatRootModals,
threadRoutes, threadRoutes,
CommunityDrawerNavigatorRouteName, CommunityDrawerNavigatorRouteName,
MessageResultsScreenRouteName,
MessageSearchRouteName,
} from './route-names.js'; } from './route-names.js';
import type { RemoveEditMode } from '../chat/message-list-types'; import type { RemoveEditMode } from '../chat/message-list-types';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
import type { NavPlusRedux } from '../types/selector-types.js'; import type { NavPlusRedux } from '../types/selector-types.js';
import type { GlobalTheme } from '../types/themes.js'; import type { GlobalTheme } from '../types/themes.js';
const baseCreateIsForegroundSelector = (routeName: string) => const baseCreateIsForegroundSelector = (routeName: string) =>
createSelector( createSelector(
▲ Show 20 Lines • Show All 343 Lines • ▼ Show 20 Linesfunction useCurrentLeafRouteName(): ?string {
return React.useMemo(() => { return React.useMemo(() => {
if (!navContext) { if (!navContext) {
return undefined; return undefined;
} }
return currentLeafRoute(navContext.state).name; return currentLeafRoute(navContext.state).name;
}, [navContext]); }, [navContext]);
} }
function useCanEditMessageNative(
threadInfo: ThreadInfo,
targetMessageInfo: ComposableMessageInfo | RobotextMessageInfo,
): boolean {
const route = useRoute();
const screenKey = route.key;
const threadCreationTime = threadInfo.creationTime;
const messageCreationTime = targetMessageInfo.time;
const canEditInThisScreen =
!screenKey.startsWith(MessageSearchRouteName) &&
!screenKey.startsWith(MessageResultsScreenRouteName) &&
messageCreationTime >= threadCreationTime;
return (
useCanEditMessage(threadInfo, targetMessageInfo) && canEditInThisScreen
);
}
export { export {
createIsForegroundSelector, createIsForegroundSelector,
useIsAppLoggedIn, useIsAppLoggedIn,
createActiveTabSelector, createActiveTabSelector,
scrollBlockingModalsClosedSelector, scrollBlockingModalsClosedSelector,
selectBackgroundIsDark, selectBackgroundIsDark,
activeThreadSelector, activeThreadSelector,
activeMessageListSelector, activeMessageListSelector,
useActiveThread, useActiveThread,
useActiveMessageList, useActiveMessageList,
calendarActiveSelector, calendarActiveSelector,
nativeCalendarQuery, nativeCalendarQuery,
nonThreadCalendarQuery, nonThreadCalendarQuery,
useCalendarQuery, useCalendarQuery,
drawerSwipeEnabledSelector, drawerSwipeEnabledSelector,
useCurrentLeafRouteName, useCurrentLeafRouteName,
getRemoveEditMode, getRemoveEditMode,
getTabNavState, getTabNavState,
getChatNavStateFromTabNavState, getChatNavStateFromTabNavState,
useCanEditMessageNative,
}; };
View Options

lib/shared/updates/update-user-spec.js

// @flow // @flow
import type { PossiblyStaleNavigationState } from '@react-navigation/native'; import type { PossiblyStaleNavigationState } from '@react-navigation/native';
import { useRoute } from '@react-navigation/native';
import _memoize from 'lodash/memoize.js'; import _memoize from 'lodash/memoize.js';
import * as React from 'react'; import * as React from 'react';
import { createSelector } from 'reselect'; import { createSelector } from 'reselect';
import { nonThreadCalendarFiltersSelector } from 'lib/selectors/calendar-filter-selectors.js'; import { nonThreadCalendarFiltersSelector } from 'lib/selectors/calendar-filter-selectors.js';
import { currentCalendarQuery } from 'lib/selectors/nav-selectors.js'; import { currentCalendarQuery } from 'lib/selectors/nav-selectors.js';
import { useCanEditMessage } from 'lib/shared/edit-messages-utils.js';
import type { CalendarQuery } from 'lib/types/entry-types.js'; import type { CalendarQuery } from 'lib/types/entry-types.js';
import type { CalendarFilter } from 'lib/types/filter-types.js'; import type { CalendarFilter } from 'lib/types/filter-types.js';
import type {
ComposableMessageInfo,
RobotextMessageInfo,
} from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js';
import type { NavContextType } from './navigation-context.js'; import type { NavContextType } from './navigation-context.js';
import { NavContext } from './navigation-context.js'; import { NavContext } from './navigation-context.js';
import { import {
getStateFromNavigatorRoute, getStateFromNavigatorRoute,
getThreadIDFromRoute, getThreadIDFromRoute,
currentLeafRoute, currentLeafRoute,
} from './navigation-utils.js'; } from './navigation-utils.js';
import { import {
AppRouteName, AppRouteName,
TabNavigatorRouteName, TabNavigatorRouteName,
MessageListRouteName, MessageListRouteName,
ChatRouteName, ChatRouteName,
CalendarRouteName, CalendarRouteName,
ThreadPickerModalRouteName, ThreadPickerModalRouteName,
ActionResultModalRouteName, ActionResultModalRouteName,
accountModals, accountModals,
scrollBlockingModals, scrollBlockingModals,
chatRootModals, chatRootModals,
threadRoutes, threadRoutes,
CommunityDrawerNavigatorRouteName, CommunityDrawerNavigatorRouteName,
MessageResultsScreenRouteName,
MessageSearchRouteName,
} from './route-names.js'; } from './route-names.js';
import type { RemoveEditMode } from '../chat/message-list-types'; import type { RemoveEditMode } from '../chat/message-list-types';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
import type { NavPlusRedux } from '../types/selector-types.js'; import type { NavPlusRedux } from '../types/selector-types.js';
import type { GlobalTheme } from '../types/themes.js'; import type { GlobalTheme } from '../types/themes.js';
const baseCreateIsForegroundSelector = (routeName: string) => const baseCreateIsForegroundSelector = (routeName: string) =>
createSelector( createSelector(
▲ Show 20 Lines • Show All 343 Lines • ▼ Show 20 Linesfunction useCurrentLeafRouteName(): ?string {
return React.useMemo(() => { return React.useMemo(() => {
if (!navContext) { if (!navContext) {
return undefined; return undefined;
} }
return currentLeafRoute(navContext.state).name; return currentLeafRoute(navContext.state).name;
}, [navContext]); }, [navContext]);
} }
function useCanEditMessageNative(
threadInfo: ThreadInfo,
targetMessageInfo: ComposableMessageInfo | RobotextMessageInfo,
): boolean {
const route = useRoute();
const screenKey = route.key;
const threadCreationTime = threadInfo.creationTime;
const messageCreationTime = targetMessageInfo.time;
const canEditInThisScreen =
!screenKey.startsWith(MessageSearchRouteName) &&
!screenKey.startsWith(MessageResultsScreenRouteName) &&
messageCreationTime >= threadCreationTime;
return (
useCanEditMessage(threadInfo, targetMessageInfo) && canEditInThisScreen
);
}
export { export {
createIsForegroundSelector, createIsForegroundSelector,
useIsAppLoggedIn, useIsAppLoggedIn,
createActiveTabSelector, createActiveTabSelector,
scrollBlockingModalsClosedSelector, scrollBlockingModalsClosedSelector,
selectBackgroundIsDark, selectBackgroundIsDark,
activeThreadSelector, activeThreadSelector,
activeMessageListSelector, activeMessageListSelector,
useActiveThread, useActiveThread,
useActiveMessageList, useActiveMessageList,
calendarActiveSelector, calendarActiveSelector,
nativeCalendarQuery, nativeCalendarQuery,
nonThreadCalendarQuery, nonThreadCalendarQuery,
useCalendarQuery, useCalendarQuery,
drawerSwipeEnabledSelector, drawerSwipeEnabledSelector,
useCurrentLeafRouteName, useCurrentLeafRouteName,
getRemoveEditMode, getRemoveEditMode,
getTabNavState, getTabNavState,
getChatNavStateFromTabNavState, getChatNavStateFromTabNavState,
useCanEditMessageNative,
}; };