CE backport for changes in EE MR 13389

This backports to ce a refactor of the shared example 'a controller that
can serve LFS files'.

The ee MR that contains the original refactor is
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/13389

The reason for the refactor was mostly the removal of the outer context
as it didn't need to be there, and prevented let(:lfs_oid) from being
overwritten.

The shared example was also renamed to be more descriptive.

spec/controllers/projects/avatars_controller_spec.rb

@@ -39,7 +39,7 @@ describe Projects::AvatarsController do
       end
 
       context 'when the avatar is stored in lfs' do
-        it_behaves_like 'repository lfs file load' do
+        it_behaves_like 'a controller that can serve LFS files' do
           let(:filename) { 'lfs_object.iso' }
           let(:filepath) { "files/lfs/#{filename}" }
         end

spec/controllers/projects/raw_controller_spec.rb

@@ -42,7 +42,7 @@ describe Projects::RawController do
       end
     end
 
-    it_behaves_like 'repository lfs file load' do
+    it_behaves_like 'a controller that can serve LFS files' do
       let(:filename) { 'lfs_object.iso' }
       let(:filepath) { "be93687/files/lfs/#{filename}" }
     end

spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb

@@ -9,87 +9,87 @@
 # - `filepath`: path of the file (contains filename)
 # - `subject`: the request to be made to the controller. Example:
 # subject { get :show, namespace_id: project.namespace, project_id: project }
-shared_examples 'repository lfs file load' do
-  context 'when file is stored in lfs' do
-    let(:lfs_oid) { '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' }
-    let(:lfs_size) { '1575078' }
-    let!(:lfs_object) { create(:lfs_object, oid: lfs_oid, size: lfs_size) }
+shared_examples 'a controller that can serve LFS files' do
+  let(:lfs_oid) { '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' }
+  let(:lfs_size) { '1575078' }
+  let!(:lfs_object) { create(:lfs_object, oid: lfs_oid, size: lfs_size) }
+
+  context 'when lfs is enabled' do
+    before do
+      allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(true)
+    end
 
-    context 'when lfs is enabled' do
+    context 'when project has access' do
       before do
-        allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(true)
+        project.lfs_objects << lfs_object
+        allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true)
+        allow(controller).to receive(:send_file) { controller.head :ok }
       end
 
-      context 'when project has access' do
-        before do
-          project.lfs_objects << lfs_object
-          allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true)
-          allow(controller).to receive(:send_file) { controller.head :ok }
-        end
+      it 'serves the file' do
+        lfs_uploader = LfsObjectUploader.new(lfs_object)
 
-        it 'serves the file' do
-          # Notice the filename= is omitted from the disposition; this is because
-          # Rails 5 will append this header in send_file
-          expect(controller).to receive(:send_file)
-                            .with(
-                              "#{LfsObjectUploader.root}/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897",
-                              filename: filename,
-                              disposition: %Q(attachment; filename*=UTF-8''#{filename}))
+        # Notice the filename= is omitted from the disposition; this is because
+        # Rails 5 will append this header in send_file
+        expect(controller).to receive(:send_file)
+                          .with(
+                            File.join(lfs_uploader.root, lfs_uploader.store_dir, lfs_uploader.filename),
+                            filename: filename,
+                            disposition: %Q(attachment; filename*=UTF-8''#{filename}))
 
-          subject
+        subject
 
-          expect(response).to have_gitlab_http_status(200)
-        end
+        expect(response).to have_gitlab_http_status(200)
+      end
 
-        context 'and lfs uses object storage' do
-          let(:lfs_object) { create(:lfs_object, :with_file, oid: lfs_oid, size: lfs_size) }
+      context 'and lfs uses object storage' do
+        let(:lfs_object) { create(:lfs_object, :with_file, oid: lfs_oid, size: lfs_size) }
 
-          before do
-            stub_lfs_object_storage
-            lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE)
-          end
+        before do
+          stub_lfs_object_storage
+          lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE)
+        end
 
-          it 'responds with redirect to file' do
-            subject
+        it 'responds with redirect to file' do
+          subject
 
-            expect(response).to have_gitlab_http_status(302)
-            expect(response.location).to include(lfs_object.reload.file.path)
-          end
+          expect(response).to have_gitlab_http_status(302)
+          expect(response.location).to include(lfs_object.reload.file.path)
+        end
 
-          it 'sets content disposition' do
-            subject
+        it 'sets content disposition' do
+          subject
 
-            file_uri = URI.parse(response.location)
-            params = CGI.parse(file_uri.query)
+          file_uri = URI.parse(response.location)
+          params = CGI.parse(file_uri.query)
 
-            expect(params["response-content-disposition"].first).to eq(%q(attachment; filename="lfs_object.iso"; filename*=UTF-8''lfs_object.iso))
-          end
+          expect(params["response-content-disposition"].first).to eq(%Q(attachment; filename="#{filename}"; filename*=UTF-8''#{filename}))
         end
       end
+    end
 
-      context 'when project does not have access' do
-        it 'does not serve the file' do
-          subject
+    context 'when project does not have access' do
+      it 'does not serve the file' do
+        subject
 
-          expect(response).to have_gitlab_http_status(404)
-        end
+        expect(response).to have_gitlab_http_status(404)
       end
     end
+  end
 
-    context 'when lfs is not enabled' do
-      before do
-        allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(false)
-      end
+  context 'when lfs is not enabled' do
+    before do
+      allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(false)
+    end
 
-      it 'delivers ASCII file' do
-        subject
+    it 'delivers ASCII file' do
+      subject
 
-        expect(response).to have_gitlab_http_status(200)
-        expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8')
-        expect(response.header['Content-Disposition'])
-            .to eq('inline')
-        expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
-      end
+      expect(response).to have_gitlab_http_status(200)
+      expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8')
+      expect(response.header['Content-Disposition'])
+          .to eq('inline')
+      expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
     end
   end
 end