enable custom Blob#key configuration

frantisekrokusekpa · frantisekrokusekpa · commit 80dfe9157e2d · 2021-09-26T23:43:14.000+02:00
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
@@ -235,3 +235,21 @@
 
 
 Please check [6-1-stable](https://github.com/rails/rails/blob/6-1-stable/activestorage/CHANGELOG.md) for previous changes.
+
+
+*   Add ability to customize the Blob#key per Model and attribute basis. Active Storage will save the Blob's attachment on the specified service at the configured key. You can also _interpolate_ values in it automatically with configurable procs.
+
+    ```ruby
+    # app/models/user.rb
+    has_one_attached :avatar,
+                     key: ':tenant/users/:record_id/avatar/:blob_checksum'
+
+    # with the following configuration
+    config.active_storage.key_interpolation_procs = {
+      tenant:        ->(record, blob) { Apartment::Tenant.current.parameterize },
+      record_id:     ->(record, blob) { record.id },
+      blob_checksum: ->(record, blob) { blob.checksum }
+    }
+    ```
+
+    *František Rokůsek*
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
@@ -37,6 +37,7 @@ class ActiveStorage::Blob < ActiveStorage::Record
   self.table_name = "active_storage_blobs"
 
   MINIMUM_TOKEN_LENGTH = 28
+  KEY_PATH_SEPARATOR = "/"
 
   has_secure_token :key, length: MINIMUM_TOKEN_LENGTH
   store :metadata, accessors: [ :analyzed, :identified ], coder: ActiveRecord::Coders::JSON
@@ -68,6 +69,11 @@ class ActiveStorage::Blob < ActiveStorage::Record
     end
   end
 
+  def move_to!(target_key)
+    service.move(key, target_key)
+    update_columns(key: target_key)
+  end
+
   class << self
     # You can use the signed ID of a blob to refer to it on the client side without fear of tampering.
     # This is particularly helpful for direct uploads where the client-side needs to refer to the blob
@@ -145,6 +151,24 @@ def scope_for_strict_loading # :nodoc:
         all
       end
     end
+
+    # Interpolates the custom storage key if needed and appends to it a unique_secure_token
+    def generate_unique_interpolated_secure_key(key:, record:, blob:, length: MINIMUM_TOKEN_LENGTH)
+      [
+        interpolate(key: key, record: record, blob: blob),
+        generate_unique_secure_token(length: length)
+      ].compact.join(KEY_PATH_SEPARATOR)
+    end
+
+    # Interpolates configured variables into keys,
+    # with procs coming from key_interpolation_procs configuration hash
+    def interpolate(key:, record:, blob:)
+      key.split(KEY_PATH_SEPARATOR).map do |key_part|
+        key_part.gsub(/:(\w*)/) do
+          ActiveStorage.key_interpolation_procs[$1.to_sym].call(record, blob)
+        end
+      end.join(KEY_PATH_SEPARATOR)
+    end
   end
 
   # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering.
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
@@ -63,6 +63,7 @@ module ActiveStorage
   mattr_accessor :urls_expire_in
 
   mattr_accessor :routes_prefix, default: "/rails/active_storage"
+  mattr_accessor :key_interpolation_procs, default: {}
   mattr_accessor :draw_routes, default: true
   mattr_accessor :resolve_model_to_route, default: :rails_storage_redirect
 
diff --git a/activestorage/lib/active_storage/attached/changes/create_many.rb b/activestorage/lib/active_storage/attached/changes/create_many.rb
@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
 module ActiveStorage
-  class Attached::Changes::CreateMany # :nodoc:
-    attr_reader :name, :record, :attachables
+  class Attached::Changes::CreateMany #:nodoc:
+    attr_reader :name, :record, :attachables, :key
 
-    def initialize(name, record, attachables)
-      @name, @record, @attachables = name, record, Array(attachables)
+    def initialize(name, record, attachables, key)
+      @name, @record, @attachables, @key = name, record, Array(attachables), key
       blobs.each(&:identify_without_saving)
       attachments
     end
@@ -25,6 +25,14 @@ def upload
     def save
       assign_associated_attachments
       reset_associated_blobs
+
+      unless key.blank?
+        blobs.each |blob| do
+          blob.move_to!(
+            ActiveStorage::Blob.generate_unique_interpolated_secure_key(key: key, record: record, blob: blob)
+          )
+        end
+      end
     end
 
     private
@@ -33,7 +41,7 @@ def subchanges
       end
 
       def build_subchange_from(attachable)
-        ActiveStorage::Attached::Changes::CreateOneOfMany.new(name, record, attachable)
+        ActiveStorage::Attached::Changes::CreateOneOfMany.new(name, record, attachable, key)
       end
 
       def assign_associated_attachments
diff --git a/activestorage/lib/active_storage/attached/changes/create_one.rb b/activestorage/lib/active_storage/attached/changes/create_one.rb
@@ -4,11 +4,11 @@
 require "action_dispatch/http/upload"
 
 module ActiveStorage
-  class Attached::Changes::CreateOne # :nodoc:
-    attr_reader :name, :record, :attachable
+  class Attached::Changes::CreateOne #:nodoc:
+    attr_reader :name, :record, :attachable, :key
 
-    def initialize(name, record, attachable)
-      @name, @record, @attachable = name, record, attachable
+    def initialize(name, record, attachable, key)
+      @name, @record, @attachable, @key = name, record, attachable, key
       blob.identify_without_saving
     end
 
@@ -32,6 +32,12 @@ def upload
     def save
       record.public_send("#{name}_attachment=", attachment)
       record.public_send("#{name}_blob=", blob)
+
+      unless key.blank?
+        blob.move_to!(
+          ActiveStorage::Blob.generate_unique_interpolated_secure_key(key: key, record: record, blob: blob)
+        )
+      end
     end
 
     private
diff --git a/activestorage/lib/active_storage/attached/model.rb b/activestorage/lib/active_storage/attached/model.rb
@@ -7,6 +7,8 @@ module ActiveStorage
   module Attached::Model
     extend ActiveSupport::Concern
 
+    KEY_PATH_SEPARATOR = "/"
+
     class_methods do
       # Specifies the relation between a single attachment and the model.
       #
@@ -47,8 +49,17 @@ module Attached::Model
       #     has_one_attached :avatar, strict_loading: true
       #   end
       #
-      def has_one_attached(name, dependent: :purge_later, service: nil, strict_loading: false)
+      # If you need the attachment to have a specific storage path, on a Model level (other than the global 'route_prefix'),
+      # pass the +:key+ option. For instance:
+      #
+      #   class User < ActiveRecord::Base
+      #     has_one_attached :avatar, key: 'tenant/:tenant_subdomain/users/:record_id/avatar'
+      #   end
+      #
+      #
+      def has_one_attached(name, dependent: :purge_later, service: nil, strict_loading: false, key: nil)
         validate_service_configuration(name, service)
+        validate_key_interpolations(name, key)
 
         generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
           # frozen_string_literal: true
@@ -62,7 +73,7 @@ def #{name}=(attachable)
               if attachable.nil?
                 ActiveStorage::Attached::Changes::DeleteOne.new("#{name}", self)
               else
-                ActiveStorage::Attached::Changes::CreateOne.new("#{name}", self, attachable)
+                ActiveStorage::Attached::Changes::CreateOne.new("#{name}", self, attachable, "#{key}")
               end
           end
         CODE
@@ -126,8 +137,16 @@ def #{name}=(attachable)
       #     has_many_attached :photos, strict_loading: true
       #   end
       #
-      def has_many_attached(name, dependent: :purge_later, service: nil, strict_loading: false)
+      # If you need the attachments to have a specific storage path, on a Model level (other than the global 'route_prefix'),
+      # pass the +:key+ option. For instance:
+      #
+      #   class User < ActiveRecord::Base
+      #     has_many_attached :photos, key: 'tenant/:tenant_subdomain/users/:record_id/photos'
+      #   end
+      #
+      def has_many_attached(name, dependent: :purge_later, service: nil, strict_loading: false, key: nil)
         validate_service_configuration(name, service)
+        validate_key_interpolations(name, key)
 
         generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
           # frozen_string_literal: true
@@ -142,7 +161,7 @@ def #{name}=(attachables)
                 if Array(attachables).none?
                   ActiveStorage::Attached::Changes::DeleteMany.new("#{name}", self)
                 else
-                  ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables)
+                  ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables, "#{key}")
                 end
             else
               ActiveSupport::Deprecation.warn \
@@ -153,7 +172,7 @@ def #{name}=(attachables)
 
               if Array(attachables).any?
                 attachment_changes["#{name}"] =
-                  ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, #{name}.blobs + attachables)
+                  ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, #{name}.blobs + attachables, "#{key}")
               end
             end
           end
@@ -215,6 +234,17 @@ def validate_service_configuration(association_name, service)
             end
           end
         end
+
+        def validate_key_interpolations(association_name, key)
+          return if key.blank?
+
+          key.scan(/:(\w+)/) do
+            key_part = $1.to_sym
+            unless ActiveStorage.key_interpolation_procs.keys.include?(key_part)
+              raise ArgumentError, "Cannot configure #{key_part} in interpolation key '#{key}' for #{name}##{association_name}"
+            end
+          end
+        end
     end
 
     def attachment_changes # :nodoc:
diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb
@@ -91,6 +91,7 @@ class Engine < Rails::Engine # :nodoc:
         ActiveStorage.routes_prefix     = app.config.active_storage.routes_prefix || "/rails/active_storage"
         ActiveStorage.draw_routes       = app.config.active_storage.draw_routes != false
         ActiveStorage.resolve_model_to_route = app.config.active_storage.resolve_model_to_route || :rails_storage_redirect
+        ActiveStorage.key_interpolation_procs = app.config.active_storage.key_interpolation_procs || {}
 
         ActiveStorage.variable_content_types = app.config.active_storage.variable_content_types || []
         ActiveStorage.web_image_content_types = app.config.active_storage.web_image_content_types || []
diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb
@@ -105,6 +105,10 @@ def exist?(key)
       raise NotImplementedError
     end
 
+    def move(source_key, target_key)
+      raise NotImplementedError
+    end
+
     # Returns the URL for the file at the +key+. This returns a permanent URL for public files, and returns a
     # short-lived URL for private files. For private files you can provide the +disposition+ (+:inline+ or +:attachment+),
     # +filename+, and +content_type+ that you wish the file to be served with on request. Additionally, you can also provide
diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb
@@ -86,6 +86,13 @@ def exist?(key)
       end
     end
 
+    def move(source_key, target_key)
+      instrument :move, source_key: source_key, target_key: target_key do
+        client.copy_blob(container, target_key, container, source_key)
+        client.delete_blob(container, source_key)
+      end
+    end
+
     def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:)
       instrument :url, key: key do |payload|
         generated_url = signer.signed_uri(
diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb
@@ -72,6 +72,10 @@ def exist?(key)
       end
     end
 
+    def move(source_key, target_key)
+      File.rename path_for(source_key), path_for(target_key)
+    end
+
     def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:)
       instrument :url, key: key do |payload|
         verified_token_with_expiration = ActiveStorage.verifier.generate(
diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb
@@ -86,6 +86,14 @@ def exist?(key)
       end
     end
 
+    def move(source_key, target_key)
+      instrument :move, source_key: source_key, target_key: target_key do
+        file = file_for(source_key)
+        file.copy target_key
+        file.delete
+      end
+    end
+
     def url_for_direct_upload(key, expires_in:, checksum:, **)
       instrument :url, key: key do |payload|
         headers = {}
diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb
@@ -77,6 +77,12 @@ def exist?(key)
       end
     end
 
+    def move(source_key, target_key)
+      instrument :move, source_key: source_key, target_key: target_key do
+        bucket.object(source_key).copy_to(bucket.object(target_key))
+      end
+    end
+
     def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:)
       instrument :url, key: key do |payload|
         generated_url = object_for(key).presigned_url :put, expires_in: expires_in.to_i,
diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb
@@ -794,6 +794,50 @@ def highlights
         @user.update! highlights: [create_blob(filename: "whenever.jpg")]
       end
     end
+
+  test "attaching a new blob from a Hash with custom storage key" do
+    @user.photos.attach(
+      { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpg" },
+      { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpeg" }
+    )
+
+    assert_equal "funky.jpg", @user.photos.first.filename.to_s
+    assert_equal "town.jpg", @user.photos.second.filename.to_s
+
+    assert_match "users/#{@user.id}/photos", @user.photos.first.key.to_s
+    assert_match "users/#{@user.id}/photos", @user.photos.second.key.to_s
+  end
+
+  test "attaching a new blob from an uploaded file with custom storage key" do
+    @user.photos.attach fixture_file_upload("racecar.jpg"), fixture_file_upload("video.mp4")
+
+    assert_equal "racecar.jpg", @user.photos.first.filename.to_s
+    assert_equal "video.mp4", @user.photos.second.filename.to_s
+
+    assert_match "users/#{@user.id}/photos", @user.photos.first.key.to_s
+    assert_match "users/#{@user.id}/photos", @user.photos.second.key.to_s
+  end
+
+  test "attaching a new blob from a Hash with a specified key does override default custom storage key" do
+    @user.photos.attach(
+      { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpg" },
+      { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpeg", key: "some/specific/key" }
+    )
+
+    assert_equal "funky.jpg", @user.photos.first.filename.to_s
+    assert_equal "town.jpg", @user.photos.second.filename.to_s
+
+    assert_match "users/#{@user.id}/photos", @user.photos.first.key.to_s
+    assert_match "some/specific/key", @user.photos.second.key.to_s
+  end
+
+  test "raises error when misconfigured interpolation key is passed" do
+    error = assert_raises ArgumentError do
+      User.class_eval do
+        has_many_attached :featured_photos, key: "some/:unconfigured/interpolation/key"
+      end
+    end
+    assert_match(/Cannot configure :unconfigured in interpolation key 'some\/:unconfigured\/interpolation\/key' for User#featured_photos/, error.message)
   end
 
   private
diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb
diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb
diff --git a/guides/source/configuring.md b/guides/source/configuring.md
