From d3ca9d574706200991f3dda79fd10a0c579d9562 Mon Sep 17 00:00:00 2001 From: Son Nguyen Date: Sat, 17 Dec 2022 22:50:39 +0700 Subject: [PATCH 1/6] [@dhealthdapps/backend] refactor: add static factory method to PaginatedResultDTO --- .../src/common/models/PaginatedResultDTO.ts | 17 +++++++++++++++++ .../backend/src/common/services/QueryService.ts | 14 ++------------ .../src/discovery/routes/AccountsController.ts | 2 +- .../src/discovery/routes/AssetsController.ts | 4 ++-- .../discovery/routes/TransactionsController.ts | 2 +- .../src/payout/routes/PayoutsController.ts | 4 ++-- .../processor/routes/OperationsController.ts | 2 +- .../statistics/routes/LeaderboardsController.ts | 2 +- .../src/statistics/routes/UsersController.ts | 2 +- .../src/users/routes/ActivitiesController.ts | 4 ++-- .../common/models/PaginatedResultDTO.spec.ts | 16 ++++++++++++++++ 11 files changed, 46 insertions(+), 23 deletions(-) diff --git a/runtime/backend/src/common/models/PaginatedResultDTO.ts b/runtime/backend/src/common/models/PaginatedResultDTO.ts index 4d8d1036..acdce6e5 100644 --- a/runtime/backend/src/common/models/PaginatedResultDTO.ts +++ b/runtime/backend/src/common/models/PaginatedResultDTO.ts @@ -98,4 +98,21 @@ export class PaginatedResultDTO { (this.pagination.total ?? 0) ); } + + /** + * Method to dynamically create and return an instance of this class + * with the provided data type, data content and pagination content. + * + * @access public + * @static + * @param {TData[]} data The results returned by the query in an array of template type `TData`. + * @param {Pageable & Countable} pagination The pagination object, contains properties such as `pageSize`, `pageNumber` and `total`. + * @returns {PaginatedResultDTO} + */ + public static create( + data: TData[], + pagination: Pageable & Countable, + ): PaginatedResultDTO { + return new PaginatedResultDTO(data, pagination); + } } diff --git a/runtime/backend/src/common/services/QueryService.ts b/runtime/backend/src/common/services/QueryService.ts index 5c890ce3..94c4cce3 100644 --- a/runtime/backend/src/common/services/QueryService.ts +++ b/runtime/backend/src/common/services/QueryService.ts @@ -192,12 +192,7 @@ export class QueryService< }; // returns wrapped entity page - // @todo move to static factory - const result: PaginatedResultDTO = - new PaginatedResultDTO(); - result.data = data; - result.pagination = pagination; - return result; + return PaginatedResultDTO.create(data, pagination); } /** @@ -410,12 +405,7 @@ export class QueryService< T4thDocument; // returns wrapped entity page - // @todo move to static factory - const result: PaginatedResultDTO = - new PaginatedResultDTO(); - result.data = data; - result.pagination = pagination; - return result; + return PaginatedResultDTO.create(data, pagination); } /** diff --git a/runtime/backend/src/discovery/routes/AccountsController.ts b/runtime/backend/src/discovery/routes/AccountsController.ts index 225b3ff9..bfb39a85 100644 --- a/runtime/backend/src/discovery/routes/AccountsController.ts +++ b/runtime/backend/src/discovery/routes/AccountsController.ts @@ -117,7 +117,7 @@ export class AccountsController { ); // wraps for transport - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: AccountDocument) => Account.fillDTO(d, new AccountDTO()), ), diff --git a/runtime/backend/src/discovery/routes/AssetsController.ts b/runtime/backend/src/discovery/routes/AssetsController.ts index 1818305d..ee869f8e 100644 --- a/runtime/backend/src/discovery/routes/AssetsController.ts +++ b/runtime/backend/src/discovery/routes/AssetsController.ts @@ -144,7 +144,7 @@ export class AssetsController { ); // wraps for transport - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: AssetDocument) => Asset.fillDTO(d, new AssetDTO())), data.pagination, ); @@ -198,7 +198,7 @@ export class AssetsController { const data = await this.assetsService.find(safeQuery); // wraps for transport using AssetDTO - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: AssetDocument) => Asset.fillDTO(d, new AssetDTO())), data.pagination, ); diff --git a/runtime/backend/src/discovery/routes/TransactionsController.ts b/runtime/backend/src/discovery/routes/TransactionsController.ts index a75b3b23..231eb67e 100644 --- a/runtime/backend/src/discovery/routes/TransactionsController.ts +++ b/runtime/backend/src/discovery/routes/TransactionsController.ts @@ -114,7 +114,7 @@ export class TransactionsController { ); // wraps for transport - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: TransactionDocument) => Transaction.fillDTO(d, new TransactionDTO()), ), diff --git a/runtime/backend/src/payout/routes/PayoutsController.ts b/runtime/backend/src/payout/routes/PayoutsController.ts index d8a3bcfc..bf3fd53a 100644 --- a/runtime/backend/src/payout/routes/PayoutsController.ts +++ b/runtime/backend/src/payout/routes/PayoutsController.ts @@ -144,7 +144,7 @@ export class PayoutsController { ); // wraps for transport - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: PayoutDocument) => Payout.fillDTO(d, new PayoutDTO())), data.pagination, ); @@ -198,7 +198,7 @@ export class PayoutsController { const data = await this.payoutsService.find(safeQuery); // wraps for transport using PayoutDTO - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: PayoutDocument) => Payout.fillDTO(d, new PayoutDTO())), data.pagination, ); diff --git a/runtime/backend/src/processor/routes/OperationsController.ts b/runtime/backend/src/processor/routes/OperationsController.ts index 7c5f22d9..1d9e1382 100644 --- a/runtime/backend/src/processor/routes/OperationsController.ts +++ b/runtime/backend/src/processor/routes/OperationsController.ts @@ -114,7 +114,7 @@ export class OperationsController { ); // wraps for transport - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: OperationDocument) => Operation.fillDTO(d, new OperationDTO()), ), diff --git a/runtime/backend/src/statistics/routes/LeaderboardsController.ts b/runtime/backend/src/statistics/routes/LeaderboardsController.ts index 74adc530..dbc1c29c 100644 --- a/runtime/backend/src/statistics/routes/LeaderboardsController.ts +++ b/runtime/backend/src/statistics/routes/LeaderboardsController.ts @@ -147,7 +147,7 @@ export class LeaderboardsController { ); // wraps for transport - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: StatisticsDocument) => { return Statistics.fillDTO(d, new StatisticsDTO()); }), diff --git a/runtime/backend/src/statistics/routes/UsersController.ts b/runtime/backend/src/statistics/routes/UsersController.ts index 97966cb5..72bd143c 100644 --- a/runtime/backend/src/statistics/routes/UsersController.ts +++ b/runtime/backend/src/statistics/routes/UsersController.ts @@ -135,7 +135,7 @@ export class UsersController { const data = await this.statisticsService.find(safeQuery); // wraps for transport using StatisticsDTO - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: StatisticsDocument) => Statistics.fillDTO(d, new StatisticsDTO()), ), diff --git a/runtime/backend/src/users/routes/ActivitiesController.ts b/runtime/backend/src/users/routes/ActivitiesController.ts index 9bbb4295..e73295c7 100644 --- a/runtime/backend/src/users/routes/ActivitiesController.ts +++ b/runtime/backend/src/users/routes/ActivitiesController.ts @@ -148,7 +148,7 @@ export class ActivitiesController { ); // wraps for transport - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: ActivityDocument) => Activity.fillDTO(d, new ActivityDTO()), ), @@ -204,7 +204,7 @@ export class ActivitiesController { const data = await this.activitiesService.find(safeQuery); // wraps for transport using ActivityDTO - return new PaginatedResultDTO( + return PaginatedResultDTO.create( data.data.map((d: ActivityDocument) => Activity.fillDTO(d, new ActivityDTO()), ), diff --git a/runtime/backend/tests/unit/common/models/PaginatedResultDTO.spec.ts b/runtime/backend/tests/unit/common/models/PaginatedResultDTO.spec.ts index 8d7dbe36..76d82029 100644 --- a/runtime/backend/tests/unit/common/models/PaginatedResultDTO.spec.ts +++ b/runtime/backend/tests/unit/common/models/PaginatedResultDTO.spec.ts @@ -31,6 +31,22 @@ describe("common/PaginatedResultDTO", () => { expect(paginatedResultDto.pagination.pageNumber).toEqual(1); }); + it("should create with default data value", () => { + // prepare + const pagination = { + pageNumber: -1, + pageSize: -1, + total: 1, + }; + + // act + const paginatedResultDto = new PaginatedResultDTO(undefined, pagination); + + // assert + expect(paginatedResultDto.data).toEqual([]); + expect(paginatedResultDto.pagination.pageNumber).toEqual(1); + }); + it("should create with default pagination values", () => { // prepare const data = ["data1", "data2"]; From 277305e5b232994524587b4fe9e7d316a14296d2 Mon Sep 17 00:00:00 2001 From: Son Nguyen Date: Sat, 17 Dec 2022 22:56:03 +0700 Subject: [PATCH 2/6] [@dhealthdapps/backend] refactor(common): return state data's hash instead of the actual content in StateDTO --- runtime/backend/src/common/models/StateDTO.ts | 18 +++++++----------- .../backend/src/common/models/StateSchema.ts | 4 +++- .../unit/common/models/StateSchema.spec.ts | 8 +++++++- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/runtime/backend/src/common/models/StateDTO.ts b/runtime/backend/src/common/models/StateDTO.ts index 35e33c88..7e0e28bc 100644 --- a/runtime/backend/src/common/models/StateDTO.ts +++ b/runtime/backend/src/common/models/StateDTO.ts @@ -11,7 +11,6 @@ import { ApiProperty } from "@nestjs/swagger"; // internal dependencies -import { StateData } from "./StateData"; import { BaseDTO } from "./BaseDTO"; /** @@ -47,22 +46,19 @@ export class StateDTO extends BaseDTO { public name: string; /** - * Contains the actual state cache data. This field is usually - * populated or updated within a service class. + * Contains the actual state cache data's hash. *

- * This field can hold **any** type of information as it extends - * the `Record` type to permit greater flexibility - * around state cache entries. + * The content is hashed with SHA3-256 to prevent revealing actual + * state data to the client. * - * @todo We probably don't want this data to be as public, instead should be the "cache hash". * @access public * @var {string} */ @ApiProperty({ - type: Object, - example: { stateKey1: 1, stateKey2: "value2", stateKey3: true }, + type: "string", + example: "", description: - "Contains the actual state cache data. This field is usually populated or updated within a service class.", + "Contains the actual state cache data's hash. The content is hashed with SHA3-256 to prevent revealing actual state data to the client.", }) - public data: StateData; + public data: string; } diff --git a/runtime/backend/src/common/models/StateSchema.ts b/runtime/backend/src/common/models/StateSchema.ts index da4ffa37..eff9d1ef 100644 --- a/runtime/backend/src/common/models/StateSchema.ts +++ b/runtime/backend/src/common/models/StateSchema.ts @@ -10,6 +10,7 @@ // external dependencies import { Prop, Schema, SchemaFactory } from "@nestjs/mongoose"; import { Model } from "mongoose"; +import { sha3_256 } from "js-sha3"; // internal dependencies import { Documentable } from "../concerns/Documentable"; @@ -134,7 +135,8 @@ export class State extends Transferable { */ public static fillDTO(doc: StateDocument, dto: StateDTO): StateDTO { dto.name = doc.name; - dto.data = doc.data; + // hash the actual state data in the dto to prevent its exposure to the client. + dto.data = sha3_256(JSON.stringify(doc.data)); return dto; } } diff --git a/runtime/backend/tests/unit/common/models/StateSchema.spec.ts b/runtime/backend/tests/unit/common/models/StateSchema.spec.ts index 5af3b748..6d64a3ec 100644 --- a/runtime/backend/tests/unit/common/models/StateSchema.spec.ts +++ b/runtime/backend/tests/unit/common/models/StateSchema.spec.ts @@ -7,6 +7,11 @@ * @author dHealth Network * @license LGPL-3.0 */ +const sha3_256Call = jest.fn().mockReturnValue("hashed-content"); +jest.mock("js-sha3", () => ({ + sha3_256: sha3_256Call +})); + // internal dependencies import { StateDTO } from "../../../../src/common/models/StateDTO"; import { State, StateDocument } from "../../../../src/common/models/StateSchema"; @@ -31,7 +36,7 @@ describe("common/StateSchema", () => { it("should return correct instance", () => { // prepare const name = "test-name"; - const data = { key: "value" }; + const data: any = "hashed-content"; const state = new State(); (state as any).name = name; (state as any).data = data; @@ -41,6 +46,7 @@ describe("common/StateSchema", () => { const result = State.fillDTO(state as StateDocument, new StateDTO()); // assert + expect(sha3_256Call).toHaveBeenCalledTimes(1); expect(result).toEqual(expectedResult); }); }); From 3ce6723a36d52e5b6175cc091977c7bbb68cf87d Mon Sep 17 00:00:00 2001 From: Son Nguyen Date: Sat, 17 Dec 2022 23:00:55 +0700 Subject: [PATCH 3/6] [@dhealthdapps/backend] refactor(common): decouple LogModule from notifier scope's dependency --- runtime/backend/src/common/Schedulers.ts | 6 +++++- runtime/backend/src/common/modules/LogModule.ts | 4 ---- runtime/backend/tests/unit/common/ScopeFactory.spec.ts | 5 +++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/runtime/backend/src/common/Schedulers.ts b/runtime/backend/src/common/Schedulers.ts index 15b90951..3d094e37 100644 --- a/runtime/backend/src/common/Schedulers.ts +++ b/runtime/backend/src/common/Schedulers.ts @@ -43,6 +43,7 @@ import { UserAggregationCommand } from "../statistics/schedulers/UserAggregation import { UserTopActivitiesCommand } from "../statistics/schedulers/UserTopActivities/UserTopActivitiesCommand"; // notifier scope +import { AlertsModule } from "../notifier/modules/AlertsModule"; import { ReportNotifierCommand } from "../notifier/schedulers/ReportNotifier/ReportNotifierCommand"; /** @@ -102,7 +103,10 @@ export const Schedulers: { [key: string]: any[] } = { UserAggregationCommand, UserTopActivitiesCommand, ], - notifier: [ReportNotifierCommand.register()], + notifier: [ + AlertsModule, + ReportNotifierCommand.register() + ], oauth: [], users: [], }; diff --git a/runtime/backend/src/common/modules/LogModule.ts b/runtime/backend/src/common/modules/LogModule.ts index 42fbf2ea..b4eea2b4 100644 --- a/runtime/backend/src/common/modules/LogModule.ts +++ b/runtime/backend/src/common/modules/LogModule.ts @@ -16,9 +16,6 @@ import { QueryModule } from "../modules/QueryModule"; import { Log, LogSchema } from "../models/LogSchema"; import { LogService } from "../services/LogService"; -// @todo decouple from notifier scope -import { AlertsModule } from "../../notifier/modules/AlertsModule"; - /** * @class LogModule * @description The main definition for the Log module. @@ -33,7 +30,6 @@ import { AlertsModule } from "../../notifier/modules/AlertsModule"; schema: LogSchema, }, // requirement from LogModule ]), - AlertsModule, QueryModule, // requirement from LogService ], providers: [LogService], diff --git a/runtime/backend/tests/unit/common/ScopeFactory.spec.ts b/runtime/backend/tests/unit/common/ScopeFactory.spec.ts index ae60e0a5..d846bf03 100644 --- a/runtime/backend/tests/unit/common/ScopeFactory.spec.ts +++ b/runtime/backend/tests/unit/common/ScopeFactory.spec.ts @@ -143,6 +143,11 @@ jest.mock("../../../src/notifier/NotifierModule", () => { return { NotifierModule: NotifierModuleMock }; }); +const AlertsModuleMock: any = jest.fn(); +jest.mock("../../../src/notifier/modules/AlertsModule", () => { + return { AlertsModule: AlertsModuleMock }; +}); + // oauth scope const OAuthModuleMock: any = jest.fn(); jest.mock("../../../src/oauth/OAuthModule", () => { From f3fd7181eb76d72148eefad317dce2482a91b14b Mon Sep 17 00:00:00 2001 From: Son Nguyen Date: Sat, 17 Dec 2022 23:02:19 +0700 Subject: [PATCH 4/6] [@dhealthdapps/backend] refactor(common): remove obsolete todo tasks --- runtime/backend/src/common/models/AccountDTO.ts | 1 - runtime/backend/src/common/models/BaseDTO.ts | 1 - runtime/backend/src/common/models/DappConfig.ts | 1 - runtime/backend/src/common/models/TransactionSchema.ts | 1 - 4 files changed, 4 deletions(-) diff --git a/runtime/backend/src/common/models/AccountDTO.ts b/runtime/backend/src/common/models/AccountDTO.ts index ba44ded3..c596c4c3 100644 --- a/runtime/backend/src/common/models/AccountDTO.ts +++ b/runtime/backend/src/common/models/AccountDTO.ts @@ -22,7 +22,6 @@ import { BaseDTO } from "./BaseDTO"; * This class shall be used in **HTTP responses** to avoid any additional * data about accounts to be revealed. * - * @todo The transaction timestamp in `firstTransactionAt` should probably be a **number** to avoid timezone issues. * @since v0.1.0 */ export class AccountDTO extends BaseDTO { diff --git a/runtime/backend/src/common/models/BaseDTO.ts b/runtime/backend/src/common/models/BaseDTO.ts index 4fced6f6..2c3fe49e 100644 --- a/runtime/backend/src/common/models/BaseDTO.ts +++ b/runtime/backend/src/common/models/BaseDTO.ts @@ -16,7 +16,6 @@ * This class shall be used in **HTTP responses** to avoid any additional * data about accounts to be revealed. * - * @todo Currently this is using a "to any"-cast trick that should be avoided. * @since v0.3.0 */ export class BaseDTO { diff --git a/runtime/backend/src/common/models/DappConfig.ts b/runtime/backend/src/common/models/DappConfig.ts index 0890dd1a..597116e3 100644 --- a/runtime/backend/src/common/models/DappConfig.ts +++ b/runtime/backend/src/common/models/DappConfig.ts @@ -51,7 +51,6 @@ export type AppConnectionPayload = { * This interface is mainly used **internally** to restrict the configuration * values provided to some modules or services and methods. * - * @todo Allow for updated discovery sources configuration (must be backwards compatible). * @link DappConfig * @since v0.1.0 */ diff --git a/runtime/backend/src/common/models/TransactionSchema.ts b/runtime/backend/src/common/models/TransactionSchema.ts index 5cc1b63f..bd9dff91 100644 --- a/runtime/backend/src/common/models/TransactionSchema.ts +++ b/runtime/backend/src/common/models/TransactionSchema.ts @@ -32,7 +32,6 @@ import { TransactionDTO } from "../../discovery/models/TransactionDTO"; * Note that this class uses the generic {@link Transferable} trait to * enable a `toDTO()` method on the model. * - * @todo Timestamp fields should be **numbers** to avoid timezone issues. * @since v0.2.0 */ @Schema({ From 8ce28788c6ab017f2787a912a6c0a0402e6d4424 Mon Sep 17 00:00:00 2001 From: Son Nguyen Date: Sat, 17 Dec 2022 23:04:05 +0700 Subject: [PATCH 5/6] [@dhealthdapps/backend] test: update unit tests --- .../unit/payout/services/MathService.spec.ts | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/runtime/backend/tests/unit/payout/services/MathService.spec.ts b/runtime/backend/tests/unit/payout/services/MathService.spec.ts index ec097260..fbe36d68 100644 --- a/runtime/backend/tests/unit/payout/services/MathService.spec.ts +++ b/runtime/backend/tests/unit/payout/services/MathService.spec.ts @@ -16,22 +16,30 @@ describe("payout/MathService", () => { beforeEach(() => { mathService = new MathService(); + }); + + afterEach(() => { + jest.clearAllMocks() }) describe("skewNormal()", () => { it("should return correct result if skewness is not defined", () => { - // prepare - const [mean, deviation] = [1, 2]; - const getRandomVariatesCall = jest - .spyOn((mathService as any), "getRandomVariates") - .mockReturnValue([1,1]); + const expectedResullts = [3, -4]; + [1, -2.5].forEach((u0: number, index: number) => { + // prepare + jest.clearAllMocks(); + const [mean, deviation] = [1, 2]; + const getRandomVariatesCall = jest + .spyOn((mathService as any), "getRandomVariates") + .mockReturnValue([u0, 1]); - // act - const result = mathService.skewNormal(mean, deviation); + // act + const result = mathService.skewNormal(mean, deviation); - // assert - expect(getRandomVariatesCall).toHaveBeenCalledTimes(1); - expect(result).toBe(3); + // assert + expect(getRandomVariatesCall).toHaveBeenCalledTimes(1); + expect(result).toBe(expectedResullts[index]); + }); }); it("should return correct result if skewness is defined", () => { @@ -54,5 +62,30 @@ describe("payout/MathService", () => { expect(mathSqrtCall).toHaveBeenNthCalledWith(2, 0); expect(result).toBe(7); }); + + it("should return correct result if first random variate number is negative", () => { + const expectedResullts = [0, 3]; + [-1, -2].forEach((u0: number, index: number) => { + // prepare + jest.clearAllMocks(); + const [mean, deviation, skewness] = [1, 2, 3]; + const getRandomVariatesCall = jest + .spyOn((mathService as any), "getRandomVariates") + .mockReturnValue([u0, 1]); + const mathSqrtCall = jest + .spyOn(Math, "sqrt") + .mockReturnValue(2); + + // act + const result = mathService.skewNormal(mean, deviation, skewness); + + // assert + expect(getRandomVariatesCall).toHaveBeenCalledTimes(1); + expect(mathSqrtCall).toHaveBeenCalledTimes(2); + expect(mathSqrtCall).toHaveBeenNthCalledWith(1, 1 + skewness * skewness); + expect(mathSqrtCall).toHaveBeenNthCalledWith(2, 1 - (skewness / 2) * (skewness/ 2)); + expect(result).toBe(expectedResullts[index]); + }); + }); }); }); \ No newline at end of file From 8f89a9566d687fdfeb72909cc60aa590eba817d3 Mon Sep 17 00:00:00 2001 From: Son Nguyen Date: Sat, 17 Dec 2022 23:04:36 +0700 Subject: [PATCH 6/6] [@dhealthdapps/backend] fix: linter --- runtime/backend/src/common/Schedulers.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/runtime/backend/src/common/Schedulers.ts b/runtime/backend/src/common/Schedulers.ts index 3d094e37..9299f450 100644 --- a/runtime/backend/src/common/Schedulers.ts +++ b/runtime/backend/src/common/Schedulers.ts @@ -103,10 +103,7 @@ export const Schedulers: { [key: string]: any[] } = { UserAggregationCommand, UserTopActivitiesCommand, ], - notifier: [ - AlertsModule, - ReportNotifierCommand.register() - ], + notifier: [AlertsModule, ReportNotifierCommand.register()], oauth: [], users: [], };