Skip to content

Commit

Permalink
presence: move all presence-reporting logic into new HeartbeatComponent
Browse files Browse the repository at this point in the history
Centralize all presence-reporting logic into a new zero-display React
component, currently (and somewhat arbitrarily) located under
AppStateHandlers.

This fixes at least two issues:

* If the user logs in and out, or otherwise performs an additional
  "initial fetch" without killing the app, the presence timer will
  be started multiple times. (This was previously concealed by
  throttling logic in usersActions.reportPresence, which was itself
  removed in a previous patch in this set.)

* If the user goes idle (e.g. by hitting the Home button), the
  presence timer will still run, and will still attempt to send
  `active` notifications. (These attempts are often suppressed while
  the app is in the background, but there's no guarantee of this.)

Fixes zulip#3699 (properly), and is work towards zulip#3659. Unfortunately,
there is no user-visible change yet: modifications will be needed on
the Zulip server side as well.
  • Loading branch information
rk-for-zulip committed Dec 16, 2019
1 parent a32a1ac commit 987a9b7
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 13 deletions.
17 changes: 8 additions & 9 deletions src/boot/AppEventHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,8 @@ import type { Dispatch, Orientation as OrientationT } from '../types';
import { connect } from '../react-redux';
import { getUnreadByHuddlesMentionsAndPMs } from '../selectors';
import { handleInitialNotification, NotificationListener } from '../notification';
import {
appOnline,
appOrientation,
appState,
initSafeAreaInsets,
reportPresence,
} from '../actions';
import { appOnline, appOrientation, appState, initSafeAreaInsets } from '../actions';
import PresenceHeartbeat from '../presence/PresenceHeartbeat';

/**
* Part of the interface from react-native-netinfo.
Expand Down Expand Up @@ -90,7 +85,6 @@ class AppEventHandlers extends PureComponent<Props> {
/** For the type, see docs: https://facebook.github.io/react-native/docs/appstate */
handleAppStateChange = (state: 'active' | 'background' | 'inactive') => {
const { dispatch, unreadCount } = this.props;
dispatch(reportPresence(state === 'active'));
dispatch(appState(state === 'active'));
if (state === 'background' && Platform.OS === 'android') {
NativeModules.BadgeCountUpdaterModule.setBadgeCount(unreadCount);
Expand Down Expand Up @@ -131,7 +125,12 @@ class AppEventHandlers extends PureComponent<Props> {
}

render() {
return <View style={styles.wrapper}>{this.props.children}</View>;
return (
<>
<PresenceHeartbeat />
<View style={styles.wrapper}>{this.props.children}</View>
</>
);
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/message/fetchActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import { tryUntilSuccessful } from '../utils/async';
import { initNotifications } from '../notification/notificationActions';
import { addToOutbox, sendOutbox } from '../outbox/outboxActions';
import { realmInit } from '../realm/realmActions';
import { reportPresence } from '../users/usersActions';
import { startEventPolling } from '../events/eventActions';
import { logout } from '../account/accountActions';

Expand Down Expand Up @@ -219,9 +218,6 @@ const fetchTopMostNarrow = () => async (dispatch: Dispatch, getState: GetState)
* we want to do when starting up, or regaining a network connection.
*/
export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetState) => {
dispatch(reportPresence());
setInterval(() => dispatch(reportPresence()), 60 * 1000);

dispatch(initialFetchStart());
const auth = getAuth(getState());

Expand Down
71 changes: 71 additions & 0 deletions src/presence/PresenceHeartbeat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// @flow strict-local
import { PureComponent } from 'react';
import { AppState } from 'react-native';
import type { Auth, Dispatch } from '../types';

import { connect } from '../react-redux';
import { tryGetAuth } from '../account/accountsSelectors';
import { reportPresence } from '../actions';
import Heartbeat from './heartbeat';

type Props = $ReadOnly<{|
dispatch: Dispatch,
auth: Auth | void,
|}>;

/**
* Component providing a recurrent `presence` signal.
*/
// Note that "PureComponent" is of questionable veracity: this component's
// entire purpose is to emit network calls for their observable side effects
// as a side effect of being rendered.
//
// However, it is at least true that there is never a need to call `render()`
// (and therefore `componentDidUpdate()`) if the props haven't changed.
class PresenceHeartbeat extends PureComponent<Props> {
/** Callback for Heartbeat object. */
onHeartbeat = (state: boolean) => {
// N.B.: If `auth` changes, we do not send out a final `false` presence
// status for the previous `auth`. It's the responsibility of our logout
// handler to determine whether that's necessary.
//
// (TODO: ensure that our logout handlers actually do that.)
if (this.props.auth) {
this.props.dispatch(reportPresence(state));
}
};

heartbeat: Heartbeat = new Heartbeat(this.onHeartbeat, 1000 * 60);

componentDidMount() {
this.updateHeartbeatState(); // conditional start
AppState.addEventListener('change', this.updateHeartbeatState);
}

componentWillUnmount() {
AppState.removeEventListener('change', this.updateHeartbeatState);
this.heartbeat.stop(); // unconditional stop
}

// React to any state change.
updateHeartbeatState = () => {
// heartbeat.toState is idempotent