Merge branch '227218-geo-remove-geo-fdw-upload-model' into 'master'

Geo - Remove Geo::Fdw::Upload model

Closes #227218

See merge request gitlab-org/gitlab!36391

ee/app/models/ee/upload.rb

@@ -9,6 +9,8 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
+      include ::Gitlab::SQL::Pattern
+
       after_destroy :log_geo_deleted_event
 
       scope :for_model, ->(model) { where(model_id: model.id, model_type: model.class.name) }
@@ -21,6 +23,16 @@ module EE
         selective_sync_scope(node).merge(object_storage_scope(node))
       end
 
+      # Searches for a list of uploads based on the query given in `query`.
+      #
+      # On PostgreSQL this method uses "ILIKE" to perform a case-insensitive
+      # search.
+      #
+      # query - The search query as a String.
+      def search(query)
+        fuzzy_search(query, [:path])
+      end
+
       private
 
       # @return [ActiveRecord::Relation<Upload>] scope observing object storage settings of the given node

ee/app/models/geo/fdw/upload.rb

-# frozen_string_literal: true
-
-module Geo
-  module Fdw
-    class Upload < ::Geo::BaseFdw
-      include Gitlab::SQL::Pattern
-      include ObjectStorable
-
-      STORE_COLUMN = :store
-
-      self.primary_key = :id
-      self.table_name = Gitlab::Geo::Fdw.foreign_table_name('uploads')
-
-      has_one :registry, class_name: 'Geo::UploadRegistry', foreign_key: :file_id
-
-      class << self
-        def for_model(model)
-          inner_join_registry
-            .where(model_id: model.id, model_type: model.class.name)
-        end
-
-        def inner_join_registry
-          joins(:registry)
-        end
-
-        def missing_registry
-          left_outer_join_registry
-            .where(registry_table[:id].eq(nil))
-        end
-
-        # Searches for a list of uploads based on the query given in `query`.
-        #
-        # On PostgreSQL this method uses "ILIKE" to perform a case-insensitive
-        # search.
-        #
-        # query - The search query as a String.
-        def search(query)
-          fuzzy_search(query, [:path])
-        end
-
-        private
-
-        def registry_table
-          Geo::UploadRegistry.arel_table
-        end
-
-        def left_outer_join_registry
-          join_statement =
-            arel_table
-              .join(registry_table, Arel::Nodes::OuterJoin)
-              .on(arel_table[:id].eq(registry_table[:file_id]))
-
-          joins(join_statement.join_sources)
-        end
-      end
-    end
-  end
-end

ee/app/models/geo/upload_registry.rb

@@ -45,7 +45,7 @@ class Geo::UploadRegistry < Geo::BaseRegistry
   def self.with_search(query)
     return all if query.nil?
 
-    where(file_id: Geo::Fdw::Upload.search(query))
+    where(file_id: Upload.search(query).limit(1000).pluck_primary_key)
   end
 
   def self.with_status(status)

ee/app/models/geo_node.rb

@@ -431,8 +431,4 @@ class GeoNode < ApplicationRecord
   def project_model
     Project
   end
-
-  def uploads_model
-    Upload
-  end
 end

ee/app/services/geo/files_expire_service.rb

@@ -25,11 +25,10 @@ module Geo
     def execute
       return unless Gitlab::Geo.secondary?
 
-      uploads = Geo::Fdw::Upload.for_model(project)
+      uploads = Upload.for_model(project)
       log_info("Expiring replicated attachments after project rename", count: uploads.count)
 
       schedule_file_removal(uploads)
-      mark_for_resync!
     end
 
     # Project's base directory for attachments storage
@@ -51,18 +50,14 @@ module Geo
 
           log_info("Scheduled to remove file", file_path: file_path)
         end
+
+        Geo::UploadRegistry.where(file_id: upload.id).delete_all
       end
 
       Geo::FileRemovalWorker.bulk_perform_async(paths_to_remove) # rubocop:disable Scalability/BulkPerformWithContext
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    def mark_for_resync!
-      Gitlab::Geo::Fdw::UploadRegistryQueryBuilder.new
-        .for_model(project)
-        .delete_all
-    end
-
     # This is called by LogHelpers to build json log with context info
     #
     # @see ::Gitlab::Geo::LogHelpers

ee/changelogs/unreleased/227218-geo-remove-geo-fdw-upload-model.yml

+---
+title: Geo - Admin Area > Geo > Uploads search is limited to 1000 records
+merge_request: 36391
+author:
+type: other

ee/lib/gitlab/geo/fdw/upload_registry_query_builder.rb

-# frozen_string_literal: true
-
-# Builder class to create composable queries using FDW to
-# retrieve file registries.
-#
-# Basic usage:
-#
-#     Gitlab::Geo::Fdw::UploadRegistryQueryBuilder.new.for_model(project)
-#
-module Gitlab
-  module Geo
-    class Fdw
-      class UploadRegistryQueryBuilder < BaseQueryBuilder
-        # rubocop:disable CodeReuse/ActiveRecord
-        def for_model(model)
-          reflect(
-            query
-              .joins(fdw_inner_join_uploads)
-              .where(
-                fdw_table[:model_id].eq(model.id)
-                  .and(fdw_table[:model_type].eq(model.class.name))
-              )
-          )
-        end
-        # rubocop:enable CodeReuse/ActiveRecord
-
-        private
-
-        def base
-          ::Geo::UploadRegistry.select(registry_table[Arel.star])
-        end
-
-        def registry_table
-          ::Geo::UploadRegistry.arel_table
-        end
-
-        def fdw_table
-          ::Geo::Fdw::Upload.arel_table
-        end
-
-        def fdw_inner_join_uploads
-          registry_table
-            .join(fdw_table, Arel::Nodes::InnerJoin)
-            .on(registry_table[:file_id].eq(fdw_table[:id]))
-            .join_sources
-        end
-      end
-    end
-  end
-end

ee/spec/lib/gitlab/geo/fdw/upload_registry_query_builder_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Geo::Fdw::UploadRegistryQueryBuilder, :geo, :geo_fdw do
-  let(:project) { create(:project) }
-  let(:upload_1) { create(:upload, :issuable_upload, model: project) }
-  let(:upload_2) { create(:upload, :issuable_upload, model: project) }
-  let(:upload_3) { create(:upload, :issuable_upload) }
-  let!(:registry_1) { create(:geo_upload_registry, file_id: upload_1.id) }
-  let!(:registry_2) { create(:geo_upload_registry, :attachment, file_id: upload_2.id) }
-  let!(:registry_3) { create(:geo_upload_registry, file_id: upload_3.id) }
-
-  describe '#for_model' do
-    it 'returns registries for uploads that belong to the model' do
-      expect(subject.for_model(project)).to match_ids(registry_1, registry_2)
-    end
-  end
-end

ee/spec/services/geo/files_expire_service_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Geo::FilesExpireService, :geo, :geo_fdw do
+RSpec.describe Geo::FilesExpireService, :geo do
   let(:project) { create(:project, :legacy_storage) }
   let!(:old_full_path) { project.full_path }
 
@@ -43,7 +43,6 @@ RSpec.describe Geo::FilesExpireService, :geo, :geo_fdw do
     context 'when not in Geo secondary node' do
       it 'no-op execute action' do
         expect(subject).not_to receive(:schedule_file_removal)
-        expect(subject).not_to receive(:mark_for_resync!)
 
         subject.execute
       end