From 71689c1497728569fc6a07e21fa7bdba68c1bac0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Mon, 6 Nov 2023 15:38:28 +0100 Subject: [PATCH] fix(syncing-server): return cursor token upon transfer limit breached (#902) * fix(syncing-server): return cursor token upon transfer limit breached * fix e2e env vars --- .github/ci.env | 2 ++ .github/workflows/e2e-home-server.yml | 1 + .../Item/ItemTransferCalculator.spec.ts | 36 ++++++++++++------- .../src/Domain/Item/ItemTransferCalculator.ts | 11 ++++-- .../Item/ItemTransferCalculatorInterface.ts | 2 +- .../UseCase/Syncing/GetItems/GetItems.spec.ts | 4 ++- .../UseCase/Syncing/GetItems/GetItems.ts | 16 ++++++--- 7 files changed, 50 insertions(+), 22 deletions(-) diff --git a/.github/ci.env b/.github/ci.env index 9541a5c6c..c1aced2e8 100644 --- a/.github/ci.env +++ b/.github/ci.env @@ -26,3 +26,5 @@ MYSQL_ROOT_PASSWORD=changeme123 AUTH_JWT_SECRET=f95259c5e441f5a4646d76422cfb3df4c4488842901aa50b6c51b8be2e0040e9 AUTH_SERVER_ENCRYPTION_SERVER_KEY=1087415dfde3093797f9a7ca93a49e7d7aa1861735eb0d32aae9c303b8c3d060 VALET_TOKEN_SECRET=4b886819ebe1e908077c6cae96311b48a8416bd60cc91c03060e15bdf6b30d1f + +SYNCING_SERVER_CONTENT_SIZE_TRANSFER_LIMIT=1000000 diff --git a/.github/workflows/e2e-home-server.yml b/.github/workflows/e2e-home-server.yml index 09fc51cc7..9b456bb08 100644 --- a/.github/workflows/e2e-home-server.yml +++ b/.github/workflows/e2e-home-server.yml @@ -70,6 +70,7 @@ jobs: echo "ACCESS_TOKEN_AGE=4" >> packages/home-server/.env echo "REFRESH_TOKEN_AGE=10" >> packages/home-server/.env echo "REVISIONS_FREQUENCY=2" >> packages/home-server/.env + echo "CONTENT_SIZE_TRANSFER_LIMIT=1000000" >> packages/home-server/.env echo "DB_HOST=localhost" >> packages/home-server/.env echo "DB_PORT=3306" >> packages/home-server/.env echo "DB_DATABASE=standardnotes" >> packages/home-server/.env diff --git a/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.spec.ts b/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.spec.ts index d03de3a5a..44926e6a0 100644 --- a/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.spec.ts +++ b/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.spec.ts @@ -25,11 +25,14 @@ describe('ItemTransferCalculator', () => { const result = await createCalculator().computeItemUuidsToFetch(itemContentSizeDescriptors, 50) - expect(result).toEqual([ - '00000000-0000-0000-0000-000000000000', - '00000000-0000-0000-0000-000000000001', - '00000000-0000-0000-0000-000000000002', - ]) + expect(result).toEqual({ + uuids: [ + '00000000-0000-0000-0000-000000000000', + '00000000-0000-0000-0000-000000000001', + '00000000-0000-0000-0000-000000000002', + ], + transferLimitBreachedBeforeEndOfItems: false, + }) }) it('should compute uuids to fetch based on transfer limit - exact limit fit', async () => { @@ -41,7 +44,10 @@ describe('ItemTransferCalculator', () => { const result = await createCalculator().computeItemUuidsToFetch(itemContentSizeDescriptors, 40) - expect(result).toEqual(['00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-000000000001']) + expect(result).toEqual({ + uuids: ['00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-000000000001'], + transferLimitBreachedBeforeEndOfItems: true, + }) }) it('should compute uuids to fetch based on transfer limit - content size not defined on an item', async () => { @@ -53,11 +59,14 @@ describe('ItemTransferCalculator', () => { const result = await createCalculator().computeItemUuidsToFetch(itemContentSizeDescriptors, 50) - expect(result).toEqual([ - '00000000-0000-0000-0000-000000000000', - '00000000-0000-0000-0000-000000000001', - '00000000-0000-0000-0000-000000000002', - ]) + expect(result).toEqual({ + uuids: [ + '00000000-0000-0000-0000-000000000000', + '00000000-0000-0000-0000-000000000001', + '00000000-0000-0000-0000-000000000002', + ], + transferLimitBreachedBeforeEndOfItems: false, + }) }) it('should compute uuids to fetch based on transfer limit - first item over the limit', async () => { @@ -69,7 +78,10 @@ describe('ItemTransferCalculator', () => { const result = await createCalculator().computeItemUuidsToFetch(itemContentSizeDescriptors, 40) - expect(result).toEqual(['00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-000000000001']) + expect(result).toEqual({ + uuids: ['00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-000000000001'], + transferLimitBreachedBeforeEndOfItems: true, + }) }) }) diff --git a/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.ts b/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.ts index 8d677ef23..a32e82cea 100644 --- a/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.ts +++ b/packages/syncing-server/src/Domain/Item/ItemTransferCalculator.ts @@ -9,16 +9,17 @@ export class ItemTransferCalculator implements ItemTransferCalculatorInterface { async computeItemUuidsToFetch( itemContentSizeDescriptors: ItemContentSizeDescriptor[], bytesTransferLimit: number, - ): Promise> { + ): Promise<{ uuids: Array; transferLimitBreachedBeforeEndOfItems: boolean }> { const itemUuidsToFetch = [] let totalContentSizeInBytes = 0 + let transferLimitBreached = false for (const itemContentSize of itemContentSizeDescriptors) { const contentSize = itemContentSize.props.contentSize ?? 0 itemUuidsToFetch.push(itemContentSize.props.uuid.value) totalContentSizeInBytes += contentSize - const transferLimitBreached = this.isTransferLimitBreached({ + transferLimitBreached = this.isTransferLimitBreached({ totalContentSizeInBytes, bytesTransferLimit, itemUuidsToFetch, @@ -30,7 +31,11 @@ export class ItemTransferCalculator implements ItemTransferCalculatorInterface { } } - return itemUuidsToFetch + return { + uuids: itemUuidsToFetch, + transferLimitBreachedBeforeEndOfItems: + transferLimitBreached && itemUuidsToFetch.length < itemContentSizeDescriptors.length, + } } async computeItemUuidBundlesToFetch( diff --git a/packages/syncing-server/src/Domain/Item/ItemTransferCalculatorInterface.ts b/packages/syncing-server/src/Domain/Item/ItemTransferCalculatorInterface.ts index 405a2493e..195e166b3 100644 --- a/packages/syncing-server/src/Domain/Item/ItemTransferCalculatorInterface.ts +++ b/packages/syncing-server/src/Domain/Item/ItemTransferCalculatorInterface.ts @@ -4,7 +4,7 @@ export interface ItemTransferCalculatorInterface { computeItemUuidsToFetch( itemContentSizeDescriptors: ItemContentSizeDescriptor[], bytesTransferLimit: number, - ): Promise> + ): Promise<{ uuids: Array; transferLimitBreachedBeforeEndOfItems: boolean }> computeItemUuidBundlesToFetch( itemContentSizeDescriptors: ItemContentSizeDescriptor[], bytesTransferLimit: number, diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.spec.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.spec.ts index 66c5cd069..d3a73234c 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.spec.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.spec.ts @@ -49,7 +49,9 @@ describe('GetItems', () => { .mockResolvedValue([ItemContentSizeDescriptor.create('00000000-0000-0000-0000-000000000000', 20).getValue()]) itemTransferCalculator = {} as jest.Mocked - itemTransferCalculator.computeItemUuidsToFetch = jest.fn().mockResolvedValue(['item-uuid']) + itemTransferCalculator.computeItemUuidsToFetch = jest + .fn() + .mockResolvedValue({ uuids: ['item-uuid'], transferLimitBreachedBeforeEndOfItems: false }) timer = {} as jest.Mocked timer.getTimestampInMicroseconds = jest.fn().mockReturnValue(123) diff --git a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.ts b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.ts index 0561dca04..7b1c2b401 100644 --- a/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.ts +++ b/packages/syncing-server/src/Domain/UseCase/Syncing/GetItems/GetItems.ts @@ -60,22 +60,22 @@ export class GetItems implements UseCaseInterface { } const itemContentSizeDescriptors = await this.itemRepository.findContentSizeForComputingTransferLimit(itemQuery) - const itemUuidsToFetch = await this.itemTransferCalculator.computeItemUuidsToFetch( + const { uuids, transferLimitBreachedBeforeEndOfItems } = await this.itemTransferCalculator.computeItemUuidsToFetch( itemContentSizeDescriptors, this.contentSizeTransferLimit, ) let items: Array = [] - if (itemUuidsToFetch.length > 0) { + if (uuids.length > 0) { items = await this.itemRepository.findAll({ - uuids: itemUuidsToFetch, + uuids, sortBy: 'updated_at_timestamp', sortOrder: 'ASC', }) } - const totalItemsCount = await this.itemRepository.countAll(itemQuery) let cursorToken = undefined - if (totalItemsCount > upperBoundLimit) { + const thereAreStillMoreItemsToFetch = await this.stillMoreItemsToFetch(itemQuery, upperBoundLimit) + if (transferLimitBreachedBeforeEndOfItems || thereAreStillMoreItemsToFetch) { const lastSyncTime = items[items.length - 1].props.timestamps.updatedAt / Time.MicrosecondsInASecond cursorToken = Buffer.from(`${this.SYNC_TOKEN_VERSION}:${lastSyncTime}`, 'utf-8').toString('base64') } @@ -87,6 +87,12 @@ export class GetItems implements UseCaseInterface { }) } + private async stillMoreItemsToFetch(itemQuery: ItemQuery, upperBoundLimit: number): Promise { + const totalItemsCount = await this.itemRepository.countAll(itemQuery) + + return totalItemsCount > upperBoundLimit + } + private getLastSyncTime(dto: GetItemsDTO): Result { let token = dto.syncToken if (dto.cursorToken !== undefined && dto.cursorToken !== null) {