From 5963766d840ddcdb577a1bd10eb1491a4ef9132f Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 9 Jul 2017 18:48:26 +0200 Subject: Explore regular polymorphic associations rather than record_gid --- lib/active_storage/attached/macros.rb | 6 ++++++ lib/active_storage/attached/many.rb | 4 ++-- lib/active_storage/attached/one.rb | 4 ++-- lib/active_storage/migration.rb | 9 +++++---- 4 files changed, 15 insertions(+), 8 deletions(-) (limited to 'lib/active_storage') diff --git a/lib/active_storage/attached/macros.rb b/lib/active_storage/attached/macros.rb index 1e0f9a6b7e..0452089a5f 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 @@ -38,6 +41,9 @@ 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" + 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..129f166776 100644 --- a/lib/active_storage/attached/many.rb +++ b/lib/active_storage/attached/many.rb @@ -7,14 +7,14 @@ 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) + @attachments ||= 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)) + ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable)) end end diff --git a/lib/active_storage/attached/one.rb b/lib/active_storage/attached/one.rb index 80e4cb6234..02fc9c9abc 100644 --- a/lib/active_storage/attached/one.rb +++ b/lib/active_storage/attached/one.rb @@ -7,13 +7,13 @@ 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) + @attachment ||= 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)) + @attachment = ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable)) end # Checks the presence of the attachment. diff --git a/lib/active_storage/migration.rb b/lib/active_storage/migration.rb index c56e7a1786..99d8b8554b 100644 --- a/lib/active_storage/migration.rb +++ b/lib/active_storage/migration.rb @@ -14,15 +14,16 @@ 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 ] + t.index [ :record_type, :record_id, :name ], name: "index_active_storage_attachments_record_and_name" + t.index [ :record_type, :record_id, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true end end end -- cgit v1.2.3 From 212f925654f944067f18429ca02d902473214722 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sun, 23 Jul 2017 21:19:34 +0200 Subject: Collapse indeces per Jeremy. --- lib/active_storage/migration.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'lib/active_storage') diff --git a/lib/active_storage/migration.rb b/lib/active_storage/migration.rb index 99d8b8554b..e843c1b630 100644 --- a/lib/active_storage/migration.rb +++ b/lib/active_storage/migration.rb @@ -21,9 +21,7 @@ class ActiveStorageCreateTables < ActiveRecord::Migration[5.1] # :nodoc: t.datetime :created_at t.index :blob_id - t.index [ :record_type, :record_id ] - t.index [ :record_type, :record_id, :name ], name: "index_active_storage_attachments_record_and_name" - t.index [ :record_type, :record_id, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true + t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true end end end -- cgit v1.2.3 From a4f36f957e013f6da34e04f0d3f1d86d86491454 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sun, 23 Jul 2017 21:41:16 +0200 Subject: Fix attaching with standard Rails associations. Removes needless ivar caching (a Rails association handles that). Inserts a reload and a nil assign, since the association proxy doesn't seem to that it's been destroyed through `purge`. --- lib/active_storage/attached/many.rb | 9 ++++----- lib/active_storage/attached/one.rb | 13 +++++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) (limited to 'lib/active_storage') diff --git a/lib/active_storage/attached/many.rb b/lib/active_storage/attached/many.rb index 129f166776..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 ||= record.public_send("#{name}_attachments") + 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| + 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) 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 02fc9c9abc..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 ||= record.public_send("#{name}_attachment") + 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: record, 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 -- cgit v1.2.3 From 68b5d274a365c1babdb92dedfcf2e600138be5eb Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 23 Jul 2017 16:47:11 -0500 Subject: Add and test preloading scope --- lib/active_storage/attached/macros.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'lib/active_storage') diff --git a/lib/active_storage/attached/macros.rb b/lib/active_storage/attached/macros.rb index 0452089a5f..5915793f8a 100644 --- a/lib/active_storage/attached/macros.rb +++ b/lib/active_storage/attached/macros.rb @@ -33,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) @@ -44,6 +48,8 @@ module ActiveStorage::Attached::Macros 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 -- cgit v1.2.3 From e16d0c9ceacd771c99048385dc886c6026c7bc45 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 23 Jul 2017 16:57:26 -0500 Subject: No more GlobalID --- lib/active_storage/attached.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib/active_storage') 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 -- cgit v1.2.3