From 26d2a2a0cc3d1ea0dfea6810783cd0d8fb0bba10 Mon Sep 17 00:00:00 2001 From: Daniel M Brasil Date: Mon, 23 Oct 2023 12:46:21 -0300 Subject: [PATCH] Migrate to request specs in `/api/v1/media` (#25543) --- .rubocop_todo.yml | 2 - .../api/v1/media_controller_spec.rb | 107 ---------- spec/requests/api/v1/media_spec.rb | 189 ++++++++++++++++++ 3 files changed, 189 insertions(+), 109 deletions(-) delete mode 100644 spec/controllers/api/v1/media_controller_spec.rb create mode 100644 spec/requests/api/v1/media_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1a21fd1560..9e04be03c2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -78,7 +78,6 @@ RSpec/AnyInstance: - 'spec/controllers/admin/accounts_controller_spec.rb' - 'spec/controllers/admin/resets_controller_spec.rb' - 'spec/controllers/admin/settings/branding_controller_spec.rb' - - 'spec/controllers/api/v1/media_controller_spec.rb' - 'spec/controllers/auth/sessions_controller_spec.rb' - 'spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb' - 'spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb' @@ -178,7 +177,6 @@ RSpec/LetSetup: RSpec/MessageChain: Exclude: - - 'spec/controllers/api/v1/media_controller_spec.rb' - 'spec/models/concerns/remotable_spec.rb' - 'spec/models/session_activation_spec.rb' - 'spec/models/setting_spec.rb' diff --git a/spec/controllers/api/v1/media_controller_spec.rb b/spec/controllers/api/v1/media_controller_spec.rb deleted file mode 100644 index b574381f90..0000000000 --- a/spec/controllers/api/v1/media_controller_spec.rb +++ /dev/null @@ -1,107 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Api::V1::MediaController do - render_views - - let(:user) { Fabricate(:user) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'write:media') } - - before do - allow(controller).to receive(:doorkeeper_token) { token } - end - - describe 'POST #create' do - describe 'with paperclip errors' do - context 'when imagemagick cant identify the file type' do - it 'returns http 422' do - allow_any_instance_of(Account).to receive_message_chain(:media_attachments, :create!).and_raise(Paperclip::Errors::NotIdentifiedByImageMagickError) - post :create, params: { file: fixture_file_upload('attachment.jpg', 'image/jpeg') } - - expect(response).to have_http_status(422) - end - end - - context 'when there is a generic error' do - it 'returns http 422' do - allow_any_instance_of(Account).to receive_message_chain(:media_attachments, :create!).and_raise(Paperclip::Error) - post :create, params: { file: fixture_file_upload('attachment.jpg', 'image/jpeg') } - - expect(response).to have_http_status(500) - end - end - end - - context 'with image/jpeg' do - before do - post :create, params: { file: fixture_file_upload('attachment.jpg', 'image/jpeg') } - end - - it 'creates a media attachment', :aggregate_failures do - expect(response).to have_http_status(200) - expect(MediaAttachment.first).to_not be_nil - expect(MediaAttachment.first).to have_attached_file(:file) - expect(body_as_json[:id]).to eq MediaAttachment.first.id.to_s - end - end - - context 'with image/gif' do - before do - post :create, params: { file: fixture_file_upload('attachment.gif', 'image/gif') } - end - - it 'creates a media attachment', :aggregate_failures do - expect(response).to have_http_status(200) - expect(MediaAttachment.first).to_not be_nil - expect(MediaAttachment.first).to have_attached_file(:file) - expect(body_as_json[:id]).to eq MediaAttachment.first.id.to_s - end - end - - context 'with video/webm' do - before do - post :create, params: { file: fixture_file_upload('attachment.webm', 'video/webm') } - end - - it 'creates a media attachment', :aggregate_failures do - expect(response).to have_http_status(200) - expect(MediaAttachment.first).to_not be_nil - expect(MediaAttachment.first).to have_attached_file(:file) - expect(body_as_json[:id]).to eq MediaAttachment.first.id.to_s - end - end - end - - describe 'PUT #update' do - context 'when somebody else\'s' do - let(:media) { Fabricate(:media_attachment, status: nil) } - - it 'returns http not found' do - put :update, params: { id: media.id, description: 'Lorem ipsum!!!' } - expect(response).to have_http_status(404) - end - end - - context 'when the author \'s' do - let(:status) { nil } - let(:media) { Fabricate(:media_attachment, status: status, account: user.account) } - - before do - put :update, params: { id: media.id, description: 'Lorem ipsum!!!' } - end - - it 'updates the description' do - expect(media.reload.description).to eq 'Lorem ipsum!!!' - end - - context 'when already attached to a status' do - let(:status) { Fabricate(:status, account: user.account) } - - it 'returns http not found' do - expect(response).to have_http_status(404) - end - end - end - end -end diff --git a/spec/requests/api/v1/media_spec.rb b/spec/requests/api/v1/media_spec.rb new file mode 100644 index 0000000000..7253a9f1e8 --- /dev/null +++ b/spec/requests/api/v1/media_spec.rb @@ -0,0 +1,189 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Media' do + let(:user) { Fabricate(:user) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + let(:scopes) { 'write:media' } + let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + + describe 'GET /api/v1/media/:id' do + subject do + get "/api/v1/media/#{media.id}", headers: headers + end + + let(:media) { Fabricate(:media_attachment, account: user.account) } + + it_behaves_like 'forbidden for wrong scope', 'read' + + it 'returns http success' do + subject + + expect(response).to have_http_status(200) + end + + it 'returns the media information' do + subject + + expect(body_as_json).to match( + a_hash_including( + id: media.id.to_s, + description: media.description, + type: media.type + ) + ) + end + + context 'when the media is still being processed' do + before do + media.update(processing: :in_progress) + end + + it 'returns http partial content' do + subject + + expect(response).to have_http_status(206) + end + end + + context 'when the media belongs to somebody else' do + let(:media) { Fabricate(:media_attachment) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + + context 'when media is attached to a status' do + let(:media) { Fabricate(:media_attachment, account: user.account, status: Fabricate.build(:status)) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end + + describe 'POST /api/v1/media' do + subject do + post '/api/v1/media', headers: headers, params: params + end + + let(:params) { {} } + + shared_examples 'a successful media upload' do |media_type| + it 'uploads the file successfully', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(MediaAttachment.first).to be_present + expect(MediaAttachment.first).to have_attached_file(:file) + end + + it 'returns the correct media content' do + subject + + body = body_as_json + + expect(body).to match( + a_hash_including(id: MediaAttachment.first.id.to_s, description: params[:description], type: media_type) + ) + end + end + + it_behaves_like 'forbidden for wrong scope', 'read read:media' + + describe 'when paperclip errors occur' do + let(:media_attachments) { double } + let(:params) { { file: fixture_file_upload('attachment.jpg', 'image/jpeg') } } + + before do + allow(User).to receive(:find).with(token.resource_owner_id).and_return(user) + allow(user.account).to receive(:media_attachments).and_return(media_attachments) + end + + context 'when imagemagick cannot identify the file type' do + it 'returns http unprocessable entity' do + allow(media_attachments).to receive(:create!).and_raise(Paperclip::Errors::NotIdentifiedByImageMagickError) + + subject + + expect(response).to have_http_status(422) + end + end + + context 'when there is a generic error' do + it 'returns http 500' do + allow(media_attachments).to receive(:create!).and_raise(Paperclip::Error) + + subject + + expect(response).to have_http_status(500) + end + end + end + + context 'with image/jpeg', paperclip_processing: true do + let(:params) { { file: fixture_file_upload('attachment.jpg', 'image/jpeg'), description: 'jpeg image' } } + + it_behaves_like 'a successful media upload', 'image' + end + + context 'with image/gif', paperclip_processing: true do + let(:params) { { file: fixture_file_upload('attachment.gif', 'image/gif') } } + + it_behaves_like 'a successful media upload', 'image' + end + + context 'with video/webm', paperclip_processing: true do + let(:params) { { file: fixture_file_upload('attachment.webm', 'video/webm') } } + + it_behaves_like 'a successful media upload', 'gifv' + end + end + + describe 'PUT /api/v1/media/:id' do + subject do + put "/api/v1/media/#{media.id}", headers: headers, params: params + end + + let(:params) { {} } + let(:media) { Fabricate(:media_attachment, status: status, account: user.account, description: 'old') } + + it_behaves_like 'forbidden for wrong scope', 'read read:media' + + context 'when the media belongs to somebody else' do + let(:media) { Fabricate(:media_attachment, status: nil) } + let(:params) { { description: 'Lorem ipsum!!!' } } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + + context 'when the requesting user owns the media' do + let(:status) { nil } + let(:params) { { description: 'Lorem ipsum!!!' } } + + it 'updates the description' do + expect { subject }.to change { media.reload.description }.from('old').to('Lorem ipsum!!!') + end + + context 'when the media is attached to a status' do + let(:status) { Fabricate(:status, account: user.account) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end + end +end