Skip to content

Commit

Permalink
Merge pull request #1675 from thostetler/reorder-requests
Browse files Browse the repository at this point in the history 
Cleanup and Ensure Sub-Requests Happen After Root
  • Loading branch information
@ehenneken
ehenneken authored Jan 2, 2019
2 parents 970441d + 254fe70 commit 7cf3a53
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 140 deletions.
3 changes: 2 additions & 1 deletion grunt/mocha-runner.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ module.exports = function (grunt) {
options: {
args: {
ignoreResourceErrors: true,
timeout: 10000
timeout: 10000,
fullTrace: true
}
},
all: {
Expand Down
63 changes: 30 additions & 33 deletions src/js/components/query_mediator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
/**
* Mediator to coordinate UI-query exchange
*/

define(['underscore',
'jquery',
'cache',
Expand Down Expand Up @@ -283,24 +282,10 @@ function (
q.lock();
ps.publish(ps.INVITING_REQUEST, q);

// give widgets some time to submit their requests
var self = this;

if (this.shortDelayInMs) {
setTimeout(function () {
self.__searchCycle.collectingRequests = false;
if (self.startExecutingQueries()) {
self.monitorExecution();
}
}, this.shortDelayInMs);
} else {
this.__searchCycle.collectingRequests = false;
if (self.startExecutingQueries()) {
setTimeout(function () {
self.monitorExecution();
}, this.shortDelayInMs);
}
}
// move this off the current stack, to add small delay
setTimeout(_.bind(function () {
this.startExecutingQueries() && this.monitorExecution();
}, this), 0);
},


Expand Down Expand Up @@ -372,14 +357,13 @@ function (
}));

self.displayTugboatMessages();
// after we are done with the first query, start executing other queries
var f = function () {
var completeWaiting = function () {
// after we are done with the first query, start executing other queries
_.each(_.keys(cycle.waiting), function (k) {
data = cycle.waiting[k];
delete cycle.waiting[k];
cycle.inprogress[k] = data;
var psk = k;

self._executeRequest.call(self, data.request, data.key)
.done(function () {
cycle.done[psk] = cycle.inprogress[psk];
Expand All @@ -403,14 +387,19 @@ function (
});
};

// for the display experience, it is better to introduce delays
if (self.longDelayInMs && self.longDelayInMs > 0) {
// wait an initial 500ms to let the widgets get their requests in,
// wait in 100ms increments until we no longer see a change in request cache
(function watchForChanges (l) {
setTimeout(function () {
f();
}, self.longDelayInMs);
} else {
f();
}
// check the current length, if equal then we can assume nothing has changed
// since last check, let the cycle finish
if (l === self.__searchCycle.waiting.length) {
self.__searchCycle.collectingRequests = false;
return completeWaiting();
}
setTimeout(watchForChanges, 100, self.__searchCycle.waiting.length);
}, 500);
})(self.__searchCycle.waiting.length);
})
.fail(function (jqXHR, textStatus, errorThrown) {
self.__searchCycle.error = true;
Expand Down Expand Up @@ -523,8 +512,17 @@ function (
apiRequest.set('target', ApiTargets.MYADS_STORAGE + '/execute_query/' + qid);
}

var ps = this.getPubSub();
var api = this.getBeeHive().getService('Api');
try {
var ps = this.getPubSub();
var api = this.getBeeHive().getService('Api');
} catch (e) {
return $.Deferred().reject().promise();
}

// reject the request if we don't have an api to call
if (!ps || !api) {
return $.Deferred().reject().promise();
}

var requestKey = this._getCacheKey(apiRequest);
var maxTry = this.failedRequestsCache.getSync(requestKey) || 0;
Expand All @@ -534,8 +532,7 @@ function (
request: apiRequest, key: senderKey, requestKey: requestKey, qm: this
},
[{ status: ApiFeedback.CODES.TOO_MANY_FAILURES }, 'Error', 'This request has reached maximum number of failures (wait before retrying)']);
var d = $.Deferred();
return d.reject();
return $.Deferred().reject().promise();
}


Expand Down
65 changes: 34 additions & 31 deletions test/mocha/js/components/discovery_mediator.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,40 +114,43 @@ define([
minsub.publish(minsub.DELIVERING_REQUEST, x.reqUnAuthorized);
});
minsub.publish(minsub.START_SEARCH, x.qError);


minsub.subscribeOnce(minsub.START_SEARCH, function() {
done()}); // if this fires, query was resurrected

x.app.getPluginOrWidgetName = sinon.stub().returns(null);
x.app = _.extend(x.app, {
getApiAccess: function() {
var defer = $.Deferred();
defer.resolve({});
return defer;
},
getController: function(name) {
if (name == 'QueryMediator')
return {
resetFailures: function() {}
}
},
getService: function(name) {
if (name == 'Api') {
return {
request: function (apiReq, options) {
var d = $.Deferred();
d.done(options.done);
d.fail(options.fail);
d.resolve();
return d;
done();
}); // if this fires, query was resurrected
var server = this.server;

// wait enough time for sub-queries to fire
setTimeout(function () {

x.app.getPluginOrWidgetName = sinon.stub().returns(null);
x.app = _.extend(x.app, {
getApiAccess: function() {
var defer = $.Deferred();
defer.resolve({});
return defer;
},
getController: function(name) {
if (name == 'QueryMediator')
return {
resetFailures: function() {}
}
},
getService: function(name) {
if (name == 'Api') {
return {
request: function (apiReq, options) {
var d = $.Deferred();
d.done(options.done);
d.fail(options.fail);
d.resolve();
return d;
}
}
}
}
}
});
this.server.respond();

});
server.respond();
}, 550);
});

it("resets ORCID settings when the ORCID API returns 401, which is passed to the feedback service", function () {
Expand Down Expand Up @@ -218,4 +221,4 @@ define([
});

})
});
});
72 changes: 32 additions & 40 deletions test/mocha/js/components/query_mediator.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,13 @@ define([

afterEach(function(done) {
if (this.beehive) {
setTimeout(function(self, xdone) {
setTimeout(function(self) {
self.server.restore();
self.server.requests = []
self.beehive.destroy();
self.beehive = null;
xdone();
}, 5, this, done)
done();
}, 5, this)
}
});

Expand Down Expand Up @@ -431,7 +431,6 @@ define([

qm.startSearchCycle(new ApiQuery({'q': 'foo'}), key);

expect(qm._cache.size).to.be.eql(0);
expect(qm.reset.callCount).to.eql(1);
expect(qm.__searchCycle.initiator).to.be.eql(key.getId());
expect(qm.__searchCycle.waiting[key.getId()]).to.be.defined;
Expand Down Expand Up @@ -500,6 +499,31 @@ define([

});

it("sends CYCLE signals when job starts and is done", function(done) {
var pubSpy = this.pubSpy;
var x = createTestQM(this.beehive);
var qm = x.qm, key1 = x.key1, key2 = x.key2, req1 = x.req1, req2 = x.req2;
var respond = _.bind(this.server.respond, this.server);
qm.__searchCycle.waiting[key1.getId()] = {key: key1, request: req1};
qm.__searchCycle.waiting[key2.getId()] = {key: key2, request: req2};
qm.startExecutingQueries();
expect(qm.__searchCycle.running).to.be.true;
expect(qm.__searchCycle.inprogress[key1.getId()]).to.be.defined;
respond();
expect(qm.__searchCycle.done[key1.getId()]).to.be.defined;
expect(pubSpy.lastCall.args[1]).to.be.instanceOf(ApiFeedback);
expect(pubSpy.lastCall.args[1].code).to.be.eql(ApiFeedback.CODES.SEARCH_CYCLE_STARTED);
expect(_.keys(qm.__searchCycle.waiting).length).to.be.eql(1);
respond();
setTimeout(function() {
respond();
expect(pubSpy.lastCall.args[1]).to.be.instanceOf(ApiFeedback);
expect(pubSpy.lastCall.args[1].code).to.be.eql(ApiFeedback.CODES.SEARCH_CYCLE_FINISHED);
qm.destroy();
done();
}, 600);
});

it("responds to GET_QTREE signal", function(done) {
var pubSpy = this.pubSpy;
var x = createTestQM(this.beehive);
Expand Down Expand Up @@ -550,9 +574,10 @@ define([
expect(qm._executeRequest.callCount).to.be.eql(1);
expect(qm.onApiResponse.callCount).to.be.eql(0);

this.server.respond();
this.server.respond();
var server = this.server;
server.respond();
setTimeout(function(qm, pubSpy, done) {
server.respond();
expect(pubSpy.firstCall.args[0]).to.be.eql(PubSubEvents.DELIVERING_RESPONSE + key1.getId());
expect(pubSpy.secondCall.args[0]).to.be.eql(PubSubEvents.FEEDBACK);
expect(pubSpy.secondCall.args[1].code).to.be.eql(ApiFeedback.CODES.SEARCH_CYCLE_STARTED);
Expand All @@ -561,7 +586,7 @@ define([
expect(qm.onApiResponse.callCount).to.be.eql(2);
qm.destroy();
done();
}, 50, qm, pubSpy, done);
}, 800, qm, pubSpy, done);

});

Expand Down Expand Up @@ -673,39 +698,6 @@ define([
done();
});


it("sends CYCLE signals when job starts and is done", function(done) {
var pubSpy = this.pubSpy;
var x = createTestQM(this.beehive);
var qm = x.qm, key1 = x.key1, key2 = x.key2, req1 = x.req1, req2 = x.req2;

this.beehive.addObject('DynamicConfig', {pskToExecuteFirst: key2.getId()});
qm.__searchCycle.waiting[key1.getId()] = {key: key1, request: req1};
qm.__searchCycle.waiting[key2.getId()] = {key: key2, request: req2};
qm.startExecutingQueries();
expect(qm.__searchCycle.running).to.be.true;
expect(qm.__searchCycle.inprogress[key2.getId()]).to.be.defined;

this.server.respond();
expect(qm.__searchCycle.done[key2.getId()]).to.be.defined;
expect(qm.__searchCycle.inprogress[key1.getId()]).to.be.defined;
expect(_.keys(qm.__searchCycle.waiting).length).to.be.eql(0);

expect(pubSpy.lastCall.args[1]).to.be.instanceOf(ApiFeedback);
expect(pubSpy.lastCall.args[1].code).to.be.eql(ApiFeedback.CODES.SEARCH_CYCLE_STARTED);

this.server.respond();
var mydone = done;

setTimeout(function(self, done, pubSpy) {
self.server.respond();
expect(pubSpy.lastCall.args[1]).to.be.instanceOf(ApiFeedback);
expect(pubSpy.lastCall.args[1].code).to.be.eql(ApiFeedback.CODES.SEARCH_CYCLE_FINISHED);
qm.destroy();
done();
}, 1, this, done, pubSpy);
});

it("knows to recover from certain errors", function(done) {
var pubSpy = this.pubSpy;
var x = createTestQM(this.beehive);
Expand Down
Loading

0 comments on commit 7cf3a53

Please sign in to comment.