redux: Store parsed ZulipVersion in Redux state, instead of string.

Storing the parsed ZulipVersion instance in Redux makes it easier to
work with, so we don't have to think about constructing a new
instance as part of every server feature check.

The one tricky bit is that our Redux state is serialized, with
JSON.stringify, by redux-persist so that it can be stored in
ZulipAsyncStorage, but the ZulipVersion instance itself is not
serializable. Thankfully, it can be fully represented by its raw
version string, which is serializable.

So, on entry into ZulipAsyncStorage, convert the ZulipVersion
instance into the raw version string with .raw() ("replace" it), and
on exit, call the ZulipVersion constructor on that raw version
string ("revive" it).

We considered two main strategies for locating the bit of state to
be transformed (in this case, the `zulipVersion` field, which stores
a ZulipVersion value, in elements of the `state.accounts` array):

1) The "outside-in" strategy, of identifying the value by the
extrinsic property of where it is; i.e., that it's at the field
named 'zulipVersion' on elements of the `state.accounts` array, or

2) The "inside-out" strategy, of identifying the value by its
intrinsic property of being `instanceof ZulipVersion`.

We chose the latter. When we work on zulip#3950, converting our
object-as-map fields to use Map or Immutable.Map, we'll be making
similar, sweeping changes to many different parts of the state, so
it's most natural for the bulk of our logic to be independent of the
location in state, and focus instead on the type of non-serializable
object being stored. This approach conveniently clears the path to
use ImmutableJS for additional reasons discussed later in this
message.

An exploration of the "outside-in" approach is
left as the un-merged PR zulip#3952. The main advantage of that approach
is that we wouldn't need to write migrations, but it had the
significant drawback of requiring a lot of code (for locating the
bit of state to be transformed) that was 1) boilerplate, and 2)
difficult to get fully type-checked, which is a bad combination.
These concerns were not sufficiently alleviated by the complexity of
that boilerplate being bounded by the complexity of the reducer(s)
in charge of the corresponding part(s) of the state.

There's nothing stopping us from mixing the two approaches, in
future, but it would be nice to stick to one as far as possible, for
simplicity.

For the "inside-out" implementation, we use `remotedev-serialize`
(added in a recent commit), with custom replacer and reviver
functions. As Greg explains at
zulip#3952 (comment):

"""
* at save time, values that are `instanceof ZulipVersion` get turned
into objects like `{__serializedType__: 'ZulipVersion', data:
'2.0.0-rc1'}`, before being JSON-serialized

* at load time, after JSON deserialization, values that are objects
with a `__serializedType__` property get transformed in a way
identified by that property's value: in particular if
`__serializedType__: 'ZulipVersion'`, then `data` is fed to `new
ZulipVersion`
"""

Since nothing has ever been stored in this `__serializedType__`
format, we have to write a migration. It turns out to be very
straightforward, and we expect it will be in the future. But we
MUST remember to do so every time we use this approach in future.

As mentioned above, this approach further clears the way for
ImmutableJS. I've neglected to mention that the primary purpose of
`remotedev-serialize` is to replace and revive ImmutableJS objects!
(Note that we added `immutable` as a dependency in a recent commit.)
So zulip#3949 and zulip#3950 will be much easier after this. The custom
replacer/reviver functions we use here are a nice bit of flexibility
provided on the side.

Fixes: zulip#3951

src/__tests__/lib/exampleData.js

@@ -5,6 +5,7 @@ import { createStore } from 'redux';
 import type { CrossRealmBot, Message, PmRecipientUser, Stream, User } from '../../api/modelTypes';
 import type { Action, GlobalState, RealmState } from '../../reduxTypes';
 import type { Auth, Account } from '../../types';
+import { ZulipVersion } from '../../utils/zulipVersion';
 import { ACCOUNT_SWITCH, LOGIN_SUCCESS, REALM_INIT } from '../../actionConstants';
 import rootReducer from '../../boot/reducers';
 import { authOfAccount } from '../../account/accountMisc';
@@ -86,7 +87,7 @@ export const makeCrossRealmBot = (args: { name?: string } = {}): CrossRealmBot =
 
 export const realm = 'https://zulip.example.org';
 
-export const zulipVersion = '2.1.0-234-g7c3acf4';
+export const zulipVersion = new ZulipVersion('2.1.0-234-g7c3acf4');
 
 export const makeAccount = (
   args: {
@@ -95,7 +96,7 @@ export const makeAccount = (
     realm?: string,
     apiKey?: string,
     shouldHaveZulipVersion?: boolean,
-    zulipVersion?: string,
+    zulipVersion?: ZulipVersion,
     ackedPushToken?: string | null,
   } = {},
 ): Account => {

src/account/__tests__/accountsReducer-test.js

@@ -10,6 +10,7 @@ import {
   ACCOUNT_REMOVE,
 } from '../../actionConstants';
 import accountsReducer from '../accountsReducer';
+import { ZulipVersion } from '../../utils/zulipVersion';
 
 import * as eg from '../../__tests__/lib/exampleData';
 
@@ -77,7 +78,7 @@ describe('accountsReducer', () => {
 
     test('records zulipVersion on active account', () => {
       const prevState = deepFreeze([account1, account2, account3]);
-      const newZulipVersion = '2.0.0';
+      const newZulipVersion = new ZulipVersion('2.0.0');
       const action = deepFreeze({ ...eg.action.realm_init, zulipVersion: newZulipVersion });
 
       const expectedState = [{ ...account1, zulipVersion: newZulipVersion }, account2, account3];

src/account/accountActions.js

@@ -7,13 +7,14 @@ import {
   LOGIN_SUCCESS,
   LOGOUT,
 } from '../actionConstants';
+import type { ZulipVersion } from '../utils/zulipVersion';
 
 export const switchAccount = (index: number): Action => ({
   type: ACCOUNT_SWITCH,
   index,
 });
 
-export const realmAdd = (realm: string, zulipVersion: string): Action => ({
+export const realmAdd = (realm: string, zulipVersion: ZulipVersion): Action => ({
   type: REALM_ADD,
   realm,
   zulipVersion,

src/account/accountsSelectors.js

@@ -162,5 +162,5 @@ export const getServerVersion = (state: GlobalState): ZulipVersion | null => {
   if (activeAccount.zulipVersion === null) {
     return null;
   }
-  return new ZulipVersion(activeAccount.zulipVersion);
+  return activeAccount.zulipVersion;
 };

src/actionTypes.js

@@ -86,6 +86,7 @@ import type {
   AlertWordsState,
   UserStatusEvent,
 } from './types';
+import type { ZulipVersion } from './utils/zulipVersion';
 
 export type { NavigationAction } from 'react-navigation';
 
@@ -154,7 +155,7 @@ type AccountSwitchAction = {|
 type RealmAddAction = {|
   type: typeof REALM_ADD,
   realm: string,
-  zulipVersion: string,
+  zulipVersion: ZulipVersion,
 |};
 
 type AccountRemoveAction = {|
@@ -176,7 +177,7 @@ type LogoutAction = {|
 type RealmInitAction = {|
   type: typeof REALM_INIT,
   data: InitialData,
-  zulipVersion: string,
+  zulipVersion: ZulipVersion,
 |};
 
 /** We learned the device token from the system.  See `SessionState`. */

src/boot/store.js

@@ -1,9 +1,12 @@
 /* @flow strict-local */
 import { applyMiddleware, compose, createStore } from 'redux';
 import type { Store } from 'redux';
+import Immutable from 'immutable';
+import * as Serialize from 'remotedev-serialize';
 import { persistStore, autoRehydrate } from '../third/redux-persist';
 import type { Config } from '../third/redux-persist';
 
+import { ZulipVersion } from '../utils/zulipVersion';
 import type { Action, GlobalState } from '../types';
 import rootReducer from './reducers';
 import middleware from './middleware';
@@ -141,12 +144,47 @@ const migrations: { [string]: (GlobalState) => GlobalState } = {
       zulipVersion: a.zulipVersion !== undefined ? a.zulipVersion : null,
     })),
   }),
+
+  // Accounts.zulipVersion is now ZulipVersion | null
+  '12': state => ({
+    ...state,
+    accounts: state.accounts.map(a => ({
+      ...a,
+      zulipVersion:
+        typeof a.zulipVersion === 'string' ? new ZulipVersion(a.zulipVersion) : a.zulipVersion,
+    })),
+  }),
+};
+
+const customReplacer = (key, value, defaultReplacer) => {
+  if (value instanceof ZulipVersion) {
+    /* eslint-disable-next-line id-match */
+    return { data: value.raw(), __serializedType__: 'ZulipVersion' };
+  }
+  return defaultReplacer(key, value);
 };
+const customReviver = (key, value, defaultReviver) => {
+  if (value !== null && typeof value === 'object' && '__serializedType__' in value) {
+    const data = value.data;
+    /* eslint-disable-next-line no-underscore-dangle */
+    switch (value.__serializedType__) {
+      case 'ZulipVersion':
+        return new ZulipVersion(data);
+      default:
+      // Fall back to defaultReviver, below
+    }
+  }
+  return defaultReviver(key, value);
+};
+
+const { stringify, parse } = Serialize.immutable(Immutable, null, customReplacer, customReviver);
 
 const reduxPersistConfig: Config = {
   whitelist: [...storeKeys, ...cacheKeys],
   // $FlowFixMe: https://github.com/rt2zz/redux-persist/issues/823
   storage: ZulipAsyncStorage,
+  serialize: stringify,
+  deserialize: parse,
 };
 
 const store: Store<GlobalState, Action> = createStore(

src/message/fetchActions.js

@@ -34,6 +34,7 @@ import { addToOutbox, sendOutbox } from '../outbox/outboxActions';
 import { realmInit } from '../realm/realmActions';
 import { startEventPolling } from '../events/eventActions';
 import { logout } from '../account/accountActions';
+import { ZulipVersion } from '../utils/zulipVersion';
 
 const messageFetchStart = (narrow: Narrow, numBefore: number, numAfter: number): Action => ({
   type: MESSAGE_FETCH_START,
@@ -249,7 +250,7 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
     return;
   }
 
-  dispatch(realmInit(initData, serverSettings.zulip_version));
+  dispatch(realmInit(initData, new ZulipVersion(serverSettings.zulip_version)));
   dispatch(fetchTopMostNarrow());
   dispatch(initialFetchComplete());
   dispatch(startEventPolling(initData.queue_id, initData.last_event_id));

src/realm/realmActions.js

@@ -1,8 +1,9 @@
 /* @flow strict-local */
 import type { InitialData, Action } from '../types';
+import type { ZulipVersion } from '../utils/zulipVersion';
 import { REALM_INIT } from '../actionConstants';
 
-export const realmInit = (data: InitialData, zulipVersion: string): Action => ({
+export const realmInit = (data: InitialData, zulipVersion: ZulipVersion): Action => ({
   type: REALM_INIT,
   data,
   zulipVersion,

src/start/RealmScreen.js

@@ -3,6 +3,7 @@ import React, { PureComponent } from 'react';
 import { ScrollView, Keyboard } from 'react-native';
 import type { NavigationScreenProp } from 'react-navigation';
 
+import { ZulipVersion } from '../utils/zulipVersion';
 import type { ApiResponseServerSettings, Dispatch } from '../types';
 import { connect } from '../react-redux';
 import { ErrorMsg, Label, SmartUrlInput, Screen, ZulipButton } from '../common';
@@ -54,7 +55,7 @@ class RealmScreen extends PureComponent<Props, State> {
 
     try {
       const serverSettings: ApiResponseServerSettings = await api.getServerSettings(realm);
-      dispatch(realmAdd(realm, serverSettings.zulip_version));
+      dispatch(realmAdd(realm, new ZulipVersion(serverSettings.zulip_version)));
       dispatch(navigateToAuth(serverSettings));
       Keyboard.dismiss();
     } catch (err) {

src/types.js

@@ -4,6 +4,7 @@ import type { DangerouslyImpreciseStyleProp } from 'react-native/Libraries/Style
 
 import type { Auth, Topic, Message, Reaction, ReactionType, Narrow } from './api/apiTypes';
 import type { AppStyles } from './styles/theme';
+import type { ZulipVersion } from './utils/zulipVersion';
 
 export type * from './reduxTypes';
 export type * from './api/apiTypes';
@@ -92,7 +93,7 @@ export type Account = {|
    *    "crunchy shell" pattern (see docs/architecture/crunchy-shell.md);
    *  * context data in Sentry reports.
    */
-  zulipVersion: string | null,
+  zulipVersion: ZulipVersion | null,
 
   /**
    * The last device token value the server has definitely heard from us.