Merge branch 'poc-upload-hashing-path' into 'master'

File uploads on objects storage should use hashed storage

Closes #4952

See merge request gitlab-org/gitlab-ee!4597

3 files changed, 34 insertions, 22 deletions

diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 0e2da64de6a..133fdf6684d 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -32,8 +32,11 @@ class FileUploader < GitlabUploader
     )
   end
 
-  def self.base_dir(model)
-    model_path_segment(model)
+  def self.base_dir(model, store = Store::LOCAL)
+    decorated_model = model
+    decorated_model = Storage::HashedProject.new(model) if store == Store::REMOTE
+
+    model_path_segment(decorated_model)
   end
 
   # used in migrations and import/exports
@@ -51,21 +54,24 @@ class FileUploader < GitlabUploader
   #
   # Returns a String without a trailing slash
   def self.model_path_segment(model)
-    if model.hashed_storage?(:attachments)
-      model.disk_path
+    case model
+    when Storage::HashedProject then model.disk_path
     else
-      model.full_path
+      model.hashed_storage?(:attachments) ? model.disk_path : model.full_path
     end
   end
 
-  def self.upload_path(secret, identifier)
-    File.join(secret, identifier)
-  end
-
   def self.generate_secret
     SecureRandom.hex
   end
 
+  def upload_paths(filename)
+    [
+      File.join(secret, filename),
+      File.join(base_dir(Store::REMOTE), secret, filename)
+    ]
+  end
+
   attr_accessor :model
 
   def initialize(model, mounted_as = nil, **uploader_context)
@@ -75,8 +81,10 @@ class FileUploader < GitlabUploader
     apply_context!(uploader_context)
   end
 
-  def base_dir
-    self.class.base_dir(@model)
+  # enforce the usage of Hashed storage when storing to
+  # remote store as the FileMover doesn't support OS
+  def base_dir(store = nil)
+    self.class.base_dir(@model, store || object_store)
   end
 
   # we don't need to know the actual path, an uploader instance should be
@@ -86,15 +94,19 @@ class FileUploader < GitlabUploader
   end
 
   def upload_path
-    self.class.upload_path(dynamic_segment, identifier)
-  end
-
-  def model_path_segment
-    self.class.model_path_segment(@model)
+    if file_storage?
+      # Legacy path relative to project.full_path
+      File.join(dynamic_segment, identifier)
+    else
+      File.join(store_dir, identifier)
+    end
   end
 
-  def store_dir
-    File.join(base_dir, dynamic_segment)
+  def store_dirs
+    {
+      Store::LOCAL => File.join(base_dir, dynamic_segment),
+      Store::REMOTE => File.join(base_dir(ObjectStorage::Store::REMOTE), dynamic_segment)
+    }
   end
 
   def markdown_link
diff --git a/app/uploaders/namespace_file_uploader.rb b/app/uploaders/namespace_file_uploader.rb
index 269415b1926..1085ecb1700 100644
--- a/app/uploaders/namespace_file_uploader.rb
+++ b/app/uploaders/namespace_file_uploader.rb
@@ -4,7 +4,7 @@ class NamespaceFileUploader < FileUploader
     options.storage_path
   end
 
-  def self.base_dir(model)
+  def self.base_dir(model, _store = nil)
     File.join(options.base_dir, 'namespace', model_path_segment(model))
   end
 
@@ -20,7 +20,7 @@ class NamespaceFileUploader < FileUploader
   def store_dirs
     {
       Store::LOCAL => File.join(base_dir, dynamic_segment),
-      Store::REMOTE => File.join('namespace', model_path_segment, dynamic_segment)
+      Store::REMOTE => File.join('namespace', self.class.model_path_segment(model), dynamic_segment)
     }
   end
 end
diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb
index d225002ad2e..e3898b07730 100644
--- a/app/uploaders/personal_file_uploader.rb
+++ b/app/uploaders/personal_file_uploader.rb
@@ -4,7 +4,7 @@ class PersonalFileUploader < FileUploader
     options.storage_path
   end
 
-  def self.base_dir(model)
+  def self.base_dir(model, _store = nil)
     File.join(options.base_dir, model_path_segment(model))
   end
 
@@ -34,7 +34,7 @@ class PersonalFileUploader < FileUploader
   def store_dirs
     {
       Store::LOCAL => File.join(base_dir, dynamic_segment),
-      Store::REMOTE => File.join(model_path_segment, dynamic_segment)
+      Store::REMOTE => File.join(self.class.model_path_segment(model), dynamic_segment)
     }
   end