narrow: Encode our own structured keys instead of JSON.

package.json

@@ -55,7 +55,6 @@
     "lodash.isequal": "^4.4.0",
     "lodash.isplainobject": "^4.0.6",
     "lodash.omit": "^4.5.0",
-    "lodash.unescape": "^4.0.1",
     "lodash.union": "^4.6.0",
     "lodash.uniqby": "^4.4.0",
     "react": "16.11.0",

src/__tests__/lib/exampleData.js

@@ -80,6 +80,23 @@ const makeUniqueRandInt = (itemsType: string, end: number): (() => number) => {
 /** Return a string that's almost surely different every time. */
 export const randString = () => randInt(2 ** 54).toString(36);
 
+const intRange = (start, len) => Array.from({ length: len }, (k, i) => i + start);
+
+/** A string with diverse characters to exercise encoding/decoding bugs. */
+/* eslint-disable prefer-template */
+export const diverseCharacters =
+  // The whole range of lowest code points, including control codes
+  // and ASCII punctuation like `"` and `&` used in various syntax...
+  String.fromCharCode(...intRange(0, 0x100))
+  // ... some characters from other scripts...
+  + 'いい文字🎇'
+  // ... some unpaired surrogates, which JS strings can have...
+  + String.fromCharCode(...intRange(0xdbf0, 0x20))
+  // ... some characters beyond the BMP (≥ U+10000)...
+  + '𐂷𠂢'
+  // ... and some code points at the very end of the Unicode range.
+  + String.fromCodePoint(...intRange(0x10fff0, 0x10));
+
 /* ========================================================================
  * Users and bots
  */

src/boot/store.js

@@ -19,6 +19,8 @@ import ZulipAsyncStorage from './ZulipAsyncStorage';
 import createMigration from '../redux-persist-migrate/index';
 import { provideLoggingContext } from './loggingContext';
 import { tryGetActiveAccount } from '../account/accountsSelectors';
+import { keyFromNarrow } from '../utils/narrow';
+import { objectFromEntries } from '../jsBackport';
 
 if (process.env.NODE_ENV === 'development') {
   // Chrome dev tools for Immutable.
@@ -225,6 +227,18 @@ const migrations: { [string]: (GlobalState) => GlobalState } = {
   // `AvatarURL`.
   '18': dropCache,
 
+  // Change format of keys representing narrows, from JSON to our format.
+  '19': state => ({
+    ...dropCache(state),
+    drafts: objectFromEntries(
+      // Note this will migrate straight to our current format, even after
+      // that changes from when this migration was written!  That saves us
+      // from duplicating `keyFromNarrow` here... but calls for care in
+      // migrations for future changes to `keyFromNarrow`.
+      Object.keys(state.drafts).map(key => [keyFromNarrow(JSON.parse(key)), state.drafts[key]]),
+    ),
+  }),
+
   // TIP: When adding a migration, consider just using `dropCache`.
 };
 

src/caughtup/caughtUpReducer.js

@@ -12,7 +12,7 @@ import {
 import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../anchor';
 import { NULL_OBJECT } from '../nullObjects';
 import { DEFAULT_CAUGHTUP } from './caughtUpSelectors';
-import { isSearchNarrow } from '../utils/narrow';
+import { isSearchNarrow, keyFromNarrow } from '../utils/narrow';
 
 const initialState: CaughtUpState = NULL_OBJECT;
 
@@ -89,7 +89,7 @@ export default (state: CaughtUpState = initialState, action: Action): CaughtUpSt
       if (isSearchNarrow(action.narrow)) {
         return state;
       }
-      const key = JSON.stringify(action.narrow);
+      const key = keyFromNarrow(action.narrow);
       let caughtUp;
       if (action.foundNewest !== undefined && action.foundOldest !== undefined) {
         /* This should always be the case for Zulip Server v1.8 or newer. */

src/caughtup/caughtUpSelectors.js

@@ -1,6 +1,7 @@
 /* @flow strict-local */
 import type { CaughtUp, CaughtUpState, GlobalState, Narrow } from '../types';
 import { NULL_OBJECT } from '../nullObjects';
+import { keyFromNarrow } from '../utils/narrow';
 
 /** The value implicitly represented by a missing entry in CaughtUpState. */
 export const DEFAULT_CAUGHTUP: CaughtUp = {
@@ -11,4 +12,4 @@ export const DEFAULT_CAUGHTUP: CaughtUp = {
 export const getCaughtUp = (state: GlobalState): CaughtUpState => state.caughtUp || NULL_OBJECT;
 
 export const getCaughtUpForNarrow = (state: GlobalState, narrow: Narrow): CaughtUp =>
-  getCaughtUp(state)[JSON.stringify(narrow)] || DEFAULT_CAUGHTUP;
+  getCaughtUp(state)[keyFromNarrow(narrow)] || DEFAULT_CAUGHTUP;

src/chat/__tests__/fetchingReducer-test.js

@@ -3,7 +3,7 @@ import deepFreeze from 'deep-freeze';
 
 import * as eg from '../../__tests__/lib/exampleData';
 import fetchingReducer from '../fetchingReducer';
-import { HOME_NARROW, HOME_NARROW_STR, streamNarrow } from '../../utils/narrow';
+import { HOME_NARROW, HOME_NARROW_STR, streamNarrow, keyFromNarrow } from '../../utils/narrow';
 import { MESSAGE_FETCH_START, MESSAGE_FETCH_ERROR } from '../../actionConstants';
 import { DEFAULT_FETCHING } from '../fetchingSelectors';
 
@@ -44,7 +44,7 @@ describe('fetchingReducer', () => {
 
       const expectedState = {
         [HOME_NARROW_STR]: { older: false, newer: false },
-        [JSON.stringify(streamNarrow('some stream'))]: { older: true, newer: false },
+        [keyFromNarrow(streamNarrow('some stream'))]: { older: true, newer: false },
       };
 
       const newState = fetchingReducer(initialState, action);

src/chat/__tests__/narrowsReducer-test.js

@@ -12,6 +12,7 @@ import {
   streamNarrow,
   topicNarrow,
   STARRED_NARROW_STR,
+  keyFromNarrow,
 } from '../../utils/narrow';
 import {
   MESSAGE_FETCH_ERROR,
@@ -23,11 +24,11 @@ import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../../anchor';
 import * as eg from '../../__tests__/lib/exampleData';
 
 describe('narrowsReducer', () => {
-  const privateNarrowStr = JSON.stringify(pm1to1NarrowFromUser(eg.otherUser));
-  const groupNarrowStr = JSON.stringify(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]));
-  const streamNarrowStr = JSON.stringify(streamNarrow(eg.stream.name));
+  const privateNarrowStr = keyFromNarrow(pm1to1NarrowFromUser(eg.otherUser));
+  const groupNarrowStr = keyFromNarrow(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]));
+  const streamNarrowStr = keyFromNarrow(streamNarrow(eg.stream.name));
   const egTopic = eg.streamMessage().subject;
-  const topicNarrowStr = JSON.stringify(topicNarrow(eg.stream.name, egTopic));
+  const topicNarrowStr = keyFromNarrow(topicNarrow(eg.stream.name, egTopic));
 
   describe('EVENT_NEW_MESSAGE', () => {
     test('if not caught up in narrow, do not add message in home narrow', () => {
@@ -145,7 +146,7 @@ describe('narrowsReducer', () => {
   });
 
   test('message sent to self is stored correctly', () => {
-    const narrowWithSelfStr = JSON.stringify(pm1to1NarrowFromUser(eg.selfUser));
+    const narrowWithSelfStr = keyFromNarrow(pm1to1NarrowFromUser(eg.selfUser));
     const initialState = Immutable.Map({
       [HOME_NARROW_STR]: [],
       [narrowWithSelfStr]: [],
@@ -381,7 +382,7 @@ describe('narrowsReducer', () => {
 
       const expectedState = Immutable.Map({
         [HOME_NARROW_STR]: [1, 2, 3],
-        [JSON.stringify(pm1to1NarrowFromUser(eg.otherUser))]: [],
+        [keyFromNarrow(pm1to1NarrowFromUser(eg.otherUser))]: [],
       });
 
       const newState = narrowsReducer(initialState, action);

src/chat/__tests__/narrowsSelectors-test.js

@@ -16,6 +16,7 @@ import {
   topicNarrow,
   STARRED_NARROW,
   pmNarrowFromUsersUnsafe,
+  keyFromNarrow,
 } from '../../utils/narrow';
 import { NULL_SUBSCRIPTION } from '../../nullObjects';
 import * as eg from '../../__tests__/lib/exampleData';
@@ -32,7 +33,7 @@ describe('getMessagesForNarrow', () => {
   test('if no outbox messages returns messages with no change', () => {
     const state = eg.reduxState({
       narrows: Immutable.Map({
-        '[]': [123],
+        [HOME_NARROW_STR]: [123],
       }),
       messages,
       outbox: [],
@@ -47,7 +48,7 @@ describe('getMessagesForNarrow', () => {
   test('combine messages and outbox in same narrow', () => {
     const state = eg.reduxState({
       narrows: Immutable.Map({
-        '[]': [123],
+        [HOME_NARROW_STR]: [123],
       }),
       messages,
       outbox: [outboxMessage],
@@ -80,7 +81,7 @@ describe('getMessagesForNarrow', () => {
   test('do not combine messages and outbox in different narrow', () => {
     const state = eg.reduxState({
       narrows: Immutable.Map({
-        [JSON.stringify(pmNarrowFromEmail('john@example.com'))]: [123],
+        [keyFromNarrow(pmNarrowFromEmail('john@example.com'))]: [123],
       }),
       messages,
       outbox: [outboxMessage],
@@ -97,7 +98,7 @@ describe('getFirstMessageId', () => {
   test('return undefined when there are no messages', () => {
     const state = eg.reduxState({
       narrows: Immutable.Map({
-        '[]': [],
+        [HOME_NARROW_STR]: [],
       }),
       outbox: [],
     });
@@ -110,7 +111,7 @@ describe('getFirstMessageId', () => {
   test('returns first message id', () => {
     const state = eg.reduxState({
       narrows: Immutable.Map({
-        '[]': [1, 2, 3],
+        [HOME_NARROW_STR]: [1, 2, 3],
       }),
       messages: {
         [1]: eg.streamMessage({ id: 1 }) /* eslint-disable-line no-useless-computed-key */,
@@ -130,7 +131,7 @@ describe('getLastMessageId', () => {
   test('return undefined when there are no messages', () => {
     const state = eg.reduxState({
       narrows: Immutable.Map({
-        '[]': [],
+        [HOME_NARROW_STR]: [],
       }),
       messages: {},
       outbox: [],
@@ -144,7 +145,7 @@ describe('getLastMessageId', () => {
   test('returns last message id', () => {
     const state = eg.reduxState({
       narrows: Immutable.Map({
-        '[]': [1, 2, 3],
+        [HOME_NARROW_STR]: [1, 2, 3],
       }),
       messages: {
         [1]: eg.streamMessage({ id: 1 }) /* eslint-disable-line no-useless-computed-key */,

src/chat/fetchingReducer.js

@@ -10,7 +10,7 @@ import {
 } from '../actionConstants';
 import { NULL_OBJECT } from '../nullObjects';
 import { DEFAULT_FETCHING } from './fetchingSelectors';
-import { isSearchNarrow } from '../utils/narrow';
+import { isSearchNarrow, keyFromNarrow } from '../utils/narrow';
 
 const initialState: FetchingState = NULL_OBJECT;
 
@@ -21,7 +21,7 @@ const messageFetchStart = (state, action) => {
     return state;
   }
 
-  const key = JSON.stringify(action.narrow);
+  const key = keyFromNarrow(action.narrow);
   const currentValue = state[key] || DEFAULT_FETCHING;
 
   return {
@@ -34,7 +34,7 @@ const messageFetchStart = (state, action) => {
 };
 
 const messageFetchError = (state, action) => {
-  const key = JSON.stringify(action.narrow);
+  const key = keyFromNarrow(action.narrow);
 
   if (isSearchNarrow(action.narrow)) {
     return state;
@@ -51,7 +51,7 @@ const messageFetchComplete = (state, action) => {
   if (isSearchNarrow(action.narrow)) {
     return state;
   }
-  const key = JSON.stringify(action.narrow);
+  const key = keyFromNarrow(action.narrow);
   const currentValue = state[key] || DEFAULT_FETCHING;
 
   return {

src/chat/fetchingSelectors.js

@@ -1,9 +1,10 @@
 /* @flow strict-local */
 import type { Fetching, GlobalState, Narrow } from '../types';
 import { getFetching } from '../directSelectors';
+import { keyFromNarrow } from '../utils/narrow';
 
 /** The value implicitly represented by a missing entry in FetchingState. */
 export const DEFAULT_FETCHING = { older: false, newer: false };
 
 export const getFetchingForNarrow = (state: GlobalState, narrow: Narrow): Fetching =>
-  getFetching(state)[JSON.stringify(narrow)] || DEFAULT_FETCHING;
+  getFetching(state)[keyFromNarrow(narrow)] || DEFAULT_FETCHING;

src/chat/narrowsReducer.js

@@ -22,6 +22,7 @@ import {
   MENTIONED_NARROW_STR,
   STARRED_NARROW_STR,
   isSearchNarrow,
+  keyFromNarrow,
 } from '../utils/narrow';
 
 const initialState: NarrowsState = Immutable.Map();
@@ -31,7 +32,7 @@ const messageFetchComplete = (state, action) => {
   if (isSearchNarrow(action.narrow)) {
     return state;
   }
-  const key = JSON.stringify(action.narrow);
+  const key = keyFromNarrow(action.narrow);
   const fetchedMessageIds = action.messages.map(message => message.id);
   const replaceExisting =
     action.anchor === FIRST_UNREAD_ANCHOR || action.anchor === LAST_MESSAGE_ANCHOR;
@@ -55,7 +56,7 @@ const eventNewMessage = (state, action) => {
     const narrowsForMessage = getNarrowsForMessage(message, action.ownUser, flags);
 
     narrowsForMessage.forEach(narrow => {
-      const key = JSON.stringify(narrow);
+      const key = keyFromNarrow(narrow);
       const value = stateMutable.get(key);
 
       if (!value) {

src/chat/narrowsSelectors.js

@@ -28,6 +28,7 @@ import {
   emailsOfGroupPmNarrow,
   isMessageInNarrow,
   caseNarrowDefault,
+  keyFromNarrow,
 } from '../utils/narrow';
 import { shouldBeMuted } from '../utils/message';
 import { NULL_ARRAY, NULL_SUBSCRIPTION } from '../nullObjects';
@@ -57,7 +58,7 @@ export const outboxMessagesForNarrow: Selector<Outbox[], Narrow> = createSelecto
 );
 
 export const getFetchedMessageIdsForNarrow = (state: GlobalState, narrow: Narrow) =>
-  getAllNarrows(state).get(JSON.stringify(narrow)) || NULL_ARRAY;
+  getAllNarrows(state).get(keyFromNarrow(narrow)) || NULL_ARRAY;
 
 const getFetchedMessagesForNarrow: Selector<Message[], Narrow> = createSelector(
   getFetchedMessageIdsForNarrow,

src/drafts/__tests__/draftsReducer-test.js

@@ -3,11 +3,11 @@ import deepFreeze from 'deep-freeze';
 import { NULL_OBJECT } from '../../nullObjects';
 import draftsReducer from '../draftsReducer';
 import { DRAFT_UPDATE } from '../../actionConstants';
-import { topicNarrow } from '../../utils/narrow';
+import { topicNarrow, keyFromNarrow } from '../../utils/narrow';
 
 describe('draftsReducer', () => {
   const testNarrow = topicNarrow('denmark', 'denmark2');
-  const testNarrowStr = JSON.stringify(testNarrow);
+  const testNarrowStr = keyFromNarrow(testNarrow);
 
   describe('DRAFT_UPDATE', () => {
     test('add a new draft key drafts', () => {

src/drafts/__tests__/draftsSelectors-test.js

@@ -1,14 +1,14 @@
 import deepFreeze from 'deep-freeze';
 
 import { getDraftForNarrow } from '../draftsSelectors';
-import { topicNarrow } from '../../utils/narrow';
+import { topicNarrow, keyFromNarrow } from '../../utils/narrow';
 
 describe('getDraftForNarrow', () => {
   test('return content of draft if exists', () => {
     const narrow = topicNarrow('stream', 'topic');
     const state = deepFreeze({
       drafts: {
-        [JSON.stringify(narrow)]: 'content',
+        [keyFromNarrow(narrow)]: 'content',
       },
     });
 
@@ -21,7 +21,7 @@ describe('getDraftForNarrow', () => {
     const narrow = topicNarrow('stream', 'topic');
     const state = deepFreeze({
       drafts: {
-        [JSON.stringify(narrow)]: 'content',
+        [keyFromNarrow(narrow)]: 'content',
       },
     });
 

src/drafts/draftsReducer.js

@@ -2,11 +2,12 @@
 import type { DraftsState, Action } from '../types';
 import { DRAFT_UPDATE, LOGOUT, ACCOUNT_SWITCH } from '../actionConstants';
 import { NULL_OBJECT } from '../nullObjects';
+import { keyFromNarrow } from '../utils/narrow';
 
 const initialState = NULL_OBJECT;
 
 const draftUpdate = (state, action) => {
-  const narrowStr = JSON.stringify(action.narrow);
+  const narrowStr = keyFromNarrow(action.narrow);
 
   if (action.content.trim().length === 0) {
     // New content is blank; delete the draft.

src/drafts/draftsSelectors.js

@@ -1,5 +1,6 @@
 /* @flow strict-local */
 import type { Narrow, GlobalState } from '../types';
+import { keyFromNarrow } from '../utils/narrow';
 
 export const getDraftForNarrow = (state: GlobalState, narrow: Narrow): string =>
-  state.drafts[JSON.stringify(narrow)] || '';
+  state.drafts[keyFromNarrow(narrow)] || '';

src/message/__tests__/fetchActions-test.js

@@ -17,14 +17,14 @@ import {
 import { FIRST_UNREAD_ANCHOR } from '../../anchor';
 import type { Message } from '../../api/modelTypes';
 import type { ServerMessage } from '../../api/messages/getMessages';
-import { streamNarrow, HOME_NARROW, HOME_NARROW_STR } from '../../utils/narrow';
+import { streamNarrow, HOME_NARROW, HOME_NARROW_STR, keyFromNarrow } from '../../utils/narrow';
 import { GravatarURL } from '../../utils/avatar';
 import * as eg from '../../__tests__/lib/exampleData';
 
 const mockStore = configureStore([thunk]);
 
 const narrow = streamNarrow('some stream');
-const streamNarrowStr = JSON.stringify(narrow);
+const streamNarrowStr = keyFromNarrow(narrow);
 
 global.FormData = class FormData {};