Refactor Message/PresenceMessage.fromValues and related

Previously, fromValues() took an options object with a 'stringifyAction'
bool property. If that was false, then it acted as a normal fromValues;
if true, then it was effectively doing a partial fromDeserialized,
parsing a wire protocol message into a message but without decoding
the message.data.

This seemed a bit silly. Removed the options object and replaced it with
an explicit Message.fromWireProtocol.

Also while doing that ended up changing a bunch of other things that
seemed broken:
- unnecessary type-assertions due to not having a wire-protocol type, or
  unnecessary use of any & unknown
- channeloptions being type-asserted as cipheroptions, then type-assert
  back to channeloptions, which made no sense
- crypto not being passed-in to history calls, so not getting decrypted

Author: SimonWoolf

ably.d.ts

@@ -2516,9 +2516,8 @@ export interface PresenceMessageStatic {
    * Initialises a `PresenceMessage` from a `PresenceMessage`-like object.
    *
    * @param values - The values to intialise the `PresenceMessage` from.
-   * @param stringifyAction - Whether to convert the `action` field from a number to a string.
    */
-  fromValues(values: PresenceMessage | Record<string, unknown>, stringifyAction?: boolean): PresenceMessage;
+  fromValues(values: Partial<Pick<PresenceMessage, 'clientId' | 'data' | 'extras'>>): PresenceMessage;
 }
 
 /**

src/common/lib/client/defaultrealtime.ts

@@ -14,6 +14,7 @@ import { FilteredSubscriptions } from './filteredsubscriptions';
 import {
   fromValues as presenceMessageFromValues,
   fromValuesArray as presenceMessagesFromValuesArray,
+  fromWireProtocol as presenceMessageFromWireProtocol,
 } from '../types/presencemessage';
 import { Http } from 'common/types/http';
 import Defaults from '../util/defaults';
@@ -39,6 +40,7 @@ export class DefaultRealtime extends BaseRealtime {
           RealtimePresence,
           presenceMessageFromValues,
           presenceMessagesFromValuesArray,
+          presenceMessageFromWireProtocol,
         },
         WebSocketTransport,
         MessageInteractions: FilteredSubscriptions,

src/common/lib/client/modularplugins.ts

@@ -8,13 +8,15 @@ import { FilteredSubscriptions } from './filteredsubscriptions';
 import {
   fromValues as presenceMessageFromValues,
   fromValuesArray as presenceMessagesFromValuesArray,
+  fromWireProtocol as presenceMessageFromWireProtocol,
 } from '../types/presencemessage';
 import { TransportCtor } from '../transport/transport';
 import * as PushPlugin from 'plugins/push';
 
 export interface PresenceMessagePlugin {
   presenceMessageFromValues: typeof presenceMessageFromValues;
   presenceMessagesFromValuesArray: typeof presenceMessagesFromValuesArray;
+  presenceMessageFromWireProtocol: typeof presenceMessageFromWireProtocol;
 }
 
 export type RealtimePresencePlugin = PresenceMessagePlugin & {

src/common/lib/client/restchannelmixin.ts

@@ -2,7 +2,7 @@ import * as API from '../../../../ably';
 import RestChannel from './restchannel';
 import RealtimeChannel from './realtimechannel';
 import * as Utils from '../util/utils';
-import Message, { fromResponseBody as messageFromResponseBody } from '../types/message';
+import Message, { WireProtocolMessage, _fromEncodedArray } from '../types/message';
 import Defaults from '../util/defaults';
 import PaginatedResource, { PaginatedResult } from './paginatedresource';
 import Resource from './resource';
@@ -30,19 +30,16 @@ export class RestChannelMixin {
 
     Utils.mixin(headers, client.options.headers);
 
-    const options = channel.channelOptions;
     return new PaginatedResource(client, this.basePath(channel) + '/messages', headers, envelope, async function (
       body,
       headers,
       unpacked,
     ) {
-      return await messageFromResponseBody(
-        body as Message[],
-        options,
-        channel.logger,
-        client._MsgPack,
-        unpacked ? undefined : format,
-      );
+      const decoded: WireProtocolMessage[] = unpacked
+        ? (body as WireProtocolMessage[])
+        : Utils.decodeBody(body, client._MsgPack, format);
+
+      return _fromEncodedArray(decoded, channel);
     }).get(params as Record<string, unknown>);
   }
 

src/common/lib/client/restpresence.ts

@@ -1,8 +1,8 @@
 import * as Utils from '../util/utils';
 import Logger from '../util/logger';
 import PaginatedResource, { PaginatedResult } from './paginatedresource';
-import PresenceMessage, { fromResponseBody as presenceMessageFromResponseBody } from '../types/presencemessage';
-import { CipherOptions } from '../types/message';
+import PresenceMessage, { WireProtocolPresenceMessage, fromEncodedArray } from '../types/presencemessage';
+import Platform from '../../platform';
 import RestChannel from './restchannel';
 import Defaults from '../util/defaults';
 
@@ -33,13 +33,11 @@ class RestPresence {
       headers,
       envelope,
       async (body, headers, unpacked) => {
-        return await presenceMessageFromResponseBody(
-          body as Record<string, unknown>[],
-          options as CipherOptions,
-          this.logger,
-          client._MsgPack,
-          unpacked ? undefined : format,
-        );
+        const decoded: WireProtocolPresenceMessage[] = unpacked
+          ? (body as WireProtocolPresenceMessage[])
+          : Utils.decodeBody(body, client._MsgPack, format);
+
+        return fromEncodedArray(this.logger, Platform.Crypto, decoded, options);
       },
     ).get(params);
   }

src/common/lib/client/restpresencemixin.ts

@@ -3,8 +3,7 @@ import RealtimePresence from './realtimepresence';
 import * as Utils from '../util/utils';
 import Defaults from '../util/defaults';
 import PaginatedResource, { PaginatedResult } from './paginatedresource';
-import PresenceMessage, { fromResponseBody as presenceMessageFromResponseBody } from '../types/presencemessage';
-import { CipherOptions } from '../types/message';
+import PresenceMessage, { WireProtocolPresenceMessage, _fromEncodedArray } from '../types/presencemessage';
 import { RestChannelMixin } from './restchannelmixin';
 
 export class RestPresenceMixin {
@@ -23,19 +22,18 @@ export class RestPresenceMixin {
 
     Utils.mixin(headers, client.options.headers);
 
-    const options = presence.channel.channelOptions;
-    return new PaginatedResource(client, this.basePath(presence) + '/history', headers, envelope, async function (
-      body,
+    return new PaginatedResource(
+      client,
+      this.basePath(presence) + '/history',
       headers,
-      unpacked,
-    ) {
-      return await presenceMessageFromResponseBody(
-        body as Record<string, unknown>[],
-        options as CipherOptions,
-        presence.logger,
-        client._MsgPack,
-        unpacked ? undefined : format,
-      );
-    }).get(params);
+      envelope,
+      async (body, headers, unpacked) => {
+        const decoded: WireProtocolPresenceMessage[] = unpacked
+          ? (body as WireProtocolPresenceMessage[])
+          : Utils.decodeBody(body, client._MsgPack, format);
+
+        return _fromEncodedArray(decoded, presence.channel);
+      },
+    ).get(params);
   }
 }

src/common/lib/types/defaultmessage.ts

@@ -1,33 +1,41 @@
 import Message, {
+  WireProtocolMessage,
   CipherOptions,
   decode,
   encode,
   EncodingDecodingContext,
   fromEncoded,
   fromEncodedArray,
   fromValues,
+  fromWireProtocol,
 } from './message';
 import * as API from '../../../../ably';
 import Platform from 'common/platform';
 import PresenceMessage from './presencemessage';
 import { ChannelOptions } from 'common/types/channel';
 import Logger from '../util/logger';
+import type { Properties } from '../util/utils';
 
 /**
  `DefaultMessage` is the class returned by `DefaultRest` and `DefaultRealtime`’s `Message` static property. It introduces the static methods described in the `MessageStatic` interface of the public API of the non tree-shakable version of the library.
  */
 export class DefaultMessage extends Message {
   static async fromEncoded(encoded: unknown, inputOptions?: API.ChannelOptions): Promise<Message> {
-    return fromEncoded(Logger.defaultLogger, Platform.Crypto, encoded, inputOptions);
+    return fromEncoded(Logger.defaultLogger, Platform.Crypto, encoded as WireProtocolMessage, inputOptions);
   }
 
   static async fromEncodedArray(encodedArray: Array<unknown>, options?: API.ChannelOptions): Promise<Message[]> {
-    return fromEncodedArray(Logger.defaultLogger, Platform.Crypto, encodedArray, options);
+    return fromEncodedArray(Logger.defaultLogger, Platform.Crypto, encodedArray as WireProtocolMessage[], options);
   }
 
   // Used by tests
-  static fromValues(values: Message | Record<string, unknown>, options?: { stringifyAction?: boolean }): Message {
-    return fromValues(values, options);
+  static fromValues(values: Properties<Message>): Message {
+    return fromValues(values);
+  }
+
+  // Used by tests
+  static fromWireProtocol(values: WireProtocolMessage): Message {
+    return fromWireProtocol(values);
   }
 
   // Used by tests

src/common/lib/types/defaultpresencemessage.ts

@@ -1,23 +1,35 @@
 import * as API from '../../../../ably';
 import Logger from '../util/logger';
-import PresenceMessage, { fromEncoded, fromEncodedArray, fromValues } from './presencemessage';
+import PresenceMessage, {
+  fromEncoded,
+  fromEncodedArray,
+  fromValues,
+  WireProtocolPresenceMessage,
+} from './presencemessage';
+import Platform from 'common/platform';
+import type { Properties } from '../util/utils';
 
 /**
  `DefaultPresenceMessage` is the class returned by `DefaultRest` and `DefaultRealtime`’s `PresenceMessage` static property. It introduces the static methods described in the `PresenceMessageStatic` interface of the public API of the non tree-shakable version of the library.
  */
 export class DefaultPresenceMessage extends PresenceMessage {
   static async fromEncoded(encoded: unknown, inputOptions?: API.ChannelOptions): Promise<PresenceMessage> {
-    return fromEncoded(Logger.defaultLogger, encoded, inputOptions);
+    return fromEncoded(Logger.defaultLogger, Platform.Crypto, encoded as WireProtocolPresenceMessage, inputOptions);
   }
 
   static async fromEncodedArray(
     encodedArray: Array<unknown>,
     options?: API.ChannelOptions,
   ): Promise<PresenceMessage[]> {
-    return fromEncodedArray(Logger.defaultLogger, encodedArray, options);
+    return fromEncodedArray(
+      Logger.defaultLogger,
+      Platform.Crypto,
+      encodedArray as WireProtocolPresenceMessage[],
+      options,
+    );
   }
 
-  static fromValues(values: PresenceMessage | Record<string, unknown>, stringifyAction?: boolean): PresenceMessage {
-    return fromValues(values, stringifyAction);
+  static fromValues(values: Properties<PresenceMessage>): PresenceMessage {
+    return fromValues(values);
   }
 }

src/common/lib/types/message.ts

@@ -1,13 +1,17 @@
 import Platform from 'common/platform';
 import Logger from '../util/logger';
 import ErrorInfo from './errorinfo';
-import { ChannelOptions } from '../../types/channel';
 import PresenceMessage from './presencemessage';
 import * as Utils from '../util/utils';
 import { Bufferlike as BrowserBufferlike } from '../../../platform/web/lib/util/bufferutils';
 import * as API from '../../../../ably';
-import { IUntypedCryptoStatic } from 'common/types/ICryptoStatic';
-import { MsgPack } from 'common/types/msgpack';
+
+import type { IUntypedCryptoStatic } from 'common/types/ICryptoStatic';
+import type { ChannelOptions } from '../../types/channel';
+import type { Properties } from '../util/utils';
+import type RestChannel from '../client/restchannel';
+import type RealtimeChannel from '../client/realtimechannel';
+type Channel = RestChannel | RealtimeChannel;
 
 const MessageActionArray: API.MessageAction[] = [
   'message.unset',
@@ -56,6 +60,8 @@ export type EncodingDecodingContext = {
   baseEncodedPreviousPayload?: Buffer | BrowserBufferlike;
 };
 
+export type WireProtocolMessage = Omit<Message, 'action'> & { action: number };
+
 function normaliseContext(context: CipherOptions | EncodingDecodingContext | ChannelOptions): EncodingDecodingContext {
   if (!context || !(context as EncodingDecodingContext).channelOptions) {
     return {
@@ -67,7 +73,7 @@ function normaliseContext(context: CipherOptions | EncodingDecodingContext | Cha
   return context as EncodingDecodingContext;
 }
 
-function normalizeCipherOptions(
+export function normalizeCipherOptions(
   Crypto: IUntypedCryptoStatic | null,
   logger: Logger,
   options: API.ChannelOptions | null,
@@ -103,10 +109,10 @@ function getMessageSize(msg: Message) {
 export async function fromEncoded(
   logger: Logger,
   Crypto: IUntypedCryptoStatic | null,
-  encoded: unknown,
+  encoded: WireProtocolMessage,
   inputOptions?: API.ChannelOptions,
 ): Promise<Message> {
-  const msg = fromValues(encoded as Message | Record<string, unknown>, { stringifyAction: true });
+  const msg = fromWireProtocol(encoded);
   const options = normalizeCipherOptions(Crypto, logger, inputOptions ?? null);
   /* if decoding fails at any point, catch and return the message decoded to
    * the fullest extent possible */
@@ -121,7 +127,7 @@ export async function fromEncoded(
 export async function fromEncodedArray(
   logger: Logger,
   Crypto: IUntypedCryptoStatic | null,
-  encodedArray: Array<unknown>,
+  encodedArray: Array<WireProtocolMessage>,
   options?: API.ChannelOptions,
 ): Promise<Message[]> {
   return Promise.all(
@@ -131,6 +137,26 @@ export async function fromEncodedArray(
   );
 }
 
+// these forms of the functions are used internally when we have a channel instance
+// already, so don't need to normalise channel options
+export async function _fromEncoded(encoded: WireProtocolMessage, channel: Channel): Promise<Message> {
+  const msg = fromWireProtocol(encoded);
+  try {
+    await decode(msg, channel.channelOptions);
+  } catch (e) {
+    Logger.logAction(channel.logger, Logger.LOG_ERROR, 'Message._fromEncoded()', (e as Error).toString());
+  }
+  return msg;
+}
+
+export async function _fromEncodedArray(encodedArray: WireProtocolMessage[], channel: Channel): Promise<Message[]> {
+  return Promise.all(
+    encodedArray.map(function (encoded) {
+      return _fromEncoded(encoded, channel);
+    }),
+  );
+}
+
 async function encrypt<T extends Message | PresenceMessage>(msg: T, options: CipherOptions): Promise<T> {
   let data = msg.data,
     encoding = msg.encoding,
@@ -272,45 +298,17 @@ export async function decode(
   context.baseEncodedPreviousPayload = lastPayload;
 }
 
-export async function fromResponseBody(
-  body: Array<Message>,
-  options: ChannelOptions | EncodingDecodingContext,
-  logger: Logger,
-  MsgPack: MsgPack | null,
-  format?: Utils.Format,
-): Promise<Message[]> {
-  if (format) {
-    body = Utils.decodeBody(body, MsgPack, format);
-  }
-
-  for (let i = 0; i < body.length; i++) {
-    const msg = (body[i] = fromValues(body[i], { stringifyAction: true }));
-    try {
-      await decode(msg, options);
-    } catch (e) {
-      Logger.logAction(logger, Logger.LOG_ERROR, 'Message.fromResponseBody()', (e as Error).toString());
-    }
-  }
-  return body;
+export function fromValues(values: Properties<Message>): Message {
+  return Object.assign(new Message(), values);
 }
 
-export function fromValues(
-  values: Message | Record<string, unknown>,
-  options?: { stringifyAction?: boolean },
-): Message {
-  const stringifyAction = options?.stringifyAction;
-  if (stringifyAction) {
-    const action = toMessageActionString(values.action as number) || values.action;
-    return Object.assign(new Message(), { ...values, action });
-  }
-  return Object.assign(new Message(), values);
+export function fromWireProtocol(values: WireProtocolMessage): Message {
+  const action = toMessageActionString(values.action as number) || values.action;
+  return Object.assign(new Message(), { ...values, action });
 }
 
-export function fromValuesArray(values: unknown[]): Message[] {
-  const count = values.length,
-    result = new Array(count);
-  for (let i = 0; i < count; i++) result[i] = fromValues(values[i] as Record<string, unknown>);
-  return result;
+export function fromValuesArray(values: Properties<Message>[]): Message[] {
+  return values.map(fromValues);
 }
 
 /* This should be called on encode()d (and encrypt()d) Messages (as it
@@ -336,7 +334,7 @@ class Message {
   encoding?: string | null;
   extras?: any;
   size?: number;
-  action?: API.MessageAction | number;
+  action?: API.MessageAction;
   serial?: string;
   refSerial?: string;
   refType?: string;