Remove GeoNode#restricted_project_ids

GeoNode#projects now returns restricted/unrestricted projects relation and only when it's really needed should be plucked to an array.

app/models/geo_node.rb

@@ -120,26 +120,10 @@ class GeoNode < ActiveRecord::Base
     end
   end
 
-  def projects_include?(project_id)
-    return true if restricted_project_ids.nil?
-
-    restricted_project_ids.include?(project_id)
-  end
-
-  def restricted_project_ids
-    return unless namespaces.presence
-
-    relations = namespaces.map { |namespace| namespace.all_projects.select(:id) }
-
-    Project.unscoped
-       .from("(#{Gitlab::SQL::Union.new(relations).to_sql}) #{Project.table_name}")
-       .pluck(:id)
-  end
-
   def lfs_objects
     relation =
-      if restricted_project_ids
-        LfsObject.joins(:projects).where(projects: { id: restricted_project_ids })
+      if selective_sync?
+        LfsObject.joins(:projects).where(projects: { id: projects })
       else
         LfsObject.all
       end
@@ -148,16 +132,25 @@ class GeoNode < ActiveRecord::Base
   end
 
   def projects
-    if restricted_project_ids
-      Project.where(id: restricted_project_ids)
+    if selective_sync?
+      relations = namespaces.map { |namespace| namespace.all_projects.select(:id) }
+
+      Project.unscoped.
+        from("(#{Gitlab::SQL::Union.new(relations).to_sql}) #{Project.table_name}")
     else
       Project.all
     end
   end
 
+  def projects_include?(project_id)
+    return true unless selective_sync?
+
+    projects.where(id: project_id).exists?
+  end
+
   def project_registries
-    if restricted_project_ids
-      Geo::ProjectRegistry.where(project_id: restricted_project_ids)
+    if selective_sync?
+      Geo::ProjectRegistry.where(project_id: projects.pluck(:id))
     else
       Geo::ProjectRegistry.all
     end
@@ -174,11 +167,15 @@ class GeoNode < ActiveRecord::Base
     end
   end
 
+  def selective_sync?
+    namespaces.present?
+  end
+
   def uploads
-    if restricted_project_ids
+    if selective_sync?
       uploads_table   = Upload.arel_table
       group_uploads   = uploads_table[:model_type].eq('Namespace').and(uploads_table[:model_id].in(Gitlab::Geo.current_node.namespace_ids))
-      project_uploads = uploads_table[:model_type].eq('Project').and(uploads_table[:model_id].in(restricted_project_ids))
+      project_uploads = uploads_table[:model_type].eq('Project').and(uploads_table[:model_id].in(projects.pluck(:id)))
       other_uploads   = uploads_table[:model_type].not_in(%w[Namespace Project])
 
       Upload.where(group_uploads.or(project_uploads).or(other_uploads))
@@ -192,7 +189,7 @@ class GeoNode < ActiveRecord::Base
 
     relation = Geo::FileRegistry.lfs_objects.synced
 
-    if restricted_project_ids
+    if selective_sync?
       relation = relation.where(file_id: lfs_objects.pluck(:id))
     end
 

app/workers/geo/repositories_clean_up_worker.rb

@@ -12,11 +12,9 @@ module Geo
       # Prevent multiple Sidekiq workers from performing repositories clean up
       try_obtain_lease do
         geo_node = GeoNode.find(geo_node_id)
+        return unless geo_node.selective_sync?
 
-        restricted_project_ids = geo_node.restricted_project_ids
-        return unless restricted_project_ids
-
-        Project.where.not(id: restricted_project_ids).find_in_batches(batch_size: BATCH_SIZE) do |batch|
+        Project.where.not(id: geo_node.projects).find_in_batches(batch_size: BATCH_SIZE) do |batch|
           batch.each do |project|
             clean_up_repositories(project)
           end

ee/app/finders/geo/attachment_registry_finder.rb

@@ -27,10 +27,10 @@ module Geo
     def uploads
       upload_model = Gitlab::Geo.fdw? ? Geo::Fdw::Upload : Upload
 
-      if selective_sync
+      if selective_sync?
         upload_table    = upload_model.arel_table
         group_uploads   = upload_table[:model_type].eq('Namespace').and(upload_table[:model_id].in(current_node.namespace_ids))
-        project_uploads = upload_table[:model_type].eq('Project').and(upload_table[:model_id].in(current_node.restricted_project_ids))
+        project_uploads = upload_table[:model_type].eq('Project').and(upload_table[:model_id].in(current_node.projects.pluck(:id)))
         other_uploads   = upload_table[:model_type].not_in(%w[Namespace Project])
 
         upload_model.where(group_uploads.or(project_uploads).or(other_uploads))

ee/app/finders/geo/registry_finder.rb

@@ -2,6 +2,8 @@ module Geo
   class RegistryFinder
     attr_reader :current_node
 
+    delegate :selective_sync?, to: :current_node, allow_nil: true
+
     def initialize(current_node: nil)
       @current_node = current_node
     end
@@ -11,11 +13,7 @@ module Geo
     def fdw?
       # Selective project replication adds a wrinkle to FDW
       # queries, so we fallback to the legacy version for now.
-      Gitlab::Geo.fdw? && !selective_sync
-    end
-
-    def selective_sync
-      current_node.restricted_project_ids
+      Gitlab::Geo.fdw? && !selective_sync?
     end
   end
 end

spec/models/geo_node_spec.rb

@@ -268,26 +268,34 @@ describe GeoNode, type: :model do
     end
   end
 
-  describe '#restricted_project_ids' do
-    context 'without namespace restriction' do
-      it 'returns nil' do
-        expect(node.restricted_project_ids).to be_nil
-      end
+  describe '#projects' do
+    let(:group_1) { create(:group) }
+    let(:group_2) { create(:group) }
+    let(:nested_group_1) { create(:group, parent: group_1) }
+    let!(:project_1) { create(:project, group: group_1) }
+    let!(:project_2) { create(:project, group: nested_group_1) }
+    let!(:project_3) { create(:project, group: group_2) }
+
+    it 'returns all projects without selective sync' do
+      expect(node.projects).to match_array([project_1, project_2, project_3])
     end
 
-    context 'with namespace restrictions' do
-      it 'returns an array with unique project ids that belong to the namespaces' do
-        group_1 = create(:group)
-        group_2 = create(:group)
-        nested_group_1 = create(:group, parent: group_1)
-        project_1 = create(:project, group: group_1)
-        project_2 = create(:project, group: nested_group_1)
-        project_3 = create(:project, group: group_2)
+    it 'returns projects that belong to the namespaces with selective sync' do
+      node.update_attribute(:namespaces, [group_1, nested_group_1])
+
+      expect(node.projects).to match_array([project_1, project_2])
+    end
+  end
 
-        node.update_attribute(:namespaces, [group_1, group_2, nested_group_1])
+  describe '#selective_sync?' do
+    it 'returns true when Geo node has namespace restrictions' do
+      node.update_attribute(:namespaces, [create(:group)])
 
-        expect(node.restricted_project_ids).to match_array([project_1.id, project_2.id, project_3.id])
-      end
+      expect(node.selective_sync?).to be true
+    end
+
+    it 'returns false when Geo node does not have namespace restrictions' do
+      expect(node.selective_sync?).to be false
     end
   end