diff options
29 files changed, 385 insertions, 227 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6b996fd6bd..fd2c5dd9bb 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,60 @@ +* Option to remove standardized column types/arguments spaces in schema dump + with `ActiveRecord::SchemaDumper.standardized_argument_widths` and + `ActiveRecord::SchemaDumper.standardized_type_widths` methods. + + *Tim Petricola* + +* Avoid loading records from database when they are already loaded using + the `pluck` method on a collection. + + Fixes #25921. + + *Ryuta Kamizono* + +* Remove text default treated as an empty string in non-strict mode for + consistency with other types. + + Strict mode controls how MySQL handles invalid or missing values in + data-change statements such as INSERT or UPDATE. If strict mode is not + in effect, MySQL inserts adjusted values for invalid or missing values + and produces warnings. + + def test_mysql_not_null_defaults_non_strict + using_strict(false) do + with_mysql_not_null_table do |klass| + record = klass.new + assert_nil record.non_null_integer + assert_nil record.non_null_string + assert_nil record.non_null_text + assert_nil record.non_null_blob + + record.save! + record.reload + + assert_equal 0, record.non_null_integer + assert_equal "", record.non_null_string + assert_equal "", record.non_null_text + assert_equal "", record.non_null_blob + end + end + end + + https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict + + *Ryuta Kamizono* + +* Sqlite3 migrations to add a column to an existing table can now be + successfully rolled back when the column was given and invalid column + type. + + Fixes #26087 + + *Travis O'Neill* + +* Deprecate `sanitize_conditions`. Use `sanitize_sql` instead. + + *Ryuta Kamizono* + * Doing count on relations that contain LEFT OUTER JOIN Arel node no longer force a DISTINCT. This solves issues when using count after a left_joins. @@ -57,8 +114,8 @@ *Xavier Noria* * Using `group` with an attribute that has a custom type will properly cast - the hash keys after calling a calculation method like `count`. - + the hash keys after calling a calculation method like `count`. + Fixes #25595. *Sean Griffin* @@ -93,7 +150,7 @@ *Sean Griffin* * Ensure hashes can be assigned to attributes created using `composed_of`. - + Fixes #25210. *Sean Griffin* diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 7c688d663c..e17ca81867 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -82,14 +82,6 @@ module ActiveRecord @target = [] end - def select(*fields) - if block_given? - load_target.select.each { |e| yield e } - else - scope.select(*fields) - end - end - def find(*args) if block_given? load_target.find(*args) { |*block_args| yield(*block_args) } @@ -111,50 +103,6 @@ module ActiveRecord end end - def first(limit = nil) - find_nth_or_last(:first, limit) - end - - def second - find_nth_or_last(:second) - end - - def third - find_nth_or_last(:third) - end - - def fourth - find_nth_or_last(:fourth) - end - - def fifth - find_nth_or_last(:fifth) - end - - def forty_two - find_nth_or_last(:forty_two) - end - - def third_to_last - find_nth_or_last(:third_to_last) - end - - def second_to_last - find_nth_or_last(:second_to_last) - end - - def last(limit = nil) - find_nth_or_last(:last, limit) - end - - def take(limit = nil) - if find_from_target? - limit ? load_target.take(limit) : load_target.first - else - scope.take(limit) - end - end - def build(attributes = {}, &block) if attributes.is_a?(Array) attributes.collect { |attr| build(attr, &block) } @@ -453,6 +401,12 @@ module ActiveRecord owner.new_record? && !foreign_key_present? end + def find_from_target? + loaded? || + owner.new_record? || + target.any? { |record| record.new_record? || record.changed? } + end + private def find_target @@ -599,21 +553,6 @@ module ActiveRecord owner.class.send(full_callback_name) end - # Should we deal with assoc.first or assoc.last by issuing an independent query to - # the database, or by getting the target, and then taking the first/last item from that? - # - # If the args is just a non-empty options hash, go to the database. - # - # Otherwise, go to the database only if none of the following are true: - # * target already loaded - # * owner is new record - # * target contains new or changed record(s) - def find_from_target? - loaded? || - owner.new_record? || - target.any? { |record| record.new_record? || record.changed? } - end - def include_in_memory?(record) if reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) assoc = owner.association(reflection.through_reflection.name) @@ -640,12 +579,6 @@ module ActiveRecord load_target.select { |r| ids.include?(r.id.to_s) } end end - - # Fetches the first/last using SQL if possible, otherwise from the target array. - def find_nth_or_last(ordinal, limit = nil) - collection = find_from_target? ? load_target : scope - limit ? collection.send(ordinal, limit) : collection.send(ordinal) - end end end end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 36f6c1b9c3..926defbb47 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -53,6 +53,12 @@ module ActiveRecord @association.loaded? end + ## + # :method: select + # + # :call-seq: + # select(*fields, &block) + # # Works in two ways. # # *First:* Specify a subset of fields to be selected from the result set. @@ -106,9 +112,6 @@ module ActiveRecord # # #<Pet id: 2, name: "Spook">, # # #<Pet id: 3, name: "Choo-Choo"> # # ] - def select(*fields, &block) - @association.select(*fields, &block) - end # Finds an object in the collection responding to the +id+. Uses the same # rules as ActiveRecord::Base.find. Returns ActiveRecord::RecordNotFound @@ -140,6 +143,12 @@ module ActiveRecord @association.find(*args, &block) end + ## + # :method: first + # + # :call-seq: + # first(limit = nil) + # # Returns the first record, or the first +n+ records, from the collection. # If the collection is empty, the first form returns +nil+, and the second # form returns an empty array. @@ -166,45 +175,63 @@ module ActiveRecord # another_person_without.pets # => [] # another_person_without.pets.first # => nil # another_person_without.pets.first(3) # => [] - def first(limit = nil) - @association.first(limit) - end + ## + # :method: second + # + # :call-seq: + # second() + # # Same as #first except returns only the second record. - def second - @association.second - end + ## + # :method: third + # + # :call-seq: + # third() + # # Same as #first except returns only the third record. - def third - @association.third - end + ## + # :method: fourth + # + # :call-seq: + # fourth() + # # Same as #first except returns only the fourth record. - def fourth - @association.fourth - end + ## + # :method: fifth + # + # :call-seq: + # fifth() + # # Same as #first except returns only the fifth record. - def fifth - @association.fifth - end + ## + # :method: forty_two + # + # :call-seq: + # forty_two() + # # Same as #first except returns only the forty second record. # Also known as accessing "the reddit". - def forty_two - @association.forty_two - end + ## + # :method: third_to_last + # + # :call-seq: + # third_to_last() + # # Same as #first except returns only the third-to-last record. - def third_to_last - @association.third_to_last - end + ## + # :method: second_to_last + # + # :call-seq: + # second_to_last() + # # Same as #first except returns only the second-to-last record. - def second_to_last - @association.second_to_last - end # Returns the last record, or the last +n+ records, from the collection. # If the collection is empty, the first form returns +nil+, and the second @@ -233,7 +260,8 @@ module ActiveRecord # another_person_without.pets.last # => nil # another_person_without.pets.last(3) # => [] def last(limit = nil) - @association.last(limit) + load_target if find_from_target? + super end # Gives a record (or N records if a parameter is supplied) from the collection @@ -262,7 +290,8 @@ module ActiveRecord # another_person_without.pets.take # => nil # another_person_without.pets.take(2) # => [] def take(limit = nil) - @association.take(limit) + load_target if find_from_target? + super end # Returns a new object of the collection type that has been instantiated @@ -1078,12 +1107,28 @@ module ActiveRecord self end + protected + + def find_nth_with_limit(index, limit) + load_target if find_from_target? + super + end + + def find_nth_from_last(index) + load_target if find_from_target? + super + end + private def null_scope? @association.null_scope? end + def find_from_target? + @association.find_from_target? + end + def exec_queries load_target end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 3946d5baa4..62acad0eda 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -274,7 +274,11 @@ module ActiveRecord construct(model, node, row, rs, seen, model_cache, aliases) else model = construct_model(ar_parent, node, row, model_cache, id, aliases) - model.readonly! + + if node.reflection.scope_for(node.base_klass).readonly_value + model.readonly! + end + seen[ar_parent.object_id][node.base_klass][id] = model construct(model, node, row, rs, seen, model_cache, aliases) end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index a8afa48865..4bb627f399 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -113,7 +113,7 @@ module ActiveRecord end def reflection_scope - @reflection_scope ||= reflection.scope ? klass.unscoped.instance_exec(nil, &reflection.scope) : klass.unscoped + @reflection_scope ||= reflection.scope_for(klass) end def build_scope diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 126047584e..def1dadb6a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -379,7 +379,7 @@ module ActiveRecord end def sql_for_insert(sql, pk, id_value, sequence_name, binds) - [sql, binds, pk, sequence_name] + [sql, binds] end def last_inserted_id(result) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index d9a799676f..ffde4f2c93 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -303,7 +303,7 @@ module ActiveRecord # end def column(name, type, options = {}) name = name.to_s - type = type.to_sym + type = type.to_sym if type options = options.dup if @columns_hash[name] && @columns_hash[name].primary_key? diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index afa0860707..45d782e45e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1047,7 +1047,8 @@ module ActiveRecord end def type_to_sql(type, limit = nil, precision = nil, scale = nil) #:nodoc: - if native = native_database_types[type.to_sym] + type = type.to_sym if type + if native = native_database_types[type] column_type_sql = (native.is_a?(Hash) ? native[:name] : native).dup if type == :decimal # ignore limit, use precision and scale diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index f09c9c9e7f..3a28879c15 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -709,7 +709,7 @@ module ActiveRecord end def fetch_type_metadata(sql_type, extra = "") - MySQL::TypeMetadata.new(super(sql_type), extra: extra, strict: strict_mode?) + MySQL::TypeMetadata.new(super(sql_type), extra: extra) end def add_index_length(quoted_columns, **options) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/column.rb b/activerecord/lib/active_record/connection_adapters/mysql/column.rb index 5452e44c84..72b57f4b8f 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/column.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/column.rb @@ -2,17 +2,7 @@ module ActiveRecord module ConnectionAdapters module MySQL class Column < ConnectionAdapters::Column # :nodoc: - delegate :strict, :extra, to: :sql_type_metadata, allow_nil: true - - def initialize(*) - super - extract_default - end - - def has_default? - return false if blob_or_text_column? # MySQL forbids defaults on blob and text columns - super - end + delegate :extra, to: :sql_type_metadata, allow_nil: true def blob_or_text_column? /\A(?:tiny|medium|long)?blob\b/ === sql_type || type == :text @@ -29,14 +19,6 @@ module ActiveRecord def auto_increment? extra == "auto_increment" end - - private - - def extract_default - if blob_or_text_column? - @default = null || strict ? nil : "" - end - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb index 1be5cb4740..24dcf852e1 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb @@ -2,13 +2,12 @@ module ActiveRecord module ConnectionAdapters module MySQL class TypeMetadata < DelegateClass(SqlTypeMetadata) # :nodoc: - attr_reader :extra, :strict + attr_reader :extra - def initialize(type_metadata, extra: "", strict: false) + def initialize(type_metadata, extra: "") super(type_metadata) @type_metadata = type_metadata @extra = extra - @strict = strict end def ==(other) @@ -24,7 +23,7 @@ module ActiveRecord protected def attributes_for_hash - [self.class, @type_metadata, extra, strict] + [self.class, @type_metadata, extra] end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 46aca2ab54..7414eba6c5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -124,14 +124,13 @@ module ActiveRecord pk = primary_key(table_ref) if table_ref end - pk = suppress_composite_primary_key(pk) - - if pk && use_insert_returning? + if pk = suppress_composite_primary_key(pk) sql = "#{sql} RETURNING #{quote_column_name(pk)}" end super end + protected :sql_for_insert def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil) if use_insert_returning? || pk == false diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 7b4b8c60f8..dd7d650207 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -62,8 +62,7 @@ module ActiveRecord # # * +sql+ - An SQL statement which should return a count query from the database, see the example above. def count_by_sql(sql) - sql = sanitize_conditions(sql) - connection.select_value(sql, "#{name} Count").to_i + connection.select_value(sanitize_sql(sql), "#{name} Count").to_i end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 3553ff4da3..8c5e4d042e 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -311,6 +311,10 @@ module ActiveRecord active_record == other_aggregation.active_record end + def scope_for(klass) + scope ? klass.unscoped.instance_exec(nil, &scope) : klass.unscoped + end + private def derive_class_name name.to_s.camelize diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index ff43def901..376867675b 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -97,7 +97,7 @@ module ActiveRecord # Person.take(5) # returns 5 objects fetched by SELECT * FROM people LIMIT 5 # Person.where(["name LIKE '%?'", name]).take def take(limit = nil) - limit ? limit(limit).to_a : find_take + limit ? find_take_with_limit(limit) : find_take end # Same as #take but raises ActiveRecord::RecordNotFound if no record @@ -526,13 +526,21 @@ module ActiveRecord end end + def find_take_with_limit(limit) + if loaded? + records.take(limit) + else + limit(limit).to_a + end + end + def find_nth(index) @offsets[offset_index + index] ||= find_nth_with_limit(index, 1).first end def find_nth_with_limit(index, limit) if loaded? - records[index, limit] + records[index, limit] || [] else relation = if order_values.empty? && primary_key order(arel_attribute(primary_key).asc) diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index c40e98715e..7f596120eb 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -29,8 +29,9 @@ module ActiveRecord else condition end end - alias_method :sanitize_sql, :sanitize_sql_for_conditions - alias_method :sanitize_conditions, :sanitize_sql + alias :sanitize_sql :sanitize_sql_for_conditions + alias :sanitize_conditions :sanitize_sql + deprecate sanitize_conditions: :sanitize_sql # Accepts an array, hash, or string of SQL conditions and sanitizes # them into a valid SQL fragment for a SET clause. diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 01f788a424..be74922453 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -16,6 +16,22 @@ module ActiveRecord cattr_accessor :ignore_tables @@ignore_tables = [] + ## + # :singleton-method: + # Define whether column arguments are lined up in dump. + # Acceptable values are true or false. + # This setting is only used if ActiveRecord::Base.schema_format == :ruby + cattr_accessor :standardized_argument_widths + @@standardized_argument_widths = true + + ## + # :singleton-method: + # Define whether columns types are lined up in dump. + # Acceptable values are true or false. + # This setting is only used if ActiveRecord::Base.schema_format == :ruby + cattr_accessor :standardized_type_widths + @@standardized_type_widths = true + class << self def dump(connection=ActiveRecord::Base.connection, stream=STDOUT, config = ActiveRecord::Base) new(connection, generate_options(config)).dump(stream) @@ -146,20 +162,32 @@ HEADER keys = @connection.migration_keys # figure out the lengths for each column based on above keys - lengths = keys.map { |key| - column_specs.map { |spec| - spec[key] ? spec[key].length + 2 : 0 - }.max - } + lengths = if standardized_argument_widths + keys.map { |key| + column_specs.map { |spec| + spec[key] ? spec[key].length + 2 : 0 + }.max + } + else + [0] * keys.length + end # the string we're going to sprintf our values against, with standardized column widths - format_string = lengths.map { |len| "%-#{len}s" } - - # find the max length for the 'type' column, which is special - type_length = column_specs.map { |column| column[:type].length }.max + format_string = if standardized_argument_widths + lengths.map { |len| "%-#{len}s" } + else + ["%s"] * keys.length + end # add column type definition to our format string - format_string.unshift " t.%-#{type_length}s " + if standardized_type_widths + # find the max length for the 'type' column, which is special + type_length = column_specs.map { |column| column[:type].length }.max + + format_string.unshift " t.%-#{type_length}s " + else + format_string.unshift " t.%s " + end format_string *= "" diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 4b8d06be4b..e6af93a53e 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -88,12 +88,6 @@ module ActiveRecord assert_equal expect.to_i, result.rows.first.first end - def test_sql_for_insert_with_returning_disabled - connection = connection_without_insert_returning - sql, binds = connection.sql_for_insert("sql", nil, nil, nil, "binds") - assert_equal ["sql", "binds"], [sql, binds] - end - def test_serial_sequence assert_equal "public.accounts_id_seq", @connection.serial_sequence("accounts", "id") diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 2c826acf96..6011c552b2 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1424,6 +1424,24 @@ class EagerAssociationTest < ActiveRecord::TestCase assert david.readonly_comments.first.readonly? end + test "eager-loading non-readonly association" do + # has_one + firm = Firm.where(id: "1").eager_load(:account).first! + assert_not firm.account.readonly? + + # has_and_belongs_to_many + project = Project.where(id: "2").eager_load(:developers).first! + assert_not project.developers.first.readonly? + + # has_many :through + david = Author.where(id: "1").eager_load(:comments).first! + assert_not david.comments.first.readonly? + + # belongs_to + post = Post.where(id: "1").eager_load(:author).first! + assert_not post.author.readonly? + end + test "eager-loading readonly association" do # has-one firm = Firm.where(id: "1").eager_load(:readonly_account).first! @@ -1438,8 +1456,8 @@ class EagerAssociationTest < ActiveRecord::TestCase assert david.readonly_comments.first.readonly? # belongs_to - post = Post.where(id: "1").eager_load(:author).first! - assert post.author.readonly? + post = Post.where(id: "1").eager_load(:readonly_author).first! + assert post.readonly_author.readonly? end test "preloading a polymorphic association with references to the associated table" do diff --git a/activerecord/test/cases/column_definition_test.rb b/activerecord/test/cases/column_definition_test.rb index 989beaa5c8..a65bb89052 100644 --- a/activerecord/test/cases/column_definition_test.rb +++ b/activerecord/test/cases/column_definition_test.rb @@ -60,14 +60,13 @@ module ActiveRecord end def test_should_not_set_default_for_blob_and_text_data_types - text_type = MySQL::TypeMetadata.new( - SqlTypeMetadata.new(type: :text)) + text_type = MySQL::TypeMetadata.new(SqlTypeMetadata.new(type: :text)) text_column = MySQL::Column.new("title", nil, text_type) - assert_equal nil, text_column.default + assert_nil text_column.default not_null_text_column = MySQL::Column.new("title", nil, text_type, false) - assert_equal "", not_null_text_column.default + assert_nil not_null_text_column.default end def test_has_default_should_return_false_for_blob_and_text_data_types diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index f94815d34c..fcaff38f82 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -127,92 +127,66 @@ if current_adapter?(:Mysql2Adapter) ActiveRecord::Base.establish_connection connection end - # MySQL cannot have defaults on text/blob columns. It reports the - # default value as null. + # Strict mode controls how MySQL handles invalid or missing values + # in data-change statements such as INSERT or UPDATE. A value can be + # invalid for several reasons. For example, it might have the wrong + # data type for the column, or it might be out of range. A value is + # missing when a new row to be inserted does not contain a value for + # a non-NULL column that has no explicit DEFAULT clause in its definition. + # (For a NULL column, NULL is inserted if the value is missing.) # - # Despite this, in non-strict mode, MySQL will use an empty string - # as the default value of the field, if no other value is - # specified. + # If strict mode is not in effect, MySQL inserts adjusted values for + # invalid or missing values and produces warnings. In strict mode, + # you can produce this behavior by using INSERT IGNORE or UPDATE IGNORE. # - # Therefore, in non-strict mode, we want column.default to report - # an empty string as its default, to be consistent with that. - # - # In strict mode, column.default should be nil. - def test_mysql_text_not_null_defaults_non_strict + # https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict + def test_mysql_not_null_defaults_non_strict using_strict(false) do - with_text_blob_not_null_table do |klass| + with_mysql_not_null_table do |klass| record = klass.new - assert_equal "", record.non_null_blob - assert_equal "", record.non_null_text - - assert_nil record.null_blob - assert_nil record.null_text + assert_nil record.non_null_integer + assert_nil record.non_null_string + assert_nil record.non_null_text + assert_nil record.non_null_blob record.save! record.reload + assert_equal 0, record.non_null_integer + assert_equal "", record.non_null_string assert_equal "", record.non_null_text assert_equal "", record.non_null_blob - - assert_nil record.null_text - assert_nil record.null_blob end end end - def test_mysql_text_not_null_defaults_strict + def test_mysql_not_null_defaults_strict using_strict(true) do - with_text_blob_not_null_table do |klass| + with_mysql_not_null_table do |klass| record = klass.new - assert_nil record.non_null_blob + assert_nil record.non_null_integer + assert_nil record.non_null_string assert_nil record.non_null_text - assert_nil record.null_blob - assert_nil record.null_text + assert_nil record.non_null_blob assert_raises(ActiveRecord::StatementInvalid) { klass.create } end end end - def with_text_blob_not_null_table + def with_mysql_not_null_table klass = Class.new(ActiveRecord::Base) - klass.table_name = "test_mysql_text_not_null_defaults" + klass.table_name = "test_mysql_not_null_defaults" klass.connection.create_table klass.table_name do |t| - t.column :non_null_text, :text, null: false - t.column :non_null_blob, :blob, null: false - t.column :null_text, :text, null: true - t.column :null_blob, :blob, null: true + t.integer :non_null_integer, null: false + t.string :non_null_string, null: false + t.text :non_null_text, null: false + t.blob :non_null_blob, null: false end yield klass ensure klass.connection.drop_table(klass.table_name) rescue nil end - - # MySQL uses an implicit default 0 rather than NULL unless in strict mode. - # We use an implicit NULL so schema.rb is compatible with other databases. - def test_mysql_integer_not_null_defaults - klass = Class.new(ActiveRecord::Base) - klass.table_name = "test_integer_not_null_default_zero" - klass.connection.create_table klass.table_name do |t| - t.column :zero, :integer, null: false, default: 0 - t.column :omit, :integer, null: false - end - - assert_equal "0", klass.columns_hash["zero"].default - assert !klass.columns_hash["zero"].null - assert_equal nil, klass.columns_hash["omit"].default - assert !klass.columns_hash["omit"].null - - assert_raise(ActiveRecord::StatementInvalid) { klass.create! } - - assert_nothing_raised do - instance = klass.create!(omit: 1) - assert_equal 0, instance.zero - assert_equal 1, instance.omit - end - ensure - klass.connection.drop_table(klass.table_name) rescue nil - end end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 76a4592ac5..151f3c8efd 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -551,6 +551,23 @@ class MigrationTest < ActiveRecord::TestCase end end + if current_adapter?(:SQLite3Adapter) + def test_allows_sqlite3_rollback_on_invalid_column_type + Person.connection.create_table :something, force: true do |t| + t.column :number, :integer + t.column :name, :string + t.column :foo, :bar + end + assert Person.connection.column_exists?(:something, :foo) + assert_nothing_raised { Person.connection.remove_column :something, :foo, :bar } + assert !Person.connection.column_exists?(:something, :foo) + assert Person.connection.column_exists?(:something, :name) + assert Person.connection.column_exists?(:something, :number) + ensure + Person.connection.drop_table :something, if_exists: true + end + end + if current_adapter? :OracleAdapter def test_create_table_with_custom_sequence_name # table name is 29 chars, the standard sequence name will diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 33baf84ef2..23cfe46d7f 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -442,3 +442,81 @@ class SchemaDumperDefaultsTest < ActiveRecord::TestCase assert_match %r{t\.time\s+"time_with_default",\s+default: '2000-01-01 07:17:04'}, output end end + +class SchemaDumperNoStandardizedArgumentWidthsTest < ActiveRecord::TestCase + include SchemaDumpingHelper + + setup do + ActiveRecord::SchemaDumper.standardized_argument_widths = false + ActiveRecord::SchemaMigration.create_table + end + + teardown do + ActiveRecord::SchemaDumper.standardized_argument_widths = true + end + + def standard_dump + @@standard_dump ||= perform_schema_dump + end + + def perform_schema_dump + dump_all_table_schema [] + end + + def assert_no_line_up(lines, pattern) + return assert(true) if lines.empty? + matches = lines.map { |line| line.match(pattern) } + matches.compact! + return assert(true) if matches.empty? + line_matches = lines.map { |line| [line, line.match(pattern)] }.select { |line, match| match } + assert line_matches.all? { |line, match| + start = match.offset(0).first + line[start - 2..start - 1] == ", " + } + end + + def column_definition_lines(output = standard_dump) + output.scan(/^( *)create_table.*?\n(.*?)^\1end/m).map { |m| m.last.split(/\n/) } + end + + def test_arguments_no_line_up + column_definition_lines.each do |column_set| + assert_no_line_up(column_set, /default: /) + assert_no_line_up(column_set, /limit: /) + assert_no_line_up(column_set, /null: /) + end + end +end + +class SchemaDumperNoStandardizedTypeWidthsTest < ActiveRecord::TestCase + include SchemaDumpingHelper + + setup do + ActiveRecord::SchemaDumper.standardized_type_widths = false + ActiveRecord::SchemaMigration.create_table + end + + teardown do + ActiveRecord::SchemaDumper.standardized_type_widths = true + end + + def standard_dump + @@standard_dump ||= perform_schema_dump + end + + def perform_schema_dump + dump_all_table_schema [] + end + + def column_definition_lines(output = standard_dump) + output.scan(/^( *)create_table.*?\n(.*?)^\1end/m).map { |m| m.last.split(/\n/) } + end + + def test_types_no_line_up + column_definition_lines.each do |column_set| + next if column_set.empty? + + assert column_set.all? { |column| !column.match(/\bt\.\w+\s{2,}/) } + end + end +end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 42eff9cff9..66a99cbcda 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -24,6 +24,7 @@ class Post < ActiveRecord::Base scope :limit_by, lambda { |l| limit(l) } belongs_to :author + belongs_to :readonly_author, -> { readonly }, class_name: "Author", foreign_key: :author_id belongs_to :author_with_posts, -> { includes(:posts) }, class_name: "Author", foreign_key: :author_id belongs_to :author_with_address, -> { includes(:author_address) }, class_name: "Author", foreign_key: :author_id diff --git a/guides/source/active_job_basics.md b/guides/source/active_job_basics.md index c9f70dc87b..c65d1e6de5 100644 --- a/guides/source/active_job_basics.md +++ b/guides/source/active_job_basics.md @@ -34,8 +34,9 @@ Delayed Job and Resque. Picking your queuing backend becomes more of an operatio concern, then. And you'll be able to switch between them without having to rewrite your jobs. -NOTE: Rails by default comes with an "immediate runner" queuing implementation. -That means that each job that has been enqueued will run immediately. +NOTE: Rails by default comes with an asynchronous queuing implementation that +runs jobs with an in-process thread pool. Jobs will run asynchronously, but any +jobs in the queue will be dropped upon restart. Creating a Job @@ -109,7 +110,7 @@ That's it! Job Execution ------------- -For enqueuing and executing jobs in production you need to set up a queuing backend, +For enqueuing and executing jobs in production you need to set up a queuing backend, that is to say you need to decide for a 3rd-party queuing library that Rails should use. Rails itself only provides an in-process queuing system, which only keeps the jobs in RAM. If the process crashes or the machine is reset, then all outstanding jobs are lost with the diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 7239105b29..3e27dfafa0 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -368,9 +368,11 @@ The MySQL adapter adds one additional configuration option: * `ActiveRecord::ConnectionAdapters::Mysql2Adapter.emulate_booleans` controls whether Active Record will consider all `tinyint(1)` columns as booleans. Defaults to `true`. -The schema dumper adds one additional configuration option: +The schema dumper adds additional configuration options: * `ActiveRecord::SchemaDumper.ignore_tables` accepts an array of tables that should _not_ be included in any generated schema file. This setting is ignored unless `config.active_record.schema_format == :ruby`. +* `ActiveRecord::SchemaDumper.standardized_argument_widths` configures whether colum arguments should be lined up or not in dump. By default this is `true`. This setting is ignored unless `config.active_record.schema_format == :ruby`. +* `ActiveRecord::SchemaDumper.standardized_type_widths` configures whether colum types should be lined up or not in dump. By default this is `true`. This setting is ignored unless `config.active_record.schema_format == :ruby`. ### Configuring Action Controller diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 70f4b84237..594d239290 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,10 @@ +* Run `before_configuration` callbacks as soon as application constant + inherits from `Rails::Application`. + + Fixes #19880. + + *Yuji Yaginuma* + * A generated app should not include Uglifier with `--skip-javascript` option. *Ben Pickles* @@ -17,7 +24,7 @@ *John Meehan* -* Display name of the class defining the initializer along with the initializer +* Display name of the class defining the initializer along with the initializer name in the output of `rails initializers`. Before: diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 9c150965bf..b01196e3ed 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -87,6 +87,7 @@ module Rails super Rails.app_class = base add_lib_to_load_path!(find_root(base.called_from)) + ActiveSupport.run_load_hooks(:before_configuration, base) end def instance @@ -146,7 +147,6 @@ module Rails def run_load_hooks! # :nodoc: return self if @ran_load_hooks @ran_load_hooks = true - ActiveSupport.run_load_hooks(:before_configuration, self) @initial_variable_values.each do |variable_name, value| if INITIAL_VARIABLES.include?(variable_name) diff --git a/railties/test/railties/railtie_test.rb b/railties/test/railties/railtie_test.rb index 994019267c..755ac23cb1 100644 --- a/railties/test/railties/railtie_test.rb +++ b/railties/test/railties/railtie_test.rb @@ -79,6 +79,13 @@ module RailtiesTest assert_equal app_path, $before_configuration end + test "before_configuration callbacks run as soon as the application constant inherits from Rails::Application" do + $before_configuration = false + class Foo < Rails::Railtie ; config.before_configuration { $before_configuration = true } ; end + class Application < Rails::Application ; end + assert $before_configuration + end + test "railtie can add after_initialize callbacks" do $after_initialize = false class Foo < Rails::Railtie ; config.after_initialize { $after_initialize = true } ; end |