From 2fe1b8d1695d8faa452a69872fde94ccc4611576 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 6 May 2024 17:19:15 +0200 Subject: [PATCH] Add API to get multiple accounts and statuses (#27871) Co-authored-by: noellabo --- app/controllers/api/v1/accounts_controller.rb | 30 ++++++++++++++++--- app/controllers/api/v1/statuses_controller.rb | 29 ++++++++++++++++-- .../concerns/status/threading_concern.rb | 27 +++++++++++------ config/routes/api.rb | 4 +-- spec/requests/api/v1/accounts_spec.rb | 16 ++++++++++ spec/requests/api/v1/statuses_spec.rb | 16 ++++++++++ 6 files changed, 104 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 23fc85b475..be7b302d3b 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -9,16 +9,22 @@ class Api::V1::AccountsController < Api::BaseController before_action -> { doorkeeper_authorize! :follow, :write, :'write:blocks' }, only: [:block, :unblock] before_action -> { doorkeeper_authorize! :write, :'write:accounts' }, only: [:create] - before_action :require_user!, except: [:show, :create] - before_action :set_account, except: [:create] - before_action :check_account_approval, except: [:create] - before_action :check_account_confirmation, except: [:create] + before_action :require_user!, except: [:index, :show, :create] + before_action :set_account, except: [:index, :create] + before_action :set_accounts, only: [:index] + before_action :check_account_approval, except: [:index, :create] + before_action :check_account_confirmation, except: [:index, :create] before_action :check_enabled_registrations, only: [:create] + before_action :check_accounts_limit, only: [:index] skip_before_action :require_authenticated_user!, only: :create override_rate_limit_headers :follow, family: :follows + def index + render json: @accounts, each_serializer: REST::AccountSerializer + end + def show cache_if_unauthenticated! render json: @account, serializer: REST::AccountSerializer @@ -79,6 +85,10 @@ class Api::V1::AccountsController < Api::BaseController @account = Account.find(params[:id]) end + def set_accounts + @accounts = Account.where(id: account_ids).without_unapproved + end + def check_account_approval raise(ActiveRecord::RecordNotFound) if @account.local? && @account.user_pending? end @@ -87,10 +97,22 @@ class Api::V1::AccountsController < Api::BaseController raise(ActiveRecord::RecordNotFound) if @account.local? && !@account.user_confirmed? end + def check_accounts_limit + raise(Mastodon::ValidationError) if account_ids.size > DEFAULT_ACCOUNTS_LIMIT + end + def relationships(**options) AccountRelationshipsPresenter.new([@account], current_user.account_id, **options) end + def account_ids + Array(accounts_params[:ids]).uniq.map(&:to_i) + end + + def accounts_params + params.permit(ids: []) + end + def account_params params.permit(:username, :email, :password, :agreement, :locale, :reason, :time_zone, :invite_code) end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 01c3718763..36a9ec6325 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -5,9 +5,11 @@ class Api::V1::StatusesController < Api::BaseController before_action -> { authorize_if_got_token! :read, :'read:statuses' }, except: [:create, :update, :destroy] before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only: [:create, :update, :destroy] - before_action :require_user!, except: [:show, :context] - before_action :set_status, only: [:show, :context] - before_action :set_thread, only: [:create] + before_action :require_user!, except: [:index, :show, :context] + before_action :set_statuses, only: [:index] + before_action :set_status, only: [:show, :context] + before_action :set_thread, only: [:create] + before_action :check_statuses_limit, only: [:index] override_rate_limit_headers :create, family: :statuses override_rate_limit_headers :update, family: :statuses @@ -23,6 +25,11 @@ class Api::V1::StatusesController < Api::BaseController DESCENDANTS_LIMIT = 60 DESCENDANTS_DEPTH_LIMIT = 20 + def index + @statuses = cache_collection(@statuses, Status) + render json: @statuses, each_serializer: REST::StatusSerializer + end + def show cache_if_unauthenticated! @status = cache_collection([@status], Status).first @@ -111,6 +118,10 @@ class Api::V1::StatusesController < Api::BaseController private + def set_statuses + @statuses = Status.permitted_statuses_from_ids(status_ids, current_account) + end + def set_status @status = Status.find(params[:id]) authorize @status, :show? @@ -125,6 +136,18 @@ class Api::V1::StatusesController < Api::BaseController render json: { error: I18n.t('statuses.errors.in_reply_not_found') }, status: 404 end + def check_statuses_limit + raise(Mastodon::ValidationError) if status_ids.size > DEFAULT_STATUSES_LIMIT + end + + def status_ids + Array(statuses_params[:ids]).uniq.map(&:to_i) + end + + def statuses_params + params.permit(ids: []) + end + def status_params params.permit( :status, diff --git a/app/models/concerns/status/threading_concern.rb b/app/models/concerns/status/threading_concern.rb index ca8c448140..478a139d63 100644 --- a/app/models/concerns/status/threading_concern.rb +++ b/app/models/concerns/status/threading_concern.rb @@ -3,6 +3,23 @@ module Status::ThreadingConcern extend ActiveSupport::Concern + class_methods do + def permitted_statuses_from_ids(ids, account, stable: false) + statuses = Status.with_accounts(ids).to_a + account_ids = statuses.map(&:account_id).uniq + domains = statuses.filter_map(&:account_domain).uniq + relations = account&.relations_map(account_ids, domains) || {} + + statuses.reject! { |status| StatusFilter.new(status, account, relations).filtered? } + + if stable + statuses.sort_by! { |status| ids.index(status.id) } + else + statuses + end + end + end + def ancestors(limit, account = nil) find_statuses_from_tree_path(ancestor_ids(limit), account) end @@ -76,15 +93,7 @@ module Status::ThreadingConcern end def find_statuses_from_tree_path(ids, account, promote: false) - statuses = Status.with_accounts(ids).to_a - account_ids = statuses.map(&:account_id).uniq - domains = statuses.filter_map(&:account_domain).uniq - relations = account&.relations_map(account_ids, domains) || {} - - statuses.reject! { |status| StatusFilter.new(status, account, relations).filtered? } - - # Order ancestors/descendants by tree path - statuses.sort_by! { |status| ids.index(status.id) } + statuses = Status.permitted_statuses_from_ids(ids, account, stable: true) # Bring self-replies to the top if promote diff --git a/config/routes/api.rb b/config/routes/api.rb index 60fb0394e7..bf3cee0c10 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -6,7 +6,7 @@ namespace :api, format: false do # JSON / REST API namespace :v1 do - resources :statuses, only: [:create, :show, :update, :destroy] do + resources :statuses, only: [:index, :create, :show, :update, :destroy] do scope module: :statuses do resources :reblogged_by, controller: :reblogged_by_accounts, only: :index resources :favourited_by, controller: :favourited_by_accounts, only: :index @@ -182,7 +182,7 @@ namespace :api, format: false do resources :familiar_followers, only: :index end - resources :accounts, only: [:create, :show] do + resources :accounts, only: [:index, :create, :show] do scope module: :accounts do resources :statuses, only: :index resources :followers, only: :index, controller: :follower_accounts diff --git a/spec/requests/api/v1/accounts_spec.rb b/spec/requests/api/v1/accounts_spec.rb index e543c41360..55f8e1c6fa 100644 --- a/spec/requests/api/v1/accounts_spec.rb +++ b/spec/requests/api/v1/accounts_spec.rb @@ -8,6 +8,22 @@ describe '/api/v1/accounts' do let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + describe 'GET /api/v1/accounts?ids[]=:id' do + let(:account) { Fabricate(:account) } + let(:other_account) { Fabricate(:account) } + let(:scopes) { 'read:accounts' } + + it 'returns expected response' do + get '/api/v1/accounts', headers: headers, params: { ids: [account.id, other_account.id, 123_123] } + + expect(response).to have_http_status(200) + expect(body_as_json).to contain_exactly( + hash_including(id: account.id.to_s), + hash_including(id: other_account.id.to_s) + ) + end + end + describe 'GET /api/v1/accounts/:id' do context 'when logged out' do let(:account) { Fabricate(:account) } diff --git a/spec/requests/api/v1/statuses_spec.rb b/spec/requests/api/v1/statuses_spec.rb index a3b84afa26..0b2d1f90cf 100644 --- a/spec/requests/api/v1/statuses_spec.rb +++ b/spec/requests/api/v1/statuses_spec.rb @@ -9,6 +9,22 @@ describe '/api/v1/statuses' do let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, application: client_app, scopes: scopes) } let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + describe 'GET /api/v1/statuses?ids[]=:id' do + let(:status) { Fabricate(:status) } + let(:other_status) { Fabricate(:status) } + let(:scopes) { 'read:statuses' } + + it 'returns expected response' do + get '/api/v1/statuses', headers: headers, params: { ids: [status.id, other_status.id, 123_123] } + + expect(response).to have_http_status(200) + expect(body_as_json).to contain_exactly( + hash_including(id: status.id.to_s), + hash_including(id: other_status.id.to_s) + ) + end + end + describe 'GET /api/v1/statuses/:id' do subject do get "/api/v1/statuses/#{status.id}", headers: headers