From afff1c8047bd8369ad73a0b5efa9169d561d2a74 Mon Sep 17 00:00:00 2001
From: Lawrence Forooghian <lawrence@forooghian.com>
Date: Thu, 17 Aug 2023 16:45:08 -0300
Subject: [PATCH] Make BaseClient._MsgPack nullable

This is preparation for #1375 (making MessagePack functionality
tree-shakable), in which there may not be a MessagePack implementation
available.

We update normaliseClientOptions so that it only sets useBinaryProtocol
to true if there is a MessagePack implementation available.
---
 src/common/lib/client/baseclient.ts | 12 ++++++++++--
 src/common/lib/util/defaults.ts     |  7 ++++---
 test/rest/defaults.test.js          | 22 ++++++++++++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/src/common/lib/client/baseclient.ts b/src/common/lib/client/baseclient.ts
index 8d8b201036..876fb71904 100644
--- a/src/common/lib/client/baseclient.ts
+++ b/src/common/lib/client/baseclient.ts
@@ -14,6 +14,7 @@ import { ModulesMap } from './modulesmap';
 import { Rest } from './rest';
 import { IUntypedCryptoStatic } from 'common/types/ICryptoStatic';
 import { throwMissingModuleError } from '../util/utils';
+import { MsgPack } from 'common/types/msgpack';
 
 /**
  `BaseClient` acts as the base class for all of the client classes exported by the SDK. It is an implementation detail and this class is not advertised publicly.
@@ -30,7 +31,14 @@ class BaseClient {
 
   private readonly _rest: Rest | null;
   readonly _Crypto: IUntypedCryptoStatic | null;
-  readonly _MsgPack = Platform.Config.msgpack;
+  private readonly __MsgPack: MsgPack | null = Platform.Config.msgpack;
+
+  get _MsgPack(): MsgPack {
+    if (!this.__MsgPack) {
+      throw new ErrorInfo('MsgPack not available', 400, 40000);
+    }
+    return this.__MsgPack;
+  }
 
   constructor(options: ClientOptions | string, modules: ModulesMap) {
     if (!options) {
@@ -47,7 +55,7 @@ class BaseClient {
       'initialized with clientOptions ' + Platform.Config.inspect(options)
     );
 
-    const normalOptions = (this.options = Defaults.normaliseOptions(optionsObj));
+    const normalOptions = (this.options = Defaults.normaliseOptions(optionsObj, this.__MsgPack));
 
     /* process options */
     if (normalOptions.key) {
diff --git a/src/common/lib/util/defaults.ts b/src/common/lib/util/defaults.ts
index 4f0ba8ee53..5d70dd2902 100644
--- a/src/common/lib/util/defaults.ts
+++ b/src/common/lib/util/defaults.ts
@@ -5,6 +5,7 @@ import ErrorInfo from 'common/lib/types/errorinfo';
 import { version } from '../../../../package.json';
 import ClientOptions, { InternalClientOptions, NormalisedClientOptions } from 'common/types/ClientOptions';
 import IDefaults from '../../types/IDefaults';
+import { MsgPack } from 'common/types/msgpack';
 
 let agent = 'ably-js/' + version;
 
@@ -41,7 +42,7 @@ type CompleteDefaults = IDefaults & {
   checkHost(host: string): void;
   getRealtimeHost(options: ClientOptions, production: boolean, environment: string): string;
   objectifyOptions(options: ClientOptions | string): ClientOptions;
-  normaliseOptions(options: InternalClientOptions): NormalisedClientOptions;
+  normaliseOptions(options: InternalClientOptions, MsgPack: MsgPack | null): NormalisedClientOptions;
   defaultGetHeaders(options: NormalisedClientOptions, headersOptions?: HeadersOptions): Record<string, string>;
   defaultPostHeaders(options: NormalisedClientOptions, headersOptions?: HeadersOptions): Record<string, string>;
 };
@@ -185,7 +186,7 @@ export function objectifyOptions(options: ClientOptions | string): ClientOptions
   return options;
 }
 
-export function normaliseOptions(options: InternalClientOptions): NormalisedClientOptions {
+export function normaliseOptions(options: InternalClientOptions, MsgPack: MsgPack | null): NormalisedClientOptions {
   if (typeof options.recover === 'function' && options.closeOnUnload === true) {
     Logger.logAction(
       Logger.LOG_ERROR,
@@ -223,7 +224,7 @@ export function normaliseOptions(options: InternalClientOptions): NormalisedClie
   const timeouts = getTimeouts(options);
 
   if ('useBinaryProtocol' in options) {
-    options.useBinaryProtocol = Platform.Config.supportsBinary && options.useBinaryProtocol;
+    options.useBinaryProtocol = Platform.Config.supportsBinary && !!MsgPack && options.useBinaryProtocol;
   } else {
     options.useBinaryProtocol = Platform.Config.preferBinary;
   }
diff --git a/test/rest/defaults.test.js b/test/rest/defaults.test.js
index 40be2e7213..173dfd684e 100644
--- a/test/rest/defaults.test.js
+++ b/test/rest/defaults.test.js
@@ -135,6 +135,28 @@ define(['ably', 'chai'], function (Ably, chai) {
       Defaults.ENVIRONMENT = '';
     });
 
+    // TODO once https://github.com/ably/ably-js/issues/1424 is fixed, this should also test the case where the useBinaryProtocol option is not specified
+    describe('normaliseOptions with useBinaryProtocol == true', () => {
+      if (Ably.Realtime.Platform.Config.supportsBinary) {
+        describe('given MsgPack implementation', () => {
+          it('maintains useBinaryProtocol as true', () => {
+            const StubMsgPack = {};
+            var normalisedOptions = Defaults.normaliseOptions({ useBinaryProtocol: true }, StubMsgPack);
+
+            expect(normalisedOptions.useBinaryProtocol).to.be.true;
+          });
+        });
+      }
+
+      describe('given no MsgPack implementation', () => {
+        it('changes useBinaryProtocol to false', () => {
+          var normalisedOptions = Defaults.normaliseOptions({ useBinaryProtocol: true }, null);
+
+          expect(normalisedOptions.useBinaryProtocol).to.be.false;
+        });
+      });
+    });
+
     it('closeOnUnload', function () {
       var options;