Opened 13 years ago

Closed 12 years ago

#1956 closed enhancement (duplicate)

dojo.widget.incrementalFinderDataProvider with "backoff" (prevents many requests)

Reported by: ialpert@… Owned by: bill
Priority: high Milestone: 1.0
Component: Dijit Version: 0.4
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

dojo.widget.incrementalComboBoxDataProvider = function(/*String*/ url, /*Number*/ limit, /*Number*/ timeout){

this.searchUrl = url; this.inFlight = false; this.activeRequest = null; this.allowCache = false; this.timeout = null; this.cache = {};

this.init = function(/*Widget*/ cbox){

this.searchUrl = cbox.dataUrl;

};

this.addToCache = function(/*String*/ keyword, /*Array*/ data){

if(this.allowCache){

this.cache[keyword] = data;

}

};

this.startSearch = function(/*String*/ searchStr, /*String*/ type, /*Boolean*/ ignoreLimit){

if(this.inFlight){

this.timeout = dojo.lang.setTimeout(this,this.startSearch,20,searchStr,type,ignoreLimit); return;

} if(this.timeout) {

dojo.lang.clearTimeout(this.timeout); this.timeout=null;

}

this.performSearch(searchStr, type, ignoreLimit);

};

this.performSearch = function(/*String*/ searchStr, /*String*/ type, /*Boolean*/ ignoreLimit){

var tss = encodeURIComponent(searchStr); var realUrl = dojo.string.substituteParams(this.searchUrl, {"searchString": tss}); var _this = this; var request = dojo.io.bind({

url: realUrl, method: "get", mimetype: "text/json", load: function(type, data, evt){

_this.inFlight = false; if(!dojo.lang.isArray(data)){

var arrData = []; for(var key in data){

arrData.push([data[key], key]);

} data = arrData;

} _this.addToCache(searchStr, data); _this.provideSearchResults(data);

}

}); this.inFlight = true;

};

};

Attachments (8)

diff.js (6.2 KB) - added by guest 13 years ago.
ComboBox.js-Diff (11.7 KB) - added by Izaak Alpert 13 years ago.
ComboBox.2.js-Diff (1.4 KB) - added by guest 13 years ago.
remoteComboBoxData.php (1.8 KB) - added by Izaak Alpert 13 years ago.
PHP file for testing request limiting functionality
test_ComboBox.html-diff (3.5 KB) - added by Izaak Alpert 13 years ago.
diff of test_ComboBox.html from dojo/tests/widgets
ComboBox.3.js-Diff (1.9 KB) - added by Izaak Alpert 13 years ago.
combo box with request prevention (no timeouts)
remoteComboBoxData.2.php (1.8 KB) - added by Izaak Alpert 13 years ago.
PHP file for testing request limiting functionality
test_ComboBox.2.html-diff (4.2 KB) - added by Izaak Alpert 13 years ago.
test for combBox with request limiting behaviour

Download all attachments as: .zip

Change History (25)

comment:1 Changed 13 years ago by bill

Description: modified (diff)
  1. Have you filed a CLA (www.dojotoolkit.org/icla.txt)?
  2. Please attach a diff file against the latest dojo 0.4.1RC code

Changed 13 years ago by guest

Attachment: diff.js added

comment:2 Changed 13 years ago by guest

  1. Yes i have -- faxed this morning (is there anything else i need to do WRT this?)
  2. couldn't find tagged version (probably my svn ignorance) so i diffed against trunk (hope this is ok)

comment:3 Changed 13 years ago by alex

Owner: changed from bill to alex
Status: newassigned

a good chunk of this patch is whitespace noise. Is that intended? I'll merge it, but I'd prefer a cleaner patch.

comment:4 Changed 13 years ago by bill

This isn't a patch against the latest. (Well technically it is, but...) He took an old version of the incrementalDataProvider and modified that, instead of modifying the newest version. So it basically lost all the recent changes. We need a patch against the newest version.

comment:5 Changed 13 years ago by Izaak Alpert

Err true, i didn't write tests for it as well, doing so now.

Changed 13 years ago by Izaak Alpert

Attachment: ComboBox.js-Diff added

comment:6 Changed 13 years ago by Izaak Alpert

Allright I've rewritten the code against latest (i apologize -- no excuse for that last diff).

Couple of problems

  1. there's still lots of whitespace in the diff file after the incrementalComboBoxDataProvider class which is the only thing i changed. Any suggestions as to how to prevent this (bit of an SVN newbie)?
  2. I've tried writing a test for this however since there's no latency in file requests it's difficult to exhibit the request limiting code, any suggestions as to how to fake out latency?

comment:7 Changed 13 years ago by bill

Milestone: 0.4.10.5

Hi Izaak,

Looks like the whitespace changes were actually things you fixed (IE, the original file had spurious whitespace). I checked in the whitespace changes as a separate checkin.

For testing, I'd suggest writing a php file. Although dojo is purely client side we can stick a few PHP files in the test directory as necessary.

As far as the algorithm goes, I'm not sure this makes sense. If I type the letter "a" and then type the letter "l", but it takes a long time to search for all words starting w/the letter "a", then eventually a drop down will be displayed with all the words that start with "a", even though I've already typed in "al". The current code will fire off request for "a" and "al" (as I type "a", and then "l"), and then just ignore the return value from the "a" request.

comment:8 Changed 13 years ago by Izaak Alpert

There may be cases where if the servers request is fast enough that 20msec value should probably be changed (I originally was going to tie this into the server response time). This number seemed to work reasonably well where the server response time was around 0.5 seconds.

What I've been trying to achieve was if somebody types "abc" quickly the b request basically doesn't get sent. It sounds like the 20msec setting is too long? I'd like to show the user results if they hesitate, but prevent unnecessary requests...

Perhaps i should build a buffer and always only send after ever X msec (where X is sufficiently small that they don't feel like it's slow)?

WRT Testing Is there any documentation/examples as to set that up I can look at (I presume there's some kind of server that gets instantiated to serve up these php files).

comment:9 Changed 13 years ago by Izaak Alpert

FYI i have had slightlyoff (from #dojo) have a look at it, seemed to think it was allright.

Also behavior where typing "a" then "l" and the "a" request getting ignored exists prior to patch.

comment:10 Changed 13 years ago by Izaak Alpert

Ok here's the latest with a test... Using no timeout

Changed 13 years ago by guest

Attachment: ComboBox.2.js-Diff added

Changed 13 years ago by Izaak Alpert

Attachment: remoteComboBoxData.php added

PHP file for testing request limiting functionality

Changed 13 years ago by Izaak Alpert

Attachment: test_ComboBox.html-diff added

diff of test_ComboBox.html from dojo/tests/widgets

comment:11 Changed 13 years ago by Izaak Alpert

ok please ignore last diff -- i've tested it out with different latiencies, i'm not convinced it works...

comment:12 Changed 13 years ago by Izaak Alpert

Ok here's a timeout less and test that actually works...

Changed 13 years ago by Izaak Alpert

Attachment: ComboBox.3.js-Diff added

combo box with request prevention (no timeouts)

Changed 13 years ago by Izaak Alpert

Attachment: remoteComboBoxData.2.php added

PHP file for testing request limiting functionality

Changed 13 years ago by Izaak Alpert

Attachment: test_ComboBox.2.html-diff added

test for combBox with request limiting behaviour

comment:13 Changed 13 years ago by Izaak Alpert

Ok this one seems to avoid race conditions and the tests is reasonably configurable

comment:14 Changed 13 years ago by Izaak Alpert

I have done some loose tests on this with the ComboBox? searchDelay set to 0 to see if it had an effect; it did not (beyond speeding up initial requests)

comment:15 Changed 12 years ago by bill

Owner: changed from alex to haysmark
Status: assignednew

Combobox/Select? have been completely rewritten; not sure if this is even an issue anymore. But we should have some kind of backoff to prevent too many requests going out. Assigning to Mark to consider.

comment:16 Changed 12 years ago by bill

Component: WidgetsDijit
Milestone: 0.91.0
Owner: changed from haysmark to bill

comment:17 Changed 12 years ago by bill

Resolution: duplicate
Status: newclosed

This ticket is really long and the patch is no longer valid because Combobox has been rewritten. Replaced with #4102.

Note: See TracTickets for help on using tickets.