aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--README.md7
-rw-r--r--app/models/active_storage/attachment.rb11
-rw-r--r--lib/active_storage/attached.rb2
-rw-r--r--lib/active_storage/attached/macros.rb12
-rw-r--r--lib/active_storage/attached/many.rb11
-rw-r--r--lib/active_storage/attached/one.rb13
-rw-r--r--lib/active_storage/migration.rb7
-rw-r--r--test/models/attachments_test.rb11
-rw-r--r--test/test_helper.rb4
9 files changed, 47 insertions, 31 deletions
diff --git a/README.md b/README.md
index 9e4749b1c6..8f340d7013 100644
--- a/README.md
+++ b/README.md
@@ -12,7 +12,7 @@ Furthermore, this repository is likely to be in heavy flux prior to the merge to
## 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/activestorage/blob/master/lib/active_storage/blob.rb) and [Attachment](https://github.com/rails/activestorage/blob/master/lib/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 GlobalID to provide 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/activestorage/blob/master/lib/active_storage/blob.rb) and [Attachment](https://github.com/rails/activestorage/blob/master/lib/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`.
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).
@@ -66,6 +66,11 @@ class MessagesController < ApplicationController
message.images.attach(params[:message][:images])
redirect_to message
end
+
+ def show
+ # Use the built-in with_attached_images scope to avoid N+1
+ @message = Message.find(params[:id]).with_attached_images
+ end
end
```
diff --git a/app/models/active_storage/attachment.rb b/app/models/active_storage/attachment.rb
index 20c619aa5a..d491c7224e 100644
--- a/app/models/active_storage/attachment.rb
+++ b/app/models/active_storage/attachment.rb
@@ -1,24 +1,15 @@
require "active_storage/blob"
-require "global_id"
require "active_support/core_ext/module/delegation"
# Schema: id, record_gid, blob_id, created_at
class ActiveStorage::Attachment < ActiveRecord::Base
self.table_name = "active_storage_attachments"
+ belongs_to :record, polymorphic: true
belongs_to :blob, class_name: "ActiveStorage::Blob"
delegate_missing_to :blob
- def record
- @record ||= GlobalID::Locator.locate(record_gid)
- end
-
- def record=(record)
- @record = record
- self.record_gid = record&.to_gid
- end
-
def purge
blob.purge
destroy
diff --git a/lib/active_storage/attached.rb b/lib/active_storage/attached.rb
index 9fa7b8e021..6b81545897 100644
--- a/lib/active_storage/attached.rb
+++ b/lib/active_storage/attached.rb
@@ -4,8 +4,6 @@ require "active_storage/attachment"
require "action_dispatch/http/upload"
require "active_support/core_ext/module/delegation"
-require "global_id/locator"
-
class ActiveStorage::Attached
attr_reader :name, :record
diff --git a/lib/active_storage/attached/macros.rb b/lib/active_storage/attached/macros.rb
index 1e0f9a6b7e..5915793f8a 100644
--- a/lib/active_storage/attached/macros.rb
+++ b/lib/active_storage/attached/macros.rb
@@ -16,6 +16,9 @@ module ActiveStorage::Attached::Macros
instance_variable_set("@active_storage_attached_#{name}", ActiveStorage::Attached::One.new(name, self))
end
+ has_one :"#{name}_attachment", -> { where(name: name) }, class_name: "ActiveStorage::Attachment", as: :record
+ has_one :"#{name}_blob", through: :"#{name}_attachment"
+
if dependent == :purge_later
before_destroy { public_send(name).purge_later }
end
@@ -30,6 +33,10 @@ module ActiveStorage::Attached::Macros
# There are no columns defined on the model side, Active Storage takes
# care of the mapping between your records and the attachments.
#
+ # To avoid N+1 queries, you can include the attached blobs in your query like so:
+ #
+ # Gallery.where(user: Current.user).with_attached_photos
+ #
# If the +:dependent+ option isn't set, all the attachments will be purged
# (i.e. destroyed) whenever the record is destroyed.
def has_many_attached(name, dependent: :purge_later)
@@ -38,6 +45,11 @@ module ActiveStorage::Attached::Macros
instance_variable_set("@active_storage_attached_#{name}", ActiveStorage::Attached::Many.new(name, self))
end
+ has_many :"#{name}_attachments", -> { where(name: name) }, as: :record, class_name: "ActiveStorage::Attachment"
+ has_many :"#{name}_blobs", through: :"#{name}_attachments"
+
+ scope :"with_attached_#{name}", -> { includes("#{name}_attachments": :blob) }
+
if dependent == :purge_later
before_destroy { public_send(name).purge_later }
end
diff --git a/lib/active_storage/attached/many.rb b/lib/active_storage/attached/many.rb
index 99d980196a..704369ba89 100644
--- a/lib/active_storage/attached/many.rb
+++ b/lib/active_storage/attached/many.rb
@@ -7,15 +7,15 @@ class ActiveStorage::Attached::Many < ActiveStorage::Attached
# You don't have to call this method to access the attachments' methods as
# they are all available at the model level.
def attachments
- @attachments ||= ActiveStorage::Attachment.where(record_gid: record.to_gid.to_s, name: name)
+ record.public_send("#{name}_attachments")
end
# Associates one or several attachments with the current record, saving
# them to the database.
def attach(*attachables)
- @attachments = attachments | Array(attachables).flatten.collect do |attachable|
- ActiveStorage::Attachment.create!(record_gid: record.to_gid.to_s, name: name, blob: create_blob_from(attachable))
- end
+ record.public_send("#{name}_attachments=", attachments | Array(attachables).flat_map do |attachable|
+ ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable))
+ end)
end
# Checks the presence of attachments.
@@ -34,7 +34,7 @@ class ActiveStorage::Attached::Many < ActiveStorage::Attached
def purge
if attached?
attachments.each(&:purge)
- @attachments = nil
+ attachments.reload
end
end
@@ -42,7 +42,6 @@ class ActiveStorage::Attached::Many < ActiveStorage::Attached
def purge_later
if attached?
attachments.each(&:purge_later)
- @attachments = nil
end
end
end
diff --git a/lib/active_storage/attached/one.rb b/lib/active_storage/attached/one.rb
index 80e4cb6234..d255412842 100644
--- a/lib/active_storage/attached/one.rb
+++ b/lib/active_storage/attached/one.rb
@@ -7,13 +7,14 @@ class ActiveStorage::Attached::One < ActiveStorage::Attached
# You don't have to call this method to access the attachment's methods as
# they are all available at the model level.
def attachment
- @attachment ||= ActiveStorage::Attachment.find_by(record_gid: record.to_gid.to_s, name: name)
+ record.public_send("#{name}_attachment")
end
# Associates a given attachment with the current record, saving it to the
# database.
def attach(attachable)
- @attachment = ActiveStorage::Attachment.create!(record_gid: record.to_gid.to_s, name: name, blob: create_blob_from(attachable))
+ write_attachment \
+ ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable))
end
# Checks the presence of the attachment.
@@ -32,7 +33,7 @@ class ActiveStorage::Attached::One < ActiveStorage::Attached
def purge
if attached?
attachment.purge
- @attachment = nil
+ write_attachment nil
end
end
@@ -40,7 +41,11 @@ class ActiveStorage::Attached::One < ActiveStorage::Attached
def purge_later
if attached?
attachment.purge_later
- @attachment = nil
end
end
+
+ private
+ def write_attachment(attachment)
+ record.public_send("#{name}_attachment=", attachment)
+ end
end
diff --git a/lib/active_storage/migration.rb b/lib/active_storage/migration.rb
index c56e7a1786..e843c1b630 100644
--- a/lib/active_storage/migration.rb
+++ b/lib/active_storage/migration.rb
@@ -14,15 +14,14 @@ class ActiveStorageCreateTables < ActiveRecord::Migration[5.1] # :nodoc:
create_table :active_storage_attachments do |t|
t.string :name
- t.string :record_gid
+ t.string :record_type
+ t.integer :record_id
t.integer :blob_id
t.datetime :created_at
- t.index :record_gid
t.index :blob_id
- t.index [ :record_gid, :name ]
- t.index [ :record_gid, :blob_id ], unique: true
+ t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true
end
end
end
diff --git a/test/models/attachments_test.rb b/test/models/attachments_test.rb
index c0f5db819d..eac3cbe680 100644
--- a/test/models/attachments_test.rb
+++ b/test/models/attachments_test.rb
@@ -68,6 +68,17 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
assert_equal "country.jpg", @user.highlights.second.filename.to_s
end
+ test "find attached blobs" do
+ @user.highlights.attach(
+ { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" },
+ { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" })
+
+ highlights = User.where(id: @user.id).with_attached_highlights.first.highlights
+
+ assert_equal "town.jpg", highlights.first.filename.to_s
+ assert_equal "country.jpg", highlights.second.filename.to_s
+ end
+
test "purge attached blobs" do
@user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
highlight_keys = @user.highlights.collect(&:key)
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 650e997205..a6e228c4d2 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -62,7 +62,3 @@ end
require "active_storage/attached"
ActiveRecord::Base.send :extend, ActiveStorage::Attached::Macros
-
-require "global_id"
-GlobalID.app = "ActiveStorageExampleApp"
-ActiveRecord::Base.send :include, GlobalID::Identification