Move to Ci::JobArtifact.replicables_for_geo_node

from GeoNode#job_artifacts. I believe the CTE is no longer
needed since the scope is not used with FDW.

ee/app/finders/geo/job_artifact_registry_finder.rb

@@ -3,7 +3,8 @@
 module Geo
   class JobArtifactRegistryFinder < FileRegistryFinder
     def replicables
-      current_node(fdw: false).job_artifacts.not_expired
+      # TODO move not_expired into replicables scope
+      ::Ci::JobArtifact.replicables_for_geo_node.not_expired
     end
 
     def syncable

ee/app/models/ee/ci/job_artifact.rb

@@ -73,6 +73,22 @@ module EE
 
         super
       end
+
+      def replicables_for_geo_node(node = ::Gitlab::Geo.current_node)
+        selective_sync_scope(node).merge(object_storage_scope(node))
+      end
+
+      def object_storage_scope(node)
+        return all if node.sync_object_storage?
+
+        with_files_stored_locally
+      end
+
+      def selective_sync_scope(node)
+        return all unless node.selective_sync?
+
+        project_id_in(node.projects)
+      end
     end
 
     def log_geo_deleted_event

ee/app/models/geo_node.rb

@@ -237,35 +237,6 @@ class GeoNode < ApplicationRecord
     end
   end
 
-  def job_artifacts
-    selective_sync_scope.merge(object_storage_scope)
-  end
-
-  def object_storage_scope
-    return Ci::JobArtifact.all if sync_object_storage?
-
-    Ci::JobArtifact.with_files_stored_locally
-  end
-
-  def selective_sync_scope
-    return Ci::JobArtifact.all unless selective_sync?
-
-    query = Ci::JobArtifact.project_id_in(projects).select(:id)
-    cte = Gitlab::SQL::CTE.new(:restricted_job_artifacts, query)
-    job_artifact_table = Ci::JobArtifact.arel_table
-
-    inner_join_restricted_job_artifacts =
-      cte.table
-        .join(job_artifact_table, Arel::Nodes::InnerJoin)
-        .on(cte.table[:id].eq(job_artifact_table[:id]))
-        .join_sources
-
-    Ci::JobArtifact
-      .with(cte.to_arel)
-      .from(cte.table)
-      .joins(inner_join_restricted_job_artifacts)
-  end
-
   def container_repositories
     return ContainerRepository.none unless Geo::ContainerRepositoryRegistry.replication_enabled?
     return ContainerRepository.all unless selective_sync?

ee/spec/models/ee/ci/job_artifact_spec.rb

@@ -2,7 +2,8 @@
 
 require 'spec_helper'
 
-RSpec.describe EE::Ci::JobArtifact do
+RSpec.describe Ci::JobArtifact do
+  using RSpec::Parameterized::TableSyntax
   include EE::GeoHelpers
 
   describe '#destroy' do
@@ -102,4 +103,75 @@ RSpec.describe EE::Ci::JobArtifact do
       it { is_expected.to eq result }
     end
   end
+
+  describe '#replicables_for_geo_node' do
+    # Selective sync is configured relative to the job artifact's project.
+    #
+    # Permutations of sync_object_storage combined with object-stored-artifacts
+    # are tested in code, because the logic is simple, and to do it in the table
+    # would quadruple its size and have too much duplication.
+    where(:selective_sync_namespaces, :selective_sync_shards, :project_factory, :include_expectation) do
+      nil                  | nil    | [:project]               | true
+      # selective sync by shard
+      nil                  | :model | [:project]               | true
+      nil                  | :other | [:project]               | false
+      # selective sync by namespace
+      :model_parent        | nil    | [:project]               | true
+      :model_parent_parent | nil    | [:project, :in_subgroup] | true
+      :other               | nil    | [:project]               | false
+      :other               | nil    | [:project, :in_subgroup] | false
+    end
+
+    with_them do
+      subject(:job_artifact_included) { described_class.replicables_for_geo_node.include?(ci_job_artifact) }
+
+      let(:factory) { [:ci_job_artifact]}
+      let(:project) { create(*project_factory) }
+      let(:ci_build) { create(:ci_build, project: project) }
+      let(:node) do
+        create_geo_node_to_test_replicables_for_geo_node(
+          project,
+          selective_sync_namespaces: selective_sync_namespaces,
+          selective_sync_shards: selective_sync_shards,
+          sync_object_storage: sync_object_storage)
+      end
+
+      before do
+        stub_artifacts_object_storage
+        stub_current_geo_node(node)
+      end
+
+      context 'when sync object storage is enabled' do
+        let(:sync_object_storage) { true }
+
+        context 'when the job artifact is locally stored' do
+          let(:ci_job_artifact) { create(*factory, job: ci_build) }
+
+          it { is_expected.to eq(include_expectation) }
+        end
+
+        context 'when the job artifact is object stored' do
+          let(:ci_job_artifact) { create(*factory, :remote_store, job: ci_build) }
+
+          it { is_expected.to eq(include_expectation) }
+        end
+      end
+
+      context 'when sync object storage is disabled' do
+        let(:sync_object_storage) { false }
+
+        context 'when the job artifact is locally stored' do
+          let(:ci_job_artifact) { create(*factory, job: ci_build) }
+
+          it { is_expected.to eq(include_expectation) }
+        end
+
+        context 'when the job artifact is object stored' do
+          let(:ci_job_artifact) { create(*factory, :remote_store, job: ci_build) }
+
+          it { is_expected.to be_falsey }
+        end
+      end
+    end
+  end
 end

ee/spec/models/geo_node_spec.rb

@@ -813,94 +813,6 @@ RSpec.describe GeoNode, :request_store, :geo, type: :model do
     end
   end
 
-  describe '#job_artifacts' do
-    context 'when selective sync is enabled' do
-      it 'applies a CTE statement' do
-        node.update!(selective_sync_type: 'namespaces')
-
-        expect(node.job_artifacts.to_sql).to match(/WITH .+restricted_job_artifacts/)
-      end
-    end
-
-    context 'when selective sync is disabled' do
-      it 'doest not apply a CTE statement' do
-        node.update!(selective_sync_type: nil)
-
-        expect(node.job_artifacts.to_sql).not_to match(/WITH .+restricted_job_artifacts/)
-      end
-    end
-  end
-
-  describe '#job_artifacts' do
-    # Selective sync is configured relative to the job artifact's project.
-    #
-    # Permutations of sync_object_storage combined with object-stored-uploads
-    # are tested in code, because the logic is simple, and to do it in the table
-    # would quadruple its size and have too much duplication.
-    where(:selective_sync_namespaces, :selective_sync_shards, :project_factory, :include_expectation) do
-      nil                  | nil    | [:project]               | true
-      # selective sync by shard
-      nil                  | :model | [:project]               | true
-      nil                  | :other | [:project]               | false
-      # selective sync by namespace
-      :model_parent        | nil    | [:project]               | true
-      :model_parent_parent | nil    | [:project, :in_subgroup] | true
-      :other               | nil    | [:project]               | false
-      :other               | nil    | [:project, :in_subgroup] | false
-    end
-
-    with_them do
-      subject(:upload_included) { node.job_artifacts.include?(upload) }
-
-      let(:factory) { [:ci_job_artifact]}
-      let(:project) { create(*project_factory) }
-      let(:ci_build) { create(:ci_build, project: project) }
-      let(:node) do
-        create_geo_node_to_test_replicables_for_geo_node(
-          project,
-          selective_sync_namespaces: selective_sync_namespaces,
-          selective_sync_shards: selective_sync_shards,
-          sync_object_storage: sync_object_storage)
-      end
-
-      before do
-        stub_artifacts_object_storage
-      end
-
-      context 'when sync object storage is enabled' do
-        let(:sync_object_storage) { true }
-
-        context 'when the upload is locally stored' do
-          let(:upload) { create(*factory, job: ci_build) }
-
-          it { is_expected.to eq(include_expectation) }
-        end
-
-        context 'when the upload is object stored' do
-          let(:upload) { create(*factory, :remote_store, job: ci_build) }
-
-          it { is_expected.to eq(include_expectation) }
-        end
-      end
-
-      context 'when sync object storage is disabled' do
-        let(:sync_object_storage) { false }
-
-        context 'when the upload is locally stored' do
-          let(:upload) { create(*factory, job: ci_build) }
-
-          it { is_expected.to eq(include_expectation) }
-        end
-
-        context 'when the upload is object stored' do
-          let(:upload) { create(*factory, :remote_store, job: ci_build) }
-
-          it { is_expected.to be_falsey }
-        end
-      end
-    end
-  end
-
   describe '#lfs_objects' do
     let_it_be(:synced_group) { create(:group) }
     let_it_be(:nested_group) { create(:group, parent: synced_group) }