Skip to content

Commit

Permalink
✨ (core) [DSDK-319]: Improvements from feedbacks (#93)
Browse files Browse the repository at this point in the history
  • Loading branch information
jiyuzhuang authored Jun 11, 2024
2 parents 5444952 + d710fac commit 10d7bfd
Show file tree
Hide file tree
Showing 19 changed files with 121 additions and 58 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:

danger:
name: Run Danger check
if: ${{ !contains(github.ref_name, 'dependencies') }}
if: ${{ github.actor != 'dependabot[bot]' }}
runs-on: [ubuntu-latest]
steps:
- uses: actions/checkout@v4
Expand Down
28 changes: 13 additions & 15 deletions packages/core/src/api/DeviceSdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,25 @@ import {
import { ApduResponse } from "@api/device-session/ApduResponse";
import { DeviceSessionState } from "@api/device-session/DeviceSessionState";
import { DeviceSessionId } from "@api/device-session/types";
import {
ConnectUseCaseArgs,
DisconnectUseCaseArgs,
DiscoveredDevice,
SendApduUseCaseArgs,
} from "@api/types";
import { ConnectedDevice } from "@api/usb/model/ConnectedDevice";
import { configTypes } from "@internal/config/di/configTypes";
import { GetSdkVersionUseCase } from "@internal/config/use-case/GetSdkVersionUseCase";
import { deviceSessionTypes } from "@internal/device-session/di/deviceSessionTypes";
import { GetDeviceSessionStateUseCase } from "@internal/device-session/use-case/GetDeviceSessionStateUseCase";
import { discoveryTypes } from "@internal/discovery/di/discoveryTypes";
import {
ConnectUseCase,
ConnectUseCaseArgs,
} from "@internal/discovery/use-case/ConnectUseCase";
import {
DisconnectUseCase,
DisconnectUseCaseArgs,
} from "@internal/discovery/use-case/DisconnectUseCase";
import { ConnectUseCase } from "@internal/discovery/use-case/ConnectUseCase";
import { DisconnectUseCase } from "@internal/discovery/use-case/DisconnectUseCase";
import type { StartDiscoveringUseCase } from "@internal/discovery/use-case/StartDiscoveringUseCase";
import type { StopDiscoveringUseCase } from "@internal/discovery/use-case/StopDiscoveringUseCase";
import { sendTypes } from "@internal/send/di/sendTypes";
import {
SendApduUseCase,
SendApduUseCaseArgs,
} from "@internal/send/use-case/SendApduUseCase";
import { SendApduUseCase } from "@internal/send/use-case/SendApduUseCase";
import { usbDiTypes } from "@internal/usb/di/usbDiTypes";
import { DiscoveredDevice } from "@internal/usb/model/DiscoveredDevice";
import {
GetConnectedDeviceUseCase,
GetConnectedDeviceUseCaseArgs,
Expand Down Expand Up @@ -127,10 +123,12 @@ export class DeviceSdk {
/**
* Sends a command to a device through a device session.
*
* @param {SendCommandUseCaseArgs<T, U>} - The device session ID, command and command parameters to send.
* @param {SendCommandUseCaseArgs<Response, Args>} args - The device session ID, command and command parameters to send.
* @returns A promise resolving with the response from the command.
*/
sendCommand<T, U>(args: SendCommandUseCaseArgs<T, U>): Promise<T> {
sendCommand<Response, Args>(
args: SendCommandUseCaseArgs<Response, Args>,
): Promise<Response> {
return this.container
.get<SendCommandUseCase>(commandTypes.SendCommandUseCase)
.execute(args);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/api/apdu/model/Apdu.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Represents an APDU command that can be sent to a device.
* An APDU command that can be sent to a device.
* DO NOT USE THIS CLASS DIRECTLY, use ApduBuilder instead.
*/
export class Apdu {
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/api/command/Command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ import { ApduResponse } from "@api/device-session/ApduResponse";
/**
* A command that can be sent to a device.
*
* @template T - The type of the response returned by the device.
* @template U - The type of the arguments passed to the command (optional).
* @template Response - The type of the response returned by the device.
* @template Args - The type of the arguments passed to the command (optional).
*/
export interface Command<T, U = void> {
export interface Command<Response, Args = void> {
/**
* 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?: U): Apdu;
getApdu(args?: Args): Apdu;

/**
* Parses the response received from the device.
Expand All @@ -27,5 +27,5 @@ export interface Command<T, U = void> {
parseResponse(
apduResponse: ApduResponse,
deviceModelId: DeviceModelId | void,
): T;
): Response;
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe("SendCommandUseCase", () => {
const useCase = new SendCommandUseCase(sessionService, () => logger);

jest
.spyOn(deviceSession, "getCommand")
.spyOn(deviceSession, "sendCommand")
.mockReturnValue(async () => Promise.resolve({ status: "success" }));

const response = await useCase.execute<{ status: string }>({
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/api/command/use-case/SendCommandUseCase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ import type { DeviceSessionService } from "@internal/device-session/service/Devi
import { loggerTypes } from "@internal/logger-publisher/di/loggerTypes";
import { LoggerPublisherService } from "@internal/logger-publisher/service/LoggerPublisherService";

export type SendCommandUseCaseArgs<T, U = void> = {
export type SendCommandUseCaseArgs<Response, Args = void> = {
/**
* The device session id.
*/
sessionId: string;
/**
* The command to send.
*/
command: Command<T, U>;
command: Command<Response, Args>;
/**
* The parameters of the command.
*/
params: U;
params: Args;
};

/**
Expand Down Expand Up @@ -46,19 +46,19 @@ export class SendCommandUseCase {
* @param params - The parameters of the command.
* @returns The response from the command.
*/
async execute<T, U = void>({
async execute<Response, Args = void>({
sessionId,
command,
params,
}: SendCommandUseCaseArgs<T, U>): Promise<T> {
}: SendCommandUseCaseArgs<Response, Args>): Promise<Response> {
const deviceSessionOrError =
this._sessionService.getDeviceSessionById(sessionId);

return deviceSessionOrError.caseOf({
// Case device session found
Right: async (deviceSession) => {
const deviceModelId = deviceSession.connectedDevice.deviceModel.id;
const action = deviceSession.getCommand<T, U>(command);
const action = deviceSession.sendCommand<Response, Args>(command);
return await action(deviceModelId, params);
},
// Case device session not found
Expand Down
20 changes: 15 additions & 5 deletions packages/core/src/api/device/DeviceModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,20 @@ export enum DeviceModelId {
*/
export type DeviceId = string;

export type DeviceModelArgs = {
id: DeviceId;
model: DeviceModelId;
name: string;
};

export class DeviceModel {
constructor(
public id: DeviceId,
public model: DeviceModelId,
public name: string,
) {}
public id: DeviceId;
public model: DeviceModelId;
public name: string;

constructor({ id, model, name }: DeviceModelArgs) {
this.id = id;
this.model = model;
this.name = name;
}
}
5 changes: 5 additions & 0 deletions packages/core/src/api/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
export type { DeviceId } from "./device/DeviceModel";
export type { ConnectionType } from "./discovery/ConnectionType";
export type { LogSubscriberOptions } from "./logger-subscriber/model/LogSubscriberOptions";
export type { DiscoveredDevice } from "./usb/model/DiscoveredDevice";
export type { DeviceModelId } from "@api/device/DeviceModel";
export type { DeviceSessionId } from "@api/device-session/types";
export type { ConnectUseCaseArgs } from "@internal/discovery/use-case/ConnectUseCase";
export type { DisconnectUseCaseArgs } from "@internal/discovery/use-case/DisconnectUseCase";
export type { SendApduUseCaseArgs } from "@internal/send/use-case/SendApduUseCase";
9 changes: 9 additions & 0 deletions packages/core/src/api/usb/model/DiscoveredDevice.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { DeviceId, DeviceModel } from "@api/device/DeviceModel";

/**
* A discovered device.
*/
export type DiscoveredDevice = {
id: DeviceId;
deviceModel: DeviceModel;
};
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,11 @@ export class DeviceSession {
});
}

getCommand<T, U>(command: Command<T, U>) {
return async (deviceModelId: DeviceModelId, getApduArgs: U): Promise<T> => {
sendCommand<Response, Args>(command: Command<Response, Args>) {
return async (
deviceModelId: DeviceModelId,
getApduArgs: Args,
): Promise<Response> => {
const apdu = command.getApdu(getApduArgs);
const response = await this.sendApdu(apdu.getRawApdu());

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { inject, injectable } from "inversify";

import { DeviceId } from "@api/device/DeviceModel";
import { DeviceSessionId } from "@api/device-session/types";
import { DeviceId } from "@api/types";
import { deviceSessionTypes } from "@internal/device-session/di/deviceSessionTypes";
import { DeviceSession } from "@internal/device-session/model/DeviceSession";
import type { DeviceSessionService } from "@internal/device-session/service/DeviceSessionService";
Expand All @@ -11,9 +11,12 @@ import { usbDiTypes } from "@internal/usb/di/usbDiTypes";
import type { UsbHidTransport } from "@internal/usb/transport/UsbHidTransport";
import type { DisconnectHandler } from "@internal/usb/transport/WebUsbHidTransport";

/**
* The arguments for the ConnectUseCase.
*/
export type ConnectUseCaseArgs = {
/**
* UUID of the device obtained through device discovery `StartDiscoveringUseCase`
* UUID of the device got from device discovery `StartDiscoveringUseCase`
*/
deviceId: DeviceId;
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import { inject, injectable } from "inversify";

import { DeviceSessionId } from "@api/device-session/types";
import type { DeviceSessionId } from "@api/types";
import { deviceSessionTypes } from "@internal/device-session/di/deviceSessionTypes";
import type { DeviceSessionService } from "@internal/device-session/service/DeviceSessionService";
import { loggerTypes } from "@internal/logger-publisher/di/loggerTypes";
import { LoggerPublisherService } from "@internal/logger-publisher/service/LoggerPublisherService";
import { usbDiTypes } from "@internal/usb/di/usbDiTypes";
import type { UsbHidTransport } from "@internal/usb/transport/UsbHidTransport";

/**
* The arguments for the DisconnectUseCase.
*/
export type DisconnectUseCaseArgs = {
/**
* Device session identifier from `DeviceSdk.connect`.
*/
sessionId: DeviceSessionId;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { of } from "rxjs";

import { DeviceModel } from "@api/index";
import { DeviceModelId, DiscoveredDevice } from "@api/types";
import { DeviceModelDataSource } from "@internal/device-model/data/DeviceModelDataSource";
import { InternalDeviceModel } from "@internal/device-model/model/DeviceModel";
import { DefaultLoggerPublisherService } from "@internal/logger-publisher/service/DefaultLoggerPublisherService";
import { LoggerPublisherService } from "@internal/logger-publisher/service/LoggerPublisherService";
import { DiscoveredDevice } from "@internal/usb/model/DiscoveredDevice";
import { InternalDiscoveredDevice } from "@internal/usb/model/InternalDiscoveredDevice";
import { usbHidDeviceConnectionFactoryStubBuilder } from "@internal/usb/service/UsbHidDeviceConnectionFactory.stub";
import { WebUsbHidTransport } from "@internal/usb/transport/WebUsbHidTransport";

Expand All @@ -14,9 +16,12 @@ let transport: WebUsbHidTransport;
let logger: LoggerPublisherService;

describe("StartDiscoveringUseCase", () => {
const stubDiscoveredDevice: DiscoveredDevice = {
id: "",
deviceModel: {} as InternalDeviceModel,
const stubDiscoveredDevice: InternalDiscoveredDevice = {
id: "internal-discovered-device-id",
deviceModel: {
id: "nanoSP" as DeviceModelId,
productName: "productName",
} as InternalDeviceModel,
};
const tag = "logger-tag";

Expand Down Expand Up @@ -47,7 +52,14 @@ describe("StartDiscoveringUseCase", () => {
expect(mockedStartDiscovering).toHaveBeenCalled();
discover.subscribe({
next: (discoveredDevice) => {
expect(discoveredDevice).toBe(stubDiscoveredDevice);
expect(discoveredDevice).toStrictEqual({
id: "internal-discovered-device-id",
deviceModel: new DeviceModel({
id: "internal-discovered-device-id",
model: "nanoSP" as DeviceModelId,
name: "productName",
}),
} as DiscoveredDevice);
done();
},
error: (error) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { inject, injectable } from "inversify";
import { Observable } from "rxjs";
import { map, Observable } from "rxjs";

import { DeviceModel } from "@api/index";
import { DiscoveredDevice } from "@api/types";
import { usbDiTypes } from "@internal/usb/di/usbDiTypes";
import { DiscoveredDevice } from "@internal/usb/model/DiscoveredDevice";
import { InternalDiscoveredDevice } from "@internal/usb/model/InternalDiscoveredDevice";
import type { UsbHidTransport } from "@internal/usb/transport/UsbHidTransport";

/**
Expand All @@ -19,6 +21,18 @@ export class StartDiscoveringUseCase {
) {}

execute(): Observable<DiscoveredDevice> {
return this.usbHidTransport.startDiscovering();
return this.usbHidTransport.startDiscovering().pipe(
map((data: InternalDiscoveredDevice) => {
const deviceModel = new DeviceModel({
id: data.id,
model: data.deviceModel.id,
name: data.deviceModel.productName,
});
return {
id: data.id,
deviceModel,
};
}),
);
}
}
9 changes: 6 additions & 3 deletions packages/core/src/internal/send/use-case/SendApduUseCase.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
import { inject, injectable } from "inversify";

import { ApduResponse } from "@api/device-session/ApduResponse";
import { DeviceSessionId } from "@api/device-session/types";
import { DeviceSessionId } from "@api/types";
import { deviceSessionTypes } from "@internal/device-session/di/deviceSessionTypes";
import type { DeviceSessionService } from "@internal/device-session/service/DeviceSessionService";
import { loggerTypes } from "@internal/logger-publisher/di/loggerTypes";
import { LoggerPublisherService } from "@internal/logger-publisher/service/LoggerPublisherService";

/**
* The arguments for the SendApduUseCase.
*/
export type SendApduUseCaseArgs = {
/**
* Device session identifier obtained through `DeviceSdk.connect`
* Device session identifier from `DeviceSdk.connect`.
*/
sessionId: DeviceSessionId;
/**
* APDU to send to the device
* Raw APDU to send to the device.
*/
apdu: Uint8Array;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { InternalDeviceModel } from "@internal/device-model/model/DeviceModel";
import { SendApduFnType } from "@internal/usb/transport/DeviceConnection";

/**
* Represents an internal connected device.
* The internal connected device.
*/
export type ConnectedDeviceConstructorArgs = {
id: DeviceId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { DeviceId } from "@api/device/DeviceModel";
import { InternalDeviceModel } from "@internal/device-model/model/DeviceModel";

/**
* Represents a discovered/scanned (not yet connected to) device.
* A discovered / scanned (not yet connected to) device.
*/
export type DiscoveredDevice = {
export type InternalDiscoveredDevice = {
// type: "web-hid", // "node-hid" in the future -> no need as we will only have 1 USB transport implementation running

/**
Expand Down
Loading

0 comments on commit 10d7bfd

Please sign in to comment.