From 245b3c8d8e9ae60f7fe92a7598923882de04ece5 Mon Sep 17 00:00:00 2001 From: Piotr Szlachciak Date: Fri, 22 Mar 2024 14:01:45 +0100 Subject: [PATCH] Make from inclusive --- .../uif-example/src/blocks/BlockIndexer.ts | 7 +- .../uif-example/src/prices/PriceIndexer.ts | 13 ++-- packages/uif/src/Indexer.ts | 24 +++---- .../src/indexers/multi/MultiIndexer.test.ts | 29 +++++--- .../uif/src/indexers/multi/MultiIndexer.ts | 66 +++++++++---------- 5 files changed, 75 insertions(+), 64 deletions(-) diff --git a/packages/uif-example/src/blocks/BlockIndexer.ts b/packages/uif-example/src/blocks/BlockIndexer.ts index 80ac9410..d0641cf0 100644 --- a/packages/uif-example/src/blocks/BlockIndexer.ts +++ b/packages/uif-example/src/blocks/BlockIndexer.ts @@ -29,14 +29,13 @@ export class BlockIndexer extends ChildIndexer { await this.blockIndexerRepository.saveHeight(height) } - override async update(currentHeight: number): Promise { - const nextHeight = currentHeight + 1 - const timestamp = nextHeight * ONE_HOUR_MS + override async update(from: number): Promise { + const timestamp = from * ONE_HOUR_MS const block = await this.blockService.getBlockNumberBefore(timestamp) await this.blockRepository.save({ number: block, timestamp }) - return nextHeight + return from } override async invalidate(targetHeight: number): Promise { diff --git a/packages/uif-example/src/prices/PriceIndexer.ts b/packages/uif-example/src/prices/PriceIndexer.ts index 467d3026..80a7e98f 100644 --- a/packages/uif-example/src/prices/PriceIndexer.ts +++ b/packages/uif-example/src/prices/PriceIndexer.ts @@ -37,18 +37,17 @@ export class PriceIndexer extends MultiIndexer { } override async multiUpdate( - currentHeight: number, - targetHeight: number, + from: number, + to: number, configurations: UpdateConfiguration[], ): Promise { - const startHour = currentHeight + 1 // we only query 24 hours at a time - const endHour = Math.min(targetHeight, startHour + 23) + const adjustedTo = Math.min(to, from + 23) const prices = await this.priceService.getHourlyPrices( this.apiId, - startHour * ONE_HOUR_MS, - endHour * ONE_HOUR_MS, + from * ONE_HOUR_MS, + adjustedTo * ONE_HOUR_MS, ) const dataToSave = configurations @@ -64,7 +63,7 @@ export class PriceIndexer extends MultiIndexer { }) await this.priceRepository.save(dataToSave) - return endHour + return adjustedTo } override async removeData( diff --git a/packages/uif/src/Indexer.ts b/packages/uif/src/Indexer.ts index 9ef664df..93248688 100644 --- a/packages/uif/src/Indexer.ts +++ b/packages/uif/src/Indexer.ts @@ -68,23 +68,23 @@ export abstract class Indexer { * Implements the main data fetching process. It is up to the indexer to * decide how much data to fetch. For example given `.update(100, 200)`, the * indexer can only fetch data up to 110 and return 110. The next time this - * method will be called with `.update(110, 200)`. + * method will be called with `.update(111, 200)`. * - * @param currentHeight The height that the indexer has synced up to - * previously. This value is exclusive so the indexer should not fetch data - * for this height. + * @param from The height for which the indexer should start syncing data. + * This value is inclusive. * - * @param targetHeight The height that the indexer should sync up to. This value is - * inclusive so the indexer should eventually fetch data for this height. + * @param to The height at which the indexer should end syncing data. This + * value is also inclusive so the indexer should eventually sync data for this + * height. * * @returns The height that the indexer has synced up to. Returning - * `currentHeight` means that the indexer has not synced any data. Returning - * a value greater than `currentHeight` means that the indexer has synced up - * to that height. Returning a value less than `currentHeight` will trigger + * `from` means that the indexer has synced a single data point. Returning + * a value greater than `from` means that the indexer has synced up + * to that height. Returning a value less than `from` will trigger * invalidation down to the returned value. Returning a value greater than - * `targetHeight` is not permitted. + * `to` is not permitted. */ - abstract update(currentHeight: number, targetHeight: number): Promise + abstract update(from: number, to: number): Promise /** * Responsible for invalidating data that was synced previously. It is @@ -216,7 +216,7 @@ export abstract class Indexer { // #region Child methods private async executeUpdate(effect: UpdateEffect): Promise { - const from = this.state.height + const from = this.state.height + 1 this.logger.info('Updating', { from, to: effect.targetHeight }) try { const newHeight = await this.update(from, effect.targetHeight) diff --git a/packages/uif/src/indexers/multi/MultiIndexer.test.ts b/packages/uif/src/indexers/multi/MultiIndexer.test.ts index 1536a89e..3b8ce0a2 100644 --- a/packages/uif/src/indexers/multi/MultiIndexer.test.ts +++ b/packages/uif/src/indexers/multi/MultiIndexer.test.ts @@ -36,6 +36,19 @@ describe(MultiIndexer.name, () => { expect(testIndexer.removeData).not.toHaveBeenCalled() expect(testIndexer.saveConfigurations).not.toHaveBeenCalled() }) + + it('no synced data', async () => { + const testIndexer = new TestMultiIndexer( + [actual('a', 100, 400), actual('b', 200, 500)], + [], + ) + + const newHeight = await testIndexer.initialize() + expect(newHeight).toEqual(99) + + expect(testIndexer.removeData).not.toHaveBeenCalled() + expect(testIndexer.saveConfigurations).not.toHaveBeenCalled() + }) }) describe(MultiIndexer.prototype.update.name, () => { @@ -103,10 +116,10 @@ describe(MultiIndexer.name, () => { ) await testIndexer.initialize() - const newHeight = await testIndexer.update(300, 600) + const newHeight = await testIndexer.update(301, 600) expect(newHeight).toEqual(400) - expect(testIndexer.multiUpdate).toHaveBeenOnlyCalledWith(300, 400, [ + expect(testIndexer.multiUpdate).toHaveBeenOnlyCalledWith(301, 400, [ update('a', 100, 400, false), update('b', 200, 500, false), ]) @@ -137,7 +150,7 @@ describe(MultiIndexer.name, () => { ) await testIndexer.initialize() - const newHeight = await testIndexer.update(400, 500) + const newHeight = await testIndexer.update(401, 500) expect(newHeight).toEqual(500) expect(testIndexer.multiUpdate).not.toHaveBeenCalled() @@ -151,7 +164,7 @@ describe(MultiIndexer.name, () => { ) await testIndexer.initialize() - const newHeight = await testIndexer.update(200, 500) + const newHeight = await testIndexer.update(201, 500) expect(newHeight).toEqual(299) expect(testIndexer.multiUpdate).not.toHaveBeenCalled() @@ -208,8 +221,8 @@ describe(MultiIndexer.name, () => { ]) // Next range - expect(await testIndexer.update(200, 500)).toEqual(400) - expect(testIndexer.multiUpdate).toHaveBeenNthCalledWith(3, 200, 400, [ + expect(await testIndexer.update(201, 500)).toEqual(400) + expect(testIndexer.multiUpdate).toHaveBeenNthCalledWith(3, 201, 400, [ update('b', 100, 400, false), ]) expect(testIndexer.saveConfigurations).toHaveBeenNthCalledWith(3, [ @@ -278,7 +291,7 @@ describe(MultiIndexer.name, () => { testIndexer.multiUpdate.resolvesTo(150) await expect(testIndexer.update(200, 300)).toBeRejectedWith( - /returned height must be between currentHeight and targetHeight/, + /returned height must be between from and to/, ) }) @@ -292,7 +305,7 @@ describe(MultiIndexer.name, () => { testIndexer.multiUpdate.resolvesTo(350) await expect(testIndexer.update(200, 300)).toBeRejectedWith( - /returned height must be between currentHeight and targetHeight/, + /returned height must be between from and to/, ) }) }) diff --git a/packages/uif/src/indexers/multi/MultiIndexer.ts b/packages/uif/src/indexers/multi/MultiIndexer.ts index efaf3668..4b6631fb 100644 --- a/packages/uif/src/indexers/multi/MultiIndexer.ts +++ b/packages/uif/src/indexers/multi/MultiIndexer.ts @@ -43,29 +43,29 @@ export abstract class MultiIndexer extends ChildIndexer { * indexer can only fetch data up to 110 and return 110. The next time this * method will be called with `.update(110, 200, [...])`. * - * @param currentHeight The height that the indexer has synced up to - * previously. This value is exclusive so the indexer should not fetch data - * for this height. If the indexer hasn't synced anything previously this - * will equal the minimum height of all configurations - 1. + * @param from The height for which the indexer should start syncing data. + * This value is inclusive. If the indexer hasn't synced anything previously + * this will equal the minimum height of all configurations. * - * @param targetHeight The height that the indexer should sync up to. This value is - * inclusive so the indexer should eventually fetch data for this height. + * @param to The height at which the indexer should end syncing data. This + * value is also inclusive so the indexer should eventually sync data for this + * height. * * @param configurations The configurations that the indexer should use to * sync data. The configurations are guaranteed to be in the range of - * `currentHeight` and `targetHeight`. Some of those configurations might - * have been synced previously for this range. Those configurations - * will include the `hasData` flag set to `true`. + * `from` and `to`. Some of those configurations might have been synced + * previously for this range. Those configurations will include the `hasData` + * flag set to `true`. * * @returns The height that the indexer has synced up to. Returning - * `currentHeight` means that the indexer has not synced any data. Returning - * a value greater than `currentHeight` means that the indexer has synced up - * to that height. Returning a value less than `currentHeight` or greater than - * `targetHeight` is not permitted. + * `from` means that the indexer has synced a single data point. Returning + * a value greater than `from` means that the indexer has synced up + * to that height. Returning a value less than `from` or greater than + * `to` is not permitted. */ abstract multiUpdate( - currentHeight: number, - targetHeight: number, + from: number, + to: number, configurations: UpdateConfiguration[], ): Promise @@ -106,31 +106,32 @@ export abstract class MultiIndexer extends ChildIndexer { return safeHeight } - async update(currentHeight: number, targetHeight: number): Promise { - const range = findRange(this.ranges, currentHeight) + async update(from: number, to: number): Promise { + const range = findRange(this.ranges, from) if (range.configurations.length === 0) { - return Math.min(range.to, targetHeight) + return Math.min(range.to, to) } const { configurations, minCurrentHeight } = getConfigurationsInRange( range, this.saved, - currentHeight, + from, ) - const minTargetHeight = Math.min(range.to, targetHeight, minCurrentHeight) - - const newHeight = await this.multiUpdate( - currentHeight, - minTargetHeight, - configurations, - ) - if (newHeight < currentHeight || newHeight > minTargetHeight) { + const adjustedTo = Math.min(range.to, to, minCurrentHeight) + + this.logger.info('Calling multiUpdate', { + from, + to: adjustedTo, + configurations: configurations.length, + }) + const newHeight = await this.multiUpdate(from, adjustedTo, configurations) + if (newHeight < from || newHeight > adjustedTo) { throw new Error( - 'Programmer error, returned height must be between currentHeight and targetHeight.', + 'Programmer error, returned height must be between from and to (both inclusive).', ) } - if (newHeight > currentHeight) { + if (newHeight > from) { updateSavedConfigurations(this.saved, configurations, newHeight) await this.saveConfigurations(this.saved) } @@ -147,13 +148,12 @@ export abstract class MultiIndexer extends ChildIndexer { } } +// TODO: test this function! function findRange( ranges: ConfigurationRange[], - currentHeight: number, + from: number, ): ConfigurationRange { - const range = ranges.find( - (range) => range.from <= currentHeight + 1 && range.to > currentHeight, - ) + const range = ranges.find((range) => range.from <= from && range.to >= from) if (!range) { throw new Error('Programmer error, there should always be a range') }