From 4d1e2dec264b156a4cfb4980ca3b486433ce64b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Tue, 11 Jul 2023 17:05:45 +0200 Subject: [PATCH] fix: unify use case usage (#654) --- .../CheckIntegrity/CheckIntegrity.spec.ts | 151 ++++++++---------- .../Syncing/CheckIntegrity/CheckIntegrity.ts | 11 +- .../CheckIntegrity/CheckIntegrityResponse.ts | 5 - .../UseCase/Syncing/GetItem/GetItem.spec.ts | 24 +-- .../Domain/UseCase/Syncing/GetItem/GetItem.ts | 23 ++- .../Syncing/GetItem/GetItemResponse.ts | 11 -- .../Syncing/SyncItems/SyncItems.spec.ts | 81 +++++----- .../UseCase/Syncing/SyncItems/SyncItems.ts | 8 +- .../UseCase/Syncing/SyncItems/SyncItemsDTO.ts | 2 + .../src/Domain/UseCase/UseCaseInterface.ts | 3 - .../HomeServer/HomeServerItemsController.ts | 24 ++- .../InversifyExpressItemsController.spec.ts | 11 +- .../InversifyExpressItemsController.ts | 5 +- 13 files changed, 168 insertions(+), 191 deletions(-) delete mode 100644 packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrityResponse.ts delete mode 100644 packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItemResponse.ts delete mode 100644 packages/syncing-server/src/Domain/UseCase/UseCaseInterface.ts diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.spec.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.spec.ts index 50a51d916..1bb196f59 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.spec.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.spec.ts @@ -42,95 +42,86 @@ describe('CheckIntegrity', () => { }) it('should return an empty result if there are no integrity mismatches', async () => { - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - freeUser: false, - integrityPayloads: [ - { - uuid: '1-2-3', - updated_at_timestamp: 1, - }, - { - uuid: '2-3-4', - updated_at_timestamp: 2, - }, - { - uuid: '3-4-5', - updated_at_timestamp: 3, - }, - { - uuid: '5-6-7', - updated_at_timestamp: 5, - }, - ], - }), - ).toEqual({ - mismatches: [], - }) - }) - - it('should return a mismatch item that has a different update at timemstap', async () => { - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - freeUser: false, - integrityPayloads: [ - { - uuid: '1-2-3', - updated_at_timestamp: 1, - }, - { - uuid: '2-3-4', - updated_at_timestamp: 1, - }, - { - uuid: '3-4-5', - updated_at_timestamp: 3, - }, - { - uuid: '5-6-7', - updated_at_timestamp: 5, - }, - ], - }), - ).toEqual({ - mismatches: [ + const result = await createUseCase().execute({ + userUuid: '1-2-3', + freeUser: false, + integrityPayloads: [ + { + uuid: '1-2-3', + updated_at_timestamp: 1, + }, { uuid: '2-3-4', updated_at_timestamp: 2, }, - ], - }) - }) - - it('should return a mismatch item that is missing on the client side', async () => { - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - freeUser: false, - integrityPayloads: [ - { - uuid: '1-2-3', - updated_at_timestamp: 1, - }, - { - uuid: '2-3-4', - updated_at_timestamp: 2, - }, - { - uuid: '5-6-7', - updated_at_timestamp: 5, - }, - ], - }), - ).toEqual({ - mismatches: [ { uuid: '3-4-5', updated_at_timestamp: 3, }, + { + uuid: '5-6-7', + updated_at_timestamp: 5, + }, ], }) + expect(result.getValue()).toEqual([]) + }) + + it('should return a mismatch item that has a different update at timemstap', async () => { + const result = await createUseCase().execute({ + userUuid: '1-2-3', + freeUser: false, + integrityPayloads: [ + { + uuid: '1-2-3', + updated_at_timestamp: 1, + }, + { + uuid: '2-3-4', + updated_at_timestamp: 1, + }, + { + uuid: '3-4-5', + updated_at_timestamp: 3, + }, + { + uuid: '5-6-7', + updated_at_timestamp: 5, + }, + ], + }) + expect(result.getValue()).toEqual([ + { + uuid: '2-3-4', + updated_at_timestamp: 2, + }, + ]) + }) + + it('should return a mismatch item that is missing on the client side', async () => { + const result = await createUseCase().execute({ + userUuid: '1-2-3', + freeUser: false, + integrityPayloads: [ + { + uuid: '1-2-3', + updated_at_timestamp: 1, + }, + { + uuid: '2-3-4', + updated_at_timestamp: 2, + }, + { + uuid: '5-6-7', + updated_at_timestamp: 5, + }, + ], + }) + expect(result.getValue()).toEqual([ + { + uuid: '3-4-5', + updated_at_timestamp: 3, + }, + ]) }) }) diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.ts index dd9ce6aa1..49c61cc5f 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrity.ts @@ -1,16 +1,15 @@ import { IntegrityPayload } from '@standardnotes/responses' +import { Result, UseCaseInterface } from '@standardnotes/domain-core' import { ContentType } from '@standardnotes/common' import { ItemRepositoryInterface } from '../../../Item/ItemRepositoryInterface' -import { UseCaseInterface } from '../../UseCaseInterface' import { CheckIntegrityDTO } from './CheckIntegrityDTO' -import { CheckIntegrityResponse } from './CheckIntegrityResponse' import { ExtendedIntegrityPayload } from '../../../Item/ExtendedIntegrityPayload' -export class CheckIntegrity implements UseCaseInterface { +export class CheckIntegrity implements UseCaseInterface { constructor(private itemRepository: ItemRepositoryInterface) {} - async execute(dto: CheckIntegrityDTO): Promise { + async execute(dto: CheckIntegrityDTO): Promise> { const serverItemIntegrityPayloads = await this.itemRepository.findItemsForComputingIntegrityPayloads(dto.userUuid) const serverItemIntegrityPayloadsMap = new Map() @@ -59,8 +58,6 @@ export class CheckIntegrity implements UseCaseInterface { } } - return { - mismatches, - } + return Result.ok(mismatches) } } diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrityResponse.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrityResponse.ts deleted file mode 100644 index 8fc5209ee..000000000 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/CheckIntegrity/CheckIntegrityResponse.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { IntegrityPayload } from '@standardnotes/responses' - -export type CheckIntegrityResponse = { - mismatches: IntegrityPayload[] -} diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.spec.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.spec.ts index 9d1beae3d..144cb2390 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.spec.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.spec.ts @@ -15,23 +15,23 @@ describe('GetItem', () => { }) it('should fail if item is not found', async () => { - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - itemUuid: '2-3-4', - }), - ).toEqual({ success: false, message: 'Could not find item with uuid 2-3-4' }) + const result = await createUseCase().execute({ + userUuid: '1-2-3', + itemUuid: '2-3-4', + }) + expect(result.isFailed()).toBeTruthy() + expect(result.getError()).toEqual('Could not find item with uuid 2-3-4') }) it('should succeed if item is found', async () => { const item = {} as jest.Mocked itemRepository.findByUuidAndUserUuid = jest.fn().mockReturnValue(item) - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - itemUuid: '2-3-4', - }), - ).toEqual({ success: true, item }) + const result = await createUseCase().execute({ + userUuid: '1-2-3', + itemUuid: '2-3-4', + }) + + expect(result.getValue()).toEqual(item) }) }) diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.ts index 7ec77ccd3..f8b56ebfa 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItem.ts @@ -1,24 +1,19 @@ -import { ItemRepositoryInterface } from '../../../Item/ItemRepositoryInterface' -import { UseCaseInterface } from '../../UseCaseInterface' -import { GetItemDTO } from './GetItemDTO' -import { GetItemResponse } from './GetItemResponse' +import { Result, UseCaseInterface } from '@standardnotes/domain-core' -export class GetItem implements UseCaseInterface { +import { ItemRepositoryInterface } from '../../../Item/ItemRepositoryInterface' +import { GetItemDTO } from './GetItemDTO' +import { Item } from '../../../Item/Item' + +export class GetItem implements UseCaseInterface { constructor(private itemRepository: ItemRepositoryInterface) {} - async execute(dto: GetItemDTO): Promise { + async execute(dto: GetItemDTO): Promise> { const item = await this.itemRepository.findByUuidAndUserUuid(dto.itemUuid, dto.userUuid) if (item === null) { - return { - success: false, - message: `Could not find item with uuid ${dto.itemUuid}`, - } + return Result.fail(`Could not find item with uuid ${dto.itemUuid}`) } - return { - success: true, - item, - } + return Result.ok(item) } } diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItemResponse.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItemResponse.ts deleted file mode 100644 index e6b9bbccd..000000000 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItem/GetItemResponse.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Item } from '../../../Item/Item' - -export type GetItemResponse = - | { - success: true - item: Item - } - | { - success: false - message: string - } diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.spec.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.spec.ts index f988041d4..230730f39 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.spec.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.spec.ts @@ -54,20 +54,20 @@ describe('SyncItems', () => { }) it('should sync items', async () => { - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - itemHashes: [itemHash], - computeIntegrityHash: false, - syncToken: 'foo', - cursorToken: 'bar', - limit: 10, - readOnlyAccess: false, - contentType: 'Note', - apiVersion: ApiVersion.v20200115, - sessionUuid: null, - }), - ).toEqual({ + const result = await createUseCase().execute({ + userUuid: '1-2-3', + itemHashes: [itemHash], + computeIntegrityHash: false, + syncToken: 'foo', + cursorToken: 'bar', + limit: 10, + readOnlyAccess: false, + contentType: 'Note', + apiVersion: ApiVersion.v20200115, + sessionUuid: null, + snjsVersion: '1.2.3', + }) + expect(result.getValue()).toEqual({ conflicts: [], cursorToken: 'asdzxc', retrievedItems: [item1], @@ -93,18 +93,18 @@ describe('SyncItems', () => { }) it('should sync items and return items keys on top for first sync', async () => { - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - itemHashes: [itemHash], - computeIntegrityHash: false, - limit: 10, - readOnlyAccess: false, - sessionUuid: '2-3-4', - contentType: 'Note', - apiVersion: ApiVersion.v20200115, - }), - ).toEqual({ + const result = await createUseCase().execute({ + userUuid: '1-2-3', + itemHashes: [itemHash], + computeIntegrityHash: false, + limit: 10, + readOnlyAccess: false, + sessionUuid: '2-3-4', + contentType: 'Note', + apiVersion: ApiVersion.v20200115, + snjsVersion: '1.2.3', + }) + expect(result.getValue()).toEqual({ conflicts: [], cursorToken: 'asdzxc', retrievedItems: [item3, item1], @@ -134,20 +134,21 @@ describe('SyncItems', () => { syncToken: 'qwerty', }) - expect( - await createUseCase().execute({ - userUuid: '1-2-3', - itemHashes: [itemHash], - computeIntegrityHash: false, - syncToken: 'foo', - readOnlyAccess: false, - sessionUuid: '2-3-4', - cursorToken: 'bar', - limit: 10, - contentType: 'Note', - apiVersion: ApiVersion.v20200115, - }), - ).toEqual({ + const result = await createUseCase().execute({ + userUuid: '1-2-3', + itemHashes: [itemHash], + computeIntegrityHash: false, + syncToken: 'foo', + readOnlyAccess: false, + sessionUuid: '2-3-4', + cursorToken: 'bar', + limit: 10, + contentType: 'Note', + apiVersion: ApiVersion.v20200115, + snjsVersion: '1.2.3', + }) + + expect(result.getValue()).toEqual({ conflicts: [ { serverItem: item2, diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.ts index 3fcbd953d..82b96c431 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItems.ts @@ -1,14 +1,14 @@ +import { Result, UseCaseInterface } from '@standardnotes/domain-core' import { Item } from '../../../Item/Item' import { ItemConflict } from '../../../Item/ItemConflict' import { ItemServiceInterface } from '../../../Item/ItemServiceInterface' -import { UseCaseInterface } from '../../UseCaseInterface' import { SyncItemsDTO } from './SyncItemsDTO' import { SyncItemsResponse } from './SyncItemsResponse' -export class SyncItems implements UseCaseInterface { +export class SyncItems implements UseCaseInterface { constructor(private itemService: ItemServiceInterface) {} - async execute(dto: SyncItemsDTO): Promise { + async execute(dto: SyncItemsDTO): Promise> { const getItemsResult = await this.itemService.getItems({ userUuid: dto.userUuid, syncToken: dto.syncToken, @@ -38,7 +38,7 @@ export class SyncItems implements UseCaseInterface { cursorToken: getItemsResult.cursorToken, } - return syncResponse + return Result.ok(syncResponse) } private isFirstSync(dto: SyncItemsDTO): boolean { diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItemsDTO.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItemsDTO.ts index c17497ed5..fd0e0312b 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItemsDTO.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/SyncItems/SyncItemsDTO.ts @@ -5,10 +5,12 @@ export type SyncItemsDTO = { itemHashes: Array computeIntegrityHash: boolean limit: number + sharedVaultUuids?: string[] | null syncToken?: string | null cursorToken?: string | null contentType?: string apiVersion: string + snjsVersion: string readOnlyAccess: boolean sessionUuid: string | null } diff --git a/packages/syncing-server/src/Domain/UseCase/UseCaseInterface.ts b/packages/syncing-server/src/Domain/UseCase/UseCaseInterface.ts deleted file mode 100644 index 7c8405a9a..000000000 --- a/packages/syncing-server/src/Domain/UseCase/UseCaseInterface.ts +++ /dev/null @@ -1,3 +0,0 @@ -export interface UseCaseInterface { - execute(...args: any[]): Promise> -} diff --git a/packages/syncing-server/src/Infra/InversifyExpressUtils/HomeServer/HomeServerItemsController.ts b/packages/syncing-server/src/Infra/InversifyExpressUtils/HomeServer/HomeServerItemsController.ts index f48ffc3d0..db1836890 100644 --- a/packages/syncing-server/src/Infra/InversifyExpressUtils/HomeServer/HomeServerItemsController.ts +++ b/packages/syncing-server/src/Infra/InversifyExpressUtils/HomeServer/HomeServerItemsController.ts @@ -10,6 +10,7 @@ import { ItemProjection } from '../../../Projection/ItemProjection' import { ProjectorInterface } from '../../../Projection/ProjectorInterface' import { ApiVersion } from '../../../Domain/Api/ApiVersion' import { SyncItems } from '../../../Domain/UseCase/Syncing/SyncItems/SyncItems' +import { HttpStatusCode } from '@standardnotes/responses' export class HomeServerItemsController extends BaseHttpController { constructor( @@ -41,16 +42,21 @@ export class HomeServerItemsController extends BaseHttpController { computeIntegrityHash: request.body.compute_integrity === true, syncToken: request.body.sync_token, cursorToken: request.body.cursor_token, + sharedVaultUuids: request.body.shared_vault_uuids, limit: request.body.limit, contentType: request.body.content_type, apiVersion: request.body.api ?? ApiVersion.v20161215, + snjsVersion: request.headers['x-snjs-version'], readOnlyAccess: response.locals.readOnlyAccess, sessionUuid: response.locals.session ? response.locals.session.uuid : null, }) + if (syncResult.isFailed()) { + return this.json({ error: { message: syncResult.getError() } }, HttpStatusCode.BadRequest) + } const syncResponse = await this.syncResponseFactoryResolver .resolveSyncResponseFactoryVersion(request.body.api) - .createResponse(syncResult) + .createResponse(syncResult.getValue()) return this.json(syncResponse) } @@ -67,19 +73,25 @@ export class HomeServerItemsController extends BaseHttpController { freeUser: response.locals.freeUser, }) - return this.json(result) + if (result.isFailed()) { + return this.json({ error: { message: result.getError() } }, HttpStatusCode.BadRequest) + } + + return this.json({ + mismatches: result.getValue(), + }) } - async getSingleItem(request: Request, response: Response): Promise { + async getSingleItem(request: Request, response: Response): Promise { const result = await this.getItem.execute({ userUuid: response.locals.user.uuid, itemUuid: request.params.uuid, }) - if (!result.success) { - return this.notFound() + if (result.isFailed()) { + return this.json({ error: { message: result.getError() } }, 404) } - return this.json({ item: await this.itemProjector.projectFull(result.item) }) + return this.json({ item: await this.itemProjector.projectFull(result.getValue()) }) } } diff --git a/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.spec.ts b/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.spec.ts index 2ce92cf22..66ede6da9 100644 --- a/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.spec.ts +++ b/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.spec.ts @@ -2,9 +2,10 @@ import 'reflect-metadata' import * as express from 'express' import { ContentType } from '@standardnotes/common' +import { Result } from '@standardnotes/domain-core' +import { results } from 'inversify-express-utils' import { InversifyExpressItemsController } from './InversifyExpressItemsController' -import { results } from 'inversify-express-utils' import { Item } from '../../Domain/Item/Item' import { ItemProjection } from '../../Projection/ItemProjection' import { ProjectorInterface } from '../../Projection/ProjectorInterface' @@ -35,13 +36,13 @@ describe('InversifyExpressItemsController', () => { itemProjector.projectFull = jest.fn().mockReturnValue({ foo: 'bar' }) syncItems = {} as jest.Mocked - syncItems.execute = jest.fn().mockReturnValue({ foo: 'bar' }) + syncItems.execute = jest.fn().mockReturnValue(Result.ok({ foo: 'bar' })) checkIntegrity = {} as jest.Mocked - checkIntegrity.execute = jest.fn().mockReturnValue({ mismatches: [{ uuid: '1-2-3', updated_at_timestamp: 2 }] }) + checkIntegrity.execute = jest.fn().mockReturnValue(Result.ok([{ uuid: '1-2-3', updated_at_timestamp: 2 }])) getItem = {} as jest.Mocked - getItem.execute = jest.fn().mockReturnValue({ success: true, item: {} as jest.Mocked }) + getItem.execute = jest.fn().mockReturnValue(Result.ok({} as jest.Mocked)) request = { headers: {}, @@ -100,7 +101,7 @@ describe('InversifyExpressItemsController', () => { it('should return 404 on a missing single item', async () => { request.params.uuid = '1-2-3' - getItem.execute = jest.fn().mockReturnValue({ success: false }) + getItem.execute = jest.fn().mockReturnValue(Result.fail('Oops')) const httpResponse = await createController().getSingleItem(request, response) const result = await httpResponse.executeAsync() diff --git a/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.ts b/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.ts index ea74cb611..b15bead42 100644 --- a/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.ts +++ b/packages/syncing-server/src/Infra/InversifyExpressUtils/InversifyExpressItemsController.ts @@ -36,10 +36,7 @@ export class InversifyExpressItemsController extends HomeServerItemsController { } @httpGet('/:uuid') - override async getSingleItem( - request: Request, - response: Response, - ): Promise { + override async getSingleItem(request: Request, response: Response): Promise { return super.getSingleItem(request, response) } }