Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that if the CachedAsyncIterable is called multiple times in parallel, it does return the correct value #4

Merged
merged 2 commits into from
Oct 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 3 additions & 23 deletions src/cached_async_iterable.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,6 @@ export default class CachedAsyncIterable extends CachedIterable {
}
}

/**
* Synchronous iterator over the cached elements.
*
* Return a generator object implementing the iterator protocol over the
* cached elements of the original (async or sync) iterable.
*/
[Symbol.iterator]() {
const cached = this;
let cur = 0;

return {
next() {
if (cached.length === cur) {
return {value: undefined, done: true};
}
return cached[cur++];
}
};
}

/**
* Asynchronous iterator caching the yielded elements.
*
Expand All @@ -60,7 +40,7 @@ export default class CachedAsyncIterable extends CachedIterable {
return {
async next() {
if (cached.length <= cur) {
cached.push(await cached.iterator.next());
cached.push(cached.iterator.next());
}
return cached[cur++];
}
Expand All @@ -77,10 +57,10 @@ export default class CachedAsyncIterable extends CachedIterable {
let idx = 0;
while (idx++ < count) {
const last = this[this.length - 1];
if (last && last.done) {
if (last && (await last).done) {
break;
}
this.push(await this.iterator.next());
this.push(this.iterator.next());
}
// Return the last cached {value, done} object to allow the calling
// code to decide if it needs to call touchNext again.
Expand Down
71 changes: 69 additions & 2 deletions test/cached_async_iterable_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ suite("CachedAsyncIterable", function() {
});
});

suite("sync iteration over cached elements", function(){
suite.skip("sync iteration over cached elements", function(){
let o1, o2;

suiteSetup(function() {
Expand Down Expand Up @@ -125,7 +125,8 @@ suite("CachedAsyncIterable", function() {

const iterable = new CachedAsyncIterable(generate());
await iterable.touchNext();
assert.deepEqual([...iterable], [o1]);
let x = [...iterable];
assert.deepEqual([...iterable], [o1])
});

test("async iterable with all cached elements", async function() {
Expand Down Expand Up @@ -170,6 +171,47 @@ suite("CachedAsyncIterable", function() {
const first = await toArray(iterable);
assert.deepEqual(await toArray(iterable), first);
});

test("lazy iterable can be called multiple times in parallel", async function() {
let counter = 0;

async function *generate() {
while (true) {
counter++;
yield null;
}
}

// We're testing that if the first call to asyncIterator has been
// made, but the value of it has not been returned yet,
// the consecutive call returns the same Promise rather than,
// attempting to fetch the next item from the iterator.
const iterable = new CachedAsyncIterable(generate());
const [val1, val2] = await Promise.all([
iterable[Symbol.asyncIterator]().next(),
iterable[Symbol.asyncIterator]().next(),
]);
assert.equal(counter, 1);
assert.equal(val1, val2);
});

test("iterable's next can be called multiple times in parallel", async function() {
let counter = 0;

async function *generate() {
while (true) {
counter++;
yield null;
}
}

const iterable = new CachedAsyncIterable(generate());
const iterator = iterable[Symbol.asyncIterator]();
let val1 = await iterator.next();
let val2 = await iterator.next();
assert.equal(counter, 2);
assert.notEqual(val1, val2);
});
});

suite("async touchNext", function(){
Expand Down Expand Up @@ -273,5 +315,30 @@ suite("CachedAsyncIterable", function() {
await iterable.touchNext(),
{value: undefined, done: true});
});

test("touchNext can be called multiple times in parallel", async function() {
let counter = 0;

async function *generate() {
let value = 5;
while (value-- > 0) {
counter++;
yield await Promise.resolve(value);
}
}

// We're testing that if the first call to asyncIterator has been
// made, but the value of it has not been returned yet,
// the consequitive call returns the same Promise rather than,
// attempting to fetch the next item from the iterator.
const iterable = new CachedAsyncIterable(generate());
await Promise.all([
iterable.touchNext(2),
iterable[Symbol.asyncIterator]().next(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add another test where touchNext is actually called multiple times :)

iterable.touchNext(2),
iterable[Symbol.asyncIterator]().next(),
]);
assert.equal(counter, 4);
});
});
});