Merge branch 'make-job-lfs-artifacts-read-only' into 'master'

app/controllers/projects/lfs_storage_controller.rb

@@ -77,8 +77,7 @@ class Projects::LfsStorageController < Projects::GitHttpClientController
 
   def link_to_project!(object)
     if object && !object.projects.exists?(storage_project.id)
-      object.projects << storage_project
-      object.save!
+      object.lfs_objects_projects.create!(project: storage_project)
     end
   end
 end

app/models/ci/job_artifact.rb

@@ -13,7 +13,7 @@ module Ci
     after_save :update_project_statistics_after_save, if: :size_changed?
     after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed?
 
-    after_save :update_file_store
+    after_save :update_file_store, if: :file_changed?
 
     scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
 

app/models/lfs_object.rb

@@ -11,7 +11,7 @@ class LfsObject < ActiveRecord::Base
 
   mount_uploader :file, LfsObjectUploader
 
-  after_save :update_file_store
+  after_save :update_file_store, if: :file_changed?
 
   def update_file_store
     # The file.object_store is set during `uploader.store!`

changelogs/unreleased/fix-file-store-artifacts-and-lfs.yml

+---
+title: Fix file_store for artifacts and lfs when saving
+merge_request:
+author:
+type: fixed

spec/controllers/projects/raw_controller_spec.rb

@@ -54,9 +54,9 @@ describe Projects::RawController do
           end
 
           context 'and lfs uses object storage' do
+            let(:lfs_object) { create(:lfs_object, :with_file, oid: '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', size: '1575078') }
+
             before do
-              lfs_object.file = fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "`/png")
-              lfs_object.save!
               stub_lfs_object_storage
               lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE)
             end

spec/models/lfs_object_spec.rb

@@ -62,9 +62,7 @@ describe LfsObject do
               .with('LfsObjectUploader', described_class.name, :file, kind_of(Numeric))
               .once
 
-            lfs_object = create(:lfs_object)
-            lfs_object.file = fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "`/png")
-            lfs_object.save!
+            create(:lfs_object, :with_file)
           end
         end
       end

spec/services/projects/update_pages_service_spec.rb

@@ -123,11 +123,13 @@ describe Projects::UpdatePagesService do
         expect(execute).not_to eq(:success)
       end
 
-      it 'fails for empty file fails' do
-        build.job_artifacts_archive.update_attributes(file: empty_file)
+      context 'when using empty file' do
+        let(:file) { empty_file }
 
-        expect { execute }
-          .to raise_error(Projects::UpdatePagesService::FailedToExtractError)
+        it 'fails to extract' do
+          expect { execute }
+            .to raise_error(Projects::UpdatePagesService::FailedToExtractError)
+        end
       end
 
       context 'when timeout happens by DNS error' do

spec/uploaders/lfs_object_uploader_spec.rb

@@ -46,8 +46,7 @@ describe LfsObjectUploader do
   end
 
   describe 'remote file' do
-    let(:remote) { described_class::Store::REMOTE }
-    let(:lfs_object) { create(:lfs_object, file_store: remote) }
+    let(:lfs_object) { create(:lfs_object, :object_storage, :with_file) }
 
     context 'with object storage enabled' do
       before do
@@ -57,16 +56,11 @@ describe LfsObjectUploader do
       it 'can store file remotely' do
         allow(ObjectStorage::BackgroundMoveWorker).to receive(:perform_async)
 
-        store_file(lfs_object)
+        lfs_object
 
-        expect(lfs_object.file_store).to eq remote
+        expect(lfs_object.file_store).to eq(described_class::Store::REMOTE)
         expect(lfs_object.file.path).not_to be_blank
       end
     end
   end
-
-  def store_file(lfs_object)
-    lfs_object.file = fixture_file_upload(Rails.root.join("spec/fixtures/dk.png"), "`/png")
-    lfs_object.save!
-  end
 end