Drop legacy artifacts usagedrop-usage-of-leagcy-artifacts

Legacy artifacts have been correctly migrated to new place -
ci_job_artifacts. Now it's time to remove the related code, but before
that we should ensure it doesn't break anything by using feature flag.

6 files changed, 50 insertions, 5 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 1bd517641ac..b8a76e662b0 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -83,8 +83,13 @@ module Ci
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
     scope :with_artifacts_archive, ->() do
-      where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)',
-        '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
+      if Feature.enabled?(:ci_enable_legacy_artifacts)
+        where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)',
+          '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
+      else
+        where('EXISTS (?)',
+          Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
+      end
     end
 
     scope :with_existing_job_artifacts, ->(query) do
@@ -135,6 +140,8 @@ module Ci
       where("EXISTS (?)", matcher)
     end
 
+    ##
+    # TODO: Remove these mounters when we remove :ci_enable_legacy_artifacts feature flag
     mount_uploader :legacy_artifacts_file, LegacyArtifactUploader, mount_on: :artifacts_file
     mount_uploader :legacy_artifacts_metadata, LegacyArtifactUploader, mount_on: :artifacts_metadata
 
@@ -775,7 +782,7 @@ module Ci
     private
 
     def erase_old_artifacts!
-      # TODO: To be removed once we get rid of
+      # TODO: To be removed once we get rid of ci_enable_legacy_artifacts feature flag
       remove_artifacts_file!
       remove_artifacts_metadata!
       save
diff --git a/app/models/concerns/artifact_migratable.rb b/app/models/concerns/artifact_migratable.rb
index cbd63ba8876..7c9f579b480 100644
--- a/app/models/concerns/artifact_migratable.rb
+++ b/app/models/concerns/artifact_migratable.rb
@@ -13,7 +13,7 @@ module ArtifactMigratable
   end
 
   def artifacts?
-    !artifacts_expired? && artifacts_file.exists?
+    !artifacts_expired? && artifacts_file&.exists?
   end
 
   def artifacts_metadata?
@@ -43,4 +43,16 @@ module ArtifactMigratable
   def artifacts_size
     read_attribute(:artifacts_size).to_i + job_artifacts.sum(:size).to_i
   end
+
+  def legacy_artifacts_file
+    return unless Feature.enabled?(:ci_enable_legacy_artifacts)
+
+    super
+  end
+
+  def legacy_artifacts_metadata
+    return unless Feature.enabled?(:ci_enable_legacy_artifacts)
+
+    super
+  end
 end
diff --git a/app/uploaders/legacy_artifact_uploader.rb b/app/uploaders/legacy_artifact_uploader.rb
index a9afc104ed1..fac3c3dcb8f 100644
--- a/app/uploaders/legacy_artifact_uploader.rb
+++ b/app/uploaders/legacy_artifact_uploader.rb
@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
+##
+# TODO: Remove this uploader when we remove :ci_enable_legacy_artifacts feature flag
+# See https://gitlab.com/gitlab-org/gitlab-ce/issues/58595
 class LegacyArtifactUploader < GitlabUploader
   extend Workhorse::UploadPath
   include ObjectStorage::Concern
diff --git a/changelogs/unreleased/drop-usage-of-leagcy-artifacts.yml b/changelogs/unreleased/drop-usage-of-leagcy-artifacts.yml
new file mode 100644
index 00000000000..d99187d8d41
--- /dev/null
+++ b/changelogs/unreleased/drop-usage-of-leagcy-artifacts.yml
@@ -0,0 +1,5 @@
+---
+title: Drop legacy artifacts usage as there are no leftovers
+merge_request: 24294
+author:
+type: performance
diff --git a/lib/gitlab/data_builder/pipeline.rb b/lib/gitlab/data_builder/pipeline.rb
index 76c8b4ec5c2..fa06fb935f7 100644
--- a/lib/gitlab/data_builder/pipeline.rb
+++ b/lib/gitlab/data_builder/pipeline.rb
@@ -47,7 +47,7 @@ module Gitlab
           user: build.user.try(:hook_attrs),
           runner: build.runner && runner_hook_attrs(build.runner),
           artifacts_file: {
-            filename: build.artifacts_file.filename,
+            filename: build.artifacts_file&.filename,
             size: build.artifacts_size
           }
         }
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 2c41dfa65cd..697fe3fda06 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -117,6 +117,16 @@ describe Ci::Build do
       it 'returns the job' do
         is_expected.to include(job)
       end
+
+      context 'when ci_enable_legacy_artifacts feature flag is disabled' do
+        before do
+          stub_feature_flags(ci_enable_legacy_artifacts: false)
+        end
+
+        it 'does not return the job' do
+          is_expected.not_to include(job)
+        end
+      end
     end
 
     context 'when job has a job artifact archive' do
@@ -471,6 +481,14 @@ describe Ci::Build do
         let(:build) { create(:ci_build, :legacy_artifacts) }
 
         it { is_expected.to be_truthy }
+
+        context 'when ci_enable_legacy_artifacts feature flag is disabled' do
+          before do
+            stub_feature_flags(ci_enable_legacy_artifacts: false)
+          end
+
+          it { is_expected.to be_falsy }
+        end
       end
     end
   end