Skip to content

Commit fd305e9

Browse files
committed
bookmarked pagination, including multi-shard
introduces a new BookmarkedCollection module with behavior similar to PaginatedCollection in the simple case. the primary advantage is that assigning to current_page (e.g. from the :page parameter to paginate) expects a bookmark token value and automatically deserializes into current_bookmark. the library client can then use current_bookmark to skip forward in the collection, rather than using (current_page - 1) * per_page as the number of items to skip. the client then calls set_next_bookmark on the pager if there's more results, and it automatically derives the bookmark for the next page and serializes it into next_page, for use by Api.paginate, etc. in addition to the PaginatedCollection.build analog, you can simply wrap an existing scope to change it from something that will paginate by page number into something that will paginate by bookmark. finally, the key reason to use bookmarked pagination is to enable composition of collections. you can merge multiple collections into one collection which when paginated will pull results from each subcollection, in order, to produce the page of results. you can also concatenate multiple collections into one collection which when paginated will exhaust the collections in order with seamless transition from one to the next when a page spans both. with collection merging available, you can paginate an association where you'd like to use with_each_shard. one collection is created per shard, and then they are merged together. this process is automated for you in the BookmarkedCollection.with_each_shard method. fixes CNVS-1169 Change-Id: Ib998eee53c33604cb6f7e338153428a157928a6d Reviewed-on: https://gerrit.instructure.com/16039 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Jacob Fugal <jacob@instructure.com> QA-Review: Clare Hetherington <clare@instructure.com>
1 parent 8195447 commit fd305e9

18 files changed

Lines changed: 1578 additions & 33 deletions

app/controllers/groups_controller.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,14 @@ def unassigned_members
146146
# @returns [Group]
147147
def index
148148
return context_index if @context
149-
@groups = @current_user.try(:current_groups)
150-
# can't use #try, cause it's not defined on the association class,
151-
# so it will try to load the association and call it on the resulting array'
152-
@groups = @groups.with_each_shard({ :include => :group_category, :order => "groups.id ASC" }) if @groups
153-
@groups ||= []
149+
if @current_user
150+
@groups = BookmarkedCollection.with_each_shard(
151+
Group::Bookmarker,
152+
@current_user.current_groups,
153+
:include => :group_category)
154+
else
155+
@groups = []
156+
end
154157
respond_to do |format|
155158
format.html {}
156159
format.json do

app/models/group.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,4 +528,22 @@ def default_collection_name
528528
def associated_shards
529529
[Shard.default]
530530
end
531+
532+
class Bookmarker
533+
def self.bookmark_for(group)
534+
group.id
535+
end
536+
537+
def self.validate(bookmark)
538+
bookmark.is_a?(Fixnum)
539+
end
540+
541+
def self.restrict_scope(scope, pager)
542+
if bookmark = pager.current_bookmark
543+
comparison = (pager.include_bookmark ? 'groups.id >= ?' : 'groups.id > ?')
544+
scope = scope.scoped(:conditions => [comparison, bookmark])
545+
end
546+
scope.scoped(:order => "groups.id ASC")
547+
end
548+
end
531549
end

app/models/page_view.rb

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -189,13 +189,25 @@ def update_without_callbacks
189189
{ :conditions => {:user_id => users} }
190190
}
191191

192+
module CassandraBookmarker
193+
def self.bookmark_for(item)
194+
[item['bucket'], item['ordered_id']]
195+
end
196+
197+
def self.validate(bookmark)
198+
bookmark.is_a?(Array) &&
199+
bookmark.size == 2
200+
end
201+
end
192202

193203
# returns a collection with very limited functionality
194204
# basically, it responds to #paginate and returns a
195205
# WillPaginate::Collection-like object
196206
def self.for_user(user)
197207
if cassandra?
198-
PaginatedCollection.build { |pager| page_view_history(user, pager) }
208+
BookmarkedCollection.build(PageView::CassandraBookmarker) do |pager|
209+
page_view_history(user, pager)
210+
end
199211
else
200212
self.scoped(:conditions => { :user_id => user.id }, :order => 'created_at desc')
201213
end
@@ -205,43 +217,56 @@ def self.page_view_history(context, pager)
205217
context_type = context.class.name
206218
context_id = context.global_id
207219
scrollback_limit = Setting.get('page_views_scrollback_limit:users', 52.weeks.to_s).to_i.ago
208-
results = []
209220

210-
if pager.current_page.to_s =~ %r{^(\d+):(\d+/\w+)$}
211-
row_ts = $1.to_i
212-
start_column = $2
221+
# get the bucket to start at from the bookmark
222+
if pager.current_bookmark
223+
bucket, ordered_id = pager.current_bookmark
213224
else
214225
# page 1
215-
row_ts = PageView.timeline_bucket_for_time(Time.now, context_type)
226+
bucket = PageView.timeline_bucket_for_time(Time.now, context_type)
227+
ordered_id = nil
216228
end
217229

218-
until pager.next_page || Time.at(row_ts) < scrollback_limit
219-
limit = pager.per_page + 1 - results.size
230+
# pull results from each bucket until the page is full or we hit the
231+
# scrollback_limit
232+
until pager.next_bookmark || Time.at(bucket) < scrollback_limit
233+
# build up the query based on the context, bucket, and ordered_id. fetch
234+
# one extra so we can tell if there are more pages
235+
limit = pager.per_page + 1 - pager.size
220236
args = []
221-
args << "#{context_type.underscore}_#{context_id}/#{row_ts}"
222-
if start_column
223-
ordered_id = "AND ordered_id <= ?"
224-
args << start_column
237+
args << "#{context_type.underscore}_#{context_id}/#{bucket}"
238+
if ordered_id
239+
ordered_id_clause = (pager.include_bookmark ? "AND ordered_id <= ?" : "AND ordered_id < ?")
240+
args << ordered_id
225241
else
226-
ordered_id = nil
242+
ordered_id_clause = nil
227243
end
228-
qs = "SELECT ordered_id, request_id FROM page_views_history_by_context where context_and_time_bucket = ? #{ordered_id} ORDER BY ordered_id DESC LIMIT #{limit}"
244+
qs = "SELECT ordered_id, request_id FROM page_views_history_by_context where context_and_time_bucket = ? #{ordered_id_clause} ORDER BY ordered_id DESC LIMIT #{limit}"
245+
246+
# execute the query collecting the results. set the bookmark iff there
247+
# was a result after the full page
229248
PageView.cassandra.execute(qs, *args).fetch do |row|
230-
if results.size == pager.per_page
231-
pager.next_page = "#{row_ts}:#{row['ordered_id']}"
249+
if pager.size == pager.per_page
250+
pager.has_more!
232251
else
233-
results << row['request_id']
252+
pager << row.to_hash.merge('bucket' => bucket)
234253
end
235254
end
236255

237-
start_column = nil
256+
ordered_id = nil
238257
# possible optimization: query page_views_counters_by_context_and_day ,
239258
# and use it as a secondary index to skip days where the user didn't
240259
# have any page views
241-
row_ts -= PageView.timeline_bucket_size(context_type)
260+
bucket -= PageView.timeline_bucket_size(context_type)
242261
end
243262

244-
pager.replace(PageView.find(results).sort_by { |pv| results.index(pv.request_id) })
263+
# now that the bookmark's been set, we only need to keep the request
264+
# ids per item returned.
265+
request_ids = pager.map{ |row| row['request_id'] }
266+
267+
# before returning, convert those request_ids into actual page views, but
268+
# preserve the order
269+
pager.replace(PageView.find(request_ids).sort_by { |pv| request_ids.index(pv.request_id) })
245270
end
246271

247272
def self.cache_queue_name

lib/api.rb

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,15 +181,20 @@ def self.paginate(collection, controller, base_url, pagination_args = {})
181181
pagination_args.reverse_merge!({ :page => controller.params[:page], :per_page => per_page })
182182
collection = collection.paginate(pagination_args)
183183
return unless collection.respond_to?(:next_page)
184-
total_pages = (pagination_args[:without_count] ? nil : collection.total_pages)
185-
total_pages = nil if total_pages.to_i <= 1
184+
185+
first_page = collection.respond_to?(:first_page) && collection.first_page
186+
first_page ||= 1
187+
188+
last_page = (pagination_args[:without_count] ? nil : collection.total_pages)
189+
last_page = nil if last_page.to_i <= 1
190+
186191
links = build_links(base_url, {
187192
:query_parameters => controller.request.query_parameters,
188193
:per_page => collection.per_page,
189194
:next => collection.next_page,
190195
:prev => collection.previous_page,
191-
:first => 1,
192-
:last => total_pages,
196+
:first => first_page,
197+
:last => last_page,
193198
})
194199
controller.response.headers["Link"] = links.join(',') if links.length > 0
195200
collection

0 commit comments

Comments
 (0)