✨ (dmk): Improve refresher after new intent queue impl

Author: aussedatlo

.changeset/cute-hands-sin.md

@@ -0,0 +1,5 @@
+---
+"@ledgerhq/device-management-kit": patch
+---
+
+Update refresher to don't ping when an intent is being processed

.changeset/odd-ducks-find.md

@@ -0,0 +1,5 @@
+---
+"@ledgerhq/device-signer-kit-solana": patch
+---
+
+Update InternalApi type in test

.changeset/vast-papayas-melt.md

@@ -0,0 +1,5 @@
+---
+"@ledgerhq/device-signer-kit-bitcoin": patch
+---
+
+Update InternalApi type in test

.changeset/wild-adults-camp.md

@@ -0,0 +1,5 @@
+---
+"@ledgerhq/device-signer-kit-ethereum": patch
+---
+
+Update InternalApi type in test

.changeset/wild-loops-act.md

@@ -0,0 +1,5 @@
+---
+"@ledgerhq/device-management-kit": minor
+---
+
+Cleanup InternalApi to remove disableRefresher function

packages/device-management-kit/src/api/device-action/DeviceAction.ts

@@ -26,7 +26,6 @@ export type InternalApi = {
   readonly setDeviceSessionState: (
     state: DeviceSessionState,
   ) => DeviceSessionState;
-  readonly disableRefresher: (blockerId: string) => () => void;
   readonly getManagerApiService: () => ManagerApiService;
   readonly getSecureChannelService: () => SecureChannelService;
 };

packages/device-management-kit/src/api/device-action/__test-utils__/makeInternalApi.ts

@@ -10,7 +10,6 @@ const apiGetDeviceSessionStateObservableMock = vi.fn();
 const setDeviceSessionStateMock = vi.fn();
 const getManagerApiServiceMock = vi.fn();
 const getSecureChannelServiceMock = vi.fn();
-const disableRefresherMock = vi.fn();
 
 export function makeDeviceActionInternalApiMock(): Mocked<InternalApi> {
   return {
@@ -22,6 +21,5 @@ export function makeDeviceActionInternalApiMock(): Mocked<InternalApi> {
     setDeviceSessionState: setDeviceSessionStateMock,
     getManagerApiService: getManagerApiServiceMock,
     getSecureChannelService: getSecureChannelServiceMock,
-    disableRefresher: disableRefresherMock,
   };
 }

packages/device-management-kit/src/api/secure-channel/task/ConnectToSecureChannelTask.ts

@@ -41,10 +41,6 @@ export class ConnectToSecureChannelTask {
   }
 
   run(): Observable<SecureChannelEvent> {
-    const reenableRefresher = this._api.disableRefresher(
-      "connectToSecureChannel",
-    );
-
     const obs = new Observable<SecureChannelEvent>((subscriber) => {
       let unsubscribed: boolean = false;
       let ignoreNetworkEvents = false;
@@ -318,7 +314,6 @@ export class ConnectToSecureChannelTask {
       };
 
       return () => {
-        reenableRefresher();
         unsubscribed = true;
         // Close the connection if it is open when unsubscribing
         if (this._connection.readyState === WebSocket.OPEN) {

packages/device-management-kit/src/internal/device-session/model/DeviceSession.test.ts

@@ -33,6 +33,7 @@ describe("DeviceSession", () => {
   let mockManagerApi: Mocked<ManagerApiService>;
   let mockSecureChannel: Mocked<SecureChannelService>;
   let mockIntentQueueService: Mocked<IntentQueueService>;
+  let mockIntentQueueServiceFactory: () => IntentQueueService;
   let refresherOptions: DeviceSessionRefresherOptions;
   const mockConnectedDevice = connectedDeviceStubBuilder();
 
@@ -66,6 +67,8 @@ describe("DeviceSession", () => {
       }),
     } as unknown as Mocked<IntentQueueService>;
 
+    mockIntentQueueServiceFactory = () => mockIntentQueueService;
+
     refresherOptions = {
       isRefresherDisabled: true,
     };
@@ -84,7 +87,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
 
       // then
@@ -104,7 +107,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
 
       // then
@@ -113,57 +116,27 @@ describe("DeviceSession", () => {
   });
 
   describe("initialiseSession", () => {
-    it("should successfully initialize session", async () => {
+    it("should successfully initialize session", () => {
       // given
-      const mockObservable = of({
-        status: "success",
-        data: { name: "Dashboard", version: "1.0.0" },
-      }).pipe(delay(1));
-
-      mockIntentQueueService.enqueue.mockReturnValue({
-        observable: mockObservable,
-        cancel: vi.fn(),
-      });
-
       deviceSession = new DeviceSession(
         { connectedDevice: mockConnectedDevice },
         mockLoggerFactory,
         mockManagerApi,
         mockSecureChannel,
-        refresherOptions,
-        mockIntentQueueService,
+        { isRefresherDisabled: false, pollingInterval: 1000 },
+        mockIntentQueueServiceFactory,
+      );
+
+      const mockRefresherStartSpy = vi.spyOn(
+        deviceSession["_deviceSessionRefresher"],
+        "startRefresher",
       );
 
       // when
-      await deviceSession.initialiseSession();
+      deviceSession.initialiseSession();
 
       // then
-      expect(mockIntentQueueService.enqueue).toHaveBeenCalled();
-    });
-
-    it("should throw error if initialization fails", async () => {
-      // given
-      const error = new Error("Initialization failed");
-      const mockObservable = throwError(() => error);
-
-      mockIntentQueueService.enqueue.mockReturnValue({
-        observable: mockObservable,
-        cancel: vi.fn(),
-      });
-
-      deviceSession = new DeviceSession(
-        { connectedDevice: mockConnectedDevice },
-        mockLoggerFactory,
-        mockManagerApi,
-        mockSecureChannel,
-        refresherOptions,
-        mockIntentQueueService,
-      );
-
-      // when/then
-      await expect(deviceSession.initialiseSession()).rejects.toThrow(
-        "Initialization failed",
-      );
+      expect(mockRefresherStartSpy).toHaveBeenCalled();
     });
   });
 
@@ -175,7 +148,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
     });
 
@@ -208,7 +181,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
     });
 
@@ -271,7 +244,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
     });
 
@@ -377,7 +350,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
     });
 
@@ -486,7 +459,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
     });
 
@@ -560,7 +533,6 @@ describe("DeviceSession", () => {
         getDeviceSessionState: expect.any(Function),
         getDeviceSessionStateObservable: expect.any(Function),
         setDeviceSessionState: expect.any(Function),
-        disableRefresher: expect.any(Function),
         getManagerApiService: expect.any(Function),
         getSecureChannelService: expect.any(Function),
       });
@@ -594,7 +566,7 @@ describe("DeviceSession", () => {
         mockManagerApi,
         mockSecureChannel,
         refresherOptions,
-        mockIntentQueueService,
+        mockIntentQueueServiceFactory,
       );
 
       let isComplete = false;
@@ -613,60 +585,4 @@ describe("DeviceSession", () => {
       expect(isComplete).toBe(true);
     });
   });
-
-  describe("disableRefresher", () => {
-    beforeEach(() => {
-      // Use proper mock for refresher tests
-      const mockCommandResult = {
-        status: "success",
-        data: { name: "Dashboard", version: "1.0.0" },
-      };
-
-      mockIntentQueueService.enqueue.mockReturnValue({
-        observable: of(mockCommandResult).pipe(delay(1)),
-        cancel: vi.fn(),
-      });
-
-      deviceSession = new DeviceSession(
-        { connectedDevice: mockConnectedDevice },
-        mockLoggerFactory,
-        mockManagerApi,
-        mockSecureChannel,
-        { isRefresherDisabled: false, pollingInterval: 1000 },
-        mockIntentQueueService,
-      );
-    });
-
-    afterEach(() => {
-      // Clean up refresher
-      if (deviceSession) {
-        deviceSession.close();
-      }
-    });
-
-    it("should return a function to re-enable refresher", () => {
-      // when
-      const enableRefresher = deviceSession.disableRefresher("test-blocker-id");
-
-      // then
-      expect(enableRefresher).toBeInstanceOf(Function);
-
-      // Should be able to call the enable function
-      enableRefresher();
-    });
-
-    it("should handle multiple refresher blockers", () => {
-      // when
-      const enable1 = deviceSession.disableRefresher("blocker-1");
-      const enable2 = deviceSession.disableRefresher("blocker-2");
-
-      // then
-      expect(enable1).toBeInstanceOf(Function);
-      expect(enable2).toBeInstanceOf(Function);
-
-      // Both should be callable
-      enable1();
-      enable2();
-    });
-  });
 });

packages/device-management-kit/src/internal/device-session/model/DeviceSession.ts

@@ -84,15 +84,18 @@ export class DeviceSession {
     managerApiService: ManagerApiService,
     secureChannelService: SecureChannelService,
     deviceSessionRefresherOptions: DeviceSessionRefresherOptions | undefined,
-    intentQueueService: IntentQueueService = new IntentQueueService(
-      loggerModuleFactory,
-    ),
+    intentQueueServiceFactory: (
+      sessionEventDispatcher: DeviceSessionEventDispatcher,
+    ) => IntentQueueService = (sessionEventDispatcher) =>
+      new IntentQueueService(loggerModuleFactory, sessionEventDispatcher),
   ) {
     this._id = id;
     this._connectedDevice = connectedDevice;
     this._logger = loggerModuleFactory("device-session");
-    this._intentQueueService = intentQueueService;
     this._managerApiService = managerApiService;
+    this._intentQueueService = intentQueueServiceFactory(
+      this._sessionEventDispatcher,
+    );
     this._secureChannelService = secureChannelService;
     this._refresherOptions = {
       ...DEVICE_SESSION_REFRESHER_DEFAULT_OPTIONS,
@@ -201,10 +204,6 @@ export class DeviceSession {
       triggersDisconnection: false,
     },
   ): Promise<Either<DmkError, ApduResponse>> {
-    this._sessionEventDispatcher.dispatch({
-      eventName: SessionEvents.DEVICE_STATE_UPDATE_BUSY,
-    });
-
     this._logger.debug(`[exchange] => ${bufferToHexaString(rawApdu, false)}`);
     const result = await this._connectedDevice.sendApdu(
       rawApdu,
@@ -228,7 +227,7 @@ export class DeviceSession {
       })
       .ifLeft(() => {
         this._sessionEventDispatcher.dispatch({
-          eventName: SessionEvents.DEVICE_STATE_UPDATE_CONNECTED,
+          eventName: SessionEvents.DEVICE_STATE_UPDATE_UNKNOWN,
         });
       });
     return result;
@@ -308,8 +307,6 @@ export class DeviceSession {
             this.setDeviceSessionState(state);
             return this._deviceState.getValue();
           },
-          disableRefresher: (blockerId: string) =>
-            this._refresherService.disableRefresher(blockerId),
           getManagerApiService: () => this._managerApiService,
           getSecureChannelService: () => this._secureChannelService,
         });
@@ -328,6 +325,7 @@ export class DeviceSession {
     this._updateDeviceStatus(DeviceStatus.NOT_CONNECTED);
     this._deviceState.complete();
     this._deviceSessionRefresher.stopRefresher();
+    this._pinger.unsubscribe();
   }
 
   public disableRefresher(id: string): () => void {