summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorSteven Danna <steve@chef.io>2015-10-05 14:18:52 +0100
committerSteven Danna <steve@chef.io>2015-10-09 14:49:18 +0100
commitecd535d7fb4036baf949833f7f88cfe0911ddca7 (patch)
tree2ff0a26401f0f5412737a13c45ae750c4a7ea130 /lib
parentc9473b61e7d5f6f48598acaef4af0cfdf821f579 (diff)
downloadchef-ecd535d7fb4036baf949833f7f88cfe0911ddca7.tar.gz
Fix search result paginationssd/search-pagination-fix
The start and rows parameter that are passed as part of the search request are passed directly to Solr on the backend. Results from Solr may contain deleted nodes no longer in the erchef database. Erchef will filter such nodes from the results. Thus, a user may receive fewer rows than they asked for. Incrementing 'start' only by the number of rows received will then result in the next Solr response overlapping the first, which can lead to duplicate results. In the case of a Solr results page that was completely filtered, it would lead to an infinite loop. This commit changes the code to always increment by the requested page size (args[:rows]) when it is non-nil. Incrementing by the length of the response set is still wrong in the case when the args[:rows] is nil, but the server doesn't give us anything else to increment by. Fixes #4027
Diffstat (limited to 'lib')
-rw-r--r--lib/chef/search/query.rb18
1 files changed, 16 insertions, 2 deletions
diff --git a/lib/chef/search/query.rb b/lib/chef/search/query.rb
index 6469a18c49..658af8779c 100644
--- a/lib/chef/search/query.rb
+++ b/lib/chef/search/query.rb
@@ -88,8 +88,21 @@ WARNDEP
if block
response["rows"].each { |row| block.call(row) if row }
- unless (response["start"] + response["rows"].length) >= response["total"]
- args_h[:start] = response["start"] + response["rows"].length
+ #
+ # args_h[:rows] and args_h[:start] are the page size and
+ # start position requested of the search index backing the
+ # search API.
+ #
+ # The response may contain fewer rows than arg_h[:rows] if
+ # the page of index results included deleted nodes which
+ # have been filtered from the returned data. In this case,
+ # we still want to start the next page at start +
+ # args_h[:rows] to avoid asking the search backend for
+ # overlapping pages (which could result in duplicates).
+ #
+ next_start = response["start"] + (args_h[:rows] || response["rows"].length)
+ unless next_start >= response["total"]
+ args_h[:start] = next_start
search(type, query, args_h, &block)
end
true
@@ -99,6 +112,7 @@ WARNDEP
end
private
+
def validate_type(t)
unless t.kind_of?(String) || t.kind_of?(Symbol)
msg = "Invalid search object type #{t.inspect} (#{t.class}), must be a String or Symbol." +