Make Array-creation behavior of Paginable more predictable (#14687)

* Make Array-creation behavior of Paginable more predictable

Paginable.paginate_by_id usually returns ActiveRecord::Relation, but it
returns an Array if min_id option is present. The behavior caused problems
fixed with the following commits:
- 552e886b64
- b63ede5005
- 64ef37b89d

To prevent from recurring similar problems, this commit introduces two
changes:
- The scope now always returns an Array whether min_id option is present
  or not.
- The scope is renamed to to_a_paginated_by_id to clarify it returns an
  Array.

* Transform Paginable.to_a_paginated_by_id from a scope to a class method

https://api.rubyonrails.org/classes/ActiveRecord/Scoping/Named/ClassMethods.html#method-i-scope
> The method is intended to return an ActiveRecord::Relation object, which
> is composable with other scopes.

Paginable.to_a_paginated_by_id returns an Array and is not appropriate
as a scope.
This commit is contained in:
Akihiko Odaki 2020-08-31 19:47:09 +09:00 committed by GitHub
parent 03b5f09ffc
commit ae871c4d46
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 13 additions and 13 deletions

View File

@ -79,7 +79,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController
private private
def set_accounts def set_accounts
@accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite]).paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) @accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
end end
def set_account def set_account

View File

@ -63,7 +63,7 @@ class Api::V1::Admin::ReportsController < Api::BaseController
private private
def set_reports def set_reports
@reports = filtered_reports.order(id: :desc).with_accounts.paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) @reports = filtered_reports.order(id: :desc).with_accounts.to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
end end
def set_report def set_report

View File

@ -21,7 +21,7 @@ class Api::V1::BookmarksController < Api::BaseController
end end
def results def results
@_results ||= account_bookmarks.eager_load(:status).paginate_by_id( @_results ||= account_bookmarks.eager_load(:status).to_a_paginated_by_id(
limit_param(DEFAULT_STATUSES_LIMIT), limit_param(DEFAULT_STATUSES_LIMIT),
params_slice(:max_id, :since_id, :min_id) params_slice(:max_id, :since_id, :min_id)
) )

View File

@ -32,7 +32,7 @@ class Api::V1::ConversationsController < Api::BaseController
def paginated_conversations def paginated_conversations
AccountConversation.where(account: current_account) AccountConversation.where(account: current_account)
.paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) .to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
end end
def insert_pagination_headers def insert_pagination_headers

View File

@ -26,7 +26,7 @@ class Api::V1::Crypto::EncryptedMessagesController < Api::BaseController
end end
def set_encrypted_messages def set_encrypted_messages
@encrypted_messages = @current_device.encrypted_messages.paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) @encrypted_messages = @current_device.encrypted_messages.to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
end end
def insert_pagination_headers def insert_pagination_headers

View File

@ -21,7 +21,7 @@ class Api::V1::FavouritesController < Api::BaseController
end end
def results def results
@_results ||= account_favourites.eager_load(:status).paginate_by_id( @_results ||= account_favourites.eager_load(:status).to_a_paginated_by_id(
limit_param(DEFAULT_STATUSES_LIMIT), limit_param(DEFAULT_STATUSES_LIMIT),
params_slice(:max_id, :since_id, :min_id) params_slice(:max_id, :since_id, :min_id)
) )

View File

@ -32,7 +32,7 @@ class Api::V1::ScheduledStatusesController < Api::BaseController
private private
def set_statuses def set_statuses
@statuses = current_account.scheduled_statuses.paginate_by_id(limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id)) @statuses = current_account.scheduled_statuses.to_a_paginated_by_id(limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id))
end end
def set_status def set_status

View File

@ -49,6 +49,6 @@ module CacheConcern
end end
def cache_collection_paginated_by_id(raw, klass, limit, options) def cache_collection_paginated_by_id(raw, klass, limit, options)
cache_collection raw.cache_ids.paginate_by_id(limit, options), klass cache_collection raw.cache_ids.to_a_paginated_by_id(limit, options), klass
end end
end end

View File

@ -36,11 +36,11 @@ class AccountConversation < ApplicationRecord
end end
class << self class << self
def paginate_by_id(limit, options = {}) def to_a_paginated_by_id(limit, options = {})
if options[:min_id] if options[:min_id]
paginate_by_min_id(limit, options[:min_id]).reverse paginate_by_min_id(limit, options[:min_id]).reverse
else else
paginate_by_max_id(limit, options[:max_id], options[:since_id]) paginate_by_max_id(limit, options[:max_id], options[:since_id]).to_a
end end
end end

View File

@ -20,12 +20,12 @@ module Paginable
query query
} }
scope :paginate_by_id, ->(limit, options = {}) { def self.to_a_paginated_by_id(limit, options = {})
if options[:min_id].present? if options[:min_id].present?
paginate_by_min_id(limit, options[:min_id]).reverse paginate_by_min_id(limit, options[:min_id]).reverse
else else
paginate_by_max_id(limit, options[:max_id], options[:since_id]) paginate_by_max_id(limit, options[:max_id], options[:since_id]).to_a
end
end end
}
end end
end end