Store paths relative to CarrierWave.root

So the path on source installs cannot be too long for our column. And fix the column length test since Route.path is limited to 255 chars, it doesn’t matter how many nested groups there are.

Store paths relative to CarrierWave.root
So the path on source installs cannot be too long for our column. And fix the column length test since Route.path is limited to 255 chars, it doesn’t matter how many nested groups there are.
a86cbc48 · Michael Kozono · 5983596a · a86cbc48 · a86cbc48 · a86cbc48
--- a/lib/gitlab/background_migration/populate_untracked_uploads.rb
+++ b/lib/gitlab/background_migration/populate_untracked_uploads.rb
@@ -90,7 +90,7 @@ def upload_path
            matchd = path_relative_to_upload_dir.match(FILE_UPLOADER_PATH_PATTERN)
            matchd[0].sub(%r{\A/}, '') # remove leading slash
          else
-            path_relative_to_carrierwave_root
+            path
          end
        end

@@ -113,20 +113,16 @@ def model_id
        end

        def file_size
-          File.size(path)
+          absolute_path = File.join(CarrierWave.root, path)
+          File.size(absolute_path)
        end

        # Not including a leading slash
        def path_relative_to_upload_dir
-          base = %r{\A#{Regexp.escape(Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR)}/}
+          base = %r{\A#{Regexp.escape(Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR)}/}
          @path_relative_to_upload_dir ||= path.sub(base, '')
        end

-        # Not including a leading slash
-        def path_relative_to_carrierwave_root
-          "uploads/#{path_relative_to_upload_dir}"
-        end
-
        private

        def matching_pattern_map

--- a/lib/gitlab/background_migration/prepare_untracked_uploads.rb
+++ b/lib/gitlab/background_migration/prepare_untracked_uploads.rb
@@ -5,8 +5,12 @@ class PrepareUntrackedUploads
      include Database::MigrationHelpers

      FILE_PATH_BATCH_SIZE = 500
-      UPLOAD_DIR = "#{CarrierWave.root}/uploads".freeze
+      RELATIVE_UPLOAD_DIR = "uploads".freeze
+      ABSOLUTE_UPLOAD_DIR = "#{CarrierWave.root}/#{RELATIVE_UPLOAD_DIR}".freeze
      FOLLOW_UP_MIGRATION = 'PopulateUntrackedUploads'.freeze
+      START_WITH_CARRIERWAVE_ROOT_REGEX = %r{\A#{CarrierWave.root}/}
+      EXCLUDED_HASHED_UPLOADS_PATH = "#{ABSOLUTE_UPLOAD_DIR}/@hashed/*".freeze
+      EXCLUDED_TMP_UPLOADS_PATH = "#{ABSOLUTE_UPLOAD_DIR}/tmp/*".freeze

      class UntrackedFile < ActiveRecord::Base
        include EachBatch
@@ -28,9 +32,9 @@ def migrate?
      end

      def store_untracked_file_paths
-        return unless Dir.exist?(UPLOAD_DIR)
+        return unless Dir.exist?(ABSOLUTE_UPLOAD_DIR)

-        each_file_batch(UPLOAD_DIR, FILE_PATH_BATCH_SIZE) do |file_paths|
+        each_file_batch(ABSOLUTE_UPLOAD_DIR, FILE_PATH_BATCH_SIZE) do |file_paths|
          insert_file_paths(file_paths)
        end
      end
@@ -49,7 +53,7 @@ def yield_paths_in_batches(stdout, batch_size, &block)
        paths = []

        stdout.each_line("\0") do |line|
-          paths << line.chomp("\0")
+          paths << line.chomp("\0").sub(START_WITH_CARRIERWAVE_ROOT_REGEX, '')

          if paths.size >= batch_size
            yield(paths)
@@ -61,9 +65,7 @@ def yield_paths_in_batches(stdout, batch_size, &block)
      end

      def build_find_command(search_dir)
-        hashed_path = "#{UPLOAD_DIR}/@hashed/*"
-        tmp_path = "#{UPLOAD_DIR}/tmp/*"
-        cmd = %W[find #{search_dir} -type f ! ( -path #{hashed_path} -prune ) ! ( -path #{tmp_path} -prune ) -print0]
+        cmd = %W[find #{search_dir} -type f ! ( -path #{EXCLUDED_HASHED_UPLOADS_PATH} -prune ) ! ( -path #{EXCLUDED_TMP_UPLOADS_PATH} -prune ) -print0]

        cmd = %w[ionice -c Idle] + cmd if ionice_is_available?


--- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb
+++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb
--- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
+++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
@@ -47,6 +47,15 @@
      end
    end

+    it 'adds files with paths relative to CarrierWave.root' do
+      Sidekiq::Testing.fake! do
+        described_class.new.perform
+        untracked_files_for_uploads.all.each do |file|
+          expect(file.path.start_with?('uploads/')).to be_truthy
+        end
+      end
+    end
+
    it 'does not add hashed files to the untracked_files_for_uploads table' do
      Sidekiq::Testing.fake! do
        described_class.new.perform
@@ -83,7 +92,7 @@
    # E.g. The installation is in use at the time of migration, and someone has
    # just uploaded a file
    context 'when there are files in /uploads/tmp' do
-      let(:tmp_file) { Rails.root.join(described_class::UPLOAD_DIR, 'tmp', 'some_file.jpg') }
+      let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') }

      before do
        FileUtils.touch(tmp_file)

--- a/spec/migrations/track_untracked_uploads_spec.rb
+++ b/spec/migrations/track_untracked_uploads_spec.rb
@@ -41,15 +41,13 @@ class UntrackedFile < ActiveRecord::Base
    component = 'a'*255

    long_path = [
-      CarrierWave.root,
      'uploads',
-      [component] * Namespace::NUMBER_OF_ANCESTORS_ALLOWED, # namespaces
-      component, # project
+      component, # project.full_path
      component  # filename
    ].flatten.join('/')

    record = UntrackedFile.create!(path: long_path)
-    expect(record.reload.path.size).to eq(5711)
+    expect(record.reload.path.size).to eq(519)
  end

  context 'with tracked and untracked uploads' do