test: add tests for signals client

Signed-off-by: Heikki Hellgren <heikki.hellgren@op.fi>

plugins/signals-react/api-report.md

@@ -8,10 +8,7 @@ import { JsonObject } from '@backstage/types';
 
 // @public (undocumented)
 export type SignalsApi = {
-  subscribe(
-    onMessage: (message: JsonObject, topic?: string) => void,
-    topic: string,
-  ): string;
+  subscribe(topic: string, onMessage: (message: JsonObject) => void): string;
   unsubscribe(subscription: string): void;
 };
 

plugins/signals-react/src/api/SignalsApi.ts

@@ -23,10 +23,7 @@ export const signalsApiRef = createApiRef<SignalsApi>({
 
 /** @public */
 export type SignalsApi = {
-  subscribe(
-    onMessage: (message: JsonObject, topic?: string) => void,
-    topic: string,
-  ): string;
+  subscribe(topic: string, onMessage: (message: JsonObject) => void): string;
 
   unsubscribe(subscription: string): void;
 };

plugins/signals-react/src/hooks/useSignalsApi.ts

@@ -27,7 +27,7 @@ export const useSignalsApi = (
   const [subscription, setSubscription] = useState<null | string>(null);
   useEffect(() => {
     if (!subscription) {
-      const sub = signals.subscribe(onMessage, topic);
+      const sub = signals.subscribe(topic, onMessage);
       setSubscription(sub);
     }
   }, [subscription, signals, onMessage, topic]);

plugins/signals/api-report.md

@@ -11,17 +11,19 @@ import { SignalsApi } from '@backstage/plugin-signals-react';
 
 // @public (undocumented)
 export class SignalsClient implements SignalsApi {
-  // (undocumented)
-  static readonly CONNECT_TIMEOUT_MS: number;
   // (undocumented)
   static create(options: {
     identity: IdentityApi;
     discoveryApi: DiscoveryApi;
+    connectTimeout?: number;
+    reconnectTimeout?: number;
   }): SignalsClient;
   // (undocumented)
-  static readonly RECONNECT_TIMEOUT_MS: number;
+  static readonly DEFAULT_CONNECT_TIMEOUT_MS: number;
   // (undocumented)
-  subscribe(onMessage: (message: JsonObject) => void, topic: string): string;
+  static readonly DEFAULT_RECONNECT_TIMEOUT_MS: number;
+  // (undocumented)
+  subscribe(topic: string, onMessage: (message: JsonObject) => void): string;
   // (undocumented)
   unsubscribe(subscription: string): void;
 }

plugins/signals/package.json

@@ -45,6 +45,7 @@
     "@testing-library/jest-dom": "^5.10.1",
     "@testing-library/react": "^12.1.3",
     "@testing-library/user-event": "^14.0.0",
+    "jest-websocket-mock": "^2.5.0",
     "msw": "^1.0.0"
   },
   "files": [

plugins/signals/src/api/SignalsClient.test.ts

@@ -0,0 +1,119 @@
+/*
+ * Copyright 2023 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { DiscoveryApi, IdentityApi } from '@backstage/core-plugin-api';
+import WS from 'jest-websocket-mock';
+import { SignalsClient } from './SignalsClient';
+
+describe('SignalsClient', () => {
+  const tokenFunction = jest.fn();
+  const baseUrlFunction = jest.fn();
+  const identity = {
+    getCredentials: tokenFunction,
+  } as unknown as IdentityApi;
+  const discoveryApi = {
+    getBaseUrl: baseUrlFunction,
+  } as unknown as DiscoveryApi;
+
+  let server: WS;
+
+  beforeEach(async () => {
+    jest.resetAllMocks();
+    tokenFunction.mockResolvedValue({ token: '12345' });
+    baseUrlFunction.mockResolvedValue('http://localhost:1234');
+    server = new WS('ws://localhost:1234', { jsonProtocol: true });
+  });
+
+  afterEach(() => {
+    WS.clean();
+  });
+
+  it('should handle single subscription correctly', async () => {
+    const messageMock = jest.fn();
+    const client = SignalsClient.create({ discoveryApi, identity });
+    const sub = client.subscribe('topic', messageMock);
+    await server.connected;
+
+    await expect(server).toReceiveMessage({
+      action: 'subscribe',
+      topic: 'topic',
+    });
+    server.send({ topic: 'topic', message: { hello: 'world' } });
+    expect(messageMock).toHaveBeenCalledWith({ hello: 'world' });
+
+    client.unsubscribe(sub);
+    await expect(server).toReceiveMessage({
+      action: 'unsubscribe',
+      topic: 'topic',
+    });
+  });
+
+  it('should handle multiple subscription correctly', async () => {
+    const messageMock1 = jest.fn();
+    const messageMock2 = jest.fn();
+    const client1 = SignalsClient.create({ discoveryApi, identity });
+    const client2 = SignalsClient.create({ discoveryApi, identity });
+    const sub1 = client1.subscribe('topic', messageMock1);
+    const sub2 = client2.subscribe('topic', messageMock2);
+
+    await server.connected;
+
+    await expect(server).toReceiveMessage({
+      action: 'subscribe',
+      topic: 'topic',
+    });
+    server.send({ topic: 'topic', message: { hello: 'world' } });
+    expect(messageMock1).toHaveBeenCalledWith({ hello: 'world' });
+    expect(messageMock2).toHaveBeenCalledWith({ hello: 'world' });
+
+    client1.unsubscribe(sub1);
+    await expect(server).not.toReceiveMessage({
+      action: 'unsubscribe',
+      topic: 'topic',
+    });
+
+    client2.unsubscribe(sub2);
+    await expect(server).toReceiveMessage({
+      action: 'unsubscribe',
+      topic: 'topic',
+    });
+  });
+
+  it('should reconnect on error', async () => {
+    const messageMock = jest.fn();
+    const client = SignalsClient.create({
+      discoveryApi,
+      identity,
+      reconnectTimeout: 10,
+      connectTimeout: 100,
+    });
+
+    client.subscribe('topic', messageMock);
+    await server.connected;
+    await expect(server).toReceiveMessage({
+      action: 'subscribe',
+      topic: 'topic',
+    });
+
+    await server.server.emit('error', null);
+
+    await new Promise(r => setTimeout(r, 50));
+    await expect(server).toReceiveMessage({
+      action: 'subscribe',
+      topic: 'topic',
+    });
+  });
+});

plugins/signals/src/api/SignalsClient.ts

@@ -31,27 +31,41 @@ const WS_CLOSE_GOING_AWAY = 1001;
 
 /** @public */
 export class SignalsClient implements SignalsApi {
-  static readonly CONNECT_TIMEOUT_MS: number = 1000;
-  static readonly RECONNECT_TIMEOUT_MS: number = 5000;
+  static readonly DEFAULT_CONNECT_TIMEOUT_MS: number = 1000;
+  static readonly DEFAULT_RECONNECT_TIMEOUT_MS: number = 5000;
   private ws: WebSocket | null = null;
   private subscriptions: Map<string, Subscription> = new Map();
   private messageQueue: string[] = [];
-  private reconnectTimeout: any;
+  private reconnectTo: any;
 
   static create(options: {
     identity: IdentityApi;
     discoveryApi: DiscoveryApi;
+    connectTimeout?: number;
+    reconnectTimeout?: number;
   }) {
-    const { identity, discoveryApi } = options;
-    return new SignalsClient(identity, discoveryApi);
+    const {
+      identity,
+      discoveryApi,
+      connectTimeout = SignalsClient.DEFAULT_CONNECT_TIMEOUT_MS,
+      reconnectTimeout = SignalsClient.DEFAULT_RECONNECT_TIMEOUT_MS,
+    } = options;
+    return new SignalsClient(
+      identity,
+      discoveryApi,
+      connectTimeout,
+      reconnectTimeout,
+    );
   }
 
   private constructor(
     private identity: IdentityApi,
     private discoveryApi: DiscoveryApi,
+    private connectTimeout: number,
+    private reconnectTimeout: number,
   ) {}
 
-  subscribe(onMessage: (message: JsonObject) => void, topic: string): string {
+  subscribe(topic: string, onMessage: (message: JsonObject) => void): string {
     const subscriptionId = uuid();
     const exists = [...this.subscriptions.values()].find(
       sub => sub.topic === topic,
@@ -118,11 +132,11 @@ export class SignalsClient implements SignalsApi {
   }
 
   private async connect() {
-    if (this.ws) {
+    if (this.ws && this.ws.readyState === WebSocket.OPEN) {
       return;
     }
 
-    const apiUrl = `${await this.discoveryApi.getBaseUrl('signals')}`;
+    const apiUrl = await this.discoveryApi.getBaseUrl('signals');
     const { token } = await this.identity.getCredentials();
 
     const url = new URL(apiUrl);
@@ -148,7 +162,7 @@ export class SignalsClient implements SignalsApi {
     while (
       this.ws &&
       this.ws.readyState !== WebSocket.OPEN &&
-      connectSleep < SignalsClient.CONNECT_TIMEOUT_MS
+      connectSleep < this.connectTimeout
     ) {
       await new Promise(r => setTimeout(r, 100));
       connectSleep += 100;
@@ -175,12 +189,12 @@ export class SignalsClient implements SignalsApi {
   }
 
   private reconnect() {
-    if (this.reconnectTimeout) {
-      clearTimeout(this.reconnectTimeout);
+    if (this.reconnectTo) {
+      clearTimeout(this.reconnectTo);
     }
 
-    this.reconnectTimeout = setTimeout(() => {
-      this.reconnectTimeout = null;
+    this.reconnectTo = setTimeout(() => {
+      this.reconnectTo = null;
       if (this.ws) {
         this.ws.close();
       }
@@ -188,13 +202,13 @@ export class SignalsClient implements SignalsApi {
       this.connect()
         .then(() => {
           // Resubscribe to existing topics in case we lost connection
-          for (const topic of this.subscriptions.keys()) {
-            this.send({ action: 'subscribe', topic });
+          for (const sub of this.subscriptions.values()) {
+            this.send({ action: 'subscribe', topic: sub.topic });
           }
         })
         .catch(() => {
           this.reconnect();
         });
-    }, SignalsClient.RECONNECT_TIMEOUT_MS);
+    }, this.reconnectTimeout);
   }
 }

yarn.lock

@@ -8912,6 +8912,7 @@ __metadata:
     "@testing-library/jest-dom": ^5.10.1
     "@testing-library/react": ^12.1.3
     "@testing-library/user-event": ^14.0.0
+    jest-websocket-mock: ^2.5.0
     msw: ^1.0.0
     react-use: ^17.2.4
     uuid: ^8.0.0