fix: fix NonJoinedBatchFetch edge cases

wise-team · Feb 7, 2019 · 620441b · 620441b
1 parent b05568b
commit 620441b
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 27 deletions.
diff --git 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
@@ -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<UnifiedSteemTransaction[] | undefined> {
-        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;
     }