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

Render cached remote suggestions immediately #156

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
11 changes: 8 additions & 3 deletions src/dataset.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,19 @@ var Dataset = (function() {
getSuggestions: function(query, cb) {
var that = this,
terms = utils.tokenizeQuery(query),
suggestions = this._getLocalSuggestions(terms).slice(0, this.limit);
suggestions = this._getLocalSuggestions(terms).slice(0, this.limit),
cacheHit = false;

cb && cb(suggestions);

if (suggestions.length < this.limit && this.transport) {
this.transport.get(query, processRemoteData);
cacheHit = this.transport.get(query, processRemoteData);
}

// if a cache hit occurred, skip rendering local suggestions
// because the rendering of local/remote suggestions is already
// in the event loop
!cacheHit && cb && cb(suggestions);

// callback for transport.get
function processRemoteData(data) {
suggestions = suggestions.slice(0);
Expand Down
58 changes: 36 additions & 22 deletions src/transport.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,41 @@ var Transport = (function() {
beforeSend: o.beforeSend
};

this.get = (/^throttle$/i.test(o.rateLimitFn) ?
utils.throttle : utils.debounce)(this.get, o.rateLimitWait || 300);
this._get = (/^throttle$/i.test(o.rateLimitFn) ?
utils.throttle : utils.debounce)(this._get, o.rateLimitWait || 300);
}

utils.mixin(Transport.prototype, {

// private methods
// ---------------

_get: function(url, cb) {
var that = this;

// under the pending request threshold, so fire off a request
if (belowPendingRequestsThreshold()) {
this._sendRequest(url).done(done);
}

// at the pending request threshold, so hang out in the on deck circle
else {
this.onDeckRequestArgs = [].slice.call(arguments, 0);
}

// success callback
function done(resp) {
var data = that.filter ? that.filter(resp) : resp;

cb && cb(data);

// cache the resp and not the results of applying filter
// in case multiple datasets use the same url and
// have different filters
requestCache.set(url, resp);
}
},

_sendRequest: function(url) {
var that = this, jqXhr = pendingRequests[url];

Expand All @@ -60,7 +86,7 @@ var Transport = (function() {

// ensures request is always made for the last query
if (that.onDeckRequestArgs) {
that.get.apply(that, that.onDeckRequestArgs);
that._get.apply(that, that.onDeckRequestArgs);
that.onDeckRequestArgs = null;
}
}
Expand All @@ -75,36 +101,24 @@ var Transport = (function() {
url,
resp;

cb = cb || utils.noop;

url = this.replace ?
this.replace(this.url, encodedQuery) :
this.url.replace(this.wildcard, encodedQuery);

// in-memory cache hit
if (resp = requestCache.get(url)) {
cb && cb(this.filter ? this.filter(resp) : resp);
}

// under the pending request threshold, so fire off a request
else if (belowPendingRequestsThreshold()) {
this._sendRequest(url).done(done);
// defer to stay consistent with behavior of ajax call
utils.defer(function() { cb(that.filter ? that.filter(resp) : resp); });
}

// at the pending request threshold, so hang out in the on deck circle
else {
this.onDeckRequestArgs = [].slice.call(arguments, 0);
this._get(url, cb);
}

// success callback
function done(resp) {
var data = that.filter ? that.filter(resp) : resp;

cb && cb(data);

// cache the resp and not the result of applying filter
// in case multiple datasets use the same url and
// have different filters
requestCache.set(url, resp);
}
// return bool indicating whether or not a cache hit occurred
return !!resp;
}
});

Expand Down
38 changes: 21 additions & 17 deletions test/dataset_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,26 +344,30 @@ describe('Dataset', function() {
var spy = jasmine.createSpy(),
remote = [fixtureDatums[0], fixtureStrings[2]];

this.dataset.transport.get.andCallFake(function(q, cb) { cb(remote); });
this.dataset.transport.get.andCallFake(function(q, cb) {
utils.defer(function() { cb(remote); });
});

this.dataset.getSuggestions('c', spy);

expect(spy.callCount).toBe(2);

// local suggestions
expect(spy.argsForCall[0][0]).toEqual([
expectedItemHash.coconut,
expectedItemHash.cake,
expectedItemHash.coffee
]);

// local + remote suggestions
expect(spy.argsForCall[1][0]).toEqual([
expectedItemHash.coconut,
expectedItemHash.cake,
expectedItemHash.coffee,
expectedItemHash.grape
]);
waitsFor(function() { return spy.callCount === 2; });

runs(function() {
// local suggestions
expect(spy.argsForCall[0][0]).toEqual([
expectedItemHash.coconut,
expectedItemHash.cake,
expectedItemHash.coffee
]);

// local + remote suggestions
expect(spy.argsForCall[1][0]).toEqual([
expectedItemHash.coconut,
expectedItemHash.cake,
expectedItemHash.coffee,
expectedItemHash.grape
]);
});
});
});

Expand Down
13 changes: 10 additions & 3 deletions test/transport_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,17 @@ describe('Transport', function() {
});

it('should call filter', function() {
expect(this.transport.filter).toHaveBeenCalled();
waitsFor(function() {
return this.transport.filter.callCount === 1;
});
});

it('should invoke callback with data', function() {
expect(this.spy).toHaveBeenCalledWith(['val']);
waitsFor(function() { return this.spy.callCount === 1; });

runs(function() {
expect(this.spy).toHaveBeenCalledWith(['val']);
});
});
});

Expand Down Expand Up @@ -120,7 +126,8 @@ describe('Transport', function() {
});

it('should set args for the on-deck request', function() {
expect(this.transport.onDeckRequestArgs).toEqual(['bad', $.noop]);
expect(this.transport.onDeckRequestArgs)
.toEqual(['http://example.com?q=bad', $.noop]);
});
});

Expand Down