Migrate to request specs in `/api/v1/media` (#25543)

.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'

spec/controllers/api/v1/media_controller_spec.rb

@@ -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

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