registerForEvents: Start sending user_avatar_url_field_optional.

See the point on `user_avatar_url_field_optional` at
https://zulipchat.com/api/register-queue.

This will reduce the volume of data that will have to be sent in the
`/register` response, which should speed up the initial load.

Now, the server may choose, at its own discretion, to omit the
`avatar_url` field for users in the `/register` response [1], and
we'll handle it by using the `/avatar/{user_id}` endpoint.

Some of the boilerplate in the new FallbackAvatarURL class is due to
a performance optimization [2] based on an observed ~1s added to the
rehydrate time on CZO when URL objects are constructed, vs. about
200ms (and maybe a bit more in the upper tail) when they aren't.
During rehydration, we don't need to expensively construct URL
objects to validate raw URL strings, as long as we do so at the edge
when we receive them from the network.

[1] As of the introduction of the client capability (Zulip 3.0,
    feature level 18), it makes this decision based on whether the
    user has been inactive for a long time (using the
    `long_term_idle` field), but this is an implementation detail
    and should be expected to change without notice.

[2] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/user_avatar_url_field_optional/near/993660

Fixes: #4157

src/api/initialDataTypes.js

@@ -111,8 +111,8 @@ export type RawInitialDataRealmUser = {|
   enter_sends: boolean,
   full_name: string,
   is_admin: boolean,
-  realm_users: Array<{| ...User, avatar_url: string | null |}>,
-  realm_non_active_users: Array<{| ...User, avatar_url: string | null |}>,
+  realm_users: Array<{| ...User, avatar_url: string | void | null |}>,
+  realm_non_active_users: Array<{| ...User, avatar_url: string | void | null |}>,
   user_id: number,
 |};
 

src/api/messages/getMessages.js

@@ -54,6 +54,7 @@ export const migrateMessages = (messages: ServerMessage[], identity: Identity):
       avatar_url: AvatarURL.fromUserOrBotData({
         rawAvatarUrl,
         email: message.sender_email,
+        userId: message.sender_id,
         realm: identity.realm,
       }),
       reactions: reactions.map(reaction => {

src/api/registerForEvents.js

@@ -18,27 +18,36 @@ type RegisterForEventsParams = {|
   client_capabilities?: {|
     notification_settings_null: boolean,
     bulk_message_deletion: boolean,
+    user_avatar_url_field_optional: boolean,
   |},
 |};
 
-const transformUser = (rawUser: {| ...User, avatar_url: string | null |}, realm: URL): User => {
+const transformUser = (
+  rawUser: {| ...User, avatar_url: string | void | null |},
+  realm: URL,
+): User => {
   const { avatar_url: rawAvatarUrl, email } = rawUser;
 
   return {
     ...rawUser,
-    avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm }),
+    avatar_url: AvatarURL.fromUserOrBotData({
+      rawAvatarUrl,
+      email,
+      userId: rawUser.user_id,
+      realm,
+    }),
   };
 };
 
 const transformCrossRealmBot = (
   rawCrossRealmBot: {| ...CrossRealmBot, avatar_url: string | void | null |},
   realm: URL,
 ): CrossRealmBot => {
-  const { avatar_url: rawAvatarUrl, email } = rawCrossRealmBot;
+  const { avatar_url: rawAvatarUrl, user_id: userId, email } = rawCrossRealmBot;
 
   return {
     ...rawCrossRealmBot,
-    avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm }),
+    avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm }),
   };
 };
 

src/boot/store.js

@@ -11,7 +11,7 @@ import { persistStore, autoRehydrate } from '../third/redux-persist';
 import type { Config } from '../third/redux-persist';
 
 import { ZulipVersion } from '../utils/zulipVersion';
-import { GravatarURL, UploadedAvatarURL } from '../utils/avatar';
+import { GravatarURL, UploadedAvatarURL, FallbackAvatarURL } from '../utils/avatar';
 import type { Action, GlobalState } from '../types';
 import config from '../config';
 import { REHYDRATE } from '../actionConstants';
@@ -298,6 +298,11 @@ const customReplacer = (key, value, defaultReplacer) => {
       data: UploadedAvatarURL.serialize(value),
       [SERIALIZED_TYPE_FIELD_NAME]: 'UploadedAvatarURL',
     };
+  } else if (value instanceof FallbackAvatarURL) {
+    return {
+      data: FallbackAvatarURL.serialize(value),
+      [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL',
+    };
   }
   return defaultReplacer(key, value);
 };
@@ -314,6 +319,8 @@ const customReviver = (key, value, defaultReviver) => {
         return GravatarURL.deserialize(data);
       case 'UploadedAvatarURL':
         return UploadedAvatarURL.deserialize(data);
+      case 'FallbackAvatarURL':
+        return FallbackAvatarURL.deserialize(data);
       default:
       // Fall back to defaultReviver, below
     }

src/events/eventToAction.js

@@ -89,6 +89,7 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => {
           avatar_url: AvatarURL.fromUserOrBotData({
             rawAvatarUrl: event.message.avatar_url,
             email: event.message.sender_email,
+            userId: event.message.sender_id,
             realm: getCurrentRealm(state),
           }),
         },
@@ -132,7 +133,7 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => {
 
       switch (event.op) {
         case 'add': {
-          const { avatar_url: rawAvatarUrl, email } = event.person;
+          const { avatar_url: rawAvatarUrl, user_id: userId, email } = event.person;
           return {
             type: EVENT_USER_ADD,
             id: event.id,
@@ -141,6 +142,7 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => {
               ...event.person,
               avatar_url: AvatarURL.fromUserOrBotData({
                 rawAvatarUrl,
+                userId,
                 email,
                 realm,
               }),
@@ -149,22 +151,23 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => {
         }
 
         case 'update': {
-          const existingUser = getAllUsersById(state).get(event.person.user_id);
+          const { user_id: userId } = event.person;
+          const existingUser = getAllUsersById(state).get(userId);
           if (!existingUser) {
             // If we get one of these events and don't have
             // information on the user, there's nothing to do about
             // it. But it's probably a bug, so, tell Sentry.
             logging.warn(
               "`realm_user` event with op `update` received for a user we don't know about",
-              { userId: event.person.user_id },
+              { userId },
             );
             return { type: 'ignore' };
           }
 
           return {
             type: EVENT_USER_UPDATE,
             id: event.id,
-            userId: event.person.user_id,
+            userId,
             // Just the fields we want to overwrite.
             person: {
               // Note: The `avatar_url` field will be out of sync with
@@ -175,6 +178,7 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => {
                 ? {
                     avatar_url: AvatarURL.fromUserOrBotData({
                       rawAvatarUrl: event.person.avatar_url,
+                      userId,
                       email: existingUser.email,
                       realm,
                     }),

src/message/fetchActions.js

@@ -326,6 +326,7 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
           client_capabilities: {
             notification_settings_null: true,
             bulk_message_deletion: true,
+            user_avatar_url_field_optional: true,
           },
         }),
       ),

src/utils/__tests__/avatar-test.js

@@ -1,42 +1,49 @@
 /* @flow strict-local */
 import md5 from 'blueimp-md5';
 
-import { AvatarURL, GravatarURL, UploadedAvatarURL } from '../avatar';
+import { AvatarURL, GravatarURL, FallbackAvatarURL, UploadedAvatarURL } from '../avatar';
 import * as eg from '../../__tests__/lib/exampleData';
 
 describe('AvatarURL', () => {
   describe('fromUserOrBotData', () => {
     const user = eg.makeUser();
-    const { email } = user;
+    const { email, user_id: userId } = user;
     const realm = eg.realm;
 
+    test('gives a `FallbackAvatarURL` if `rawAvatarURL` is undefined', () => {
+      const rawAvatarUrl = undefined;
+      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf(
+        FallbackAvatarURL,
+      );
+    });
+
     test('gives a `GravatarURL` if `rawAvatarURL` is null', () => {
       const rawAvatarUrl = null;
-      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf(
+      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf(
         GravatarURL,
       );
     });
 
     test('gives a `GravatarURL` if `rawAvatarURL` is a URL string on Gravatar origin', () => {
       const rawAvatarUrl =
         'https://secure.gravatar.com/avatar/2efaec12efd9bea8a089299208117786?d=identicon&version=3';
-      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf(
+      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf(
         GravatarURL,
       );
     });
 
     test('gives an `UploadedAvatarURL` if `rawAvatarURL` is a non-Gravatar absolute URL string', () => {
       const rawAvatarUrl =
         'https://zulip-avatars.s3.amazonaws.com/13/430713047f2cffed661f84e139a64f864f17f286?x=x&version=5';
-      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf(
+      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf(
         UploadedAvatarURL,
       );
     });
 
     test('gives an `UploadedAvatarURL` if `rawAvatarURL` is a relative URL string', () => {
       const rawAvatarUrl =
         '/user_avatars/2/08fb6d007eb10a56efee1d64760fbeb6111c4352.png?x=x&version=2';
-      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf(
+      expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf(
         UploadedAvatarURL,
       );
     });
@@ -144,3 +151,28 @@ describe('UploadedAvatarURL', () => {
     });
   });
 });
+
+describe('FallbackAvatarURL', () => {
+  test('serializes/deserializes correctly', () => {
+    const instance = FallbackAvatarURL.validateAndConstructInstance({
+      realm: eg.realm,
+      userId: eg.selfUser.user_id,
+    });
+
+    const roundTripped = FallbackAvatarURL.deserialize(FallbackAvatarURL.serialize(instance));
+
+    SIZES_WE_USE.forEach(size => {
+      expect(instance.get(size).toString()).toEqual(roundTripped.get(size).toString());
+    });
+  });
+
+  test('gives the `/avatar/{user_id}` URL, on the provided realm', () => {
+    const userId = eg.selfUser.user_id;
+    const instance = FallbackAvatarURL.validateAndConstructInstance({
+      realm: new URL('https://chat.zulip.org'),
+      userId,
+    });
+
+    expect(instance.get().toString()).toEqual(`https://chat.zulip.org/avatar/${userId.toString()}`);
+  });
+});

src/utils/avatar.js

@@ -18,15 +18,25 @@ export class AvatarURL {
    */
   static fromUserOrBotData(args: {|
     rawAvatarUrl: string | void | null,
+    userId: number,
     email: string,
     realm: URL,
   |}): AvatarURL {
-    const { rawAvatarUrl, email, realm } = args;
+    const { rawAvatarUrl, userId, email, realm } = args;
     if (rawAvatarUrl === undefined) {
-      // `rawAvatarUrl` will be undefined for cross-realm bots from
-      // servers prior to 58ee3fa8c (1.9.0). Fall back to Gravatar for
-      // this case; it should be pretty rare.
-      return GravatarURL.validateAndConstructInstance({ email });
+      // New in Zulip 3.0, feature level 18, the field may be missing
+      // on user objects in the register response, at the server's
+      // discretion, if we announce the
+      // `user_avatar_url_field_optional` client capability, which we
+      // do. See the note about `user_avatar_url_field_optional` at
+      // https://zulipchat.com/api/register-queue.
+      //
+      // It will also be absent on cross-realm bots from servers prior
+      // to 58ee3fa8c (1.9.0). The effect of using FallbackAvatarURL for
+      // this case isn't thoroughly considered, but at worst, it means a
+      // 404. We could plumb through the server version and
+      // conditionalize on that.
+      return FallbackAvatarURL.validateAndConstructInstance({ realm, userId });
     } else if (rawAvatarUrl === null) {
       // If we announce `client_gravatar`, which we do, `rawAvatarUrl`
       // might be null. In that case, we take responsibility for
@@ -156,6 +166,75 @@ export class GravatarURL extends AvatarURL {
   }
 }
 
+/**
+ * The /avatar/{user_id} redirect.
+ *
+ * See the point on `user_avatar_url_field_optional` at
+ * https://zulipchat.com/api/register-queue.
+ *
+ * This endpoint does not currently support size customization.
+ */
+export class FallbackAvatarURL extends AvatarURL {
+  /**
+   * Serialize to a special string; reversible with `deserialize`.
+   */
+  static serialize(instance: FallbackAvatarURL): string {
+    return instance._standardUrl instanceof URL
+      ? instance._standardUrl.toString()
+      : instance._standardUrl;
+  }
+
+  /**
+   * Use a special string from `serialize` to make a new instance.
+   */
+  static deserialize(serialized: string): FallbackAvatarURL {
+    return new FallbackAvatarURL(serialized);
+  }
+
+  /**
+   * Construct from raw server data, or throw an error.
+   */
+  static validateAndConstructInstance(args: {| realm: URL, userId: number |}): FallbackAvatarURL {
+    const { realm, userId } = args;
+    return new FallbackAvatarURL(new URL(`/avatar/${userId.toString()}`, realm.origin));
+  }
+
+  /**
+   * Standard URL from which to generate others. PRIVATE.
+   *
+   * May be a string if the instance was constructed at rehydrate
+   * time, when URL validation is unnecessary.
+   */
+  _standardUrl: string | URL;
+
+  /**
+   * PRIVATE: Make an instance from already-validated data.
+   *
+   * Not part of the public interface; use the static methods instead.
+   *
+   * It's private because we need a path to constructing an instance
+   * without constructing URL objects, which takes more time than is
+   * acceptable when we can avoid it, e.g., during rehydration.
+   * Constructing URL objects is a necessary part of validating data
+   * from the server, but we only need to validate the data once, when
+   * it's first received.
+   */
+  constructor(standardUrl: string | URL) {
+    super();
+    this._standardUrl = standardUrl;
+  }
+
+  get(size: number | void): URL {
+    // `this._standardUrl` may have begun its life as a string, to
+    // avoid computing a URL object during rehydration
+    if (typeof this._standardUrl === 'string') {
+      this._standardUrl = new URL(this._standardUrl);
+    }
+
+    return this._standardUrl;
+  }
+}
+
 /**
  * An avatar that was uploaded to the Zulip server.
  *