Page MenuHomePhabricator
Differential D5179

[services] Tunnelbroker - Add `updateSessionItemDeviceToken` database method
ClosedPublic
Actions

Authored by • max on Sep 19 2022, 7:04 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 1, 10:21 PM
Unknown Object (File)
Tue, Mar 25, 4:04 AM
Unknown Object (File)
Tue, Mar 18, 8:40 AM
Unknown Object (File)
Tue, Mar 18, 8:40 AM
Unknown Object (File)
Tue, Mar 18, 8:40 AM
Unknown Object (File)
Tue, Mar 18, 1:48 AM
Unknown Object (File)
Tue, Mar 18, 1:48 AM
Unknown Object (File)
Tue, Mar 18, 1:48 AM
View All Files
Subscribers
• abosh
ashoat
atul
• karol
tomek

Details

Reviewers
• jon
tomek
Commits
rCOMMdf8907048ae7: [services] Tunnelbroker - Add `updateSessionItemDeviceToken` database method
Summary

This diff introduces a database method to update the device notification token field.
The purpose is to update the device notification token when the token is expired or wrong.

Related Linear task: ENG-1782

Test Plan
  1. Successfully built the server.
  2. Run the test in D5184 using yarn run-unit-tests and yarn run-integration-tests.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• max created this revision.Sep 19 2022, 7:04 AM
• max held this revision as a draft.
Herald added a reviewer: • jon. · View Herald TranscriptSep 19 2022, 7:04 AM
Herald added subscribers: • abosh, • karol, atul and 2 others. · View Herald Transcript
• max published this revision for review.Sep 19 2022, 7:08 AM
• max edited the summary of this revision. (Show Details)
• max edited the test plan for this revision. (Show Details)
• max added a reviewer: tomek.
Herald added a reviewer: • karol. · View Herald TranscriptSep 19 2022, 7:09 AM
Harbormaster completed remote builds in B12275: Diff 16837.Sep 19 2022, 7:14 AM
• jon added a comment.Sep 19 2022, 11:49 AM

not familiar with Aws sdk, but the C++ looks about right. I feel like having a test case for this would help a lot of asserting that it has the desired behavior

• max mentioned this in D5184: [services] Tunnelbroker - Add test for `updateSessionItemDeviceToken` database method.Sep 19 2022, 3:56 PM
• max added a child revision: D5184: [services] Tunnelbroker - Add test for `updateSessionItemDeviceToken` database method.
• max edited the test plan for this revision. (Show Details)Sep 19 2022, 3:58 PM
• max removed a reviewer: • karol.
In D5179#151896, @jon wrote:

I feel like having a test case for this would help a lot of asserting that it has the desired behavior

Sounds reasonable! D5184 was created and the test description was updated as well.

tomek requested changes to this revision.Sep 20 2022, 7:09 AM
tomek added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
141–145 ↗(On Diff #16837)

Is there a way to inform the caller that this operation failed? Assuming that it always succeeds sounds dangerous.

This revision now requires changes to proceed.Sep 20 2022, 7:09 AM
• max updated this revision to Diff 16949.Sep 21 2022, 7:07 AM

Changes to return bool result instead of void.

• max marked an inline comment as done.Sep 21 2022, 7:10 AM
• max added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
141–145 ↗(On Diff #16837)

Is there a way to inform the caller that this operation failed? Assuming that it always succeeds sounds dangerous.

I think it's a good idea to get rid of voiding everywhere practice in the internal database functions. I've changed it to the bool, and we will return grpc::Status(grpc::StatusCode::INTERNAL, "Can not update the token in the Database"); to the client in the follow-up diff where it handles.

Harbormaster completed remote builds in B12347: Diff 16949.Sep 21 2022, 7:17 AM
• max added a child revision: D5206: [services] Tunnelbroker - Update handling of the `newNotifyToken` in `Get` stream request.Sep 21 2022, 3:38 PM
• max added a child revision: D5210: [services] Tunnelbroker - Updating `Send` handler with using of the Rust notification library.Sep 22 2022, 5:00 AM
• jon accepted this revision.Sep 22 2022, 11:32 AM

We can always revisit if we want more detail than just a bool to denote success/failure. But that decision can be deferred.

This looks fine to me.

tomek accepted this revision.Sep 23 2022, 4:49 AM
This revision is now accepted and ready to land.Sep 23 2022, 4:49 AM
• max updated this revision to Diff 17038.Sep 23 2022, 11:20 AM
• max marked an inline comment as done.

Rebasing.

Harbormaster failed remote builds in B12416: Diff 17038!Sep 23 2022, 11:21 AM
Harbormaster completed remote builds in B12416: Diff 17038.Sep 23 2022, 11:29 AM
Closed by commit rCOMMdf8907048ae7: [services] Tunnelbroker - Add `updateSessionItemDeviceToken` database method (authored by • max). · Explain WhySep 23 2022, 11:30 AM
This revision was automatically updated to reflect the committed changes.
• max added a commit: rCOMMdf8907048ae7: [services] Tunnelbroker - Add `updateSessionItemDeviceToken` database method.
• max mentioned this in rCOMM258f3e8b6a95: [services] Tunnelbroker - Add test for `updateSessionItemDeviceToken` database….Sep 23 2022, 11:49 AM

Revision Contents
Changeset List

PathSize
services/
tunnelbroker/
src/
Database/
3 lines
39 lines

Diff 17043

View Options

services/tunnelbroker/src/Database/DatabaseManager.h

// @flow // @flow
import { useRoute } from '@react-navigation/native';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View } from 'react-native';
import { useStringForUser } from 'lib/hooks/ens-cache.js'; import { useStringForUser } from 'lib/hooks/ens-cache.js';
import { clusterEndHeight, avatarOffset } from './chat-constants.js'; import { clusterEndHeight, avatarOffset } from './chat-constants.js';
import type { DisplayType } from './timestamp.react.js'; import type { DisplayType } from './timestamp.react.js';
import { Timestamp, timestampHeight } from './timestamp.react.js'; import { Timestamp, timestampHeight } from './timestamp.react.js';
import { SingleLine } from '../components/single-line.react.js'; import { SingleLine } from '../components/single-line.react.js';
import { MessageListRouteName } from '../navigation/route-names.js';
import { useStyles } from '../themes/colors.js'; import { useStyles } from '../themes/colors.js';
import type { ChatMessageInfoItemWithHeight } from '../types/chat-types.js'; import type { ChatMessageInfoItemWithHeight } from '../types/chat-types.js';
import { useShouldRenderAvatars } from '../utils/avatar-utils.js'; import { useShouldRenderAvatars } from '../utils/avatar-utils.js';
type Props = { type Props = {
+item: ChatMessageInfoItemWithHeight, +item: ChatMessageInfoItemWithHeight,
+focused: boolean, +focused: boolean,
+display: DisplayType, +display: DisplayType,
}; };
function MessageHeader(props: Props): React.Node { function MessageHeader(props: Props): React.Node {
const styles = useStyles(unboundStyles); const styles = useStyles(unboundStyles);
const { item, focused, display } = props; const { item, focused, display } = props;
const { creator, time } = item.messageInfo; const { creator, time } = item.messageInfo;
const { isViewer } = creator; const { isViewer } = creator;
const route = useRoute();
const modalDisplay = display === 'modal'; const modalDisplay = display === 'modal';
const shouldShowUsername = !isViewer && (modalDisplay || item.startsCluster); const shouldShowUsername = !isViewer && (modalDisplay || item.startsCluster);
const stringForUser = useStringForUser(shouldShowUsername ? creator : null); const stringForUser = useStringForUser(shouldShowUsername ? creator : null);
const shouldRenderAvatars = useShouldRenderAvatars(); const shouldRenderAvatars = useShouldRenderAvatars();
let authorName = null; let authorName = null;
if (stringForUser) { if (stringForUser) {
const style = [styles.authorName]; const style = [styles.authorName];
if (modalDisplay) { if (modalDisplay) {
style.push(styles.modal); style.push(styles.modal);
} }
if (shouldRenderAvatars) { if (shouldRenderAvatars) {
style.push({ marginLeft: 12 + avatarOffset }); style.push({ marginLeft: 12 + avatarOffset });
} else { } else {
style.push({ marginLeft: 12 }); style.push({ marginLeft: 12 });
} }
authorName = <SingleLine style={style}>{stringForUser}</SingleLine>; authorName = <SingleLine style={style}>{stringForUser}</SingleLine>;
} }
// We only want to render the top-placed timestamp for a message if it's
// rendered in the message list, and not any separate screens (i.e.
// the MessageResultsScreen).
const presentedFromMessageList =
typeof route.params?.presentedFrom === 'string' &&
route.params.presentedFrom.startsWith(MessageListRouteName);
const messageInMessageList =
route.name === MessageListRouteName || presentedFromMessageList;
const timestamp = const timestamp =
modalDisplay || item.startsConversation ? ( messageInMessageList && (modalDisplay || item.startsConversation) ? (
<Timestamp time={time} display={display} /> <Timestamp time={time} display={display} />
) : null; ) : null;
let style = null; let style = null;
if (focused && !modalDisplay) { if (focused && !modalDisplay) {
let topMargin = 0; let topMargin = 0;
if (!item.startsCluster && !item.messageInfo.creator.isViewer) { if (!item.startsCluster && !item.messageInfo.creator.isViewer) {
topMargin += authorNameHeight + clusterEndHeight; topMargin += authorNameHeight + clusterEndHeight;
Show All 34 Lines
View Options

services/tunnelbroker/src/Database/DatabaseManager.cpp

// @flow // @flow
import { useRoute } from '@react-navigation/native';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View } from 'react-native';
import { useStringForUser } from 'lib/hooks/ens-cache.js'; import { useStringForUser } from 'lib/hooks/ens-cache.js';
import { clusterEndHeight, avatarOffset } from './chat-constants.js'; import { clusterEndHeight, avatarOffset } from './chat-constants.js';
import type { DisplayType } from './timestamp.react.js'; import type { DisplayType } from './timestamp.react.js';
import { Timestamp, timestampHeight } from './timestamp.react.js'; import { Timestamp, timestampHeight } from './timestamp.react.js';
import { SingleLine } from '../components/single-line.react.js'; import { SingleLine } from '../components/single-line.react.js';
import { MessageListRouteName } from '../navigation/route-names.js';
import { useStyles } from '../themes/colors.js'; import { useStyles } from '../themes/colors.js';
import type { ChatMessageInfoItemWithHeight } from '../types/chat-types.js'; import type { ChatMessageInfoItemWithHeight } from '../types/chat-types.js';
import { useShouldRenderAvatars } from '../utils/avatar-utils.js'; import { useShouldRenderAvatars } from '../utils/avatar-utils.js';
type Props = { type Props = {
+item: ChatMessageInfoItemWithHeight, +item: ChatMessageInfoItemWithHeight,
+focused: boolean, +focused: boolean,
+display: DisplayType, +display: DisplayType,
}; };
function MessageHeader(props: Props): React.Node { function MessageHeader(props: Props): React.Node {
const styles = useStyles(unboundStyles); const styles = useStyles(unboundStyles);
const { item, focused, display } = props; const { item, focused, display } = props;
const { creator, time } = item.messageInfo; const { creator, time } = item.messageInfo;
const { isViewer } = creator; const { isViewer } = creator;
const route = useRoute();
const modalDisplay = display === 'modal'; const modalDisplay = display === 'modal';
const shouldShowUsername = !isViewer && (modalDisplay || item.startsCluster); const shouldShowUsername = !isViewer && (modalDisplay || item.startsCluster);
const stringForUser = useStringForUser(shouldShowUsername ? creator : null); const stringForUser = useStringForUser(shouldShowUsername ? creator : null);
const shouldRenderAvatars = useShouldRenderAvatars(); const shouldRenderAvatars = useShouldRenderAvatars();
let authorName = null; let authorName = null;
if (stringForUser) { if (stringForUser) {
const style = [styles.authorName]; const style = [styles.authorName];
if (modalDisplay) { if (modalDisplay) {
style.push(styles.modal); style.push(styles.modal);
} }
if (shouldRenderAvatars) { if (shouldRenderAvatars) {
style.push({ marginLeft: 12 + avatarOffset }); style.push({ marginLeft: 12 + avatarOffset });
} else { } else {
style.push({ marginLeft: 12 }); style.push({ marginLeft: 12 });
} }
authorName = <SingleLine style={style}>{stringForUser}</SingleLine>; authorName = <SingleLine style={style}>{stringForUser}</SingleLine>;
} }
// We only want to render the top-placed timestamp for a message if it's
// rendered in the message list, and not any separate screens (i.e.
// the MessageResultsScreen).
const presentedFromMessageList =
typeof route.params?.presentedFrom === 'string' &&
route.params.presentedFrom.startsWith(MessageListRouteName);
const messageInMessageList =
route.name === MessageListRouteName || presentedFromMessageList;
const timestamp = const timestamp =
modalDisplay || item.startsConversation ? ( messageInMessageList && (modalDisplay || item.startsConversation) ? (
<Timestamp time={time} display={display} /> <Timestamp time={time} display={display} />
) : null; ) : null;
let style = null; let style = null;
if (focused && !modalDisplay) { if (focused && !modalDisplay) {
let topMargin = 0; let topMargin = 0;
if (!item.startsCluster && !item.messageInfo.creator.isViewer) { if (!item.startsCluster && !item.messageInfo.creator.isViewer) {
topMargin += authorNameHeight + clusterEndHeight; topMargin += authorNameHeight + clusterEndHeight;
Show All 34 Lines