♻️ (core): Command signature: pass params to command constructor

LedgerHQ · Jun 12, 2024 · 4c4d93f · 4c4d93f
1 parent d57b00c
commit 4c4d93f
Show file tree

Hide file tree

Showing 12 changed files with 95 additions and 68 deletions.
diff --git a/packages/core/src/api/command/Command.ts b/packages/core/src/api/command/Command.ts
@@ -9,13 +9,14 @@ import { ApduResponse } from "@api/device-session/ApduResponse";
  * @template Args - The type of the arguments passed to the command (optional).
  */
 export interface Command<Response, Args = void> {
+  args: Args;
+
   /**
    * Gets the APDU (Application Protocol Data Unit) for the command.
    *
-   * @param args - The arguments passed to the command (optional).
    * @returns The APDU for the command.
    */
-  getApdu(args?: Args): Apdu;
+  getApdu(): Apdu;
 
   /**
    * Parses the response received from the device.

diff --git a/packages/core/src/api/command/os/CloseAppCommand.ts b/packages/core/src/api/command/os/CloseAppCommand.ts
@@ -10,6 +10,8 @@ import { ApduResponse } from "@api/device-session/ApduResponse";
  * The command to close a runnint application on the device.
  */
 export class CloseAppCommand implements Command<void> {
+  args: void = undefined;
+
   getApdu(): Apdu {
     const closeAppApduArgs: ApduBuilderArgs = {
       cla: 0xb0,

diff --git a/packages/core/src/api/command/os/GetAppAndVersionCommand.test.ts b/packages/core/src/api/command/os/GetAppAndVersionCommand.test.ts
@@ -29,7 +29,7 @@ const FAILED_RESPONSE_HEX = Uint8Array.from([0x67, 0x00]);
 const ERROR_RESPONSE_HEX = Uint8Array.from([0x04, 0x90, 0x00]);
 
 describe("GetAppAndVersionCommand", () => {
-  let command: Command<void, GetAppAndVersionResponse>;
+  let command: Command<GetAppAndVersionResponse>;
 
   beforeEach(() => {
     command = new GetAppAndVersionCommand();

diff --git a/packages/core/src/api/command/os/GetAppAndVersionCommand.ts b/packages/core/src/api/command/os/GetAppAndVersionCommand.ts
@@ -28,6 +28,8 @@ export type GetAppAndVersionResponse = {
 export class GetAppAndVersionCommand
   implements Command<GetAppAndVersionResponse>
 {
+  args = undefined;
+
   getApdu(): Apdu {
     const getAppAndVersionApduArgs: ApduBuilderArgs = {
       cla: 0xb0,

diff --git a/packages/core/src/api/command/os/GetBatteryStatusCommand.test.ts b/packages/core/src/api/command/os/GetBatteryStatusCommand.test.ts
@@ -1,15 +1,10 @@
-import { Command } from "@api/command/Command";
-import {
-  InvalidBatteryStatusTypeError,
-  InvalidStatusWordError,
-} from "@api/command/Errors";
+import { InvalidStatusWordError } from "@api/command/Errors";
 import { ApduResponse } from "@api/device-session/ApduResponse";
 
 import {
   BatteryStatusType,
   ChargingMode,
   GetBatteryStatusCommand,
-  GetBatteryStatusResponse,
 } from "./GetBatteryStatusCommand";
 
 const GET_BATTERY_STATUS_APDU_PERCENTAGE = Uint8Array.from([
@@ -37,28 +32,42 @@ const FLAGS_RESPONSE_HEX = Uint8Array.from([
 const FAILED_RESPONSE_HEX = Uint8Array.from([0x67, 0x00]);
 
 describe("GetBatteryStatus", () => {
-  let command: Command<GetBatteryStatusResponse, BatteryStatusType>;
-
-  beforeEach(() => {
-    command = new GetBatteryStatusCommand();
-  });
-
   describe("getApdu", () => {
     it("should return the GetBatteryStatus APDU", () => {
       expect(
-        command.getApdu(BatteryStatusType.BATTERY_PERCENTAGE).getRawApdu(),
+        new GetBatteryStatusCommand({
+          statusType: BatteryStatusType.BATTERY_PERCENTAGE,
+        })
+          .getApdu()
+          .getRawApdu(),
       ).toStrictEqual(GET_BATTERY_STATUS_APDU_PERCENTAGE);
       expect(
-        command.getApdu(BatteryStatusType.BATTERY_VOLTAGE).getRawApdu(),
+        new GetBatteryStatusCommand({
+          statusType: BatteryStatusType.BATTERY_VOLTAGE,
+        })
+          .getApdu()
+          .getRawApdu(),
       ).toStrictEqual(GET_BATTERY_STATUS_APDU_VOLTAGE);
       expect(
-        command.getApdu(BatteryStatusType.BATTERY_TEMPERATURE).getRawApdu(),
+        new GetBatteryStatusCommand({
+          statusType: BatteryStatusType.BATTERY_TEMPERATURE,
+        })
+          .getApdu()
+          .getRawApdu(),
       ).toStrictEqual(GET_BATTERY_STATUS_APDU_TEMPERATURE);
       expect(
-        command.getApdu(BatteryStatusType.BATTERY_CURRENT).getRawApdu(),
+        new GetBatteryStatusCommand({
+          statusType: BatteryStatusType.BATTERY_CURRENT,
+        })
+          .getApdu()
+          .getRawApdu(),
       ).toStrictEqual(GET_BATTERY_STATUS_APDU_CURRENT);
       expect(
-        command.getApdu(BatteryStatusType.BATTERY_FLAGS).getRawApdu(),
+        new GetBatteryStatusCommand({
+          statusType: BatteryStatusType.BATTERY_FLAGS,
+        })
+          .getApdu()
+          .getRawApdu(),
       ).toStrictEqual(GET_BATTERY_STATUS_APDU_FLAGS);
     });
   });
@@ -68,7 +77,10 @@ describe("GetBatteryStatus", () => {
         statusCode: PERCENTAGE_RESPONSE_HEX.slice(-2),
         data: PERCENTAGE_RESPONSE_HEX.slice(0, -2),
       });
-      command.getApdu(BatteryStatusType.BATTERY_PERCENTAGE);
+      const command = new GetBatteryStatusCommand({
+        statusType: BatteryStatusType.BATTERY_PERCENTAGE,
+      });
+      command.getApdu();
       const parsed = command.parseResponse(PERCENTAGE_RESPONSE);
       expect(parsed).toStrictEqual(55);
     });
@@ -77,7 +89,10 @@ describe("GetBatteryStatus", () => {
         statusCode: VOLTAGE_RESPONSE_HEX.slice(-2),
         data: VOLTAGE_RESPONSE_HEX.slice(0, -2),
       });
-      command.getApdu(BatteryStatusType.BATTERY_VOLTAGE);
+      const command = new GetBatteryStatusCommand({
+        statusType: BatteryStatusType.BATTERY_VOLTAGE,
+      });
+      command.getApdu();
       const parsed = command.parseResponse(VOLTAGE_RESPONSE);
       expect(parsed).toStrictEqual(4095);
     });
@@ -86,7 +101,10 @@ describe("GetBatteryStatus", () => {
         statusCode: TEMPERATURE_RESPONSE_HEX.slice(-2),
         data: TEMPERATURE_RESPONSE_HEX.slice(0, -2),
       });
-      command.getApdu(BatteryStatusType.BATTERY_TEMPERATURE);
+      const command = new GetBatteryStatusCommand({
+        statusType: BatteryStatusType.BATTERY_TEMPERATURE,
+      });
+      command.getApdu();
       const parsed = command.parseResponse(TEMPERATURE_RESPONSE);
       expect(parsed).toStrictEqual(16);
     });
@@ -95,7 +113,10 @@ describe("GetBatteryStatus", () => {
         statusCode: FLAGS_RESPONSE_HEX.slice(-2),
         data: FLAGS_RESPONSE_HEX.slice(0, -2),
       });
-      command.getApdu(BatteryStatusType.BATTERY_FLAGS);
+      const command = new GetBatteryStatusCommand({
+        statusType: BatteryStatusType.BATTERY_FLAGS,
+      });
+      command.getApdu();
       const parsed = command.parseResponse(FLAGS_RESPONSE);
       expect(parsed).toStrictEqual({
         charging: ChargingMode.USB,
@@ -104,21 +125,15 @@ describe("GetBatteryStatus", () => {
         issueBattery: false,
       });
     });
-    it("should not parse the response when getApdu not called", () => {
-      const PERCENTAGE_RESPONSE = new ApduResponse({
-        statusCode: PERCENTAGE_RESPONSE_HEX.slice(-2),
-        data: PERCENTAGE_RESPONSE_HEX.slice(0, -2),
-      });
-      expect(() => command.parseResponse(PERCENTAGE_RESPONSE)).toThrow(
-        InvalidBatteryStatusTypeError,
-      );
-    });
     it("should throw an error if the response returned unsupported format", () => {
       const FAILED_RESPONSE = new ApduResponse({
         statusCode: FAILED_RESPONSE_HEX.slice(-2),
         data: FAILED_RESPONSE_HEX.slice(0, -2),
       });
-      command.getApdu(BatteryStatusType.BATTERY_PERCENTAGE);
+      const command = new GetBatteryStatusCommand({
+        statusType: BatteryStatusType.BATTERY_PERCENTAGE,
+      });
+      command.getApdu();
       expect(() => command.parseResponse(FAILED_RESPONSE)).toThrow(
         InvalidStatusWordError,
       );

diff --git a/packages/core/src/api/command/os/GetBatteryStatusCommand.ts b/packages/core/src/api/command/os/GetBatteryStatusCommand.ts
@@ -66,6 +66,10 @@ export type BatteryStatusFlags = {
  */
 export type GetBatteryStatusResponse = number | BatteryStatusFlags;
 
+type Arguments = {
+  statusType: BatteryStatusType;
+};
+
 /**
  * Command to get the battery status of the device.
  * The parameter statusType defines the type of information to retrieve, cf.
@@ -75,28 +79,25 @@ export type GetBatteryStatusResponse = number | BatteryStatusFlags;
  * going to decrease the overall performance of the communication with the device.
  */
 export class GetBatteryStatusCommand
-  implements Command<GetBatteryStatusResponse, BatteryStatusType>
+  implements Command<GetBatteryStatusResponse, Arguments>
 {
-  private _statusType: BatteryStatusType | undefined = undefined;
+  args: Arguments;
+
+  constructor(args: Arguments) {
+    this.args = args;
+  }
 
-  getApdu(statusType: BatteryStatusType): Apdu {
-    this._statusType = statusType;
+  getApdu(): Apdu {
     const getBatteryStatusArgs: ApduBuilderArgs = {
       cla: 0xe0,
       ins: 0x10,
       p1: 0x00,
-      p2: statusType,
+      p2: this.args.statusType,
     } as const;
     return new ApduBuilder(getBatteryStatusArgs).build();
   }
 
   parseResponse(apduResponse: ApduResponse): GetBatteryStatusResponse {
-    if (this._statusType === undefined) {
-      throw new InvalidBatteryStatusTypeError(
-        "Call getApdu to initialise battery status type.",
-      );
-    }
-
     const parser = new ApduParser(apduResponse);
     if (!CommandUtils.isSuccessResponse(apduResponse)) {
       throw new InvalidStatusWordError(
@@ -106,7 +107,7 @@ export class GetBatteryStatusCommand
       );
     }
 
-    switch (this._statusType) {
+    switch (this.args.statusType) {
       case BatteryStatusType.BATTERY_PERCENTAGE: {
         const percentage = parser.extract8BitUint();
         if (!percentage) {
@@ -148,7 +149,7 @@ export class GetBatteryStatusCommand
         };
       }
       default:
-        this._exhaustiveMatchingGuard(this._statusType);
+        this._exhaustiveMatchingGuard(this.args.statusType);
     }
   }
 

diff --git a/packages/core/src/api/command/os/GetOsVersionCommand.ts b/packages/core/src/api/command/os/GetOsVersionCommand.ts
@@ -64,6 +64,8 @@ export type GetOsVersionResponse = {
  * Command to get information about the device firmware.
  */
 export class GetOsVersionCommand implements Command<GetOsVersionResponse> {
+  args = undefined;
+
   getApdu(): Apdu {
     const getOsVersionApduArgs: ApduBuilderArgs = {
       cla: 0xe0,

diff --git a/packages/core/src/api/command/os/OpenAppCommand.test.ts b/packages/core/src/api/command/os/OpenAppCommand.test.ts
@@ -3,18 +3,12 @@ import { ApduResponse } from "@api/device-session/ApduResponse";
 import { OpenAppCommand } from "./OpenAppCommand";
 
 describe("OpenAppCommand", () => {
-  let openAppCommand: OpenAppCommand;
-
-  beforeEach(() => {
-    openAppCommand = new OpenAppCommand();
-  });
-
+  const appName = "MyApp";
   it("should return the correct APDU for opening an application", () => {
-    const appName = "MyApp";
     const expectedApdu = Uint8Array.from([
       0xe0, 0xd8, 0x00, 0x00, 0x05, 0x4d, 0x79, 0x41, 0x70, 0x70,
     ]);
-    const apdu = openAppCommand.getApdu(appName);
+    const apdu = new OpenAppCommand({ appName }).getApdu();
     expect(apdu.getRawApdu()).toStrictEqual(expectedApdu);
   });
 
@@ -23,14 +17,18 @@ describe("OpenAppCommand", () => {
       statusCode: new Uint8Array([0x90, 0x00]),
       data: new Uint8Array([]),
     });
-    expect(() => openAppCommand.parseResponse(apduResponse)).not.toThrow();
+    expect(() =>
+      new OpenAppCommand({ appName }).parseResponse(apduResponse),
+    ).not.toThrow();
   });
 
   it("should throw error when command is unsuccessful", () => {
     const apduResponse: ApduResponse = new ApduResponse({
       statusCode: new Uint8Array([0x6a, 0x81]),
       data: new Uint8Array([]),
     });
-    expect(() => openAppCommand.parseResponse(apduResponse)).toThrow();
+    expect(() =>
+      new OpenAppCommand({ appName }).parseResponse(apduResponse),
+    ).toThrow();
   });
 });
diff --git a/packages/core/src/api/command/os/OpenAppCommand.ts b/packages/core/src/api/command/os/OpenAppCommand.ts
@@ -6,19 +6,29 @@ import { InvalidStatusWordError } from "@api/command/Errors";
 import { CommandUtils } from "@api/command/utils/CommandUtils";
 import { ApduResponse } from "@api/device-session/ApduResponse";
 
+type Arguments = {
+  appName: string;
+};
+
 /**
  * The command to open an application on the device.
  */
-export class OpenAppCommand implements Command<void, string> {
-  getApdu(appName: string): Apdu {
+export class OpenAppCommand implements Command<void, Arguments> {
+  args: Arguments;
+
+  constructor(args: Arguments) {
+    this.args = args;
+  }
+
+  getApdu(): Apdu {
     const openAppApduArgs: ApduBuilderArgs = {
       cla: 0xe0,
       ins: 0xd8,
       p1: 0x00,
       p2: 0x00,
     } as const;
     return new ApduBuilder(openAppApduArgs)
-      .addAsciiStringToData(appName)
+      .addAsciiStringToData(this.args.appName)
       .build();
   }
 

diff --git a/packages/core/src/api/command/use-case/SendCommandUseCase.test.ts b/packages/core/src/api/command/use-case/SendCommandUseCase.test.ts
@@ -19,6 +19,7 @@ describe("SendCommandUseCase", () => {
     logger = new DefaultLoggerPublisherService([], "send-command-use-case");
     sessionService = new DefaultDeviceSessionService(() => logger);
     command = {
+      args: undefined,
       getApdu: jest.fn(),
       parseResponse: jest.fn(),
     };

diff --git a/packages/core/src/api/command/use-case/SendCommandUseCase.ts b/packages/core/src/api/command/use-case/SendCommandUseCase.ts
@@ -43,13 +43,11 @@ export class SendCommandUseCase {
    *
    * @param sessionId - The device session id.
    * @param command - The command to send.
-   * @param params - The parameters of the command.
    * @returns The response from the command.
    */
   async execute<Response, Args = void>({
     sessionId,
     command,
-    params,
   }: SendCommandUseCaseArgs<Response, Args>): Promise<Response> {
     const deviceSessionOrError =
       this._sessionService.getDeviceSessionById(sessionId);
@@ -59,7 +57,7 @@ export class SendCommandUseCase {
       Right: async (deviceSession) => {
         const deviceModelId = deviceSession.connectedDevice.deviceModel.id;
         const action = deviceSession.sendCommand<Response, Args>(command);
-        return await action(deviceModelId, params);
+        return await action(deviceModelId);
       },
       // Case device session not found
       Left: (error) => {

diff --git a/packages/core/src/internal/device-session/model/DeviceSession.ts b/packages/core/src/internal/device-session/model/DeviceSession.ts
@@ -98,11 +98,8 @@ export class DeviceSession {
   }
 
   sendCommand<Response, Args>(command: Command<Response, Args>) {
-    return async (
-      deviceModelId: DeviceModelId,
-      getApduArgs: Args,
-    ): Promise<Response> => {
-      const apdu = command.getApdu(getApduArgs);
+    return async (deviceModelId: DeviceModelId): Promise<Response> => {
+      const apdu = command.getApdu();
       const response = await this.sendApdu(apdu.getRawApdu());
 
       return response.caseOf({