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

diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb
index 90f42251e13531b7fe6a0eb0cc29e8bbf82f81c8..ac8de5fc0f48776c86cb63621d04b25edb94601f 100644 (file)
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@@ -53,6 +53,10 @@ class Api::BaseController < ApplicationController
     [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 06fa6c762399f70603e8e6203011489558b517e4..b68a8805fa17baa6d1f7040b435dbd8e81d297af 100644 (file)
--- a/app/controllers/api/v1/accounts/statuses_controller.rb
+++ b/app/controllers/api/v1/accounts/statuses_controller.rb
@@ -28,10 +28,9 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
 
   def 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),
-      params[:max_id],
-      params[:since_id]
+      params_slice(:max_id, :since_id, :min_id)
     )
 
     statuses.merge!(only_media_scope) if truthy_param?(:only_media)
@@ -82,7 +81,7 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
 
   def prev_path
     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
 

diff --git a/app/controllers/api/v1/favourites_controller.rb b/app/controllers/api/v1/favourites_controller.rb
index ab5204355cd620a4b6f5765c6ed4d51e07b7eb35..db827f9d4aa2aae2452c42fe2f475520e73e266f 100644 (file)
--- a/app/controllers/api/v1/favourites_controller.rb
+++ b/app/controllers/api/v1/favourites_controller.rb
@@ -26,10 +26,9 @@ class Api::V1::FavouritesController < Api::BaseController
   end
 
   def results
-    @_results ||= account_favourites.paginate_by_max_id(
+    @_results ||= account_favourites.paginate_by_id(
       limit_param(DEFAULT_STATUSES_LIMIT),
-      params[:max_id],
-      params[:since_id]
+      params_slice(:max_id, :since_id, :min_id)
     )
   end
 
@@ -49,7 +48,7 @@ class Api::V1::FavouritesController < Api::BaseController
 
   def prev_path
     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
 

diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb
index 593c8f9a9c8a201e4218479bd642a5f0a3a67fca..e2dec62afaef4a4d07cf982649369f6042515239 100644 (file)
--- a/app/controllers/api/v1/notifications_controller.rb
+++ b/app/controllers/api/v1/notifications_controller.rb
@@ -37,10 +37,9 @@ class Api::V1::NotificationsController < Api::BaseController
   end
 
   def paginated_notifications
-    browserable_account_notifications.paginate_by_max_id(
+    browserable_account_notifications.paginate_by_id(
       limit_param(DEFAULT_NOTIFICATIONS_LIMIT),
-      params[:max_id],
-      params[:since_id]
+      params_slice(:max_id, :since_id, :min_id)
     )
   end
 
@@ -64,7 +63,7 @@ class Api::V1::NotificationsController < Api::BaseController
 
   def prev_path
     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
 

diff --git a/app/controllers/api/v1/reports_controller.rb b/app/controllers/api/v1/reports_controller.rb
index a954101cb8fee46da9ed91f1cd807dc96e5d22ac..9c6ee0a50c6e933709acf284b1a621cfd0ed00b6 100644 (file)
--- a/app/controllers/api/v1/reports_controller.rb
+++ b/app/controllers/api/v1/reports_controller.rb
@@ -7,11 +7,6 @@ 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 4412aaaa391c7f0dca66d03e7741e476c5aee86e..fcd0757f1a87c0d6792cfb12de536d9ef46c4643 100644 (file)
--- a/app/controllers/api/v1/timelines/home_controller.rb
+++ b/app/controllers/api/v1/timelines/home_controller.rb
@@ -30,7 +30,8 @@ class Api::V1::Timelines::HomeController < Api::BaseController
     account_home_feed.get(
       limit_param(DEFAULT_STATUSES_LIMIT),
       params[:max_id],
-      params[:since_id]
+      params[:since_id],
+      params[:min_id]
     )
   end
 
@@ -51,7 +52,7 @@ class Api::V1::Timelines::HomeController < Api::BaseController
   end
 
   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
 
   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 cfc5f3b5e4d68326ffe03161f96ec5dd65863c93..a15eae468d92b44f0c3228192d144d9e045ba034 100644 (file)
--- a/app/controllers/api/v1/timelines/list_controller.rb
+++ b/app/controllers/api/v1/timelines/list_controller.rb
@@ -32,7 +32,8 @@ class Api::V1::Timelines::ListController < Api::BaseController
     list_feed.get(
       limit_param(DEFAULT_STATUSES_LIMIT),
       params[:max_id],
-      params[:since_id]
+      params[:since_id],
+      params[:min_id]
     )
   end
 
@@ -53,7 +54,7 @@ class Api::V1::Timelines::ListController < Api::BaseController
   end
 
   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
 
   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 13fe015b7df335b550b1678e313517d11238a2cf..aabe24324312421b5f9c0765b3a7c037907ab9df 100644 (file)
--- a/app/controllers/api/v1/timelines/public_controller.rb
+++ b/app/controllers/api/v1/timelines/public_controller.rb
@@ -21,10 +21,9 @@ class Api::V1::Timelines::PublicController < Api::BaseController
   end
 
   def public_statuses
-    statuses = public_timeline_statuses.paginate_by_max_id(
+    statuses = public_timeline_statuses.paginate_by_id(
       limit_param(DEFAULT_STATUSES_LIMIT),
-      params[:max_id],
-      params[:since_id]
+      params_slice(:max_id, :since_id, :min_id)
     )
 
     if truthy_param?(:only_media)
@@ -53,7 +52,7 @@ class Api::V1::Timelines::PublicController < Api::BaseController
   end
 
   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
 
   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 7de49a5ed683598a6fc2392fb2edbd88399dad63..cf58d5cf40b79198ef0b1cadd1bf6af8bdb56422 100644 (file)
--- a/app/controllers/api/v1/timelines/tag_controller.rb
+++ b/app/controllers/api/v1/timelines/tag_controller.rb
@@ -29,10 +29,9 @@ class Api::V1::Timelines::TagController < Api::BaseController
     if @tag.nil?
       []
     else
-      statuses = tag_timeline_statuses.paginate_by_max_id(
+      statuses = tag_timeline_statuses.paginate_by_id(
         limit_param(DEFAULT_STATUSES_LIMIT),
-        params[:max_id],
-        params[:since_id]
+        params_slice(:max_id, :since_id, :min_id)
       )
 
       if truthy_param?(:only_media)
@@ -62,7 +61,7 @@ class Api::V1::Timelines::TagController < Api::BaseController
   end
 
   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
 
   def pagination_max_id

diff --git a/app/models/concerns/paginable.rb b/app/models/concerns/paginable.rb
index 66695677e9af6990bca0382c2b76d45bc2990a45..52f1869184fa074bace587c30eef497b4b386488 100644 (file)
--- a/app/models/concerns/paginable.rb
+++ b/app/models/concerns/paginable.rb
@@ -19,5 +19,13 @@ 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 d99f1ffb2c7620025fc0cdaf627d73ddd7a5d3d5..5bce88f25522df0a9797a30672e46195e30c2793 100644 (file)
--- a/app/models/feed.rb
+++ b/app/models/feed.rb
@@ -6,16 +6,20 @@ class Feed
     @id   = id
   end
 
-  def get(limit, max_id = nil, since_id = nil)
-    from_redis(limit, max_id, since_id)
+  def get(limit, max_id = nil, since_id = nil, min_id = nil)
+    from_redis(limit, max_id, since_id, min_id)
   end
 
   protected
 
-  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)
+  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
 
     Status.where(id: unhydrated).cache_ids
   end

diff --git a/app/models/home_feed.rb b/app/models/home_feed.rb
index b943a34ce19ab4889ca13c801cbde91a97efe01a..ba7564983b0f8bc9820759773c7d53b0ec075578 100644 (file)
--- a/app/models/home_feed.rb
+++ b/app/models/home_feed.rb
@@ -7,9 +7,9 @@ class HomeFeed < Feed
     @account = account
   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")
-      from_database(limit, max_id, since_id)
+      from_database(limit, max_id, since_id, min_id)
     else
       super
     end
@@ -17,9 +17,9 @@ class HomeFeed < Feed
 
   private
 
-  def from_database(limit, max_id, since_id)
+  def from_database(limit, max_id, since_id, min_id)
     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) }
   end
 end

diff --git a/config/routes.rb b/config/routes.rb
index 877823c7fca5e27282586d82d31c3617fb8c31b7..35e4bdbf9b5ef207128ed89e36d05a9d5dca1c82 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -268,7 +268,7 @@ Rails.application.routes.draw do
       resources :blocks,       only: [:index]
       resources :mutes,        only: [:index]
       resources :favourites,   only: [:index]
-      resources :reports,      only: [:index, :create]
+      resources :reports,      only: [: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 2bdf927f21f7db2b75451ae87da9339896fefae3..231f765008ed2792cc9d51342dea95b680a0c142 100644 (file)
--- a/spec/controllers/api/v1/favourites_controller_spec.rb
+++ b/spec/controllers/api/v1/favourites_controller_spec.rb
@@ -64,7 +64,7 @@ RSpec.describe Api::V1::FavouritesController, type: :controller do
           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&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
 
         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 ac93998c6901d0b2be8ad4b7df1593416ba57d65..a3596cf8a0387640c4deca777327d3ac3f6f2e21 100644 (file)
--- a/spec/controllers/api/v1/reports_controller_spec.rb
+++ b/spec/controllers/api/v1/reports_controller_spec.rb
@@ -12,16 +12,6 @@ RSpec.describe Api::V1::ReportsController, type: :controller do
     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) }