From 5f2659708696406793af8baf90fae82da6e24cd9 Mon Sep 17 00:00:00 2001 From: Martin Fenner Date: Fri, 13 Dec 2019 11:09:01 +0000 Subject: [PATCH] correctly parse cursor when a DOI contains a comma. datacite/datacite#897 --- app/controllers/activities_controller.rb | 6 ++++-- app/controllers/concerns/paginatable.rb | 8 +++++++- app/controllers/dois_controller.rb | 2 +- app/controllers/events_controller.rb | 4 ++-- app/controllers/old_events_controller.rb | 2 +- app/models/event.rb | 2 +- 6 files changed, 16 insertions(+), 8 deletions(-) diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index 7a955b9a3..a4cd94269 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -49,6 +49,8 @@ def index render json: ActivitySerializer.new(results, options).serialized_json, status: :ok else + results = response.results + options = {} options[:meta] = { total: total, @@ -60,7 +62,7 @@ def index self: request.original_url, next: response.results.size < page[:size] ? nil : request.base_url + "/activities?" + { query: params[:query], - "page[cursor]" => page[:cursor] ? Base64.urlsafe_encode64(Array.wrap(@activities.to_a.last[:sort]).join(","), padding: false) : nil, + "page[cursor]" => page[:cursor] ? make_cursor(results) : nil, "page[number]" => page[:cursor].nil? && page[:number].present? ? page[:number] + 1 : nil, "page[size]" => page[:size], sort: params[:sort] }.compact.to_query @@ -68,7 +70,7 @@ def index options[:include] = @include options[:is_collection] = true - render json: ActivitySerializer.new(response.results, options).serialized_json, status: :ok + render json: ActivitySerializer.new(results, options).serialized_json, status: :ok end rescue Elasticsearch::Transport::Transport::Errors::BadRequest => exception Raven.capture_exception(exception) diff --git a/app/controllers/concerns/paginatable.rb b/app/controllers/concerns/paginatable.rb index a9e43e3bd..bf4587193 100644 --- a/app/controllers/concerns/paginatable.rb +++ b/app/controllers/concerns/paginatable.rb @@ -20,7 +20,8 @@ def page_from_params(params) begin # When we decode and split, we'll always end up with an array # use urlsafe_decode to not worry about url-unsafe characters + and / - page[:cursor] = Base64.urlsafe_decode64(page[:cursor].to_s).split(",") + # split into two strings so that DOIs with comma in them are left intact + page[:cursor] = Base64.urlsafe_decode64(page[:cursor].to_s).split(",", 2) rescue ArgumentError # If we fail to decode we'll just default back to an empty cursor page[:cursor] = [] @@ -40,5 +41,10 @@ def page_from_params(params) page end + + def make_cursor(results) + # Base64-encode cursor + Base64.urlsafe_encode64(results.to_a.last[:sort], padding: false) + end end end diff --git a/app/controllers/dois_controller.rb b/app/controllers/dois_controller.rb index 3a5256bf6..443066933 100644 --- a/app/controllers/dois_controller.rb +++ b/app/controllers/dois_controller.rb @@ -195,7 +195,7 @@ def index "client-id" => params[:client_id], certificate: params[:certificate], # The cursor link should be an array of values, but we want to encode it into a single string for the URL - "page[cursor]" => page[:cursor] ? Base64.urlsafe_encode64(Array.wrap(results.to_a.last[:sort]).join(','), padding: false) : nil, + "page[cursor]" => page[:cursor] ? make_cursor(results) : nil, "page[number]" => page[:cursor].nil? && page[:number].present? ? page[:number] + 1 : nil, "page[size]" => page[:size] }.compact.to_query }.compact diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index b81ec8bd0..4c30555f3 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -204,7 +204,7 @@ def index "registrant-id" => params[:registrant_id], "publication-year" => params[:publication_year], "year-month" => params[:year_month], - "page[cursor]" => page[:cursor] ? Base64.urlsafe_encode64(Array.wrap(results.to_a.last[:sort]).join(","), padding: false) : nil, + "page[cursor]" => page[:cursor] ? make_cursor(results) : nil, "page[number]" => page[:cursor].nil? && page[:number].present? ? page[:number] + 1 : nil, "page[size]" => page[:size] }.compact.to_query }.compact @@ -216,7 +216,7 @@ def index if @include.include?(:dois) options[:include] = [] doi_names = (results.map { |event| event.doi}).uniq().join(",") - events_serialized[:included] = DoiSerializer.new((Doi.find_by_id(doi_names).results), {is_collection: true}).serializable_hash.dig(:data) + events_serialized[:included] = DoiSerializer.new((Doi.find_by_id(doi_names).results), is_collection: true).serializable_hash.dig(:data) end render json: events_serialized, status: :ok diff --git a/app/controllers/old_events_controller.rb b/app/controllers/old_events_controller.rb index 2809e3626..dba7d2b62 100644 --- a/app/controllers/old_events_controller.rb +++ b/app/controllers/old_events_controller.rb @@ -168,7 +168,7 @@ def index "registrant-id" => params[:registrant_id], "publication-year" => params[:publication_year], "year-month" => params[:year_month], - "page[cursor]" => page[:cursor] ? Base64.strict_encode64(Array.wrap(results.to_a.last[:sort]).join(',')) : nil, + "page[cursor]" => page[:cursor] ? make_cursor(results) : nil, "page[number]" => page[:cursor].nil? && page[:number].present? ? page[:number] + 1 : nil, "page[size]" => page[:size] }.compact.to_query }.compact diff --git a/app/models/event.rb b/app/models/event.rb index b33481850..70f23efa8 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -476,7 +476,7 @@ def self.update_datacite_orcid_auto_update(options = {}) break unless response.results.results.length > 0 logger.info "[Update] Updating #{response.results.results.length} datacite-orcid-auto-update events starting with _id #{response.results.to_a.first[:_id]}." - cursor = response.results.to_a.last[:sort].first.to_i + cursor = response.results.to_a.last[:sort] ids = response.results.results.map(&:obj_id).uniq OrcidAutoUpdateJob.perform_later(ids, options)