Support min_id-based pagination in REST API (#8736)
* Allow min_id pagination in Feed#get * Add min_id pagination to home and list timeline APIs * Add min_id pagination to account statuses, public and tag APIs * Remove unused stub in reports API * Use min_id pagination in notifications, favourites, and fix order * Fix HomeFeed#from_database not using paginate_by_id
This commit is contained in:
		
							parent
							
								
									3d7f68c273
								
							
						
					
					
						commit
						f0fff3eb10
					
				
					 15 changed files with 49 additions and 51 deletions
				
			
		| 
						 | 
					@ -53,6 +53,10 @@ class Api::BaseController < ApplicationController
 | 
				
			||||||
    [params[:limit].to_i.abs, default_limit * 2].min
 | 
					    [params[:limit].to_i.abs, default_limit * 2].min
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def params_slice(*keys)
 | 
				
			||||||
 | 
					    params.slice(*keys).permit(*keys)
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def current_resource_owner
 | 
					  def current_resource_owner
 | 
				
			||||||
    @current_user ||= User.find(doorkeeper_token.resource_owner_id) if doorkeeper_token
 | 
					    @current_user ||= User.find(doorkeeper_token.resource_owner_id) if doorkeeper_token
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -28,10 +28,9 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def account_statuses
 | 
					  def account_statuses
 | 
				
			||||||
    statuses = truthy_param?(:pinned) ? pinned_scope : permitted_account_statuses
 | 
					    statuses = truthy_param?(:pinned) ? pinned_scope : permitted_account_statuses
 | 
				
			||||||
    statuses = statuses.paginate_by_max_id(
 | 
					    statuses = statuses.paginate_by_id(
 | 
				
			||||||
      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
					      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
				
			||||||
      params[:max_id],
 | 
					      params_slice(:max_id, :since_id, :min_id)
 | 
				
			||||||
      params[:since_id]
 | 
					 | 
				
			||||||
    )
 | 
					    )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    statuses.merge!(only_media_scope) if truthy_param?(:only_media)
 | 
					    statuses.merge!(only_media_scope) if truthy_param?(:only_media)
 | 
				
			||||||
| 
						 | 
					@ -82,7 +81,7 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def prev_path
 | 
					  def prev_path
 | 
				
			||||||
    unless @statuses.empty?
 | 
					    unless @statuses.empty?
 | 
				
			||||||
      api_v1_account_statuses_url pagination_params(since_id: pagination_since_id)
 | 
					      api_v1_account_statuses_url pagination_params(min_id: pagination_since_id)
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -26,10 +26,9 @@ class Api::V1::FavouritesController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def results
 | 
					  def results
 | 
				
			||||||
    @_results ||= account_favourites.paginate_by_max_id(
 | 
					    @_results ||= account_favourites.paginate_by_id(
 | 
				
			||||||
      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
					      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
				
			||||||
      params[:max_id],
 | 
					      params_slice(:max_id, :since_id, :min_id)
 | 
				
			||||||
      params[:since_id]
 | 
					 | 
				
			||||||
    )
 | 
					    )
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -49,7 +48,7 @@ class Api::V1::FavouritesController < Api::BaseController
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def prev_path
 | 
					  def prev_path
 | 
				
			||||||
    unless results.empty?
 | 
					    unless results.empty?
 | 
				
			||||||
      api_v1_favourites_url pagination_params(since_id: pagination_since_id)
 | 
					      api_v1_favourites_url pagination_params(min_id: pagination_since_id)
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -37,10 +37,9 @@ class Api::V1::NotificationsController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def paginated_notifications
 | 
					  def paginated_notifications
 | 
				
			||||||
    browserable_account_notifications.paginate_by_max_id(
 | 
					    browserable_account_notifications.paginate_by_id(
 | 
				
			||||||
      limit_param(DEFAULT_NOTIFICATIONS_LIMIT),
 | 
					      limit_param(DEFAULT_NOTIFICATIONS_LIMIT),
 | 
				
			||||||
      params[:max_id],
 | 
					      params_slice(:max_id, :since_id, :min_id)
 | 
				
			||||||
      params[:since_id]
 | 
					 | 
				
			||||||
    )
 | 
					    )
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -64,7 +63,7 @@ class Api::V1::NotificationsController < Api::BaseController
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def prev_path
 | 
					  def prev_path
 | 
				
			||||||
    unless @notifications.empty?
 | 
					    unless @notifications.empty?
 | 
				
			||||||
      api_v1_notifications_url pagination_params(since_id: pagination_since_id)
 | 
					      api_v1_notifications_url pagination_params(min_id: pagination_since_id)
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -7,11 +7,6 @@ class Api::V1::ReportsController < Api::BaseController
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  respond_to :json
 | 
					  respond_to :json
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def index
 | 
					 | 
				
			||||||
    @reports = current_account.reports
 | 
					 | 
				
			||||||
    render json: @reports, each_serializer: REST::ReportSerializer
 | 
					 | 
				
			||||||
  end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  def create
 | 
					  def create
 | 
				
			||||||
    @report = ReportService.new.call(
 | 
					    @report = ReportService.new.call(
 | 
				
			||||||
      current_account,
 | 
					      current_account,
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -30,7 +30,8 @@ class Api::V1::Timelines::HomeController < Api::BaseController
 | 
				
			||||||
    account_home_feed.get(
 | 
					    account_home_feed.get(
 | 
				
			||||||
      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
					      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
				
			||||||
      params[:max_id],
 | 
					      params[:max_id],
 | 
				
			||||||
      params[:since_id]
 | 
					      params[:since_id],
 | 
				
			||||||
 | 
					      params[:min_id]
 | 
				
			||||||
    )
 | 
					    )
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -51,7 +52,7 @@ class Api::V1::Timelines::HomeController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def prev_path
 | 
					  def prev_path
 | 
				
			||||||
    api_v1_timelines_home_url pagination_params(since_id: pagination_since_id)
 | 
					    api_v1_timelines_home_url pagination_params(min_id: pagination_since_id)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def pagination_max_id
 | 
					  def pagination_max_id
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -32,7 +32,8 @@ class Api::V1::Timelines::ListController < Api::BaseController
 | 
				
			||||||
    list_feed.get(
 | 
					    list_feed.get(
 | 
				
			||||||
      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
					      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
				
			||||||
      params[:max_id],
 | 
					      params[:max_id],
 | 
				
			||||||
      params[:since_id]
 | 
					      params[:since_id],
 | 
				
			||||||
 | 
					      params[:min_id]
 | 
				
			||||||
    )
 | 
					    )
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -53,7 +54,7 @@ class Api::V1::Timelines::ListController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def prev_path
 | 
					  def prev_path
 | 
				
			||||||
    api_v1_timelines_list_url params[:id], pagination_params(since_id: pagination_since_id)
 | 
					    api_v1_timelines_list_url params[:id], pagination_params(min_id: pagination_since_id)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def pagination_max_id
 | 
					  def pagination_max_id
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -21,10 +21,9 @@ class Api::V1::Timelines::PublicController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def public_statuses
 | 
					  def public_statuses
 | 
				
			||||||
    statuses = public_timeline_statuses.paginate_by_max_id(
 | 
					    statuses = public_timeline_statuses.paginate_by_id(
 | 
				
			||||||
      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
					      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
				
			||||||
      params[:max_id],
 | 
					      params_slice(:max_id, :since_id, :min_id)
 | 
				
			||||||
      params[:since_id]
 | 
					 | 
				
			||||||
    )
 | 
					    )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if truthy_param?(:only_media)
 | 
					    if truthy_param?(:only_media)
 | 
				
			||||||
| 
						 | 
					@ -53,7 +52,7 @@ class Api::V1::Timelines::PublicController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def prev_path
 | 
					  def prev_path
 | 
				
			||||||
    api_v1_timelines_public_url pagination_params(since_id: pagination_since_id)
 | 
					    api_v1_timelines_public_url pagination_params(min_id: pagination_since_id)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def pagination_max_id
 | 
					  def pagination_max_id
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -29,10 +29,9 @@ class Api::V1::Timelines::TagController < Api::BaseController
 | 
				
			||||||
    if @tag.nil?
 | 
					    if @tag.nil?
 | 
				
			||||||
      []
 | 
					      []
 | 
				
			||||||
    else
 | 
					    else
 | 
				
			||||||
      statuses = tag_timeline_statuses.paginate_by_max_id(
 | 
					      statuses = tag_timeline_statuses.paginate_by_id(
 | 
				
			||||||
        limit_param(DEFAULT_STATUSES_LIMIT),
 | 
					        limit_param(DEFAULT_STATUSES_LIMIT),
 | 
				
			||||||
        params[:max_id],
 | 
					        params_slice(:max_id, :since_id, :min_id)
 | 
				
			||||||
        params[:since_id]
 | 
					 | 
				
			||||||
      )
 | 
					      )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      if truthy_param?(:only_media)
 | 
					      if truthy_param?(:only_media)
 | 
				
			||||||
| 
						 | 
					@ -62,7 +61,7 @@ class Api::V1::Timelines::TagController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def prev_path
 | 
					  def prev_path
 | 
				
			||||||
    api_v1_timelines_tag_url params[:id], pagination_params(since_id: pagination_since_id)
 | 
					    api_v1_timelines_tag_url params[:id], pagination_params(min_id: pagination_since_id)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def pagination_max_id
 | 
					  def pagination_max_id
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -19,5 +19,13 @@ module Paginable
 | 
				
			||||||
      query = query.where(arel_table[:id].gt(min_id)) if min_id.present?
 | 
					      query = query.where(arel_table[:id].gt(min_id)) if min_id.present?
 | 
				
			||||||
      query
 | 
					      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
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -6,16 +6,20 @@ class Feed
 | 
				
			||||||
    @id   = id
 | 
					    @id   = id
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def get(limit, max_id = nil, since_id = nil)
 | 
					  def get(limit, max_id = nil, since_id = nil, min_id = nil)
 | 
				
			||||||
    from_redis(limit, max_id, since_id)
 | 
					    from_redis(limit, max_id, since_id, min_id)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  protected
 | 
					  protected
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def from_redis(limit, max_id, since_id)
 | 
					  def from_redis(limit, max_id, since_id, min_id)
 | 
				
			||||||
 | 
					    if min_id.blank?
 | 
				
			||||||
      max_id     = '+inf' if max_id.blank?
 | 
					      max_id     = '+inf' if max_id.blank?
 | 
				
			||||||
      since_id   = '-inf' if since_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)
 | 
					      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
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    Status.where(id: unhydrated).cache_ids
 | 
					    Status.where(id: unhydrated).cache_ids
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -7,9 +7,9 @@ class HomeFeed < Feed
 | 
				
			||||||
    @account = account
 | 
					    @account = account
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def get(limit, max_id = nil, since_id = nil)
 | 
					  def get(limit, max_id = nil, since_id = nil, min_id = nil)
 | 
				
			||||||
    if redis.exists("account:#{@account.id}:regeneration")
 | 
					    if redis.exists("account:#{@account.id}:regeneration")
 | 
				
			||||||
      from_database(limit, max_id, since_id)
 | 
					      from_database(limit, max_id, since_id, min_id)
 | 
				
			||||||
    else
 | 
					    else
 | 
				
			||||||
      super
 | 
					      super
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
| 
						 | 
					@ -17,9 +17,9 @@ class HomeFeed < Feed
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  private
 | 
					  private
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def from_database(limit, max_id, since_id)
 | 
					  def from_database(limit, max_id, since_id, min_id)
 | 
				
			||||||
    Status.as_home_timeline(@account)
 | 
					    Status.as_home_timeline(@account)
 | 
				
			||||||
          .paginate_by_max_id(limit, max_id, since_id)
 | 
					          .paginate_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
 | 
				
			||||||
          .reject { |status| FeedManager.instance.filter?(:home, status, @account.id) }
 | 
					          .reject { |status| FeedManager.instance.filter?(:home, status, @account.id) }
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -268,7 +268,7 @@ Rails.application.routes.draw do
 | 
				
			||||||
      resources :blocks,       only: [:index]
 | 
					      resources :blocks,       only: [:index]
 | 
				
			||||||
      resources :mutes,        only: [:index]
 | 
					      resources :mutes,        only: [:index]
 | 
				
			||||||
      resources :favourites,   only: [:index]
 | 
					      resources :favourites,   only: [:index]
 | 
				
			||||||
      resources :reports,      only: [:index, :create]
 | 
					      resources :reports,      only: [:create]
 | 
				
			||||||
      resources :filters,      only: [:index, :create, :show, :update, :destroy]
 | 
					      resources :filters,      only: [:index, :create, :show, :update, :destroy]
 | 
				
			||||||
      resources :endorsements, only: [:index]
 | 
					      resources :endorsements, only: [:index]
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -64,7 +64,7 @@ RSpec.describe Api::V1::FavouritesController, type: :controller do
 | 
				
			||||||
          get :index, params: { limit: 1 }
 | 
					          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', '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&since_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}"
 | 
				
			||||||
        end
 | 
					        end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        it 'does not add pagination headers if not necessary' do
 | 
					        it 'does not add pagination headers if not necessary' do
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -12,16 +12,6 @@ RSpec.describe Api::V1::ReportsController, type: :controller do
 | 
				
			||||||
    allow(controller).to receive(:doorkeeper_token) { token }
 | 
					    allow(controller).to receive(:doorkeeper_token) { token }
 | 
				
			||||||
  end
 | 
					  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
 | 
					  describe 'POST #create' do
 | 
				
			||||||
    let(:scopes)  { 'write:reports' }
 | 
					    let(:scopes)  { 'write:reports' }
 | 
				
			||||||
    let!(:status) { Fabricate(:status) }
 | 
					    let!(:status) { Fabricate(:status) }
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue