minor tweaks in Active Storage after a walkthrough

15 files changed, 59 insertions, 48 deletions

diff --git a/activestorage/README.md b/activestorage/README.md
index d4ffe0c484..17d98978bb 100644
--- a/activestorage/README.md
+++ b/activestorage/README.md
@@ -1,17 +1,16 @@
 # Active Storage
 
-Active Storage makes it simple to upload and reference files in cloud services, like Amazon S3, Google Cloud Storage or Microsoft Azure Storage and attach those files to Active Records. It also provides a disk service for testing or local deployments, but the focus is on cloud storage.
+Active Storage makes it simple to upload and reference files in cloud services like Amazon S3, Google Cloud Storage, or Microsoft Azure Storage, and attach those files to Active Records. Supports having one main service and mirrors in other services for redundancy. It also provides a disk service for testing or local deployments, but the focus is on cloud storage.
 
 Files can be uploaded from the server to the cloud or directly from the client to the cloud.
 
-Image files can further more be transformed using on-demand variants for quality, aspect ratio, size, or any other
-MiniMagick supported transformation.
+Image files can furthermore be transformed using on-demand variants for quality, aspect ratio, size, or any other [MiniMagick](https://github.com/minimagick/minimagick) supported transformation.
 
 ## Compared to other storage solutions
 
-A key difference to how Active Storage works compared to other attachment solutions in Rails is through the use of built-in [Blob](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/blob.rb) and [Attachment](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/attachment.rb) models (backed by Active Record). This means existing application models do not need to be modified with additional columns to associate with files. Active Storage uses polymorphic associations via the join model of `Attachment`, which then connects to the actual `Blob`.
+A key difference to how Active Storage works compared to other attachment solutions in Rails is through the use of built-in [Blob](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/blob.rb) and [Attachment](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/attachment.rb) models (backed by Active Record). This means existing application models do not need to be modified with additional columns to associate with files. Active Storage uses polymorphic associations via the `Attachment` join model, which then connects to the actual `Blob`.
 
-These `Blob` models are intended to be immutable in spirit. One file, one blob. You can associate the same blob with multiple application models as well. And if you want to do transformations of a given `Blob`, the idea is that you'll simply create a new one, rather than attempt to mutate the existing (though of course you can delete that later if you don't need it).
+`Blob` models store attachment metadata (filename, content-type, etc.), and their identifier key in the storage service. Blob models do not store the actual binary data. They are intended to be immutable in spirit. One file, one blob. You can associate the same blob with multiple application models as well. And if you want to do transformations of a given `Blob`, the idea is that you'll simply create a new one, rather than attempt to mutate the existing one (though of course you can delete the previous version later if you don't need it).
 
 ## Examples
 
@@ -19,16 +18,32 @@ One attachment:
 
 ```ruby
 class User < ApplicationRecord
+  # Associates an attachment and a blob. When the user is destroyed they are
+  # purged by default (models destroyed, and resource files deleted).
   has_one_attached :avatar
 end
 
-user.avatar.attach io: File.open("~/face.jpg"), filename: "avatar.jpg", content_type: "image/jpg"
+# Attach an avatar to the user.
+user.avatar.attach(io: File.open("~/face.jpg"), filename: "avatar.jpg", content_type: "image/jpg")
+
+# Does the user have an avatar?
 user.avatar.attached? # => true
 
+# Synchronously destroy the avatar and actual resource files.
 user.avatar.purge
+
+# Destroy the associated models and actual resource files async, via Active Job.
+user.avatar.purge_later
+
+# Does the user have an avatar?
 user.avatar.attached? # => false
 
-url_for(user.avatar) # Generate a permanent URL for the blob, which upon access will redirect to a temporary service URL.
+# Generate a permanent URL for the blob that points to the application.
+# Upon access, a redirect to the actual service endpoint is returned.
+# This indirection decouples the public URL from the actual one, and
+# allows for example mirroring attachments in different services for
+# high-availability. The redirection has an HTTP expiration of 5 min.
+url_for(user.avatar)
 
 class AvatarsController < ApplicationController
   def update
diff --git a/activestorage/app/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb
index b10d4e2cac..dee88054da 100644
--- a/activestorage/app/controllers/active_storage/disk_controller.rb
+++ b/activestorage/app/controllers/active_storage/disk_controller.rb
@@ -33,7 +33,6 @@ class ActiveStorage::DiskController < ActionController::Base
       ActiveStorage::Blob.service
     end
 
-
     def decode_verified_key
       ActiveStorage.verifier.verified(params[:encoded_key], purpose: :blob_key)
     end
@@ -42,7 +41,6 @@ class ActiveStorage::DiskController < ActionController::Base
       params[:disposition].presence_in(%w( inline attachment )) || "inline"
     end
 
-
     def decode_verified_token
       ActiveStorage.verifier.verified(params[:encoded_token], purpose: :blob_token)
     end
diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb
index 404ccefd05..369c07929d 100644
--- a/activestorage/app/jobs/active_storage/purge_job.rb
+++ b/activestorage/app/jobs/active_storage/purge_job.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Provides delayed purging of attachments or blobs using their +#purge_later+ method.
+# Provides delayed purging of attachments or blobs using their +purge_later+ method.
 class ActiveStorage::PurgeJob < ActiveJob::Base
   # FIXME: Limit this to a custom ActiveStorage error
   retry_on StandardError
diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb
index adfa6d4aa5..ad43845e4e 100644
--- a/activestorage/app/models/active_storage/attachment.rb
+++ b/activestorage/app/models/active_storage/attachment.rb
@@ -4,7 +4,7 @@ require "active_support/core_ext/module/delegation"
 
 # Attachments associate records with blobs. Usually that's a one record-many blobs relationship,
 # but it is possible to associate many different records with the same blob. If you're doing that,
-# you'll want to declare with `has_one/many_attached :thingy, dependent: false`, so that destroying
+# you'll want to declare with <tt>has_one/many_attached :thingy, dependent: false</tt>, so that destroying
 # any one record won't destroy the blob as well. (Then you'll need to do your own garbage collecting, though).
 class ActiveStorage::Attachment < ActiveRecord::Base
   self.table_name = "active_storage_attachments"
@@ -22,8 +22,8 @@ class ActiveStorage::Attachment < ActiveRecord::Base
   end
 
   # Purging an attachment means purging the blob, which means talking to the service, which means
-  # talking over the internet. Whenever you're doing that, it's a good idea to put that work in a job,
-  # so it doesn't hold up other operations. That's what +#purge_later+ provides.
+  # talking over the Internet. Whenever you're doing that, it's a good idea to put that work in a job,
+  # so it doesn't hold up other operations. That's what +purge_later+ provides.
   def purge_later
     ActiveStorage::PurgeJob.perform_later(self)
   end
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index dc91a9265f..36b8a35778 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -3,8 +3,8 @@
 # A blob is a record that contains the metadata about a file and a key for where that file resides on the service.
 # Blobs can be created in two ways:
 #
-# 1) Subsequent to the file being uploaded server-side to the service via #create_after_upload!
-# 2) Ahead of the file being directly uploaded client-side to the service via #create_before_direct_upload!
+# 1) Subsequent to the file being uploaded server-side to the service via <tt>create_after_upload!</tt>.
+# 2) Ahead of the file being directly uploaded client-side to the service via <tt>create_before_direct_upload!</tt>.
 #
 # The first option doesn't require any client-side JavaScript integration, and can be used by any other back-end
 # service that deals with files. The second option is faster, since you're not using your own server as a staging
@@ -12,7 +12,7 @@
 #
 # Blobs are intended to be immutable in as-so-far as their reference to a specific file goes. You're allowed to
 # update a blob's metadata on a subsequent pass, but you should not update the key or change the uploaded file.
-# If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old.
+# If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old one.
 class ActiveStorage::Blob < ActiveRecord::Base
   self.table_name = "active_storage_blobs"
 
@@ -22,11 +22,11 @@ class ActiveStorage::Blob < ActiveRecord::Base
   class_attribute :service
 
   class << self
-    # You can used 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
+    # You can used 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
     # that was created ahead of the upload itself on form submission.
     #
-    # The signed id is also used to create stable URLs for the blob through the BlobsController.
+    # The signed ID is also used to create stable URLs for the blob through the BlobsController.
     def find_signed(id)
       find ActiveStorage.verifier.verify(id, purpose: :blob_id)
     end
@@ -43,8 +43,8 @@ class ActiveStorage::Blob < ActiveRecord::Base
     end
 
     # Returns a saved blob instance after the +io+ has been uploaded to the service. Note, the blob is first built,
-    # then the +io+ is uploaded, then the blob is saved. This is doing to avoid opening a transaction and talking to
-    # the service during that (which is a bad idea and leads to deadlocks).
+    # then the +io+ is uploaded, then the blob is saved. This is done this way to avoid uploading (which may take
+    # time), while having an open database transaction.
     def create_after_upload!(io:, filename:, content_type: nil, metadata: nil)
       build_after_upload(io: io, filename: filename, content_type: content_type, metadata: metadata).tap(&:save!)
     end
@@ -59,9 +59,8 @@ class ActiveStorage::Blob < ActiveRecord::Base
     end
   end
 
-
   # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering.
-  # It uses the framework-wide verifier on `ActiveStorage.verifier`, but with a dedicated purpose.
+  # It uses the framework-wide verifier on <tt>ActiveStorage.verifier</tt>, but with a dedicated purpose.
   def signed_id
     ActiveStorage.verifier.generate(id, purpose: :blob_id)
   end
@@ -74,8 +73,9 @@ class ActiveStorage::Blob < ActiveRecord::Base
     self[:key] ||= self.class.generate_unique_secure_token
   end
 
-  # Returns a ActiveStorage::Filename instance of the filename that can be queried for basename, extension, and
-  # a sanitized version of the filename that's safe to use in URLs.
+  # Returns an ActiveStorage::Filename instance of the filename that can be
+  # queried for basename, extension, and a sanitized version of the filename
+  # that's safe to use in URLs.
   def filename
     ActiveStorage::Filename.new(self[:filename])
   end
@@ -100,8 +100,9 @@ class ActiveStorage::Blob < ActiveRecord::Base
     content_type.start_with?("text")
   end
 
-  # Returns a ActiveStorage::Variant instance with the set of +transformations+ passed in. This is only relevant
-  # for image files, and it allows any image to be transformed for size, colors, and the like. Example:
+  # Returns an ActiveStorage::Variant instance with the set of +transformations+
+  # passed in. This is only relevant for image files, and it allows any image to
+  # be transformed for size, colors, and the like. Example:
   #
   #   avatar.variant(resize: "100x100").processed.service_url
   #
@@ -119,7 +120,6 @@ class ActiveStorage::Blob < ActiveRecord::Base
     ActiveStorage::Variant.new(self, ActiveStorage::Variation.new(transformations))
   end
 
-
   # Returns the URL of the blob on the service. This URL is intended to be short-lived for security and not used directly
   # with users. Instead, the +service_url+ should only be exposed as a redirect from a stable, possibly authenticated URL.
   # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And
@@ -162,7 +162,6 @@ class ActiveStorage::Blob < ActiveRecord::Base
     service.download key, &block
   end
 
-
   # Deletes the file on the service that's associated with this blob. This should only be done if the blob is going to be
   # deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+
   # methods in most circumstances.
@@ -178,7 +177,7 @@ class ActiveStorage::Blob < ActiveRecord::Base
     destroy
   end
 
-  # Enqueues a ActiveStorage::PurgeJob job that'll call +#purge+. This is the recommended way to purge blobs when the call
+  # Enqueues an ActiveStorage::PurgeJob job that'll call +purge+. This is the recommended way to purge blobs when the call
   # needs to be made from a transaction, a callback, or any other real-time scenario.
   def purge_later
     ActiveStorage::PurgeJob.perform_later(self)
diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb
index 7b4ca18c8c..92fa445095 100644
--- a/activestorage/app/models/active_storage/variant.rb
+++ b/activestorage/app/models/active_storage/variant.rb
@@ -4,8 +4,8 @@
 # These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the
 # original.
 #
-# Variants rely on `MiniMagick` for the actual transformations of the file, so you must add `gem "mini_magick"`
-# to your Gemfile if you wish to use variants.
+# Variants rely on {MiniMagick}(https://github.com/minimagick/minimagick) for the actual transformations
+# of the file, so you must add <tt>gem "mini_magick"</tt> to your Gemfile if you wish to use variants.
 #
 # Note that to create a variant it's necessary to download the entire blob file from the service and load it
 # into memory. The larger the image, the more memory is used. Because of this process, you also want to be
@@ -21,7 +21,7 @@
 # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::VariantsController
 # can then produce on-demand.
 #
-# When you do want to actually produce the variant needed, call +#processed+. This will check that the variant
+# When you do want to actually produce the variant needed, call +processed+. This will check that the variant
 # has already been processed and uploaded to the service, and, if so, just return that. Otherwise it will perform
 # the transformations, upload the variant to the service, and return itself again. Example:
 #
@@ -58,14 +58,13 @@ class ActiveStorage::Variant
   # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And
   # it allows permanent URLs that redirect to the +service_url+ to be cached in the view.
   #
-  # Use `url_for(variant)` (or the implied form, like `link_to variant` or `redirect_to variant`) to get the stable URL
-  # for a variant that points to the ActiveStorage::VariantsController, which in turn will use this +#service_call+ method
+  # Use <tt>url_for(variant)</tt> (or the implied form, like +link_to variant+ or +redirect_to variant+) to get the stable URL
+  # for a variant that points to the ActiveStorage::VariantsController, which in turn will use this +service_call+ method
   # for its redirection.
   def service_url(expires_in: 5.minutes, disposition: :inline)
     service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type
   end
 
-
   private
     def processed?
       service.exist?(key)
diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb
index 396ce85ef1..f657d90db4 100644
--- a/activestorage/app/models/active_storage/variation.rb
+++ b/activestorage/app/models/active_storage/variation.rb
@@ -15,13 +15,13 @@ class ActiveStorage::Variation
   attr_reader :transformations
 
   class << self
-    # Returns a variation instance with the transformations that were encoded by +#encode+.
+    # Returns a variation instance with the transformations that were encoded by +encode+.
     def decode(key)
       new ActiveStorage.verifier.verify(key, purpose: :variation)
     end
 
     # Returns a signed key for the +transformations+, which can be used to refer to a specific
-    # variation in a URL or combined key (like `ActiveStorage::Variant#key`).
+    # variation in a URL or combined key (like <tt>ActiveStorage::Variant#key</tt>).
     def encode(transformations)
       ActiveStorage.verifier.generate(transformations, purpose: :variation)
     end
@@ -31,7 +31,7 @@ class ActiveStorage::Variation
     @transformations = transformations
   end
 
-  # Accepts an open MiniMagick image instance, like what's return by `MiniMagick::Image.read(io)`,
+  # Accepts an open MiniMagick image instance, like what's returned by <tt>MiniMagick::Image.read(io)</tt>,
   # and performs the +transformations+ against it. The transformed image instance is then returned.
   def transform(image)
     transformations.each do |(method, argument)|
diff --git a/activestorage/lib/active_storage/attached.rb b/activestorage/lib/active_storage/attached.rb
index e90b75afd0..9c4b6d5d1d 100644
--- a/activestorage/lib/active_storage/attached.rb
+++ b/activestorage/lib/active_storage/attached.rb
@@ -5,8 +5,8 @@ require "action_dispatch/http/upload"
 require "active_support/core_ext/module/delegation"
 
 module ActiveStorage
-# Abstract baseclass for the concrete ActiveStorage::Attached::One and ActiveStorage::Attached::Many
-# classes that both provide proxy access to the blob association for a record.
+  # Abstract base class for the concrete ActiveStorage::Attached::One and ActiveStorage::Attached::Many
+  # classes that both provide proxy access to the blob association for a record.
   class Attached
     attr_reader :name, :record
 
diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb
index 027112195f..f3879ee2e3 100644
--- a/activestorage/lib/active_storage/attached/macros.rb
+++ b/activestorage/lib/active_storage/attached/macros.rb
@@ -19,7 +19,7 @@ module ActiveStorage
     # most circumstances.
     #
     # The system has been designed to having you go through the ActiveStorage::Attached::One
-    # proxy that provides the dynamic proxy to the associations and factory methods, like +#attach+.
+    # proxy that provides the dynamic proxy to the associations and factory methods, like +attach+.
     #
     # If the +:dependent+ option isn't set, the attachment will be purged
     # (i.e. destroyed) whenever the record is destroyed.
diff --git a/activestorage/lib/active_storage/gem_version.rb b/activestorage/lib/active_storage/gem_version.rb
index bb6241ae47..e1d7b3493a 100644
--- a/activestorage/lib/active_storage/gem_version.rb
+++ b/activestorage/lib/active_storage/gem_version.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module ActiveStorage
-  # Returns the version of the currently loaded Active Storage as a <tt>Gem::Version</tt>
+  # Returns the version of the currently loaded Active Storage as a <tt>Gem::Version</tt>.
   def self.gem_version
     Gem::Version.new VERSION::STRING
   end
diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb
index 80cc5d5c80..0d00a75c0e 100644
--- a/activestorage/lib/active_storage/log_subscriber.rb
+++ b/activestorage/lib/active_storage/log_subscriber.rb
@@ -19,7 +19,7 @@ module ActiveStorage
     end
 
     def service_exist(event)
-      debug event, color("Checked if file exist at key: #{key_in(event)} (#{event.payload[:exist] ? "yes" : "no"})", BLUE)
+      debug event, color("Checked if file exists at key: #{key_in(event)} (#{event.payload[:exist] ? "yes" : "no"})", BLUE)
     end
 
     def service_url(event)
diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb
index d066deaa20..d0ee2273c5 100644
--- a/activestorage/lib/active_storage/service/azure_storage_service.rb
+++ b/activestorage/lib/active_storage/service/azure_storage_service.rb
@@ -5,7 +5,7 @@ require "azure/storage"
 require "azure/storage/core/auth/shared_access_signature"
 
 module ActiveStorage
-  # Wraps the Microsoft Azure Storage Blob Service as a Active Storage service.
+  # Wraps the Microsoft Azure Storage Blob Service as an Active Storage service.
   # See ActiveStorage::Service for the generic API documentation that applies to all services.
   class Service::AzureStorageService < Service
     attr_reader :client, :path, :blobs, :container, :signer
diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb
index cf4019966f..0e38f88138 100644
--- a/activestorage/lib/active_storage/service/disk_service.rb
+++ b/activestorage/lib/active_storage/service/disk_service.rb
@@ -6,7 +6,7 @@ require "digest/md5"
 require "active_support/core_ext/numeric/bytes"
 
 module ActiveStorage
-  # Wraps a local disk path as a Active Storage service. See ActiveStorage::Service for the generic API
+  # Wraps a local disk path as an Active Storage service. See ActiveStorage::Service for the generic API
   # documentation that applies to all services.
   class Service::DiskService < Service
     attr_reader :root
diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb
index 32dfb4851b..e656f2c69f 100644
--- a/activestorage/lib/active_storage/service/gcs_service.rb
+++ b/activestorage/lib/active_storage/service/gcs_service.rb
@@ -4,7 +4,7 @@ require "google/cloud/storage"
 require "active_support/core_ext/object/to_query"
 
 module ActiveStorage
-  # Wraps the Google Cloud Storage as a Active Storage service. See ActiveStorage::Service for the generic API
+  # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API
   # documentation that applies to all services.
   class Service::GCSService < Service
     attr_reader :client, :bucket
diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb
index 98c9dd2972..bef2ecbea9 100644
--- a/activestorage/lib/active_storage/service/s3_service.rb
+++ b/activestorage/lib/active_storage/service/s3_service.rb
@@ -4,7 +4,7 @@ require "aws-sdk"
 require "active_support/core_ext/numeric/bytes"
 
 module ActiveStorage
-  # Wraps the Amazon Simple Storage Service (S3) as a Active Storage service.
+  # Wraps the Amazon Simple Storage Service (S3) as an Active Storage service.
   # See ActiveStorage::Service for the generic API documentation that applies to all services.
   class Service::S3Service < Service
     attr_reader :client, :bucket, :upload_options