[courier] resolve looper iteration when requests are aborted (#10410)

Backports PR #10392

**Commit 1:**
[courier] resolve looper iteration when requests are aborted

The search looper returns the promise from search executions to ensure that subsequent loops do not run until the previous loop has completed. This causes the looper to get stuck when all of the aborted requests are completed, as completed requests do not resolve or reject their promises, causing the looper to never allow another search loop. Since this behavior is desirable for pages where we don't want to execute the `.then()` or `.catch()` callbacks for requests that were aborted, we work around it in this specific case by creating a promise that is resolved when each request is either aborted or completed.

* Original sha: 1683e03b53
* Authored by spalger <spalger@users.noreply.github.com> on 2017-02-16T06:01:20Z

**Commit 2:**
[ui/promises] add Promise.race() method to Promises util

* Original sha: faa7771e12
* Authored by spalger <spalger@users.noreply.github.com> on 2017-02-16T18:22:21Z

**Commit 3:**
[courier/request] use angular promises in courier

* Original sha: 3b4e3a687b
* Authored by spalger <spalger@users.noreply.github.com> on 2017-02-16T18:22:49Z

**Commit 4:**
[courier/looper] no single-letter variables

* Original sha: 4b1dbf8413
* Authored by spalger <spalger@users.noreply.github.com> on 2017-02-16T18:23:15Z

**Commit 5:**
[ui/promises] add tests for Promise.race()

* Original sha: bc82b29623
* Authored by spalger <spalger@users.noreply.github.com> on 2017-02-16T19:15:51Z
This commit is contained in:
jasper 2017-02-16 15:40:08 -05:00 committed by Court Ewing
parent d7c639fa0b
commit ae4a6f070a
7 changed files with 114 additions and 9 deletions

View file

@ -185,7 +185,7 @@ export default function SourceAbstractFactory(Private, Promise, PromiseEmitter)
courierFetch.these([req]);
return req.defer.promise;
return req.getCompletePromise();
};
/**

View file

@ -175,7 +175,7 @@ export default function SearchSourceFactory(Promise, Private, config) {
// return promises created by the completion handler so that
// errors will bubble properly
return req.defer.promise.then(addRequest);
return req.getCompletePromise().then(addRequest);
});
};

View file

@ -23,7 +23,7 @@ export default function fetchService(Private, Promise) {
const fetchTheseSoon = (requests) => {
requests.forEach(req => req._setFetchRequested());
debouncedFetchThese();
return Promise.all(requests.map(req => req.defer.promise));
return Promise.all(requests.map(req => req.getCompletePromise()));
};
this.fetchQueued = (strategy) => {

View file

@ -14,7 +14,7 @@ export default function AbstractReqProvider(Private, Promise) {
constructor(source, defer) {
this.source = source;
this.defer = defer || Promise.defer();
this._whenAbortedHandlers = [];
this.abortedDefer = Promise.defer();
requestQueue.push(this);
}
@ -120,11 +120,12 @@ export default function AbstractReqProvider(Private, Promise) {
this._markStopped();
this.defer = null;
this.aborted = true;
_.callEach(this._whenAbortedHandlers);
this.abortedDefer.resolve();
this.abortedDefer = null;
}
whenAborted(cb) {
this._whenAbortedHandlers.push(cb);
this.abortedDefer.promise.then(cb);
}
complete() {
@ -133,6 +134,14 @@ export default function AbstractReqProvider(Private, Promise) {
this.defer.resolve(this.resp);
}
getCompletePromise() {
return this.defer.promise;
}
getCompleteOrAbortedPromise() {
return Promise.race([ this.defer.promise, this.abortedDefer.promise ]);
}
clone() {
return new this.constructor(this.source, this.defer);
}

View file

@ -17,9 +17,12 @@ export default function SearchLooperService(Private, Promise, Notifier, $rootSco
*/
const searchLooper = new Looper(null, function () {
$rootScope.$broadcast('courier:searchRefresh');
return fetch.these(
requestQueue.getInactive(searchStrategy)
);
const requests = requestQueue.getInactive(searchStrategy);
// promise returned from fetch.these() only resolves when
// the requests complete, but we want to continue even if
// the requests abort so we make our own
fetch.these(requests);
return Promise.all(requests.map(request => request.getCompleteOrAbortedPromise()));
});
searchLooper.onHastyLoop = function () {

View file

@ -78,4 +78,90 @@ describe('Promise service', function () {
expect(thenback.getCall(0).args[0][1]).to.be(undefined);
});
});
describe('Promise.race()', () => {
let crankTimeout;
beforeEach(() => {
// constantly call $rootScope.$apply() in a loop so we can
// pretend that these are real promises
(function crank$apply() {
$rootScope.$apply();
crankTimeout = setTimeout(crank$apply, 1);
}());
});
afterEach(() => {
clearTimeout(crankTimeout);
});
it(`resolves with the first resolved promise's value`, async () => {
const p1 = new Promise(resolve => setTimeout(resolve, 100, 1));
const p2 = new Promise(resolve => setTimeout(resolve, 200, 2));
expect(await Promise.race([p1, p2])).to.be(1);
});
it(`rejects with the first rejected promise's rejection reason`, async () => {
const p1 = new Promise((r, reject) => setTimeout(reject, 200, new Error(1)));
const p2 = new Promise((r, reject) => setTimeout(reject, 100, new Error(2)));
expect(await Promise.race([p1, p2]).catch(e => e.message)).to.be('2');
});
it('does not wait for subsequent promises to resolve/reject', async () => {
const start = Date.now();
const p1 = new Promise(resolve => setTimeout(resolve, 100));
const p2 = new Promise(resolve => setTimeout(resolve, 5000));
await Promise.race([p1, p2]);
const time = Date.now() - start;
expect(time).to.be.lessThan(1000);
expect(time).to.be.greaterThan(100);
});
it('allows non-promises in the array', async () => {
expect(await Promise.race([1,2,3])).to.be(1);
});
context('argument is undefined', () => {
it('rejects the promise', async () => {
const football = {};
expect(await Promise.race().catch(e => football)).to.be(football);
});
});
context('argument is a string', () => {
it(`resolves with the first character`, async () => {
expect(await Promise.race('abc')).to.be('a');
});
});
context('argument is a non-iterable object', () => {
it('reject the promise', async () => {
const football = {};
expect(await Promise.race({}).catch(e => football)).to.be(football);
});
});
context('argument is a generator', () => {
it('resolves with the first resolved value', async () => {
function *gen() {
yield new Promise(resolve => setTimeout(resolve, 100, 1));
yield new Promise(resolve => setTimeout(resolve, 200, 2));
}
expect(await Promise.race(gen())).to.be(1);
});
it('resolves with the first non-promise value', async () => {
function *gen() {
yield 1;
yield new Promise(resolve => setTimeout(resolve, 200, 2));
}
expect(await Promise.race(gen())).to.be(1);
});
it('iterates all values from the generator, even if one is already "resolved"', async () => {
let yieldCount = 0;
function *gen() {
yieldCount += 1;
yield 1;
yieldCount += 1;
yield new Promise(resolve => setTimeout(resolve, 200, 2));
}
expect(await Promise.race(gen())).to.be(1);
expect(yieldCount).to.be(2);
});
});
});
});

View file

@ -95,6 +95,13 @@ module.service('Promise', function ($q, $timeout) {
});
});
};
Promise.race = function (iterable) {
return new Promise((resolve, reject) => {
for (const i of iterable) {
Promise.resolve(i).then(resolve, reject);
}
});
};
return Promise;
});