diff options
17 files changed, 176 insertions, 100 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 83510c40c8..21075d0636 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Add a `:required` option to singular associations, providing a nicer + API for presence validations on associations. + + *Sean Griffin* + * Fixed error in `reset_counters` when associations have `select` scope. (Call to `count` generates invalid SQL.) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6222bfe903..d9b339a1f6 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1309,6 +1309,10 @@ module ActiveRecord # that is the inverse of this <tt>has_one</tt> association. Does not work in combination # with <tt>:through</tt> or <tt>:as</tt> options. # See ActiveRecord::Associations::ClassMethods's overview on Bi-directional associations for more detail. + # [:required] + # When set to +true+, the association will also have its presence validated. + # This will validate the association itself, not the id. You can use + # +:inverse_of+ to avoid an extra query during validation. # # Option examples: # has_one :credit_card, dependent: :destroy # destroys the associated credit card @@ -1320,6 +1324,7 @@ module ActiveRecord # has_one :boss, readonly: :true # has_one :club, through: :membership # has_one :primary_address, -> { where primary: true }, through: :addressables, source: :addressable + # has_one :credit_card, required: true def has_one(name, scope = nil, options = {}) reflection = Builder::HasOne.build(self, name, scope, options) Reflection.add_reflection self, name, reflection @@ -1421,6 +1426,10 @@ module ActiveRecord # object that is the inverse of this <tt>belongs_to</tt> association. Does not work in # combination with the <tt>:polymorphic</tt> options. # See ActiveRecord::Associations::ClassMethods's overview on Bi-directional associations for more detail. + # [:required] + # When set to +true+, the association will also have its presence validated. + # This will validate the association itself, not the id. You can use + # +:inverse_of+ to avoid an extra query during validation. # # Option examples: # belongs_to :firm, foreign_key: "client_of" @@ -1433,6 +1442,7 @@ module ActiveRecord # belongs_to :post, counter_cache: true # belongs_to :company, touch: true # belongs_to :company, touch: :employees_last_updated_at + # belongs_to :company, required: true def belongs_to(name, scope = nil, options = {}) reflection = Builder::BelongsTo.build(self, name, scope, options) Reflection.add_reflection self, name, reflection diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index e474236939..947d61ee7b 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -36,6 +36,7 @@ module ActiveRecord::Associations::Builder reflection = builder.build(model) define_accessors model, reflection define_callbacks model, reflection + define_validations model, reflection builder.define_extensions model reflection end @@ -124,6 +125,10 @@ module ActiveRecord::Associations::Builder CODE end + def self.define_validations(model, reflection) + # noop + end + def self.valid_dependent_options raise NotImplementedError end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index e655c389a6..17f36acf40 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -3,7 +3,7 @@ module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: def valid_options - super + [:remote, :dependent, :primary_key, :inverse_of] + super + [:remote, :dependent, :primary_key, :inverse_of, :required] end def self.define_accessors(model, reflection) @@ -27,5 +27,12 @@ module ActiveRecord::Associations::Builder end CODE end + + def self.define_validations(model, reflection) + super + if reflection.options[:required] + model.validates_presence_of reflection.name + end + end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb deleted file mode 100644 index 5b3e88fb08..0000000000 --- a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb +++ /dev/null @@ -1,13 +0,0 @@ -module ActiveRecord - module ConnectionAdapters - module PostgreSQL - module Cast # :nodoc: - def range_to_string(object) # :nodoc: - from = object.begin.respond_to?(:infinite?) && object.begin.infinite? ? '' : object.begin - to = object.end.respond_to?(:infinite?) && object.end.infinite? ? '' : object.end - "[#{from},#{to}#{object.exclude_end? ? ')' : ']'}" - end - end - end - end -end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb index 847fd4dded..37e5c3859c 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb @@ -1,11 +1,7 @@ -require 'active_record/connection_adapters/postgresql/cast' - module ActiveRecord module ConnectionAdapters # PostgreSQL-specific extensions to column definitions in a table. class PostgreSQLColumn < Column #:nodoc: - extend PostgreSQL::Cast - attr_accessor :array def initialize(name, default, cast_type, sql_type = nil, null = true, default_function = nil) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb index 991cdd0913..ae967d5167 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb @@ -10,28 +10,10 @@ module ActiveRecord @type = type end - def extract_bounds(value) - from, to = value[1..-2].split(',') - { - from: (value[1] == ',' || from == '-infinity') ? @subtype.infinity(negative: true) : from, - to: (value[-2] == ',' || to == 'infinity') ? @subtype.infinity : to, - exclude_start: (value[0] == '('), - exclude_end: (value[-1] == ')') - } - end - - def infinity?(value) - value.respond_to?(:infinite?) && value.infinite? - end - def type_cast_for_schema(value) value.inspect.gsub('Infinity', '::Float::INFINITY') end - def type_cast_single(value) - infinity?(value) ? value : @subtype.type_cast_from_database(value) - end - def cast_value(value) return if value == 'empty' return value if value.is_a?(::Range) @@ -53,6 +35,40 @@ This is not reliable and will be removed in the future. end ::Range.new(from, to, extracted[:exclude_end]) end + + def type_cast_for_database(value) + if value.is_a?(::Range) + from = type_cast_single_for_database(value.begin) + to = type_cast_single_for_database(value.end) + "[#{from},#{to}#{value.exclude_end? ? ')' : ']'}" + else + super + end + end + + private + + def type_cast_single(value) + infinity?(value) ? value : @subtype.type_cast_from_database(value) + end + + def type_cast_single_for_database(value) + infinity?(value) ? '' : @subtype.type_cast_for_database(value) + end + + def extract_bounds(value) + from, to = value[1..-2].split(',') + { + from: (value[1] == ',' || from == '-infinity') ? @subtype.infinity(negative: true) : from, + to: (value[-2] == ',' || to == 'infinity') ? @subtype.infinity : to, + exclude_start: (value[0] == '('), + exclude_end: (value[-1] == ')') + } + end + + def infinity?(value) + value.respond_to?(:infinite?) && value.infinite? + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 39b36731cd..be598f997f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -21,13 +21,6 @@ module ActiveRecord sql_type = type_to_sql(column.type, column.limit, column.precision, column.scale) case value - when Range - if /range$/ =~ sql_type - escaped = quote_string(PostgreSQLColumn.range_to_string(value)) - "'#{escaped}'::#{sql_type}" - else - super - end when Float if value.infinite? || value.nan? "'#{value.to_s}'" @@ -54,12 +47,6 @@ module ActiveRecord return super unless column case value - when Range - if /range$/ =~ column.sql_type - PostgreSQLColumn.range_to_string(value) - else - super - end when NilClass if column.array value diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 360922dfaa..e6163771e8 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -50,34 +50,6 @@ module ActiveRecord end end - class SQLite3String < Type::String # :nodoc: - def initialize(logger, *args) - @logger = logger - super(*args) - end - - def type_cast_for_database(value) - return unless value - - if value.encoding == Encoding::ASCII_8BIT - @logger.error "Binary data inserted for `string` type" if @logger - value.encode Encoding::UTF_8 - else - value - end - end - - class Factory - def initialize(logger) - @logger = logger - end - - def new(*args) - SQLite3String.new(@logger, *args) - end - end - end - # The SQLite3 adapter works SQLite 3.6.16 or newer # with the sqlite3-ruby drivers (available as gem from https://rubygems.org/gems/sqlite3). # @@ -255,14 +227,6 @@ module ActiveRecord end end - def _type_cast(value) # :nodoc: - if value.is_a?(BigDecimal) - value.to_f - else - super - end - end - def quote_string(s) #:nodoc: @connection.class.quote(s) end @@ -285,6 +249,19 @@ module ActiveRecord end end + def type_cast(value, column) # :nodoc: + return value.to_f if BigDecimal === value + return super unless String === value + return super unless column && value + + value = super + if column.type == :string && value.encoding == Encoding::ASCII_8BIT + logger.error "Binary data inserted for `string` type on column `#{column.name}`" if logger + value = value.encode Encoding::UTF_8 + end + value + end + # DATABASE STATEMENTS ====================================== def explain(arel, binds = []) @@ -526,7 +503,6 @@ module ActiveRecord def initialize_type_map(m) super m.register_type(/binary/i, SQLite3Binary.new) - register_class_with_limit m, %r(char)i, SQLite3String::Factory.new(logger) end def select(sql, name = nil, binds = []) #:nodoc: diff --git a/activerecord/test/cases/adapters/postgresql/quoting_test.rb b/activerecord/test/cases/adapters/postgresql/quoting_test.rb index 59c274c759..11d5173d37 100644 --- a/activerecord/test/cases/adapters/postgresql/quoting_test.rb +++ b/activerecord/test/cases/adapters/postgresql/quoting_test.rb @@ -61,7 +61,7 @@ module ActiveRecord def test_quote_range range = "1,2]'; SELECT * FROM users; --".."a" c = PostgreSQLColumn.new(nil, nil, OID::Range.new(Type::Integer.new, :int8range)) - assert_equal "'[1,2]''; SELECT * FROM users; --,a]'::int8range", @conn.quote(range, c) + assert_equal "'[1,0]'", @conn.quote(range, c) end def test_quote_bit_string diff --git a/activerecord/test/cases/adapters/postgresql/range_test.rb b/activerecord/test/cases/adapters/postgresql/range_test.rb index 02d0a9b483..d812cd01c4 100644 --- a/activerecord/test/cases/adapters/postgresql/range_test.rb +++ b/activerecord/test/cases/adapters/postgresql/range_test.rb @@ -271,12 +271,12 @@ _SQL end def test_ranges_correctly_escape_input - e = assert_raises(ActiveRecord::StatementInvalid) do - range = "1,2]'; SELECT * FROM users; --".."a" - PostgresqlRange.update_all(int8_range: range) - end + range = "-1,2]'; DROP TABLE postgresql_ranges; --".."a" + PostgresqlRange.update_all(int8_range: range) - assert e.message.starts_with?("PG::InvalidTextRepresentation") + assert_nothing_raised do + PostgresqlRange.first + end end private diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 4238681905..c9bdfb88ae 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -936,7 +936,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end end -unless current_adapter?(:MysqlAdapter, :Mysql2Adapter) class BelongsToWithForeignKeyTest < ActiveRecord::TestCase def test_destroy_linked_models address = AuthorAddress.create! @@ -945,4 +944,3 @@ class BelongsToWithForeignKeyTest < ActiveRecord::TestCase author.destroy! end end -end diff --git a/activerecord/test/cases/associations/required_test.rb b/activerecord/test/cases/associations/required_test.rb new file mode 100644 index 0000000000..a6934a056e --- /dev/null +++ b/activerecord/test/cases/associations/required_test.rb @@ -0,0 +1,82 @@ +require "cases/helper" + +class RequiredAssociationsTest < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + class Parent < ActiveRecord::Base + end + + class Child < ActiveRecord::Base + end + + setup do + @connection = ActiveRecord::Base.connection + @connection.create_table :parents, force: true + @connection.create_table :children, force: true do |t| + t.belongs_to :parent + end + end + + teardown do + @connection.execute("DROP TABLE IF EXISTS parents") + @connection.execute("DROP TABLE IF EXISTS children") + end + + test "belongs_to associations are not required by default" do + model = subclass_of(Child) do + belongs_to :parent, inverse_of: false, + class_name: "RequiredAssociationsTest::Parent" + end + + assert model.new.save + assert model.new(parent: Parent.new).save + end + + test "required belongs_to associations have presence validated" do + model = subclass_of(Child) do + belongs_to :parent, required: true, inverse_of: false, + class_name: "RequiredAssociationsTest::Parent" + end + + record = model.new + assert_not record.save + assert_equal ["Parent can't be blank"], record.errors.full_messages + + record.parent = Parent.new + assert record.save + end + + test "has_one associations are not required by default" do + model = subclass_of(Parent) do + has_one :child, inverse_of: false, + class_name: "RequiredAssociationsTest::Child" + end + + assert model.new.save + assert model.new(child: Child.new).save + end + + test "required has_one associations have presence validated" do + model = subclass_of(Parent) do + has_one :child, required: true, inverse_of: false, + class_name: "RequiredAssociationsTest::Child" + end + + record = model.new + assert_not record.save + assert_equal ["Child can't be blank"], record.errors.full_messages + + record.child = Child.new + assert record.save + end + + private + + def subclass_of(klass, &block) + subclass = Class.new(klass, &block) + def subclass.name + superclass.name + end + subclass + end +end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 9b762fc0d5..2170fe6118 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -20,7 +20,7 @@ require 'models/toy' require 'rexml/document' class PersistenceTest < ActiveRecord::TestCase - fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts, :minivans, :pets, :toys + fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :author_addresses, :categorizations, :categories, :posts, :minivans, :pets, :toys # Oracle UPDATE does not support ORDER BY unless current_adapter?(:OracleAdapter) diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 7dbb29d9b0..932c9ba5d9 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -84,9 +84,7 @@ ActiveRecord::Schema.define do create_table :author_addresses, force: true do |t| end - unless current_adapter?(:MysqlAdapter, :Mysql2Adapter) - add_foreign_key :authors, :author_addresses - end + add_foreign_key :authors, :author_addresses create_table :author_favorites, force: true do |t| t.column :author_id, :integer diff --git a/guides/source/4_2_release_notes.md b/guides/source/4_2_release_notes.md index 1bb912b678..7b5cd5c799 100644 --- a/guides/source/4_2_release_notes.md +++ b/guides/source/4_2_release_notes.md @@ -262,6 +262,10 @@ for detailed changes. * Added support for user-created range types in PostgreSQL adapter. ([Commit](https://github.com/rails/rails/commit/4cb47167e747e8f9dc12b0ddaf82bdb68c03e032)) +* Added a `:required` option to singular associations, which defines a + presence validation on the association. + ([Pull Request](https://github.com/rails/rails/pull/16056)) + Active Model ------------ @@ -315,6 +319,11 @@ for detailed changes. as well as `Kernel#concern` to avoid the `module Foo; extend ActiveSupport::Concern; end` boilerplate. ([Commit](https://github.com/rails/rails/commit/b16c36e688970df2f96f793a759365b248b582ad)) +* Added `Hash#transform_values` and `Hash#transform_values!` to simplify a + common pattern where the values of a hash must change, but the keys are left + the same. + ([Pull Request](https://github.com/rails/rails/pull/15819)) + Credits ------- diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 4f37bf971a..9537d9718c 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -3672,9 +3672,9 @@ t.advance(seconds: 1) #### `Time.current` -Active Support defines `Time.current` to be today in the current time zone. That's like `Time.now`, except that it honors the user time zone, if defined. It also defines `Time.yesterday` and `Time.tomorrow`, and the instance predicates `past?`, `today?`, and `future?`, all of them relative to `Time.current`. +Active Support defines `Time.current` to be today in the current time zone. That's like `Time.now`, except that it honors the user time zone, if defined. It also defines the instance predicates `past?`, `today?`, and `future?`, all of them relative to `Time.current`. -When making Time comparisons using methods which honor the user time zone, make sure to use `Time.current` and not `Time.now`. There are cases where the user time zone might be in the future compared to the system time zone, which `Time.today` uses by default. This means `Time.now` may equal `Time.yesterday`. +When making Time comparisons using methods which honor the user time zone, make sure to use `Time.current` instead of `Time.now`. There are cases where the user time zone might be in the future compared to the system time zone, which `Time.now` uses by default. This means `Time.now.to_date` may equal `Date.yesterday`. #### `all_day`, `all_week`, `all_month`, `all_quarter` and `all_year` |