From 620441bf1a31f9ff9761eb7883b0af19cdb670e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Lewandowski?= Date: Thu, 7 Feb 2019 17:23:32 +0100 Subject: [PATCH] fix: fix NonJoinedBatchFetch edge cases --- .../NonJoinedBatchFetch.spec.test.ts | 62 ++++++++++++------- src/accounthistory/NonJoinedBatchFetch.ts | 19 ++++-- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/accounthistory/NonJoinedBatchFetch.spec.test.ts b/src/accounthistory/NonJoinedBatchFetch.spec.test.ts index 2df5fdf..1ecd66b 100644 --- a/src/accounthistory/NonJoinedBatchFetch.spec.test.ts +++ b/src/accounthistory/NonJoinedBatchFetch.spec.test.ts @@ -55,7 +55,7 @@ describe("NonJoinedBatchFetch", function() { it("queries with correct batchSize", async () => { const batchSize = _.random(10, 1000); const { batchFetch, getAccountHistoryAsyncSpy, account } = prepare({ - accountHistoryLength: _.random(0, 999), + accountHistoryLength: _.random(1, 999), batchSize, }); @@ -66,26 +66,46 @@ describe("NonJoinedBatchFetch", function() { expect(getAccountHistoryAsyncSpy.firstCall.args).to.deep.equal([account, -1, realBatchSize]); }); - it("query batches does not overlap", async () => { - const batchSize = _.random(50, 60); - const numBatches = _.random(5, 10); - const { account, batchFetch, getAccountHistoryAsyncSpy, fakeAccountHistoryOps } = prepare({ - accountHistoryLength: batchSize * (numBatches - 1) + _.random(1, 5), - batchSize, - }); - - let finished = false; - while (!finished) { - finished = !(await batchFetch.getNextBatch()); - } - - let sum = 0; - for (const call of getAccountHistoryAsyncSpy.getCalls()) { - sum += (await call.returnValue).length; - } - expect(sum).to.be.equal(fakeAccountHistoryOps.length); - expect(getAccountHistoryAsyncSpy.callCount).to.be.equal(numBatches); - }); + const baseBatchSize = _.random(40, 60); + const numOfFullBatches = _.random(5, 10); + [ + { + name: "Query batches does not overlap when account history length is multiplication of batch size", + accountHistoryLength: baseBatchSize * numOfFullBatches, // + 0 + numOfBatches: numOfFullBatches, + }, + { + name: "Query batches does not overlap when account history length is multiplication of batch size plus 1", + accountHistoryLength: baseBatchSize * numOfFullBatches + 1, + numOfBatches: numOfFullBatches + 1, + }, + { + name: "Query batches does not overlap when account history length is multiplication of batch size minus 1", + accountHistoryLength: baseBatchSize * numOfFullBatches - 1, + numOfBatches: numOfFullBatches, + }, + ].forEach(test => + it(test.name, async () => { + const accountHistoryLength = test.accountHistoryLength; + const { batchFetch, getAccountHistoryAsyncSpy, fakeAccountHistoryOps } = prepare({ + accountHistoryLength, + batchSize: baseBatchSize, + }); + + let finished = false; + while (!finished) { + finished = !(await batchFetch.getNextBatch()); + } + + let sum = 0; + for (const call of getAccountHistoryAsyncSpy.getCalls()) { + sum += (await call.returnValue).length; + } + expect(fakeAccountHistoryOps.length).to.be.equal(accountHistoryLength); + expect(sum, "sum").to.be.equal(fakeAccountHistoryOps.length); + expect(getAccountHistoryAsyncSpy.callCount).to.be.equal(test.numOfBatches); + }), + ); it("does not loop endlessly when accountHistoryLength is a multiplication of batchSize", async () => { const batchSize = Math.floor(Math.random() * 1000); diff --git a/src/accounthistory/NonJoinedBatchFetch.ts b/src/accounthistory/NonJoinedBatchFetch.ts index 7551923..dc6408f 100644 --- a/src/accounthistory/NonJoinedBatchFetch.ts +++ b/src/accounthistory/NonJoinedBatchFetch.ts @@ -11,6 +11,7 @@ export class NonJoinedBatchFetch { private account: string; private steemAdapter: SteemAdapter; private nextFrom: number = -1; + private hasMore: boolean = true; public constructor(props: { steemAdapter: SteemAdapter; account: string; batchSize: number }) { ow(props.steemAdapter, ow.object.is(o => SteemAdapter.isSteemAdapter(o)).label("steemAdapter")); @@ -28,12 +29,18 @@ export class NonJoinedBatchFetch { } public async getNextBatch(): Promise { - if (this.nextFrom === 0) { + if (!this.hasMore) { return undefined; } const batchRaw = await this.loadFrom(this.nextFrom); - this.nextFrom = this.calculateNextFrom(batchRaw); + + const nextWouldBe = this.calculateNextFrom(batchRaw); + if (typeof nextWouldBe !== "undefined") { + this.nextFrom = nextWouldBe; + } else { + this.hasMore = false; + } const batch = batchRaw.map(op => this.opToTrx(op)); @@ -56,7 +63,6 @@ export class NonJoinedBatchFetch { batchLimit, exclusiveBatchSize: this.exclusiveBatchSize, resultLength: result.length, - nextFrom: this.nextFrom, range: [this.getIndexOfOp(result[0]), this.getIndexOfOp(result[result.length - 1])], }); @@ -75,15 +81,16 @@ export class NonJoinedBatchFetch { return await this.steemAdapter.getAccountHistoryAsync(this.account, from, batchLimit); } - private calculateNextFrom(batch: steem.AccountHistory.Operation[]): number { + private calculateNextFrom(batch: steem.AccountHistory.Operation[]): number | undefined { const accountHistoryDoesNotHaveMoreOperations = batch.length < this.exclusiveBatchSize; if (accountHistoryDoesNotHaveMoreOperations) { - return 0; + return undefined; } const oldestOp = batch[batch.length - 1]; const oldestOpIndex = this.getIndexOfOp(oldestOp); - const nextBatchFrom = Math.max(oldestOpIndex - 1, 0); + const nextBatchWouldHaveIndex = oldestOpIndex - 1; + const nextBatchFrom = nextBatchWouldHaveIndex < 0 ? undefined : nextBatchWouldHaveIndex; return nextBatchFrom; }