From 2511af65334ad4fcfda39dfad3f2ebf34fb6ee93 Mon Sep 17 00:00:00 2001 From: zunda Date: Fri, 28 Sep 2018 16:14:26 -1000 Subject: [PATCH] Revert "Support min_id-based pagination in REST API (#8736)" This reverts commit f0fff3eb1051ff77ec3f37aa75f8c56720b626a3. --- app/controllers/api/base_controller.rb | 4 ---- .../api/v1/accounts/statuses_controller.rb | 7 ++++--- app/controllers/api/v1/favourites_controller.rb | 7 ++++--- .../api/v1/notifications_controller.rb | 7 ++++--- app/controllers/api/v1/reports_controller.rb | 5 +++++ .../api/v1/timelines/home_controller.rb | 5 ++--- .../api/v1/timelines/list_controller.rb | 5 ++--- .../api/v1/timelines/public_controller.rb | 7 ++++--- .../api/v1/timelines/tag_controller.rb | 7 ++++--- app/models/concerns/paginable.rb | 8 -------- app/models/feed.rb | 16 ++++++---------- app/models/home_feed.rb | 8 ++++---- config/routes.rb | 2 +- .../api/v1/favourites_controller_spec.rb | 2 +- .../api/v1/reports_controller_spec.rb | 10 ++++++++++ 15 files changed, 51 insertions(+), 49 deletions(-) diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index ac8de5fc0f4877..90f42251e13531 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -53,10 +53,6 @@ def limit_param(default_limit) [params[:limit].to_i.abs, default_limit * 2].min end - def params_slice(*keys) - params.slice(*keys).permit(*keys) - end - def current_resource_owner @current_user ||= User.find(doorkeeper_token.resource_owner_id) if doorkeeper_token end diff --git a/app/controllers/api/v1/accounts/statuses_controller.rb b/app/controllers/api/v1/accounts/statuses_controller.rb index b68a8805fa17ba..06fa6c762399f7 100644 --- a/app/controllers/api/v1/accounts/statuses_controller.rb +++ b/app/controllers/api/v1/accounts/statuses_controller.rb @@ -28,9 +28,10 @@ def cached_account_statuses def account_statuses statuses = truthy_param?(:pinned) ? pinned_scope : permitted_account_statuses - statuses = statuses.paginate_by_id( + statuses = statuses.paginate_by_max_id( limit_param(DEFAULT_STATUSES_LIMIT), - params_slice(:max_id, :since_id, :min_id) + params[:max_id], + params[:since_id] ) statuses.merge!(only_media_scope) if truthy_param?(:only_media) @@ -81,7 +82,7 @@ def next_path def prev_path unless @statuses.empty? - api_v1_account_statuses_url pagination_params(min_id: pagination_since_id) + api_v1_account_statuses_url pagination_params(since_id: pagination_since_id) end end diff --git a/app/controllers/api/v1/favourites_controller.rb b/app/controllers/api/v1/favourites_controller.rb index db827f9d4aa2aa..ab5204355cd620 100644 --- a/app/controllers/api/v1/favourites_controller.rb +++ b/app/controllers/api/v1/favourites_controller.rb @@ -26,9 +26,10 @@ def cached_favourites end def results - @_results ||= account_favourites.paginate_by_id( + @_results ||= account_favourites.paginate_by_max_id( limit_param(DEFAULT_STATUSES_LIMIT), - params_slice(:max_id, :since_id, :min_id) + params[:max_id], + params[:since_id] ) end @@ -48,7 +49,7 @@ def next_path def prev_path unless results.empty? - api_v1_favourites_url pagination_params(min_id: pagination_since_id) + api_v1_favourites_url pagination_params(since_id: pagination_since_id) end end diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index e2dec62afaef4a..593c8f9a9c8a20 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -37,9 +37,10 @@ def load_notifications end def paginated_notifications - browserable_account_notifications.paginate_by_id( + browserable_account_notifications.paginate_by_max_id( limit_param(DEFAULT_NOTIFICATIONS_LIMIT), - params_slice(:max_id, :since_id, :min_id) + params[:max_id], + params[:since_id] ) end @@ -63,7 +64,7 @@ def next_path def prev_path unless @notifications.empty? - api_v1_notifications_url pagination_params(min_id: pagination_since_id) + api_v1_notifications_url pagination_params(since_id: pagination_since_id) end end diff --git a/app/controllers/api/v1/reports_controller.rb b/app/controllers/api/v1/reports_controller.rb index 9c6ee0a50c6e93..a954101cb8fee4 100644 --- a/app/controllers/api/v1/reports_controller.rb +++ b/app/controllers/api/v1/reports_controller.rb @@ -7,6 +7,11 @@ class Api::V1::ReportsController < Api::BaseController respond_to :json + def index + @reports = current_account.reports + render json: @reports, each_serializer: REST::ReportSerializer + end + def create @report = ReportService.new.call( current_account, diff --git a/app/controllers/api/v1/timelines/home_controller.rb b/app/controllers/api/v1/timelines/home_controller.rb index fcd0757f1a87c0..4412aaaa391c7f 100644 --- a/app/controllers/api/v1/timelines/home_controller.rb +++ b/app/controllers/api/v1/timelines/home_controller.rb @@ -30,8 +30,7 @@ def home_statuses account_home_feed.get( limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], - params[:since_id], - params[:min_id] + params[:since_id] ) end @@ -52,7 +51,7 @@ def next_path end def prev_path - api_v1_timelines_home_url pagination_params(min_id: pagination_since_id) + api_v1_timelines_home_url pagination_params(since_id: pagination_since_id) end def pagination_max_id diff --git a/app/controllers/api/v1/timelines/list_controller.rb b/app/controllers/api/v1/timelines/list_controller.rb index a15eae468d92b4..cfc5f3b5e4d683 100644 --- a/app/controllers/api/v1/timelines/list_controller.rb +++ b/app/controllers/api/v1/timelines/list_controller.rb @@ -32,8 +32,7 @@ def list_statuses list_feed.get( limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], - params[:since_id], - params[:min_id] + params[:since_id] ) end @@ -54,7 +53,7 @@ def next_path end def prev_path - api_v1_timelines_list_url params[:id], pagination_params(min_id: pagination_since_id) + api_v1_timelines_list_url params[:id], pagination_params(since_id: pagination_since_id) end def pagination_max_id diff --git a/app/controllers/api/v1/timelines/public_controller.rb b/app/controllers/api/v1/timelines/public_controller.rb index aabe2432431242..13fe015b7df335 100644 --- a/app/controllers/api/v1/timelines/public_controller.rb +++ b/app/controllers/api/v1/timelines/public_controller.rb @@ -21,9 +21,10 @@ def cached_public_statuses end def public_statuses - statuses = public_timeline_statuses.paginate_by_id( + statuses = public_timeline_statuses.paginate_by_max_id( limit_param(DEFAULT_STATUSES_LIMIT), - params_slice(:max_id, :since_id, :min_id) + params[:max_id], + params[:since_id] ) if truthy_param?(:only_media) @@ -52,7 +53,7 @@ def next_path end def prev_path - api_v1_timelines_public_url pagination_params(min_id: pagination_since_id) + api_v1_timelines_public_url pagination_params(since_id: pagination_since_id) end def pagination_max_id diff --git a/app/controllers/api/v1/timelines/tag_controller.rb b/app/controllers/api/v1/timelines/tag_controller.rb index cf58d5cf40b791..7de49a5ed68359 100644 --- a/app/controllers/api/v1/timelines/tag_controller.rb +++ b/app/controllers/api/v1/timelines/tag_controller.rb @@ -29,9 +29,10 @@ def tagged_statuses if @tag.nil? [] else - statuses = tag_timeline_statuses.paginate_by_id( + statuses = tag_timeline_statuses.paginate_by_max_id( limit_param(DEFAULT_STATUSES_LIMIT), - params_slice(:max_id, :since_id, :min_id) + params[:max_id], + params[:since_id] ) if truthy_param?(:only_media) @@ -61,7 +62,7 @@ def next_path end def prev_path - api_v1_timelines_tag_url params[:id], pagination_params(min_id: pagination_since_id) + api_v1_timelines_tag_url params[:id], pagination_params(since_id: pagination_since_id) end def pagination_max_id diff --git a/app/models/concerns/paginable.rb b/app/models/concerns/paginable.rb index 52f1869184fa07..66695677e9af69 100644 --- a/app/models/concerns/paginable.rb +++ b/app/models/concerns/paginable.rb @@ -19,13 +19,5 @@ module Paginable query = query.where(arel_table[:id].gt(min_id)) if min_id.present? query } - - scope :paginate_by_id, ->(limit, **options) { - if options[:min_id].present? - paginate_by_min_id(limit, options[:min_id]).reverse - else - paginate_by_max_id(limit, options[:max_id], options[:since_id]) - end - } end end diff --git a/app/models/feed.rb b/app/models/feed.rb index 5bce88f25522df..d99f1ffb2c7620 100644 --- a/app/models/feed.rb +++ b/app/models/feed.rb @@ -6,20 +6,16 @@ def initialize(type, id) @id = id end - def get(limit, max_id = nil, since_id = nil, min_id = nil) - from_redis(limit, max_id, since_id, min_id) + def get(limit, max_id = nil, since_id = nil) + from_redis(limit, max_id, since_id) end protected - def from_redis(limit, max_id, since_id, min_id) - if min_id.blank? - max_id = '+inf' if max_id.blank? - since_id = '-inf' if since_id.blank? - unhydrated = redis.zrevrangebyscore(key, "(#{max_id}", "(#{since_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i) - else - unhydrated = redis.zrangebyscore(key, "(#{min_id}", '+inf', limit: [0, limit], with_scores: true).map(&:first).map(&:to_i) - end + def from_redis(limit, max_id, since_id) + max_id = '+inf' if max_id.blank? + since_id = '-inf' if since_id.blank? + unhydrated = redis.zrevrangebyscore(key, "(#{max_id}", "(#{since_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i) Status.where(id: unhydrated).cache_ids end diff --git a/app/models/home_feed.rb b/app/models/home_feed.rb index ba7564983b0f8b..b943a34ce19ab4 100644 --- a/app/models/home_feed.rb +++ b/app/models/home_feed.rb @@ -7,9 +7,9 @@ def initialize(account) @account = account end - def get(limit, max_id = nil, since_id = nil, min_id = nil) + def get(limit, max_id = nil, since_id = nil) if redis.exists("account:#{@account.id}:regeneration") - from_database(limit, max_id, since_id, min_id) + from_database(limit, max_id, since_id) else super end @@ -17,9 +17,9 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil) private - def from_database(limit, max_id, since_id, min_id) + def from_database(limit, max_id, since_id) Status.as_home_timeline(@account) - .paginate_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id) + .paginate_by_max_id(limit, max_id, since_id) .reject { |status| FeedManager.instance.filter?(:home, status, @account.id) } end end diff --git a/config/routes.rb b/config/routes.rb index 35e4bdbf9b5ef2..877823c7fca5e2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -268,7 +268,7 @@ resources :blocks, only: [:index] resources :mutes, only: [:index] resources :favourites, only: [:index] - resources :reports, only: [:create] + resources :reports, only: [:index, :create] resources :filters, only: [:index, :create, :show, :update, :destroy] resources :endorsements, only: [:index] diff --git a/spec/controllers/api/v1/favourites_controller_spec.rb b/spec/controllers/api/v1/favourites_controller_spec.rb index 231f765008ed27..2bdf927f21f7db 100644 --- a/spec/controllers/api/v1/favourites_controller_spec.rb +++ b/spec/controllers/api/v1/favourites_controller_spec.rb @@ -64,7 +64,7 @@ get :index, params: { limit: 1 } expect(response.headers['Link'].find_link(['rel', 'next']).href).to eq "http://test.host/api/v1/favourites?limit=1&max_id=#{favourite.id}" - expect(response.headers['Link'].find_link(['rel', 'prev']).href).to eq "http://test.host/api/v1/favourites?limit=1&min_id=#{favourite.id}" + expect(response.headers['Link'].find_link(['rel', 'prev']).href).to eq "http://test.host/api/v1/favourites?limit=1&since_id=#{favourite.id}" end it 'does not add pagination headers if not necessary' do diff --git a/spec/controllers/api/v1/reports_controller_spec.rb b/spec/controllers/api/v1/reports_controller_spec.rb index a3596cf8a03876..ac93998c6901d0 100644 --- a/spec/controllers/api/v1/reports_controller_spec.rb +++ b/spec/controllers/api/v1/reports_controller_spec.rb @@ -12,6 +12,16 @@ allow(controller).to receive(:doorkeeper_token) { token } end + describe 'GET #index' do + let(:scopes) { 'read:reports' } + + it 'returns http success' do + get :index + + expect(response).to have_http_status(200) + end + end + describe 'POST #create' do let(:scopes) { 'write:reports' } let!(:status) { Fabricate(:status) }