diff options
| -rw-r--r-- | actionview/lib/action_view/lookup_context.rb | 8 | ||||
| -rw-r--r-- | actionview/lib/action_view/rendering.rb | 2 | ||||
| -rw-r--r-- | actionview/test/template/lookup_context_test.rb | 10 | ||||
| -rw-r--r-- | activerecord/lib/active_record/insert_all.rb | 51 | ||||
| -rw-r--r-- | activerecord/lib/active_record/persistence.rb | 217 | ||||
| -rw-r--r-- | activerecord/test/cases/insert_all_test.rb | 36 | ||||
| -rw-r--r-- | railties/lib/rails/mailers_controller.rb | 2 |
7 files changed, 184 insertions, 142 deletions
diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 2eaf188ecf..fd3d025cbf 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -280,7 +280,15 @@ module ActionView # add :html as fallback to :js. def formats=(values) if values + values = values.dup values.concat(default_formats) if values.delete "*/*" + values.uniq! + + invalid_values = (values - Template::Types.symbols) + unless invalid_values.empty? + raise ArgumentError, "Invalid formats: #{invalid_values.map(&:inspect).join(", ")}" + end + if values == [:js] values << :html @html_fallback_for_js = true diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index ac861c44d4..5a06bd9da6 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -127,7 +127,7 @@ module ActionView # Assign the rendered format to look up context. def _process_format(format) super - lookup_context.formats = [format.to_sym] + lookup_context.formats = [format.to_sym] if format.to_sym end # Normalize args by converting render "foo" to render :action => "foo" and diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 537f8ee163..3e357fe1a7 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -67,7 +67,7 @@ class LookupContextTest < ActiveSupport::TestCase test "handles explicitly defined */* formats fallback to :js" do @lookup_context.formats = [:js, Mime::ALL] - assert_equal [:js, *Mime::SET.symbols], @lookup_context.formats + assert_equal [:js, *Mime::SET.symbols].uniq, @lookup_context.formats end test "adds :html fallback to :js formats" do @@ -75,6 +75,14 @@ class LookupContextTest < ActiveSupport::TestCase assert_equal [:js, :html], @lookup_context.formats end + test "raises on invalid format assignment" do + ex = assert_raises ArgumentError do + @lookup_context.formats = [:html, :invalid, "also bad"] + end + + assert_equal 'Invalid formats: :invalid, "also bad"', ex.message + end + test "provides getters and setters for locale" do @lookup_context.locale = :pt assert_equal :pt, @lookup_context.locale diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index 98c98d61cd..d30aee7c00 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -14,6 +14,7 @@ module ActiveRecord @returning = (connection.supports_insert_returning? ? primary_keys : false) if @returning.nil? @returning = false if @returning == [] + @unique_by = find_unique_index_for(unique_by) if unique_by @on_duplicate = :skip if @on_duplicate == :update && updatable_columns.empty? ensure_valid_options_for_connection! @@ -27,6 +28,11 @@ module ActiveRecord keys - readonly_columns - unique_by_columns end + def primary_keys + Array(model.primary_key) + end + + def skip_duplicates? on_duplicate == :skip end @@ -47,6 +53,21 @@ module ActiveRecord end private + def find_unique_index_for(unique_by) + match = Array(unique_by).map(&:to_s) + + if index = unique_indexes.find { |i| match.include?(i.name) || i.columns == match } + index + else + raise ArgumentError, "No unique index found for #{unique_by}" + end + end + + def unique_indexes + connection.schema_cache.indexes(model.table_name).select(&:unique) + end + + def ensure_valid_options_for_connection! if returning && !connection.supports_insert_returning? raise ArgumentError, "#{connection.class} does not support :returning" @@ -69,21 +90,20 @@ module ActiveRecord end end + def to_sql connection.build_insert_sql(ActiveRecord::InsertAll::Builder.new(self)) end + def readonly_columns primary_keys + model.readonly_attributes.to_a end def unique_by_columns - unique_by ? unique_by.fetch(:columns).map(&:to_s) : [] + Array(unique_by&.columns) end - def primary_keys - Array.wrap(model.primary_key) - end def verify_attributes(attributes) if keys != attributes.keys.to_set @@ -121,10 +141,13 @@ module ActiveRecord end def conflict_target - return unless conflict_columns - sql = +"(#{quote_columns(conflict_columns).join(',')})" - sql << " WHERE #{where}" if where - sql + if index = insert_all.unique_by + sql = +"(#{quote_columns(index.columns).join(',')})" + sql << " WHERE #{index.where}" if index.where + sql + elsif update_duplicates? + "(#{quote_columns(insert_all.primary_keys).join(',')})" + end end def updatable_columns @@ -150,18 +173,6 @@ module ActiveRecord def quote_columns(columns) columns.map(&connection.method(:quote_column_name)) end - - def conflict_columns - @conflict_columns ||= begin - conflict_columns = insert_all.unique_by.fetch(:columns) if insert_all.unique_by - conflict_columns ||= Array.wrap(model.primary_key) if update_duplicates? - conflict_columns - end - end - - def where - insert_all.unique_by && insert_all.unique_by[:where] - end end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 0c31f0f57e..ba03a3773a 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -57,203 +57,188 @@ module ActiveRecord end end - # Inserts a single record into the database. This method constructs a single SQL INSERT - # statement and sends it straight to the database. It does not instantiate the involved - # models and it does not trigger Active Record callbacks or validations. However, values - # passed to #insert will still go through Active Record's normal type casting and - # serialization. + # Inserts a single record into the database in a single SQL INSERT + # statement. It does not instantiate any models nor does it trigger + # Active Record callbacks or validations. Though passed values + # go through Active Record's type casting and serialization. # # See <tt>ActiveRecord::Persistence#insert_all</tt> for documentation. def insert(attributes, returning: nil, unique_by: nil) insert_all([ attributes ], returning: returning, unique_by: unique_by) end - # Inserts multiple records into the database. This method constructs a single SQL INSERT - # statement and sends it straight to the database. It does not instantiate the involved - # models and it does not trigger Active Record callbacks or validations. However, values - # passed to #insert_all will still go through Active Record's normal type casting and - # serialization. + # Inserts multiple records into the database in a single SQL INSERT + # statement. It does not instantiate any models nor does it trigger + # Active Record callbacks or validations. Though passed values + # go through Active Record's type casting and serialization. # - # The +attributes+ parameter is an Array of Hashes. These Hashes describe the - # attributes on the objects that are to be created. All of the Hashes must have - # same keys. + # The +attributes+ parameter is an Array of Hashes. Every Hash determines + # the attributes for a single row and must have the same keys. # - # Records that would violate a unique constraint on the table are skipped. + # Rows are considered to be unique by every unique index on the table. Any + # duplicate rows are skipped. + # Override with <tt>:unique_by</tt> (see below). # - # Returns an <tt>ActiveRecord::Result</tt>. The contents of the result depend on the - # value of <tt>:returning</tt> (see below). + # Returns an <tt>ActiveRecord::Result</tt> with its contents based on + # <tt>:returning</tt> (see below). # # ==== Options # # [:returning] - # (Postgres-only) An array of attributes that should be returned for all successfully - # inserted records. For databases that support <tt>INSERT ... RETURNING</tt>, this will default - # to returning the primary keys of the successfully inserted records. Pass - # <tt>returning: %w[ id name ]</tt> to return the id and name of every successfully inserted - # record or pass <tt>returning: false</tt> to omit the clause. + # (Postgres-only) An array of attributes to return for all successfully + # inserted records, which by default is the primary key. + # Pass <tt>returning: %w[ id name ]</tt> for both id and name + # or <tt>returning: false</tt> to omit the underlying RETURNING SQL + # clause entirely. # # [:unique_by] - # (Postgres and SQLite only) In a table with more than one unique constraint or index, - # new records may be considered duplicates according to different criteria. By default, - # new rows will be skipped if they violate _any_ unique constraint or index. By defining - # <tt>:unique_by</tt>, you can skip rows that would create duplicates according to the given - # constraint but raise <tt>ActiveRecord::RecordNotUnique</tt> if rows violate other constraints. + # (Postgres and SQLite only) By default rows are considered to be unique + # by every unique index on the table. Any duplicate rows are skipped. # - # (For example, maybe you assume a client will try to import the same ISBNs more than - # once and want to silently ignore the duplicate records, but you don't except any of - # your code to attempt to create two rows with the same primary key and would appreciate - # an exception report in that scenario.) + # To skip rows according to just one unique index pass <tt>:unique_by</tt>. # - # Indexes can be identified by an array of columns: + # Consider a Book model where no duplicate ISBNs make sense, but if any + # row has an existing id, or is not unique by another unique index, + # <tt>ActiveRecord::RecordNotUnique</tt> is raised. # - # unique_by: { columns: %w[ isbn ] } + # Unique indexes can be identified by columns or name: # - # Partial indexes can be identified by an array of columns and a <tt>:where</tt> condition: + # unique_by: :isbn + # unique_by: %i[ author_id name ] + # unique_by: :index_books_on_isbn # - # unique_by: { columns: %w[ isbn ], where: "published_on IS NOT NULL" } + # Because it relies on the index information from the database + # <tt>:unique_by</tt> is recommended to be paired with + # Active Record's schema_cache. # # ==== Example # - # # Insert multiple records and skip duplicates - # # ('Eloquent Ruby' will be skipped because its id is duplicate) + # # Insert records and skip inserting any duplicates. + # # Here "Eloquent Ruby" is skipped because its id is not unique. + # # Book.insert_all([ - # { id: 1, title: 'Rework', author: 'David' }, - # { id: 1, title: 'Eloquent Ruby', author: 'Russ' } + # { id: 1, title: "Rework", author: "David" }, + # { id: 1, title: "Eloquent Ruby", author: "Russ" } # ]) - # def insert_all(attributes, returning: nil, unique_by: nil) InsertAll.new(self, attributes, on_duplicate: :skip, returning: returning, unique_by: unique_by).execute end - # Inserts a single record into the database. This method constructs a single SQL INSERT - # statement and sends it straight to the database. It does not instantiate the involved - # models and it does not trigger Active Record callbacks or validations. However, values - # passed to #insert! will still go through Active Record's normal type casting and - # serialization. + # Inserts a single record into the database in a single SQL INSERT + # statement. It does not instantiate any models nor does it trigger + # Active Record callbacks or validations. Though passed values + # go through Active Record's type casting and serialization. # - # See <tt>ActiveRecord::Persistence#insert_all!</tt> for documentation. + # See <tt>ActiveRecord::Persistence#insert_all!</tt> for more. def insert!(attributes, returning: nil) insert_all!([ attributes ], returning: returning) end - # Inserts multiple records into the database. This method constructs a single SQL INSERT - # statement and sends it straight to the database. It does not instantiate the involved - # models and it does not trigger Active Record callbacks or validations. However, values - # passed to #insert_all! will still go through Active Record's normal type casting and - # serialization. + # Inserts multiple records into the database in a single SQL INSERT + # statement. It does not instantiate any models nor does it trigger + # Active Record callbacks or validations. Though passed values + # go through Active Record's type casting and serialization. # - # The +attributes+ parameter is an Array of Hashes. These Hashes describe the - # attributes on the objects that are to be created. All of the Hashes must have - # same keys. + # The +attributes+ parameter is an Array of Hashes. Every Hash determines + # the attributes for a single row and must have the same keys. # - # #insert_all! will raise <tt>ActiveRecord::RecordNotUnique</tt> if any of the records being - # inserts would violate a unique constraint on the table. In that case, no records - # would be inserted. + # Raises <tt>ActiveRecord::RecordNotUnique</tt> if any rows violate a + # unique index on the table. In that case, no rows are inserted. # - # To skip duplicate records, see <tt>ActiveRecord::Persistence#insert_all</tt>. + # To skip duplicate rows, see <tt>ActiveRecord::Persistence#insert_all</tt>. # To replace them, see <tt>ActiveRecord::Persistence#upsert_all</tt>. # - # Returns an <tt>ActiveRecord::Result</tt>. The contents of the result depend on the - # value of <tt>:returning</tt> (see below). + # Returns an <tt>ActiveRecord::Result</tt> with its contents based on + # <tt>:returning</tt> (see below). # # ==== Options # # [:returning] - # (Postgres-only) An array of attributes that should be returned for all successfully - # inserted records. For databases that support <tt>INSERT ... RETURNING</tt>, this will default - # to returning the primary keys of the successfully inserted records. Pass - # <tt>returning: %w[ id name ]</tt> to return the id and name of every successfully inserted - # record or pass <tt>returning: false</tt> to omit the clause. + # (Postgres-only) An array of attributes to return for all successfully + # inserted records, which by default is the primary key. + # Pass <tt>returning: %w[ id name ]</tt> for both id and name + # or <tt>returning: false</tt> to omit the underlying RETURNING SQL + # clause entirely. # # ==== Examples # # # Insert multiple records # Book.insert_all!([ - # { title: 'Rework', author: 'David' }, - # { title: 'Eloquent Ruby', author: 'Russ' } + # { title: "Rework", author: "David" }, + # { title: "Eloquent Ruby", author: "Russ" } # ]) # - # # Raises ActiveRecord::RecordNotUnique because 'Eloquent Ruby' - # # does not have a unique ID + # # Raises ActiveRecord::RecordNotUnique because "Eloquent Ruby" + # # does not have a unique id. # Book.insert_all!([ - # { id: 1, title: 'Rework', author: 'David' }, - # { id: 1, title: 'Eloquent Ruby', author: 'Russ' } + # { id: 1, title: "Rework", author: "David" }, + # { id: 1, title: "Eloquent Ruby", author: "Russ" } # ]) - # def insert_all!(attributes, returning: nil) InsertAll.new(self, attributes, on_duplicate: :raise, returning: returning).execute end - # Upserts (updates or inserts) a single record into the database. This method constructs - # a single SQL INSERT statement and sends it straight to the database. It does not - # instantiate the involved models and it does not trigger Active Record callbacks or - # validations. However, values passed to #upsert will still go through Active Record's - # normal type casting and serialization. + # Updates or inserts (upserts) multiple records into the database in a + # single SQL INSERT statement. It does not instantiate any models nor does + # it trigger Active Record callbacks or validations. Though passed values + # go through Active Record's type casting and serialization. # # See <tt>ActiveRecord::Persistence#upsert_all</tt> for documentation. def upsert(attributes, returning: nil, unique_by: nil) upsert_all([ attributes ], returning: returning, unique_by: unique_by) end - # Upserts (updates or inserts) multiple records into the database. This method constructs - # a single SQL INSERT statement and sends it straight to the database. It does not - # instantiate the involved models and it does not trigger Active Record callbacks or - # validations. However, values passed to #upsert_all will still go through Active Record's - # normal type casting and serialization. + # Updates or inserts (upserts) multiple records into the database in a + # single SQL INSERT statement. It does not instantiate any models nor does + # it trigger Active Record callbacks or validations. Though passed values + # go through Active Record's type casting and serialization. # - # The +attributes+ parameter is an Array of Hashes. These Hashes describe the - # attributes on the objects that are to be created. All of the Hashes must have - # same keys. + # The +attributes+ parameter is an Array of Hashes. Every Hash determines + # the attributes for a single row and must have the same keys. # - # Returns an <tt>ActiveRecord::Result</tt>. The contents of the result depend on the - # value of <tt>:returning</tt> (see below). + # Returns an <tt>ActiveRecord::Result</tt> with its contents based on + # <tt>:returning</tt> (see below). # # ==== Options # # [:returning] - # (Postgres-only) An array of attributes that should be returned for all successfully - # inserted records. For databases that support <tt>INSERT ... RETURNING</tt>, this will default - # to returning the primary keys of the successfully inserted records. Pass - # <tt>returning: %w[ id name ]</tt> to return the id and name of every successfully inserted - # record or pass <tt>returning: false</tt> to omit the clause. + # (Postgres-only) An array of attributes to return for all successfully + # inserted records, which by default is the primary key. + # Pass <tt>returning: %w[ id name ]</tt> for both id and name + # or <tt>returning: false</tt> to omit the underlying RETURNING SQL + # clause entirely. # # [:unique_by] - # (Postgres and SQLite only) In a table with more than one unique constraint or index, - # new records may be considered duplicates according to different criteria. For MySQL, - # an upsert will take place if a new record violates _any_ unique constraint. For - # Postgres and SQLite, new rows will replace existing rows when the new row has the - # same primary key as the existing row. In case of SQLite, an upsert operation causes - # an insert to behave as an update or a no-op if the insert would violate - # a uniqueness constraint. By defining <tt>:unique_by</tt>, you can supply - # a different unique constraint for matching new records to existing ones than the - # primary key. + # (Postgres and SQLite only) By default rows are considered to be unique + # by every unique index on the table. Any duplicate rows are skipped. # - # (For example, if you have a unique index on the ISBN column and use that as - # the <tt>:unique_by</tt>, a new record with the same ISBN as an existing record - # will replace the existing record but a new record with the same primary key - # as an existing record will raise <tt>ActiveRecord::RecordNotUnique</tt>.) + # To skip rows according to just one unique index pass <tt>:unique_by</tt>. # - # Indexes can be identified by an array of columns: + # Consider a Book model where no duplicate ISBNs make sense, but if any + # row has an existing id, or is not unique by another unique index, + # <tt>ActiveRecord::RecordNotUnique</tt> is raised. # - # unique_by: { columns: %w[ isbn ] } + # Unique indexes can be identified by columns or name: # - # Partial indexes can be identified by an array of columns and a <tt>:where</tt> condition: + # unique_by: :isbn + # unique_by: %i[ author_id name ] + # unique_by: :index_books_on_isbn # - # unique_by: { columns: %w[ isbn ], where: "published_on IS NOT NULL" } + # Because it relies on the index information from the database + # <tt>:unique_by</tt> is recommended to be paired with + # Active Record's schema_cache. # # ==== Examples # - # # Given a unique index on <tt>books.isbn</tt> and the following record: - # Book.create!(title: 'Rework', author: 'David', isbn: '1') + # # Inserts multiple records, performing an upsert when records have duplicate ISBNs. + # # Here "Eloquent Ruby" overwrites "Rework" because its ISBN is duplicate. # - # # Insert multiple records, allowing new records with the same ISBN - # # as an existing record to overwrite the existing record. - # # ('Eloquent Ruby' will overwrite 'Rework' because its ISBN is duplicate) # Book.upsert_all([ - # { title: 'Eloquent Ruby', author: 'Russ', isbn: '1' }, - # { title: 'Clean Code', author: 'Robert', isbn: '2' } - # ], unique_by: { columns: %w[ isbn ] }) + # { title: "Rework", author: "David", isbn: "1" }, + # { title: "Eloquent Ruby", author: "Russ", isbn: "1" } + # ], unique_by: :isbn) # + # Book.find_by(isbn: "1").title # => "Eloquent Ruby" def upsert_all(attributes, returning: nil, unique_by: nil) InsertAll.new(self, attributes, on_duplicate: :update, returning: returning, unique_by: unique_by).execute end diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index a3e920969d..0818d7c1ab 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -95,7 +95,37 @@ class InsertAllTest < ActiveRecord::TestCase assert_raise ActiveRecord::RecordNotUnique do Book.insert_all [{ id: 1, name: "Agile Web Development with Rails" }], - unique_by: { columns: %i{author_id name} } + unique_by: :index_books_on_author_id_and_name + end + end + + def test_insert_all_and_upsert_all_with_index_finding_options + skip unless supports_insert_conflict_target? + + assert_difference "Book.count", +3 do + Book.insert_all [{ name: "Rework", author_id: 1 }], unique_by: :isbn + Book.insert_all [{ name: "Remote", author_id: 1 }], unique_by: %i( author_id name ) + Book.insert_all [{ name: "Renote", author_id: 1 }], unique_by: :index_books_on_isbn + end + + assert_raise ActiveRecord::RecordNotUnique do + Book.upsert_all [{ name: "Rework", author_id: 1 }], unique_by: :isbn + end + end + + def test_insert_all_and_upsert_all_raises_when_index_is_missing + skip unless supports_insert_conflict_target? + + [ :cats, %i( author_id isbn ), :author_id ].each do |missing_or_non_unique_by| + error = assert_raises ArgumentError do + Book.insert_all [{ name: "Rework", author_id: 1 }], unique_by: missing_or_non_unique_by + end + assert_match "No unique index", error.message + + error = assert_raises ArgumentError do + Book.upsert_all [{ name: "Rework", author_id: 1 }], unique_by: missing_or_non_unique_by + end + assert_match "No unique index", error.message end end @@ -120,7 +150,7 @@ class InsertAllTest < ActiveRecord::TestCase Book.upsert_all [{ id: 101, name: "Perelandra", author_id: 7 }] Book.upsert_all [{ id: 103, name: "Perelandra", author_id: 7, isbn: "1974522598" }], - unique_by: { columns: %i{author_id name} } + unique_by: :index_books_on_author_id_and_name book = Book.find_by(name: "Perelandra") assert_equal 101, book.id, "Should not have updated the ID" @@ -132,7 +162,7 @@ class InsertAllTest < ActiveRecord::TestCase Book.upsert_all [{ name: "Out of the Silent Planet", author_id: 7, isbn: "1974522598", published_on: Date.new(1938, 4, 1) }] Book.upsert_all [{ name: "Perelandra", author_id: 7, isbn: "1974522598" }], - unique_by: { columns: %w[ isbn ], where: "published_on IS NOT NULL" } + unique_by: :index_books_on_isbn assert_equal ["Out of the Silent Planet", "Perelandra"], Book.where(isbn: "1974522598").order(:name).pluck(:name) end diff --git a/railties/lib/rails/mailers_controller.rb b/railties/lib/rails/mailers_controller.rb index 95dae3ec2d..5cffa52860 100644 --- a/railties/lib/rails/mailers_controller.rb +++ b/railties/lib/rails/mailers_controller.rb @@ -38,7 +38,7 @@ class Rails::MailersController < Rails::ApplicationController # :nodoc: end else @part = find_preferred_part(request.format, Mime[:html], Mime[:text]) - render action: "email", layout: false, formats: %w[html] + render action: "email", layout: false, formats: [:html] end else raise AbstractController::ActionNotFound, "Email '#{@email_action}' not found in #{@preview.name}" |