diff --git a/plugins/signals-react/api-report.md b/plugins/signals-react/api-report.md index 75aeb5d77d..21f070c35d 100644 --- a/plugins/signals-react/api-report.md +++ b/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; }; diff --git a/plugins/signals-react/src/api/SignalsApi.ts b/plugins/signals-react/src/api/SignalsApi.ts index e592b56df0..2aefa5d166 100644 --- a/plugins/signals-react/src/api/SignalsApi.ts +++ b/plugins/signals-react/src/api/SignalsApi.ts @@ -23,10 +23,7 @@ export const signalsApiRef = createApiRef({ /** @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; }; diff --git a/plugins/signals-react/src/hooks/useSignalsApi.ts b/plugins/signals-react/src/hooks/useSignalsApi.ts index 327ca08ff9..0fcc69efed 100644 --- a/plugins/signals-react/src/hooks/useSignalsApi.ts +++ b/plugins/signals-react/src/hooks/useSignalsApi.ts @@ -27,7 +27,7 @@ export const useSignalsApi = ( const [subscription, setSubscription] = useState(null); useEffect(() => { if (!subscription) { - const sub = signals.subscribe(onMessage, topic); + const sub = signals.subscribe(topic, onMessage); setSubscription(sub); } }, [subscription, signals, onMessage, topic]); diff --git a/plugins/signals/api-report.md b/plugins/signals/api-report.md index 2c9777a5c0..08c52ef418 100644 --- a/plugins/signals/api-report.md +++ b/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; } diff --git a/plugins/signals/package.json b/plugins/signals/package.json index 35ffc8e212..e708d46329 100644 --- a/plugins/signals/package.json +++ b/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": [ diff --git a/plugins/signals/src/api/SignalsClient.test.ts b/plugins/signals/src/api/SignalsClient.test.ts new file mode 100644 index 0000000000..03b25d580d --- /dev/null +++ b/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', + }); + }); +}); diff --git a/plugins/signals/src/api/SignalsClient.ts b/plugins/signals/src/api/SignalsClient.ts index 1fbe374ad9..4051468a9e 100644 --- a/plugins/signals/src/api/SignalsClient.ts +++ b/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 = 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); } } diff --git a/yarn.lock b/yarn.lock index c89f106c57..d44cfeab72 100644 --- a/yarn.lock +++ b/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