diff options
Diffstat (limited to 'activerecord')
160 files changed, 3168 insertions, 1817 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c7928e503b..e15756e834 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,275 @@ -* Fix PredicateBuilder so polymorhic association keys in `where` clause can - also accept not only `ActiveRecord::Base` direct descendances (decorated +* Allow for the name of the schema_migrations table to be configured. + + *Jerad Phelps* + +* Do not add to scope includes values from through associations. + Fixed bug when providing `includes` in through association scope, and fetching targets. + + Example: + class Vendor < ActiveRecord::Base + has_many :relationships, -> { includes(:user) } + has_many :users, through: :relationships + end + + vendor = Vendor.first + + # Before + + vendor.users.to_a # => Raises exception: not found `:user` for `User` + + # After + + vendor.users.to_a # => No exception is raised + + + Fixes: #12242, #9517, #10240 + + *Paul Nikitochkin* + +* Type cast json values on write, so that the value is consistent + with reading from the database. + + Example: + + x = JsonDataType.new tags: {"string" => "foo", :symbol => :bar} + + # Before: + x.tags # => {"string" => "foo", :symbol => :bar} + + # After: + x.tags # => {"string" => "foo", "symbol" => "bar"} + + *Severin Schoepke* + +* `ActiveRecord::Store` works together with PG `hstore` columns. + Fixes #12452. + + *Yves Senn* + +* Fix bug where `ActiveRecord::Store` used a global `Hash` to keep track of + all registered `stored_attributes`. Now every subclass of + `ActiveRecord::Base` has it's own `Hash`. + + *Yves Senn* + +* Save `has_one` association when primary key is manually set. + + Fixes #12302. + + *Lauro Caetano* + +* Allow any version of BCrypt when using `has_secure_password`. + + *Mike Perham* + +* Sub-query generated for `Relation` passed as array condition did not take in account + bind values and have invalid syntax. + + Generate sub-query with inline bind values. + + Fixes #12586. + + *Paul Nikitochkin* + +* Fix a bug where rake db:structure:load crashed when the path contained + spaces. + + *Kevin Mook* + +* `ActiveRecord::QueryMethods#unscope` unscopes negative equality + + Allows you to call `#unscope` on a relation with negative equality + operators, i.e. `Arel::Nodes::NotIn` and `Arel::Nodes::NotEqual` that have + been generated through the use of `where.not`. + + *Eric Hankins* + +* Raise an exception when model without primary key calls `.find_with_ids`. + + *Shimpei Makimoto* + +* Make `Relation#empty?` use `exists?` instead of `count`. + + *Szymon Nowak* + +* `rake db:structure:dump` no longer crashes when the port was specified as `Fixnum`. + + *Kenta Okamoto* + +* `NullRelation#pluck` takes a list of columns + + The method signature in `NullRelation` was updated to mimic that in + `Calculations`. + + *Derek Prior* + +* `scope_chain` should not be mutated for other reflections. + + Currently `scope_chain` uses same array for building different + `scope_chain` for different associations. During processing + these arrays are sometimes mutated and because of in-place + mutation the changed `scope_chain` impacts other reflections. + + Fix is to dup the value before adding to the `scope_chain`. + + Fixes #3882. + + *Neeraj Singh* + +* Prevent the inversed association from being reloaded on save. + + Fixes #9499. + + *Dmitry Polushkin* + +* Generate subquery for `Relation` if it passed as array condition for `where` + method. + + Example: + + # Before + Blog.where('id in (?)', Blog.where(id: 1)) + # => SELECT "blogs".* FROM "blogs" WHERE "blogs"."id" = 1 + # => SELECT "blogs".* FROM "blogs" WHERE (id IN (1)) + + # After + Blog.where('id in (?)', Blog.where(id: 1).select(:id)) + # => SELECT "blogs".* FROM "blogs" + # WHERE "blogs"."id" IN (SELECT "blogs"."id" FROM "blogs" WHERE "blogs"."id" = 1) + + Fixes #12415. + + *Paul Nikitochkin* + +* For missed association exception message + which is raised in `ActiveRecord::Associations::Preloader` class + added owner record class name in order to simplify to find problem code. + + *Paul Nikitochkin* + +* `has_and_belongs_to_many` is now transparently implemented in terms of + `has_many :through`. Behavior should remain the same, if not, it is a bug. + +* `create_savepoint`, `rollback_to_savepoint` and `release_savepoint` accept + a savepoint name. + + *Yves Senn* + +* Make `next_migration_number` accessible for third party generators. + + *Yves Senn* + +* Objects instantiated using a null relationship will now retain the + attributes of the where clause. + + Fixes #11676, #11675, #11376. + + *Paul Nikitochkin*, *Peter Brown*, *Nthalk* + +* Fixed `ActiveRecord::Associations::CollectionAssociation#find` + when using `has_many` association with `:inverse_of` and finding an array of one element, + it should return an array of one element too. + + *arthurnn* + +* Callbacks on has_many should access the in memory parent if a inverse_of is set. + + *arthurnn* + +* `ActiveRecord::ConnectionAdapters.string_to_time` respects + string with timezone (e.g. Wed, 04 Sep 2013 20:30:00 JST). + + Fixes #12278. + + *kennyj* + +* Calling `update_attributes` will now throw an `ArgumentError` whenever it + gets a `nil` argument. More specifically, it will throw an error if the + argument that it gets passed does not respond to to `stringify_keys`. + + Example: + + @my_comment.update_attributes(nil) # => raises ArgumentError + + *John Wang* + +* Deprecate `quoted_locking_column` method, which isn't used anywhere. + + *kennyj* + +* Migration dump UUID default functions to schema.rb. + + Fixes #10751. + + *kennyj* + +* Fixed a bug in `ActiveRecord::Associations::CollectionAssociation#find_by_scan` + when using `has_many` association with `:inverse_of` option and UUID primary key. + + Fixes #10450. + + *kennyj* + +* ActiveRecord::Base#<=> has been removed. Primary keys may not be in order, + or even be numbers, so sorting by id doesn't make sense. Please use `sort_by` + and specify the attribute you wish to sort with. For example, change: + + Post.all.to_a.sort + + to: + + Post.all.to_a.sort_by(&:id) + + *Aaron Patterson* + +* Fix: joins association, with defined in the scope block constraints by using several + where constraints and at least of them is not `Arel::Nodes::Equality`, + generates invalid SQL expression. + + Fixes #11963. + + *Paul Nikitochkin* + +* Deprecate the delegation of Array bang methods for associations. + To use them, instead first call `#to_a` on the association to access the + array to be acted on. + + *Ben Woosley* + +* `CollectionAssociation#first`/`#last` (e.g. `has_many`) use a `LIMIT`ed + query to fetch results rather than loading the entire collection. + + *Lann Martin* + +* Make possible to run SQLite rake tasks without the `Rails` constant defined. + + *Damien Mathieu* + +* Allow Relation#from to accept other relations with bind values. + + *Ryan Wallace* + +* Fix inserts with prepared statements disabled. + + Fixes #12023. + + *Rafael Mendonça França* + +* Setting a has_one association on a new record no longer causes an empty + transaction. + + *Dylan Thacker-Smith* + +* Fix `AR::Relation#merge` sometimes failing to preserve `readonly(false)` flag. + + *thedarkone* + +* Re-use `order` argument pre-processing for `reorder`. + + *Paul Nikitochkin* + +* Fix PredicateBuilder so polymorphic association keys in `where` clause can + accept objects other than direct descendants of `ActiveRecord::Base` (decorated models, for example). *Mikhail Dieterle* @@ -78,6 +348,12 @@ *Yves Senn* +* Fixes bug when using includes combined with select, the select statement was overwritten. + + Fixes #11773. + + *Edo Balvers* + * Load fixtures from linked folders. *Kassio Borges* @@ -129,13 +405,13 @@ to allow the connection adapter to properly determine how to quote the value. This was affecting certain databases that use specific column types. - Fixes: #6763 + Fixes #6763. *Alfred Wong* * rescue from all exceptions in `ConnectionManagement#call` - Fixes #11497 + Fixes #11497. As `ActiveRecord::ConnectionAdapters::ConnectionManagement` middleware does not rescue from Exception (but only from StandardError), the Connection @@ -157,27 +433,27 @@ * Remove extra decrement of transaction deep level. - Fixes: #4566 + Fixes #4566. *Paul Nikitochkin* * Reset @column_defaults when assigning `locking_column`. We had a potential problem. For example: - class Post < ActiveRecord::Base - self.column_defaults # if we call this unintentionally before setting locking_column ... - self.locking_column = 'my_locking_column' - end + class Post < ActiveRecord::Base + self.column_defaults # if we call this unintentionally before setting locking_column ... + self.locking_column = 'my_locking_column' + end - Post.column_defaults["my_locking_column"] - => nil # expected value is 0 ! + Post.column_defaults["my_locking_column"] + => nil # expected value is 0 ! *kennyj* * Remove extra select and update queries on save/touch/destroy ActiveRecord model with belongs to reflection with option `touch: true`. - Fixes: #11288 + Fixes #11288. *Paul Nikitochkin* @@ -417,7 +693,7 @@ *Neeraj Singh* -* Fixture setup does no longer depend on `ActiveRecord::Base.configurations`. +* Fixture setup no longer depends on `ActiveRecord::Base.configurations`. This is relevant when `ENV["DATABASE_URL"]` is used in place of a `database.yml`. *Yves Senn* diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 9d418dacaf..f19f5ecdf9 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -145,10 +145,6 @@ module ActiveRecord autoload :MySQLDatabaseTasks, 'active_record/tasks/mysql_database_tasks' autoload :PostgreSQLDatabaseTasks, 'active_record/tasks/postgresql_database_tasks' - - autoload :FirebirdDatabaseTasks, 'active_record/tasks/firebird_database_tasks' - autoload :SqlserverDatabaseTasks, 'active_record/tasks/sqlserver_database_tasks' - autoload :OracleDatabaseTasks, 'active_record/tasks/oracle_database_tasks' end autoload :TestFixtures, 'active_record/fixtures' diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index d075edc159..0d5313956b 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -224,7 +224,7 @@ module ActiveRecord writer_method(name, class_name, mapping, allow_nil, converter) reflection = ActiveRecord::Reflection.create(:composed_of, part_id, nil, options, self) - Reflection.add_reflection self, part_id, reflection + Reflection.add_aggregate_reflection self, part_id, reflection end private diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 33cbafc6aa..74e2774626 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -73,12 +73,6 @@ module ActiveRecord end end - class HasAndBelongsToManyAssociationForeignKeyNeeded < ActiveRecordError #:nodoc: - def initialize(reflection) - super("Cannot create self referential has_and_belongs_to_many association on '#{reflection.class_name rescue nil}##{reflection.name rescue nil}'. :association_foreign_key cannot be the same as the :foreign_key.") - end - end - class EagerLoadPolymorphicError < ActiveRecordError #:nodoc: def initialize(reflection) super("Can not eagerly load the polymorphic association #{reflection.name.inspect}") @@ -114,7 +108,6 @@ module ActiveRecord autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association' autoload :BelongsToPolymorphicAssociation, 'active_record/associations/belongs_to_polymorphic_association' - autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association' autoload :HasManyAssociation, 'active_record/associations/has_many_association' autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association' autoload :HasOneAssociation, 'active_record/associations/has_one_association' @@ -1560,8 +1553,39 @@ module ActiveRecord # has_and_belongs_to_many :categories, join_table: "prods_cats" # has_and_belongs_to_many :categories, -> { readonly } def has_and_belongs_to_many(name, scope = nil, options = {}, &extension) - reflection = Builder::HasAndBelongsToMany.build(self, name, scope, options, &extension) - Reflection.add_reflection self, name, reflection + if scope.is_a?(Hash) + options = scope + scope = nil + end + + builder = Builder::HasAndBelongsToMany.new name, self, options + + join_model = builder.through_model + + middle_reflection = builder.middle_reflection join_model + + Builder::HasMany.define_callbacks self, middle_reflection + Reflection.add_reflection self, middle_reflection.name, middle_reflection + + include Module.new { + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def destroy_associations + association(:#{middle_reflection.name}).delete_all(:delete_all) + association(:#{name}).reset + super + end + RUBY + } + + hm_options = {} + hm_options[:through] = middle_reflection.name + hm_options[:source] = join_model.right_reflection.name + + [:before_add, :after_add, :before_remove, :after_remove].each do |k| + hm_options[k] = options[k] if options.key? k + end + + has_many name, scope, hm_options, &extension end end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 67d24b35d1..e6a45487d0 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -13,11 +13,11 @@ module ActiveRecord # BelongsToAssociation # BelongsToPolymorphicAssociation # CollectionAssociation - # HasAndBelongsToManyAssociation # HasManyAssociation # HasManyThroughAssociation + ThroughAssociation class Association #:nodoc: attr_reader :owner, :target, :reflection + attr_accessor :inversed delegate :options, :to => :reflection @@ -43,6 +43,7 @@ module ActiveRecord @loaded = false @target = nil @stale_state = nil + @inversed = false end # Reloads the \target and returns +self+ on success. @@ -60,8 +61,9 @@ module ActiveRecord # Asserts the \target has been loaded setting the \loaded flag to +true+. def loaded! - @loaded = true + @loaded = true @stale_state = stale_state + @inversed = false end # The target is stale if the target no longer points to the record(s) that the @@ -71,7 +73,7 @@ module ActiveRecord # # Note that if the target has not been loaded, it is not considered stale. def stale_target? - loaded? && @stale_state != stale_state + !inversed && loaded? && @stale_state != stale_state end # Sets the target of this association to <tt>\target</tt>, and the \loaded flag to +true+. @@ -105,6 +107,7 @@ module ActiveRecord if record && invertible_for?(record) inverse = record.association(inverse_reflection_for(record).name) inverse.target = owner + inverse.inversed = true end end diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 8027acfb83..17f056e764 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -44,18 +44,6 @@ module ActiveRecord chain.each_with_index do |reflection, i| table, foreign_table = tables.shift, tables.first - if reflection.source_macro == :has_and_belongs_to_many - join_table = tables.shift - - scope = scope.joins(join( - join_table, - table[reflection.association_primary_key]. - eq(join_table[reflection.association_foreign_key]) - )) - - table, foreign_table = join_table, tables.first - end - if reflection.source_macro == :belongs_to if reflection.options[:polymorphic] key = reflection.association_primary_key(self.klass) @@ -90,7 +78,8 @@ module ActiveRecord scope = scope.joins(join(foreign_table, constraint)) end - klass = i == 0 ? self.klass : reflection.klass + is_first_chain = i == 0 + klass = is_first_chain ? self.klass : reflection.klass # Exclude the scope of the association itself, because that # was already merged in the #scope method. @@ -101,7 +90,10 @@ module ActiveRecord scope.merge! item.except(:where, :includes, :bind) end - scope.includes! item.includes_values + if is_first_chain + scope.includes! item.includes_values + end + scope.where_values += item.where_values scope.order_values |= item.order_values end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 8eec4f56af..e1fa5225b5 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -50,8 +50,8 @@ module ActiveRecord # Checks whether record is different to the current target, without loading it def different_target?(record) - if record.nil? - owner[reflection.foreign_key] + if record.nil? + owner[reflection.foreign_key] else record.id != owner[reflection.foreign_key] end diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 34de1a1f32..d8d68eb908 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -8,7 +8,6 @@ # - HasOneAssociation # - CollectionAssociation # - HasManyAssociation -# - HasAndBelongsToManyAssociation module ActiveRecord::Associations::Builder class Association #:nodoc: @@ -17,11 +16,17 @@ module ActiveRecord::Associations::Builder end self.extensions = [] - VALID_OPTIONS = [:class_name, :foreign_key, :validate] - - attr_reader :name, :scope, :options + VALID_OPTIONS = [:class_name, :class, :foreign_key, :validate] def self.build(model, name, scope, options, &block) + extension = define_extensions model, name, &block + reflection = create_reflection model, name, scope, options, extension + define_accessors model, reflection + define_callbacks model, reflection + reflection + end + + def self.create_reflection(model, name, scope, options, extension = nil) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) if scope.is_a?(Hash) @@ -29,47 +34,48 @@ module ActiveRecord::Associations::Builder scope = nil end - builder = new(name, scope, options, &block) - reflection = builder.build(model) - builder.define_accessors model - builder.define_callbacks model, reflection - builder.define_extensions model - reflection - end + validate_options(options) - def initialize(name, scope, options) - @name = name - @scope = scope - @options = options + scope = build_scope(scope, extension) - validate_options + ActiveRecord::Reflection.create(macro, name, scope, options, model) + end + + def self.build_scope(scope, extension) + new_scope = scope if scope && scope.arity == 0 - @scope = proc { instance_exec(&scope) } + new_scope = proc { instance_exec(&scope) } + end + + if extension + new_scope = wrap_scope new_scope, extension end + + new_scope end - def build(model) - ActiveRecord::Reflection.create(macro, name, scope, options, model) + def self.wrap_scope(scope, extension) + scope end - def macro + def self.macro raise NotImplementedError end - def valid_options + def self.valid_options(options) VALID_OPTIONS + Association.extensions.flat_map(&:valid_options) end - def validate_options - options.assert_valid_keys(valid_options) + def self.validate_options(options) + options.assert_valid_keys(valid_options(options)) end - def define_extensions(model) + def self.define_extensions(model, name) end - def define_callbacks(model, reflection) - add_before_destroy_callbacks(model, name) if options[:dependent] + def self.define_callbacks(model, reflection) + add_before_destroy_callbacks(model, reflection) if reflection.options[:dependent] Association.extensions.each do |extension| extension.build model, reflection end @@ -81,14 +87,14 @@ module ActiveRecord::Associations::Builder # end # # Post.first.comments and Post.first.comments= methods are defined by this method... - - def define_accessors(model) + def self.define_accessors(model, reflection) mixin = model.generated_feature_methods - define_readers(mixin) - define_writers(mixin) + name = reflection.name + define_readers(mixin, name) + define_writers(mixin, name) end - def define_readers(mixin) + def self.define_readers(mixin, name) mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{name}(*args) association(:#{name}).reader(*args) @@ -96,7 +102,7 @@ module ActiveRecord::Associations::Builder CODE end - def define_writers(mixin) + def self.define_writers(mixin, name) mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{name}=(value) association(:#{name}).writer(value) @@ -104,17 +110,16 @@ module ActiveRecord::Associations::Builder CODE end - def valid_dependent_options + def self.valid_dependent_options raise NotImplementedError end - private - - def add_before_destroy_callbacks(model, name) - unless valid_dependent_options.include? options[:dependent] - raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{options[:dependent]}" + def self.add_before_destroy_callbacks(model, reflection) + unless valid_dependent_options.include? reflection.options[:dependent] + raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{reflection.options[:dependent]}" end + name = reflection.name model.before_destroy lambda { |o| o.association(name).handle_dependency } end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 4e88b50ec5..aa43c34d86 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,50 +1,44 @@ module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: - def macro + def self.macro :belongs_to end - def valid_options + def self.valid_options(options) super + [:foreign_type, :polymorphic, :touch] end - def constructable? - !options[:polymorphic] - end - - def valid_dependent_options + def self.valid_dependent_options [:destroy, :delete] end - def define_callbacks(model, reflection) + def self.define_callbacks(model, reflection) super - add_counter_cache_callbacks(model, reflection) if options[:counter_cache] - add_touch_callbacks(model, reflection) if options[:touch] + add_counter_cache_callbacks(model, reflection) if reflection.options[:counter_cache] + add_touch_callbacks(model, reflection) if reflection.options[:touch] end - def define_accessors(mixin) + def self.define_accessors(mixin, reflection) super add_counter_cache_methods mixin end - private - - def add_counter_cache_methods(mixin) + def self.add_counter_cache_methods(mixin) return if mixin.method_defined? :belongs_to_counter_cache_after_create mixin.class_eval do - def belongs_to_counter_cache_after_create(association, reflection) - if record = send(association.name) + def belongs_to_counter_cache_after_create(reflection) + if record = send(reflection.name) cache_column = reflection.counter_cache_column record.class.increment_counter(cache_column, record.id) @_after_create_counter_called = true end end - def belongs_to_counter_cache_before_destroy(association, reflection) + def belongs_to_counter_cache_before_destroy(reflection) foreign_key = reflection.foreign_key.to_sym unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key - record = send association.name + record = send reflection.name if record && !self.destroyed? cache_column = reflection.counter_cache_column record.class.decrement_counter(cache_column, record.id) @@ -52,13 +46,13 @@ module ActiveRecord::Associations::Builder end end - def belongs_to_counter_cache_after_update(association, reflection) + def belongs_to_counter_cache_after_update(reflection) foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column if (@_after_create_counter_called ||= false) @_after_create_counter_called = false - elsif attribute_changed?(foreign_key) && !new_record? && association.constructable? + elsif attribute_changed?(foreign_key) && !new_record? && reflection.constructable? model = reflection.klass foreign_key_was = attribute_was foreign_key foreign_key = attribute foreign_key @@ -74,20 +68,19 @@ module ActiveRecord::Associations::Builder end end - def add_counter_cache_callbacks(model, reflection) + def self.add_counter_cache_callbacks(model, reflection) cache_column = reflection.counter_cache_column - association = self model.after_create lambda { |record| - record.belongs_to_counter_cache_after_create(association, reflection) + record.belongs_to_counter_cache_after_create(reflection) } model.before_destroy lambda { |record| - record.belongs_to_counter_cache_before_destroy(association, reflection) + record.belongs_to_counter_cache_before_destroy(reflection) } model.after_update lambda { |record| - record.belongs_to_counter_cache_after_update(association, reflection) + record.belongs_to_counter_cache_after_update(reflection) } klass = reflection.class_name.safe_constantize @@ -120,10 +113,10 @@ module ActiveRecord::Associations::Builder end end - def add_touch_callbacks(model, reflection) + def self.add_touch_callbacks(model, reflection) foreign_key = reflection.foreign_key - n = name - touch = options[:touch] + n = reflection.name + touch = reflection.options[:touch] callback = lambda { |record| BelongsTo.touch_record(record, foreign_key, n, touch) diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 7bd0687c0b..2ff67f904d 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -1,4 +1,4 @@ -# This class is inherited by the has_many and has_many_and_belongs_to_many association classes +# This class is inherited by the has_many and has_many_and_belongs_to_many association classes require 'active_record/associations' @@ -7,35 +7,29 @@ module ActiveRecord::Associations::Builder CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] - def valid_options + def self.valid_options(options) super + [:table_name, :before_add, :after_add, :before_remove, :after_remove, :extend] end - attr_reader :block_extension - - def initialize(name, scope, options) + def self.define_callbacks(model, reflection) super - @mod = nil - if block_given? - @mod = Module.new(&Proc.new) - @scope = wrap_scope @scope, @mod - end + name = reflection.name + options = reflection.options + CALLBACKS.each { |callback_name| + define_callback(model, callback_name, name, options) + } end - def define_callbacks(model, reflection) - super - CALLBACKS.each { |callback_name| define_callback(model, callback_name) } - end - - def define_extensions(model) - if @mod + def self.define_extensions(model, name) + if block_given? extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" - model.parent.const_set(extension_module_name, @mod) + extension = Module.new(&Proc.new) + model.parent.const_set(extension_module_name, extension) end end - def define_callback(model, callback_name) + def self.define_callback(model, callback_name, name, options) full_callback_name = "#{callback_name}_for_#{name}" # TODO : why do i need method_defined? I think its because of the inheritance chain @@ -54,8 +48,7 @@ module ActiveRecord::Associations::Builder end # Defines the setter and getter methods for the collection_singular_ids. - - def define_readers(mixin) + def self.define_readers(mixin, name) super mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 @@ -65,7 +58,7 @@ module ActiveRecord::Associations::Builder CODE end - def define_writers(mixin) + def self.define_writers(mixin, name) super mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 @@ -75,9 +68,7 @@ module ActiveRecord::Associations::Builder CODE end - private - - def wrap_scope(scope, mod) + def self.wrap_scope(scope, mod) if scope proc { |owner| instance_exec(owner, &scope).extending(mod) } else diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 55ec3bf4f1..1c9c04b044 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -1,24 +1,121 @@ module ActiveRecord::Associations::Builder - class HasAndBelongsToMany < CollectionAssociation #:nodoc: - def macro - :has_and_belongs_to_many + class HasAndBelongsToMany # :nodoc: + class JoinTableResolver + KnownTable = Struct.new :join_table + + class KnownClass + def initialize(lhs_class, rhs_class_name) + @lhs_class = lhs_class + @rhs_class_name = rhs_class_name + @join_table = nil + end + + def join_table + @join_table ||= [@lhs_class.table_name, klass.table_name].sort.join("\0").gsub(/^(.*_)(.+)\0\1(.+)/, '\1\2_\3').gsub("\0", "_") + end + + private + def klass; @rhs_class_name.constantize; end + end + + def self.build(lhs_class, name, options) + if options[:join_table] + KnownTable.new options[:join_table] + else + class_name = options.fetch(:class_name) { + name.to_s.camelize.singularize + } + KnownClass.new lhs_class, class_name + end + end + end + + attr_reader :lhs_model, :association_name, :options + + def initialize(association_name, lhs_model, options) + @association_name = association_name + @lhs_model = lhs_model + @options = options + end + + def through_model + habtm = JoinTableResolver.build lhs_model, association_name, options + + join_model = Class.new(ActiveRecord::Base) { + class << self; + attr_accessor :class_resolver + attr_accessor :name + attr_accessor :table_name_resolver + attr_accessor :left_reflection + attr_accessor :right_reflection + end + + def self.table_name + table_name_resolver.join_table + end + + def self.compute_type(class_name) + class_resolver.compute_type class_name + end + + def self.add_left_association(name, options) + belongs_to name, options + self.left_reflection = reflect_on_association(name) + end + + def self.add_right_association(name, options) + rhs_name = name.to_s.singularize.to_sym + belongs_to rhs_name, options + self.right_reflection = reflect_on_association(rhs_name) + end + + } + + join_model.name = "HABTM_#{association_name.to_s.camelize}" + join_model.table_name_resolver = habtm + join_model.class_resolver = lhs_model + + join_model.add_left_association :left_side, class: lhs_model + join_model.add_right_association association_name, belongs_to_options(options) + join_model + end + + def middle_reflection(join_model) + middle_name = [lhs_model.name.downcase.pluralize, + association_name].join('_').gsub(/::/, '_').to_sym + middle_options = middle_options join_model + + HasMany.create_reflection(lhs_model, + middle_name, + nil, + middle_options) end - def valid_options - super + [:join_table, :association_foreign_key] + private + + def middle_options(join_model) + middle_options = {} + middle_options[:class] = join_model + middle_options[:source] = join_model.left_reflection.name + if options.key? :foreign_key + middle_options[:foreign_key] = options[:foreign_key] + end + middle_options end - def define_callbacks(model, reflection) - super - name = self.name - model.send(:include, Module.new { - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def destroy_associations - association(:#{name}).delete_all - super - end - RUBY - }) + def belongs_to_options(options) + rhs_options = {} + + if options.key? :class_name + rhs_options[:foreign_key] = options[:class_name].foreign_key + rhs_options[:class_name] = options[:class_name] + end + + if options.key? :association_foreign_key + rhs_options[:foreign_key] = options[:association_foreign_key] + end + + rhs_options end end end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index a60cb4769a..227184cd19 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,14 +1,14 @@ module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: - def macro + def self.macro :has_many end - def valid_options + def self.valid_options(options) super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache] end - def valid_dependent_options + def self.valid_dependent_options [:destroy, :delete_all, :nullify, :restrict_with_error, :restrict_with_exception] end end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 62d454ce55..064a3c8b51 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,27 +1,21 @@ module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: - def macro + def self.macro :has_one end - def valid_options + def self.valid_options(options) valid = super + [:order, :as] valid += [:through, :source, :source_type] if options[:through] valid end - def constructable? - !options[:through] - end - - def valid_dependent_options + def self.valid_dependent_options [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception] end - private - - def add_before_destroy_callbacks(model, name) - super unless options[:through] + def self.add_before_destroy_callbacks(model, reflection) + super unless reflection.options[:through] end end end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index d97c0e9afd..2a4b1c441f 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -1,23 +1,18 @@ -# This class is inherited by the has_one and belongs_to association classes +# This class is inherited by the has_one and belongs_to association classes module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: - def valid_options + def self.valid_options(options) super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of] end - def constructable? - true - end - - def define_accessors(model) + def self.define_accessors(model, reflection) super - define_constructors(model.generated_feature_methods) if constructable? + define_constructors(model.generated_feature_methods, reflection.name) if reflection.constructable? end # Defines the (build|create)_association methods for belongs_to or has_one association - - def define_constructors(mixin) + def self.define_constructors(mixin, name) mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def build_#{name}(*args, &block) association(:#{name}).build(*args, &block) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 228c500f0a..6b06a5f7fd 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -7,7 +7,6 @@ module ActiveRecord # collections. See the class hierarchy in AssociationProxy. # # CollectionAssociation: - # HasAndBelongsToManyAssociation => has_and_belongs_to_many # HasManyAssociation => has_many # HasManyThroughAssociation + ThroughAssociation => has_many :through # @@ -80,14 +79,13 @@ module ActiveRecord load_target.find(*args) { |*block_args| yield(*block_args) } else if options[:inverse_of] && loaded? - args = args.flatten - raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args.blank? - + args_flatten = args.flatten + raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args_flatten.blank? result = find_by_scan(*args) result_size = Array(result).size - if !result || result_size != args.size - scope.raise_record_not_found_exception!(args, result_size, args.size) + if !result || result_size != args_flatten.size + scope.raise_record_not_found_exception!(args_flatten, result_size, args_flatten.size) else result end @@ -197,9 +195,7 @@ module ActiveRecord # Count all records using SQL. Construct options and pass them with # scope to the target class's +count+. - def count(column_name = nil, count_options = {}) - column_name, count_options = nil, column_name if column_name.is_a?(Hash) - + def count(column_name = nil) relation = scope if association_scope.distinct_value # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL. @@ -528,21 +524,20 @@ module ActiveRecord # * target already loaded # * owner is new record # * target contains new or changed record(s) - # * the first arg is an integer (which indicates the number of records to be returned) def fetch_first_or_last_using_find?(args) if args.first.is_a?(Hash) true else !(loaded? || owner.new_record? || - target.any? { |record| record.new_record? || record.changed? } || - args.first.kind_of?(Integer)) + target.any? { |record| record.new_record? || record.changed? }) end end def include_in_memory?(record) if reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) - owner.send(reflection.through_reflection.name).any? { |source| + assoc = owner.association(reflection.through_reflection.name) + assoc.reader.any? { |source| target = source.send(reflection.source_reflection.name) target.respond_to?(:include?) ? target.include?(record) : target == record } || target.include?(record) @@ -555,14 +550,14 @@ module ActiveRecord # specified, then #find scans the entire collection. def find_by_scan(*args) expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.map{ |arg| arg.to_i }.uniq + ids = args.flatten.compact.map{ |arg| arg.to_s }.uniq if ids.size == 1 id = ids.first - record = load_target.detect { |r| id == r.id } + record = load_target.detect { |r| id == r.id.to_s } expects_array ? [ record ] : record else - load_target.select { |r| ids.include?(r.id) } + load_target.select { |r| ids.include?(r.id.to_s) } end end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index ea7f768a68..2e70a07962 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -281,7 +281,7 @@ module ActiveRecord # so method calls may be chained. # # class Person < ActiveRecord::Base - # pets :has_many + # has_many :pets # end # # person.pets.size # => 0 @@ -669,8 +669,8 @@ module ActiveRecord # # #<Pet id: 2, name: "Spook", person_id: 1>, # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> # # ] - def count(column_name = nil, options = {}) - @association.count(column_name, options) + def count(column_name = nil) + @association.count(column_name) end # Returns the size of the collection. If the collection hasn't been loaded, diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb deleted file mode 100644 index b2e6c708bf..0000000000 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ /dev/null @@ -1,56 +0,0 @@ -module ActiveRecord - # = Active Record Has And Belongs To Many Association - module Associations - class HasAndBelongsToManyAssociation < CollectionAssociation #:nodoc: - attr_reader :join_table - - def initialize(owner, reflection) - @join_table = Arel::Table.new(reflection.join_table) - super - end - - def insert_record(record, validate = true, raise = false) - if record.new_record? - if raise - record.save!(:validate => validate) - else - return unless record.save(:validate => validate) - end - end - - stmt = join_table.compile_insert( - join_table[reflection.foreign_key] => owner.id, - join_table[reflection.association_foreign_key] => record.id - ) - - owner.class.connection.insert stmt - - record - end - - private - - def count_records - load_target.size - end - - def delete_records(records, method) - relation = join_table - condition = relation[reflection.foreign_key].eq(owner.id) - - unless records == :all - condition = condition.and( - relation[reflection.association_foreign_key] - .in(records.map { |x| x.id }.compact) - ) - end - - owner.class.connection.delete(relation.where(condition).compile_delete) - end - - def invertible_for?(record) - false - end - end - end -end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 607ed0da46..0a23109b9b 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -32,6 +32,7 @@ module ActiveRecord def insert_record(record, validate = true, raise = false) set_owner_attributes(record) + set_inverse_instance(record) if raise record.save!(:validate => validate) @@ -125,7 +126,11 @@ module ActiveRecord end def foreign_key_present? - owner.attribute_present?(reflection.association_primary_key) + if reflection.klass.primary_key + owner.attribute_present?(reflection.association_primary_key) + else + false + end end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index a74dd1cdab..56331bbb0b 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -140,7 +140,21 @@ module ActiveRecord case method when :destroy - count = scope.destroy_all.length + if scope.klass.primary_key + count = scope.destroy_all.length + else + scope.to_a.each do |record| + record.run_callbacks :destroy + end + + arel = scope.arel + + stmt = Arel::DeleteManager.new arel.engine + stmt.from scope.klass.arel_table + stmt.wheres = arel.constraints + + count = scope.klass.connection.delete(stmt, 'SQL', scope.bind_values) + end when :nullify count = scope.update_all(source_reflection.foreign_key => nil) else diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 3ab1ea1ff4..0008600418 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -27,6 +27,8 @@ module ActiveRecord return self.target if !(target || record) if (target != record) || record.changed? + save &&= owner.persisted? + transaction_if(save) do remove_target!(options[:dependent]) if target && !target.destroyed? @@ -34,7 +36,7 @@ module ActiveRecord set_owner_attributes(record) set_inverse_instance(record) - if owner.persisted? && save && !record.save + if save && !record.save nullify_owner_attributes(record) set_owner_attributes(target) if target raise RecordNotSaved, "Failed to save the new associated #{reflection.name}." diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 5aa17e5fbb..c3ac0680ea 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -1,11 +1,75 @@ module ActiveRecord module Associations class JoinDependency # :nodoc: - autoload :JoinPart, 'active_record/associations/join_dependency/join_part' autoload :JoinBase, 'active_record/associations/join_dependency/join_base' autoload :JoinAssociation, 'active_record/associations/join_dependency/join_association' - attr_reader :join_parts, :reflections, :alias_tracker, :base_klass + class Aliases # :nodoc: + def initialize(tables) + @tables = tables + @alias_cache = tables.each_with_object({}) { |table,h| + h[table.node] = table.columns.each_with_object({}) { |column,i| + i[column.name] = column.alias + } + } + @name_and_alias_cache = tables.each_with_object({}) { |table,h| + h[table.node] = table.columns.map { |column| + [column.name, column.alias] + } + } + end + + def columns + @tables.flat_map { |t| t.column_aliases } + end + + # An array of [column_name, alias] pairs for the table + def column_aliases(node) + @name_and_alias_cache[node] + end + + def column_alias(node, column) + @alias_cache[node][column] + end + + class Table < Struct.new(:node, :columns) + def table + Arel::Nodes::TableAlias.new node.table, node.aliased_table_name + end + + def column_aliases + t = table + columns.map { |column| t[column.name].as Arel.sql column.alias } + end + end + Column = Struct.new(:name, :alias) + end + + attr_reader :alias_tracker, :base_klass, :join_root + + def self.make_tree(associations) + hash = {} + walk_tree associations, hash + hash + end + + def self.walk_tree(associations, hash) + case associations + when Symbol, String + hash[associations.to_sym] ||= {} + when Array + associations.each do |assoc| + walk_tree assoc, hash + end + when Hash + associations.each do |k,v| + cache = hash[k] ||= {} + walk_tree v, cache + end + else + raise ConfigurationError, associations.inspect + end + end # base is the base class on which operation is taking place. # associations is the list of associations which are joined using hash, symbol or array. @@ -29,211 +93,181 @@ module ActiveRecord # joins #=> [] # def initialize(base, associations, joins) - @base_klass = base - @table_joins = joins - @join_parts = [JoinBase.new(base)] - @associations = {} - @reflections = [] @alias_tracker = AliasTracker.new(base.connection, joins) @alias_tracker.aliased_name_for(base.table_name) # Updates the count for base.table_name to 1 - build(associations) + tree = self.class.make_tree associations + @join_root = JoinBase.new base, build(tree, base) + @join_root.children.each { |child| construct_tables! @join_root, child } end - def graft(*associations) - associations.each do |association| - join_associations.detect {|a| association == a} || - build(association.reflection.name, association.find_parent_in(self) || join_base, association.join_type) - end - self + def reflections + join_root.drop(1).map!(&:reflection) end - def join_associations - join_parts.drop 1 - end + def join_constraints(outer_joins) + joins = join_root.children.flat_map { |child| + make_inner_joins join_root, child + } - def join_base - join_parts.first + joins.concat outer_joins.flat_map { |oj| + if join_root.match? oj.join_root + walk join_root, oj.join_root + else + oj.join_root.children.flat_map { |child| + make_outer_joins join_root, child + } + end + } end - def join_relation(relation) - join_associations.inject(relation) do |rel,association| - association.join_relation(rel) - end + def aliases + Aliases.new join_root.each_with_index.map { |join_part,i| + columns = join_part.column_names.each_with_index.map { |column_name,j| + Aliases::Column.new column_name, "t#{i}_r#{j}" + } + Aliases::Table.new(join_part, columns) + } end - def columns - join_parts.collect { |join_part| - table = join_part.aliased_table - join_part.column_names_with_alias.collect{ |column_name, aliased_name| - table[column_name].as Arel.sql(aliased_name) + def instantiate(result_set, aliases) + primary_key = aliases.column_alias(join_root, join_root.primary_key) + type_caster = result_set.column_type primary_key + + seen = Hash.new { |h,parent_klass| + h[parent_klass] = Hash.new { |i,parent_id| + i[parent_id] = Hash.new { |j,child_klass| j[child_klass] = {} } } - }.flatten - end + } - def instantiate(rows) - primary_key = join_base.aliased_primary_key - parents = {} + model_cache = Hash.new { |h,klass| h[klass] = {} } + parents = model_cache[join_root] + column_aliases = aliases.column_aliases join_root - records = rows.map { |model| - primary_id = model[primary_key] - parent = parents[primary_id] ||= join_base.instantiate(model) - construct(parent, @associations, join_associations, model) - parent - }.uniq + result_set.each { |row_hash| + primary_id = type_caster.type_cast row_hash[primary_key] + parent = parents[primary_id] ||= join_root.instantiate(row_hash, column_aliases) + construct(parent, join_root, row_hash, result_set, seen, model_cache, aliases) + } - remove_duplicate_results!(base_klass, records, @associations) - records + parents.values end - protected - - def remove_duplicate_results!(base, records, associations) - case associations - when Symbol, String - reflection = base.reflections[associations] - remove_uniq_by_reflection(reflection, records) - when Array - associations.each do |association| - remove_duplicate_results!(base, records, association) - end - when Hash - associations.each_key do |name| - reflection = base.reflections[name] - remove_uniq_by_reflection(reflection, records) - - parent_records = [] - records.each do |record| - if descendant = record.send(reflection.name) - if reflection.collection? - parent_records.concat descendant.target.uniq - else - parent_records << descendant - end - end - end + private - remove_duplicate_results!(reflection.klass, parent_records, associations[name]) unless parent_records.empty? - end - end + def make_constraints(parent, child, tables, join_type) + chain = child.reflection.chain + foreign_table = parent.table + foreign_klass = parent.base_klass + child.join_constraints(foreign_table, foreign_klass, child, join_type, tables, child.reflection.scope_chain, chain) end - def cache_joined_association(association) - associations = [] - parent = association.parent - while parent != join_base - associations.unshift(parent.reflection.name) - parent = parent.parent - end - ref = @associations - associations.each do |key| - ref = ref[key] - end - ref[association.reflection.name] ||= {} - end + def make_outer_joins(parent, child) + tables = table_aliases_for(parent, child) + join_type = Arel::OuterJoin + joins = make_constraints parent, child, tables, join_type - def build(associations, parent = join_parts.last, join_type = Arel::InnerJoin) - case associations - when Symbol, String - reflection = parent.reflections[associations.intern] or - raise ConfigurationError, "Association named '#{ associations }' was not found on #{ parent.base_klass.name }; perhaps you misspelled it?" - unless join_association = find_join_association(reflection, parent) - @reflections << reflection - join_association = build_join_association(reflection, parent) - join_association.join_type = join_type - @join_parts << join_association - cache_joined_association(join_association) - end - join_association - when Array - associations.each do |association| - build(association, parent, join_type) - end - when Hash - associations.keys.sort_by { |a| a.to_s }.each do |name| - join_association = build(name, parent, join_type) - build(associations[name], join_association, join_type) - end - else - raise ConfigurationError, associations.inspect - end + joins.concat child.children.flat_map { |c| make_outer_joins(child, c) } end - def find_join_association(name_or_reflection, parent) - if String === name_or_reflection - name_or_reflection = name_or_reflection.to_sym - end + def make_inner_joins(parent, child) + tables = child.tables + join_type = Arel::InnerJoin + joins = make_constraints parent, child, tables, join_type - join_associations.detect { |j| - j.reflection == name_or_reflection && j.parent == parent + joins.concat child.children.flat_map { |c| make_inner_joins(child, c) } + end + + def table_aliases_for(parent, node) + node.reflection.chain.map { |reflection| + alias_tracker.aliased_table_for( + reflection.table_name, + table_alias_for(reflection, parent, reflection != node.reflection) + ) } end - def remove_uniq_by_reflection(reflection, records) - if reflection && reflection.collection? - records.each { |record| record.send(reflection.name).target.uniq! } - end + def construct_tables!(parent, node) + node.tables = table_aliases_for(parent, node) + node.children.each { |child| construct_tables! node, child } end - def build_join_association(reflection, parent) - JoinAssociation.new(reflection, self, parent) + def table_alias_for(reflection, parent, join) + name = "#{reflection.plural_name}_#{parent.table_name}" + name << "_join" if join + name end - def construct(parent, associations, join_parts, row) - case associations - when Symbol, String - name = associations.to_s + def walk(left, right) + intersection, missing = right.children.map { |node1| + [left.children.find { |node2| node1.match? node2 }, node1] + }.partition(&:first) - join_part = join_parts.detect { |j| - j.reflection.name.to_s == name && - j.parent_table_name == parent.class.table_name } + ojs = missing.flat_map { |_,n| make_outer_joins left, n } + intersection.flat_map { |l,r| walk l, r }.concat ojs + end - raise(ConfigurationError, "No such association") unless join_part + def find_reflection(klass, name) + klass.reflect_on_association(name) or + raise ConfigurationError, "Association named '#{ name }' was not found on #{ klass.name }; perhaps you misspelled it?" + end - join_parts.delete(join_part) - construct_association(parent, join_part, row) - when Array - associations.each do |association| - construct(parent, association, join_parts, row) - end - when Hash - associations.sort_by { |k,_| k.to_s }.each do |association_name, assoc| - association = construct(parent, association_name, join_parts, row) - construct(association, assoc, join_parts, row) if association + def build(associations, base_klass) + associations.map do |name, right| + reflection = find_reflection base_klass, name + reflection.check_validity! + + if reflection.options[:polymorphic] + raise EagerLoadPolymorphicError.new(reflection) end - else - raise ConfigurationError, associations.inspect + + JoinAssociation.new reflection, build(right, reflection.klass) end end - def construct_association(record, join_part, row) - return if record.id.to_s != join_part.parent.record_id(row).to_s + def construct(ar_parent, parent, row, rs, seen, model_cache, aliases) + primary_id = ar_parent.id - macro = join_part.reflection.macro - if macro == :has_one - return record.association(join_part.reflection.name).target if record.association_cache.key?(join_part.reflection.name) - association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? - set_target_and_inverse(join_part, association, record) - else - association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? - case macro - when :has_many, :has_and_belongs_to_many - other = record.association(join_part.reflection.name) + parent.children.each do |node| + if node.reflection.collection? + other = ar_parent.association(node.reflection.name) other.loaded! - other.target.push(association) if association - other.set_inverse_instance(association) - when :belongs_to - set_target_and_inverse(join_part, association, record) else - raise ConfigurationError, "unknown macro: #{join_part.reflection.macro}" + if ar_parent.association_cache.key?(node.reflection.name) + model = ar_parent.association(node.reflection.name).target + construct(model, node, row, rs, seen, model_cache, aliases) + next + end + end + + key = aliases.column_alias(node, node.primary_key) + id = row[key] + next if id.nil? + + model = seen[parent.base_klass][primary_id][node.base_klass][id] + + if model + construct(model, node, row, rs, seen, model_cache, aliases) + else + model = construct_model(ar_parent, node, row, model_cache, id, aliases) + seen[parent.base_klass][primary_id][node.base_klass][id] = model + construct(model, node, row, rs, seen, model_cache, aliases) end end - association end - def set_target_and_inverse(join_part, association, record) - other = record.association(join_part.reflection.name) - other.target = association - other.set_inverse_instance(association) + def construct_model(record, node, row, model_cache, id, aliases) + model = model_cache[node][id] ||= node.instantiate(row, + aliases.column_aliases(node)) + other = record.association(node.reflection.name) + + if node.reflection.collection? + other.target.push(model) + else + other.target = model + end + + other.set_inverse_instance(model) + model end end end diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 58fc00d811..191d430636 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -1,77 +1,31 @@ +require 'active_record/associations/join_dependency/join_part' + module ActiveRecord module Associations class JoinDependency # :nodoc: class JoinAssociation < JoinPart # :nodoc: - include JoinHelper - # The reflection of the association represented attr_reader :reflection - # The JoinDependency object which this JoinAssociation exists within. This is mainly - # relevant for generating aliases which do not conflict with other joins which are - # part of the query. - attr_reader :join_dependency - - # A JoinBase instance representing the active record we are joining onto. - # (So in Author.has_many :posts, the Author would be that base record.) - attr_reader :parent - - # What type of join will be generated, either Arel::InnerJoin (default) or Arel::OuterJoin - attr_accessor :join_type - - # These implement abstract methods from the superclass - attr_reader :aliased_prefix - - attr_reader :tables + attr_accessor :tables - delegate :options, :through_reflection, :source_reflection, :chain, :to => :reflection - delegate :alias_tracker, :to => :join_dependency - - def initialize(reflection, join_dependency, parent = nil) - reflection.check_validity! - - if reflection.options[:polymorphic] - raise EagerLoadPolymorphicError.new(reflection) - end - - super(reflection.klass) + def initialize(reflection, children) + super(reflection.klass, children) @reflection = reflection - @join_dependency = join_dependency - @parent = parent - @join_type = Arel::InnerJoin - @aliased_prefix = "t#{ join_dependency.join_parts.size }" - @tables = construct_tables.reverse - end - - def parent_table_name; parent.table_name; end - alias :alias_suffix :parent_table_name - - def ==(other) - other.class == self.class && - other.reflection == reflection && - other.parent == parent + @tables = nil end - def find_parent_in(other_join_dependency) - other_join_dependency.join_parts.detect do |join_part| - case parent - when JoinBase - parent.base_klass == join_part.base_klass - else - parent == join_part - end - end + def match?(other) + return true if self == other + super && reflection == other.reflection end - def join_constraints + def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scope_chain, chain) joins = [] - tables = @tables.dup - - foreign_table = parent.table - foreign_klass = parent.base_klass + tables = tables.reverse - scope_chain_iter = reflection.scope_chain.reverse_each + scope_chain_iter = scope_chain.reverse_each # The chain starts with the target table, but we want to end with it here (makes # more sense in this context), so we reverse @@ -83,17 +37,6 @@ module ActiveRecord when :belongs_to key = reflection.association_primary_key foreign_key = reflection.foreign_key - when :has_and_belongs_to_many - # Join the join table first... - joins << join( - table, - table[reflection.foreign_key]. - eq(foreign_table[reflection.active_record_primary_key])) - - foreign_table, table = table, tables.shift - - key = reflection.association_primary_key - foreign_key = reflection.association_foreign_key else key = reflection.foreign_key foreign_key = reflection.active_record_primary_key @@ -105,7 +48,7 @@ module ActiveRecord if item.is_a?(Relation) item else - ActiveRecord::Relation.create(klass, table).instance_exec(self, &item) + ActiveRecord::Relation.create(klass, table).instance_exec(node, &item) end end @@ -125,7 +68,7 @@ module ActiveRecord constraint = constraint.and rel.arel.constraints end - joins << join(table, constraint) + joins << table.create_join(table, table.create_on(constraint), join_type) # The current table in this iteration becomes the foreign table in the next foreign_table, foreign_klass = table, klass @@ -162,13 +105,8 @@ module ActiveRecord constraint end - def join_relation(joining_relation) - self.join_type = Arel::OuterJoin - joining_relation.joins(self) - end - def table - tables.last + tables.first end def aliased_table_name diff --git a/activerecord/lib/active_record/associations/join_dependency/join_base.rb b/activerecord/lib/active_record/associations/join_dependency/join_base.rb index a7dacdbfd6..3a26c25737 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_base.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_base.rb @@ -1,18 +1,16 @@ +require 'active_record/associations/join_dependency/join_part' + module ActiveRecord module Associations class JoinDependency # :nodoc: class JoinBase < JoinPart # :nodoc: - def ==(other) - other.class == self.class && - other.base_klass == base_klass - end - - def aliased_prefix - "t0" + def match?(other) + return true if self == other + super && base_klass == other.base_klass end def table - Arel::Table.new(table_name, arel_engine) + base_klass.arel_table end def aliased_table_name diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb index b534569063..91e1c6a9d7 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -8,34 +8,36 @@ module ActiveRecord # operations (for example a has_and_belongs_to_many JoinAssociation would result in # two; one for the join table and one for the target table). class JoinPart # :nodoc: + include Enumerable + # The Active Record class which this join part is associated 'about'; for a JoinBase # this is the actual base model, for a JoinAssociation this is the target model of the # association. - attr_reader :base_klass + attr_reader :base_klass, :children - delegate :table_name, :column_names, :primary_key, :reflections, :arel_engine, :to => :base_klass + delegate :table_name, :column_names, :primary_key, :to => :base_klass - def initialize(base_klass) + def initialize(base_klass, children) @base_klass = base_klass - @cached_record = {} @column_names_with_alias = nil + @children = children end - def aliased_table - Arel::Nodes::TableAlias.new table, aliased_table_name + def name + reflection.name end - def ==(other) - raise NotImplementedError + def match?(other) + self.class == other.class end - # An Arel::Table for the active_record - def table - raise NotImplementedError + def each(&block) + yield self + children.each { |child| child.each(&block) } end - # The prefix to be used when aliasing columns in the active_record's table - def aliased_prefix + # An Arel::Table for the active_record + def table raise NotImplementedError end @@ -44,33 +46,25 @@ module ActiveRecord raise NotImplementedError end - # The alias for the primary key of the active_record's table - def aliased_primary_key - "#{aliased_prefix}_r0" - end + def extract_record(row, column_names_with_alias) + # This code is performance critical as it is called per row. + # see: https://github.com/rails/rails/pull/12185 + hash = {} - # An array of [column_name, alias] pairs for the table - def column_names_with_alias - unless @column_names_with_alias - @column_names_with_alias = [] + index = 0 + length = column_names_with_alias.length - ([primary_key] + (column_names - [primary_key])).compact.each_with_index do |column_name, i| - @column_names_with_alias << [column_name, "#{aliased_prefix}_r#{i}"] - end + while index < length + column_name, alias_name = column_names_with_alias[index] + hash[column_name] = row[alias_name] + index += 1 end - @column_names_with_alias - end - - def extract_record(row) - Hash[column_names_with_alias.map{|cn, an| [cn, row[an]]}] - end - def record_id(row) - row[aliased_primary_key] + hash end - def instantiate(row) - @cached_record[record_id(row)] ||= base_klass.instantiate(extract_record(row)) + def instantiate(row, aliases) + base_klass.instantiate(extract_record(row, aliases)) end end end diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb index 27b70edf1a..f345d16841 100644 --- a/activerecord/lib/active_record/associations/join_helper.rb +++ b/activerecord/lib/active_record/associations/join_helper.rb @@ -10,21 +10,12 @@ module ActiveRecord private def construct_tables - tables = [] - chain.each do |reflection| - tables << alias_tracker.aliased_table_for( + chain.map do |reflection| + alias_tracker.aliased_table_for( table_name_for(reflection), table_alias_for(reflection, reflection != self.reflection) ) - - if reflection.source_macro == :has_and_belongs_to_many - tables << alias_tracker.aliased_table_for( - reflection.source_reflection.join_table, - table_alias_for(reflection, true) - ) - end end - tables end def table_name_for(reflection) diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 2317e34bc0..2393667ac8 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -42,12 +42,9 @@ module ActiveRecord autoload :HasManyThrough, 'active_record/associations/preloader/has_many_through' autoload :HasOne, 'active_record/associations/preloader/has_one' autoload :HasOneThrough, 'active_record/associations/preloader/has_one_through' - autoload :HasAndBelongsToMany, 'active_record/associations/preloader/has_and_belongs_to_many' autoload :BelongsTo, 'active_record/associations/preloader/belongs_to' end - attr_reader :records, :associations, :preload_scope, :model - # Eager loads the named associations for the given Active Record record(s). # # In this description, 'association name' shall refer to the name passed @@ -82,38 +79,47 @@ module ActiveRecord # [ :books, :author ] # { author: :avatar } # [ :books, { author: :avatar } ] - def initialize(records, associations, preload_scope = nil) - @records = Array.wrap(records).compact.uniq - @associations = Array.wrap(associations) - @preload_scope = preload_scope || Relation.create(nil, nil) - end - def run - unless records.empty? - associations.each { |association| preload(association) } + NULL_RELATION = Struct.new(:values).new({}) + + def preload(records, associations, preload_scope = nil) + records = Array.wrap(records).compact.uniq + associations = Array.wrap(associations) + preload_scope = preload_scope || NULL_RELATION + + if records.empty? + [] + else + associations.flat_map { |association| + preloaders_on association, records, preload_scope + } end end private - def preload(association) + def preloaders_on(association, records, scope) case association when Hash - preload_hash(association) + preloaders_for_hash(association, records, scope) when Symbol - preload_one(association) + preloaders_for_one(association, records, scope) when String - preload_one(association.to_sym) + preloaders_for_one(association.to_sym, records, scope) else raise ArgumentError, "#{association.inspect} was not recognised for preload" end end - def preload_hash(association) - association.each do |parent, child| - Preloader.new(records, parent, preload_scope).run - Preloader.new(records.map { |record| record.send(parent) }.flatten, child).run - end + def preloaders_for_hash(association, records, scope) + parent, child = association.to_a.first # hash should only be of length 1 + + loaders = preloaders_for_one parent, records, scope + + recs = loaders.flat_map(&:preloaded_records).uniq + loaders.concat Array.wrap(child).flat_map { |assoc| + preloaders_on assoc, recs, scope + } end # Not all records have the same class, so group then preload group on the reflection @@ -123,52 +129,81 @@ module ActiveRecord # Additionally, polymorphic belongs_to associations can have multiple associated # classes, depending on the polymorphic_type field. So we group by the classes as # well. - def preload_one(association) - grouped_records(association).each do |reflection, klasses| - klasses.each do |klass, records| - preloader_for(reflection).new(klass, records, reflection, preload_scope).run + def preloaders_for_one(association, records, scope) + grouped_records(association, records).flat_map do |reflection, klasses| + klasses.map do |rhs_klass, rs| + loader = preloader_for(reflection, rs, rhs_klass).new(rhs_klass, rs, reflection, scope) + loader.run self + loader end end end - def grouped_records(association) - Hash[ - records_by_reflection(association).map do |reflection, records| - [reflection, records.group_by { |record| association_klass(reflection, record) }] - end - ] + def grouped_records(association, records) + reflection_records = records_by_reflection(association, records) + + reflection_records.each_with_object({}) do |(reflection, r_records),h| + h[reflection] = r_records.group_by { |record| + association_klass(reflection, record) + } + end end - def records_by_reflection(association) + def records_by_reflection(association, records) records.group_by do |record| - reflection = record.class.reflections[association] + reflection = record.class.reflect_on_association(association) - unless reflection - raise ActiveRecord::ConfigurationError, "Association named '#{association}' was not found; " \ - "perhaps you misspelled it?" - end - - reflection + reflection || raise_config_error(record, association) end end + def raise_config_error(record, association) + raise ActiveRecord::ConfigurationError, + "Association named '#{association}' was not found on #{record.class.name}; " \ + "perhaps you misspelled it?" + end + def association_klass(reflection, record) if reflection.macro == :belongs_to && reflection.options[:polymorphic] - klass = record.send(reflection.foreign_type) + klass = record.read_attribute(reflection.foreign_type.to_s) klass && klass.constantize else reflection.klass end end - def preloader_for(reflection) + class AlreadyLoaded + attr_reader :owners, :reflection + + def initialize(klass, owners, reflection, preload_scope) + @owners = owners + @reflection = reflection + end + + def run(preloader); end + + def preloaded_records + owners.flat_map { |owner| owner.read_attribute reflection.name } + end + end + + class NullPreloader + def self.new(klass, owners, reflection, preload_scope); self; end + def self.run(preloader); end + end + + def preloader_for(reflection, owners, rhs_klass) + return NullPreloader unless rhs_klass + + if owners.first.association(reflection.name).loaded? + return AlreadyLoaded + end + case reflection.macro when :has_many reflection.options[:through] ? HasManyThrough : HasMany when :has_one reflection.options[:through] ? HasOneThrough : HasOne - when :has_and_belongs_to_many - HasAndBelongsToMany when :belongs_to BelongsTo end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 928da71eed..69b65982b3 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -3,6 +3,7 @@ module ActiveRecord class Preloader class Association #:nodoc: attr_reader :owners, :reflection, :preload_scope, :model, :klass + attr_reader :preloaded_records def initialize(klass, owners, reflection, preload_scope) @klass = klass @@ -12,15 +13,14 @@ module ActiveRecord @model = owners.first && owners.first.class @scope = nil @owners_by_key = nil + @preloaded_records = [] end - def run - unless owners.first.association(reflection.name).loaded? - preload - end + def run(preloader) + preload(preloader) end - def preload + def preload(preloader) raise NotImplementedError end @@ -68,36 +68,39 @@ module ActiveRecord private - def associated_records_by_owner + def associated_records_by_owner(preloader) owners_map = owners_by_key owner_keys = owners_map.keys.compact # Each record may have multiple owners, and vice-versa - records_by_owner = Hash[owners.map { |owner| [owner, []] }] + records_by_owner = owners.each_with_object({}) do |owner,h| + h[owner] = [] + end - if klass && owner_keys.any? + if owner_keys.any? # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) # Make several smaller queries if necessary or make one query if the adapter supports it sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) - sliced.each { |slice| - records = records_for(slice) - caster = type_caster(records, association_key_name) - records.each do |record| - owner_key = caster.call record[association_key_name] - - owners_map[owner_key].each do |owner| - records_by_owner[owner] << record - end + + records = load_slices sliced + records.each do |record, owner_key| + owners_map[owner_key].each do |owner| + records_by_owner[owner] << record end - } + end end records_by_owner end - IDENTITY = lambda { |value| value } - def type_caster(results, name) - IDENTITY + def load_slices(slices) + @preloaded_records = slices.flat_map { |slice| + records_for(slice) + } + + @preloaded_records.map { |record| + [record, record[association_key_name]] + } end def reflection_scope @@ -116,6 +119,14 @@ module ActiveRecord scope.select! preload_values[:select] || values[:select] || table[Arel.star] scope.includes! preload_values[:includes] || values[:includes] + if preload_values.key? :order + scope.order! preload_values[:order] + else + if values.key? :order + scope.order! values[:order] + end + end + if options[:as] scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name }) end diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb index e6cd35e7a1..5adffcd831 100644 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ b/activerecord/lib/active_record/associations/preloader/collection_association.rb @@ -9,8 +9,8 @@ module ActiveRecord super.order(preload_scope.values[:order] || reflection_scope.values[:order]) end - def preload - associated_records_by_owner.each do |owner, records| + def preload(preloader) + associated_records_by_owner(preloader).each do |owner, records| association = owner.association(reflection.name) association.loaded! association.target.concat(records) diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb deleted file mode 100644 index c042a44b21..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb +++ /dev/null @@ -1,65 +0,0 @@ -module ActiveRecord - module Associations - class Preloader - class HasAndBelongsToMany < CollectionAssociation #:nodoc: - attr_reader :join_table - - def initialize(klass, records, reflection, preload_options) - super - @join_table = Arel::Table.new(reflection.join_table).alias('t0') - end - - # Unlike the other associations, we want to get a raw array of rows so that we can - # access the aliased column on the join table - def records_for(ids) - scope = query_scope ids - klass.connection.select_all(scope.arel, 'SQL', scope.bind_values) - end - - def owner_key_name - reflection.active_record_primary_key - end - - def association_key_name - 'ar_association_key_name' - end - - def association_key - join_table[reflection.foreign_key] - end - - private - - # Once we have used the join table column (in super), we manually instantiate the - # actual records, ensuring that we don't create more than one instances of the same - # record - def associated_records_by_owner - records = {} - super.each_value do |rows| - rows.map! { |row| records[row[klass.primary_key]] ||= klass.instantiate(row) } - end - end - - def type_caster(results, name) - caster = results.column_types.fetch(name, results.identity_type) - lambda { |value| caster.type_cast value } - end - - def build_scope - super.joins(join).select(join_select) - end - - def join_select - association_key.as(Arel.sql(association_key_name)) - end - - def join - condition = table[reflection.association_primary_key].eq( - join_table[reflection.association_foreign_key]) - - table.create_join(join_table, table.create_on(condition)) - end - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb index 157b627ad5..7b37b5942d 100644 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ b/activerecord/lib/active_record/associations/preloader/has_many_through.rb @@ -4,7 +4,7 @@ module ActiveRecord class HasManyThrough < CollectionAssociation #:nodoc: include ThroughAssociation - def associated_records_by_owner + def associated_records_by_owner(preloader) records_by_owner = super if reflection_scope.distinct_value diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb index 44e804d785..2b5cfda8ce 100644 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ b/activerecord/lib/active_record/associations/preloader/singular_association.rb @@ -5,8 +5,8 @@ module ActiveRecord private - def preload - associated_records_by_owner.each do |owner, associated_records| + def preload(preloader) + associated_records_by_owner(preloader).each do |owner, associated_records| record = associated_records.first association = owner.association(reflection.name) diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 2c625cec04..2a8530af62 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -2,7 +2,6 @@ module ActiveRecord module Associations class Preloader module ThroughAssociation #:nodoc: - def through_reflection reflection.through_reflection end @@ -11,34 +10,68 @@ module ActiveRecord reflection.source_reflection end - def associated_records_by_owner - through_records = through_records_by_owner + def associated_records_by_owner(preloader) + preloader.preload(owners, + through_reflection.name, + through_scope) + + through_records = owners.map do |owner| + association = owner.association through_reflection.name + + [owner, Array(association.reader)] + end + + reset_association owners, through_reflection.name + + middle_records = through_records.map { |(_,rec)| rec }.flatten + + preloaders = preloader.preload(middle_records, + source_reflection.name, + reflection_scope) - Preloader.new(through_records.values.flatten, source_reflection.name, reflection_scope).run + @preloaded_records = preloaders.flat_map(&:preloaded_records) + + middle_to_pl = preloaders.each_with_object({}) do |pl,h| + pl.owners.each { |middle| + h[middle] = pl + } + end - through_records.each do |owner, records| - records.map! { |r| r.send(source_reflection.name) }.flatten! - records.compact! + record_offset = {} + @preloaded_records.each_with_index do |record,i| + record_offset[record] = i end + + through_records.each_with_object({}) { |(lhs,center),records_by_owner| + pl_to_middle = center.group_by { |record| middle_to_pl[record] } + + records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles| + rhs_records = middles.flat_map { |r| + association = r.association source_reflection.name + + association.reader + }.compact + + rhs_records.sort_by { |rhs| record_offset[rhs] } + end + } end private - def through_records_by_owner - Preloader.new(owners, through_reflection.name, through_scope).run - + def reset_association(owners, association_name) should_reset = (through_scope != through_reflection.klass.unscoped) || (reflection.options[:source_type] && through_reflection.collection?) - owners.each_with_object({}) do |owner, h| - association = owner.association through_reflection.name - h[owner] = Array(association.reader) - - # Dont cache the association - we would only be caching a subset - association.reset if should_reset + # Dont cache the association - we would only be caching a subset + if should_reset + owners.each { |owner| + owner.association(association_name).reset + } end end + def through_scope scope = through_reflection.klass.unscoped diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index 10238555f0..02dc464536 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -42,7 +42,6 @@ module ActiveRecord scope.first.tap { |record| set_inverse_instance(record) } end - # Implemented by subclasses def replace(record) raise NotImplementedError, "Subclasses must implement a replace(record) method" end diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index 4f06955406..30fa2c8ba5 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -12,6 +12,9 @@ module ActiveRecord # of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt> # exception is raised. def assign_attributes(new_attributes) + if !new_attributes.respond_to?(:stringify_keys) + raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." + end return if new_attributes.blank? attributes = new_attributes.stringify_keys diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 208da2cb77..43419efc75 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/enumerable' require 'mutex_m' +require 'thread_safe' module ActiveRecord # = Active Record Attribute Methods @@ -29,23 +30,18 @@ module ActiveRecord } class AttributeMethodCache - include Mutex_m - def initialize - super @module = Module.new - @method_cache = {} + @method_cache = ThreadSafe::Cache.new end def [](name) - synchronize do - @method_cache.fetch(name) { - safe_name = name.unpack('h*').first - temp_method = "__temp__#{safe_name}" - ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name - @module.module_eval method_body(temp_method, safe_name), __FILE__, __LINE__ - @method_cache[name] = @module.instance_method temp_method - } + @method_cache.compute_if_absent(name) do + safe_name = name.unpack('h*').first + temp_method = "__temp__#{safe_name}" + ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name + @module.module_eval method_body(temp_method, safe_name), __FILE__, __LINE__ + @module.instance_method temp_method end end @@ -165,12 +161,9 @@ module ActiveRecord # If we haven't generated any methods yet, generate them, then # see if we've created the method we're looking for. def method_missing(method, *args, &block) # :nodoc: - if self.class.define_attribute_methods - if respond_to_without_attributes?(method) - send(method, *args, &block) - else - super - end + self.class.define_attribute_methods + if respond_to_without_attributes?(method) + send(method, *args, &block) else super end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index dc2399643c..19e81abba5 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -19,8 +19,7 @@ module ActiveRecord # Attempts to +save+ the record and clears changed attributes if successful. def save(*) if status = super - @previously_changed = changes - @changed_attributes.clear + changes_applied end status end @@ -28,16 +27,14 @@ module ActiveRecord # Attempts to <tt>save!</tt> the record and clears changed attributes if successful. def save!(*) super.tap do - @previously_changed = changes - @changed_attributes.clear + changes_applied end end # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do - @previously_changed.clear - @changed_attributes.clear + reset_changes end end @@ -48,11 +45,11 @@ module ActiveRecord # The attribute already has an unsaved change. if attribute_changed?(attr) - old = @changed_attributes[attr] - @changed_attributes.delete(attr) unless _field_changed?(attr, old, value) + old = changed_attributes[attr] + changed_attributes.delete(attr) unless _field_changed?(attr, old, value) else old = clone_attribute_value(:read_attribute, attr) - @changed_attributes[attr] = old if _field_changed?(attr, old, value) + changed_attributes[attr] = old if _field_changed?(attr, old, value) end # Carry on. diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 1cf3aba41c..c152a246b5 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -109,13 +109,14 @@ module ActiveRecord # We use #[] first as a perf optimization for non-nil values. See https://gist.github.com/jonleighton/3552829. name = attr_name.to_s @attributes_cache[name] || @attributes_cache.fetch(name) { - column = @columns_hash.fetch(name) { - return @attributes.fetch(name) { - if name == 'id' && self.class.primary_key != name - read_attribute(self.class.primary_key) - end - } - } + column = @column_types_override[name] if @column_types_override + column ||= @column_types[name] + + return @attributes.fetch(name) { + if name == 'id' && self.class.primary_key != name + read_attribute(self.class.primary_key) + end + } unless column value = @attributes.fetch(name) { return block_given? ? yield(name) : nil diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb index 1287de0d0d..5701804168 100644 --- a/activerecord/lib/active_record/attribute_methods/serialization.rb +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -66,6 +66,10 @@ module ActiveRecord def type @column.type end + + def accessor + ActiveRecord::Store::IndifferentHashAccessor + end end class Attribute < Struct.new(:coder, :value, :state) # :nodoc: diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index b30d1eb0a6..e9622ca0c1 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -143,71 +143,71 @@ module ActiveRecord module ClassMethods private - def define_non_cyclic_method(name, reflection, &block) - define_method(name) do |*args| - result = true; @_already_called ||= {} - # Loop prevention for validation of associations - unless @_already_called[[name, reflection.name]] - begin - @_already_called[[name, reflection.name]]=true - result = instance_eval(&block) - ensure - @_already_called[[name, reflection.name]]=false + def define_non_cyclic_method(name, &block) + define_method(name) do |*args| + result = true; @_already_called ||= {} + # Loop prevention for validation of associations + unless @_already_called[name] + begin + @_already_called[name]=true + result = instance_eval(&block) + ensure + @_already_called[name]=false + end end - end - result + result + end end - end - # Adds validation and save callbacks for the association as specified by - # the +reflection+. - # - # For performance reasons, we don't check whether to validate at runtime. - # However the validation and callback methods are lazy and those methods - # get created when they are invoked for the very first time. However, - # this can change, for instance, when using nested attributes, which is - # called _after_ the association has been defined. Since we don't want - # the callbacks to get defined multiple times, there are guards that - # check if the save or validation methods have already been defined - # before actually defining them. - def add_autosave_association_callbacks(reflection) - save_method = :"autosave_associated_records_for_#{reflection.name}" - validation_method = :"validate_associated_records_for_#{reflection.name}" - collection = reflection.collection? - - unless method_defined?(save_method) - if collection - before_save :before_save_collection_association - - define_non_cyclic_method(save_method, reflection) { save_collection_association(reflection) } - # Doesn't use after_save as that would save associations added in after_create/after_update twice - after_create save_method - after_update save_method - elsif reflection.macro == :has_one - define_method(save_method) { save_has_one_association(reflection) } - # Configures two callbacks instead of a single after_save so that - # the model may rely on their execution order relative to its - # own callbacks. - # - # For example, given that after_creates run before after_saves, if - # we configured instead an after_save there would be no way to fire - # a custom after_create callback after the child association gets - # created. - after_create save_method - after_update save_method - else - define_non_cyclic_method(save_method, reflection) { save_belongs_to_association(reflection) } - before_save save_method + # Adds validation and save callbacks for the association as specified by + # the +reflection+. + # + # For performance reasons, we don't check whether to validate at runtime. + # However the validation and callback methods are lazy and those methods + # get created when they are invoked for the very first time. However, + # this can change, for instance, when using nested attributes, which is + # called _after_ the association has been defined. Since we don't want + # the callbacks to get defined multiple times, there are guards that + # check if the save or validation methods have already been defined + # before actually defining them. + def add_autosave_association_callbacks(reflection) + save_method = :"autosave_associated_records_for_#{reflection.name}" + validation_method = :"validate_associated_records_for_#{reflection.name}" + collection = reflection.collection? + + unless method_defined?(save_method) + if collection + before_save :before_save_collection_association + + define_non_cyclic_method(save_method) { save_collection_association(reflection) } + # Doesn't use after_save as that would save associations added in after_create/after_update twice + after_create save_method + after_update save_method + elsif reflection.macro == :has_one + define_method(save_method) { save_has_one_association(reflection) } + # Configures two callbacks instead of a single after_save so that + # the model may rely on their execution order relative to its + # own callbacks. + # + # For example, given that after_creates run before after_saves, if + # we configured instead an after_save there would be no way to fire + # a custom after_create callback after the child association gets + # created. + after_create save_method + after_update save_method + else + define_non_cyclic_method(save_method) { save_belongs_to_association(reflection) } + before_save save_method + end end - end - if reflection.validate? && !method_defined?(validation_method) - method = (collection ? :validate_collection_association : :validate_single_association) - define_non_cyclic_method(validation_method, reflection) { send(method, reflection) } - validate validation_method + if reflection.validate? && !method_defined?(validation_method) + method = (collection ? :validate_collection_association : :validate_single_association) + define_non_cyclic_method(validation_method) { send(method, reflection) } + validate validation_method + end end - end end # Reloads the attributes of the object as usual and clears <tt>marked_for_destruction</tt> flag. @@ -254,173 +254,179 @@ module ActiveRecord private - # Returns the record for an association collection that should be validated - # or saved. If +autosave+ is +false+ only new records will be returned, - # unless the parent is/was a new record itself. - def associated_records_to_validate_or_save(association, new_record, autosave) - if new_record - association && association.target - elsif autosave - association.target.find_all { |record| record.changed_for_autosave? } - else - association.target.find_all { |record| record.new_record? } + # Returns the record for an association collection that should be validated + # or saved. If +autosave+ is +false+ only new records will be returned, + # unless the parent is/was a new record itself. + def associated_records_to_validate_or_save(association, new_record, autosave) + if new_record + association && association.target + elsif autosave + association.target.find_all { |record| record.changed_for_autosave? } + else + association.target.find_all { |record| record.new_record? } + end end - end - # go through nested autosave associations that are loaded in memory (without loading - # any new ones), and return true if is changed for autosave - def nested_records_changed_for_autosave? - self.class.reflect_on_all_autosave_associations.any? do |reflection| - association = association_instance_get(reflection.name) - association && Array.wrap(association.target).any? { |a| a.changed_for_autosave? } + # go through nested autosave associations that are loaded in memory (without loading + # any new ones), and return true if is changed for autosave + def nested_records_changed_for_autosave? + self.class.reflect_on_all_autosave_associations.any? do |reflection| + association = association_instance_get(reflection.name) + association && Array.wrap(association.target).any? { |a| a.changed_for_autosave? } + end end - end - # Validate the association if <tt>:validate</tt> or <tt>:autosave</tt> is - # turned on for the association. - def validate_single_association(reflection) - association = association_instance_get(reflection.name) - record = association && association.reader - association_valid?(reflection, record) if record - end + # Validate the association if <tt>:validate</tt> or <tt>:autosave</tt> is + # turned on for the association. + def validate_single_association(reflection) + association = association_instance_get(reflection.name) + record = association && association.reader + association_valid?(reflection, record) if record + end - # Validate the associated records if <tt>:validate</tt> or - # <tt>:autosave</tt> is turned on for the association specified by - # +reflection+. - def validate_collection_association(reflection) - if association = association_instance_get(reflection.name) - if records = associated_records_to_validate_or_save(association, new_record?, reflection.options[:autosave]) - records.each { |record| association_valid?(reflection, record) } + # Validate the associated records if <tt>:validate</tt> or + # <tt>:autosave</tt> is turned on for the association specified by + # +reflection+. + def validate_collection_association(reflection) + if association = association_instance_get(reflection.name) + if records = associated_records_to_validate_or_save(association, new_record?, reflection.options[:autosave]) + records.each { |record| association_valid?(reflection, record) } + end end end - end - # Returns whether or not the association is valid and applies any errors to - # the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt> - # enabled records if they're marked_for_destruction? or destroyed. - def association_valid?(reflection, record) - return true if record.destroyed? || record.marked_for_destruction? - - unless valid = record.valid? - if reflection.options[:autosave] - record.errors.each do |attribute, message| - attribute = "#{reflection.name}.#{attribute}" - errors[attribute] << message - errors[attribute].uniq! + # Returns whether or not the association is valid and applies any errors to + # the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt> + # enabled records if they're marked_for_destruction? or destroyed. + def association_valid?(reflection, record) + return true if record.destroyed? || record.marked_for_destruction? + + unless valid = record.valid? + if reflection.options[:autosave] + record.errors.each do |attribute, message| + attribute = "#{reflection.name}.#{attribute}" + errors[attribute] << message + errors[attribute].uniq! + end + else + errors.add(reflection.name) end - else - errors.add(reflection.name) end + valid end - valid - end - # Is used as a before_save callback to check while saving a collection - # association whether or not the parent was a new record before saving. - def before_save_collection_association - @new_record_before_save = new_record? - true - end + # Is used as a before_save callback to check while saving a collection + # association whether or not the parent was a new record before saving. + def before_save_collection_association + @new_record_before_save = new_record? + true + end - # Saves any new associated records, or all loaded autosave associations if - # <tt>:autosave</tt> is enabled on the association. - # - # In addition, it destroys all children that were marked for destruction - # with mark_for_destruction. - # - # This all happens inside a transaction, _if_ the Transactions module is included into - # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. - def save_collection_association(reflection) - if association = association_instance_get(reflection.name) - autosave = reflection.options[:autosave] - - if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) - - if autosave - records_to_destroy = records.select(&:marked_for_destruction?) - records_to_destroy.each { |record| association.destroy(record) } - records -= records_to_destroy - end + # Saves any new associated records, or all loaded autosave associations if + # <tt>:autosave</tt> is enabled on the association. + # + # In addition, it destroys all children that were marked for destruction + # with mark_for_destruction. + # + # This all happens inside a transaction, _if_ the Transactions module is included into + # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. + def save_collection_association(reflection) + if association = association_instance_get(reflection.name) + autosave = reflection.options[:autosave] + + if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) + + if autosave + records_to_destroy = records.select(&:marked_for_destruction?) + records_to_destroy.each { |record| association.destroy(record) } + records -= records_to_destroy + end - records.each do |record| - next if record.destroyed? + records.each do |record| + next if record.destroyed? - saved = true + saved = true - if autosave != false && (@new_record_before_save || record.new_record?) - if autosave - saved = association.insert_record(record, false) - else - association.insert_record(record) unless reflection.nested? + if autosave != false && (@new_record_before_save || record.new_record?) + if autosave + saved = association.insert_record(record, false) + else + association.insert_record(record) unless reflection.nested? + end + elsif autosave + saved = record.save(:validate => false) end - elsif autosave - saved = record.save(:validate => false) - end - raise ActiveRecord::Rollback unless saved + raise ActiveRecord::Rollback unless saved + end end - end - # reconstruct the scope now that we know the owner's id - association.reset_scope if association.respond_to?(:reset_scope) + # reconstruct the scope now that we know the owner's id + association.reset_scope if association.respond_to?(:reset_scope) + end end - end - # Saves the associated record if it's new or <tt>:autosave</tt> is enabled - # on the association. - # - # In addition, it will destroy the association if it was marked for - # destruction with mark_for_destruction. - # - # This all happens inside a transaction, _if_ the Transactions module is included into - # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. - def save_has_one_association(reflection) - association = association_instance_get(reflection.name) - record = association && association.load_target - if record && !record.destroyed? - autosave = reflection.options[:autosave] - - if autosave && record.marked_for_destruction? - record.destroy - else - key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id - if autosave != false && (new_record? || record.new_record? || record[reflection.foreign_key] != key || autosave) - unless reflection.through_reflection - record[reflection.foreign_key] = key - end + # Saves the associated record if it's new or <tt>:autosave</tt> is enabled + # on the association. + # + # In addition, it will destroy the association if it was marked for + # destruction with mark_for_destruction. + # + # This all happens inside a transaction, _if_ the Transactions module is included into + # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. + def save_has_one_association(reflection) + association = association_instance_get(reflection.name) + record = association && association.load_target + if record && !record.destroyed? + autosave = reflection.options[:autosave] - saved = record.save(:validate => !autosave) - raise ActiveRecord::Rollback if !saved && autosave - saved + if autosave && record.marked_for_destruction? + record.destroy + else + key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id + if autosave != false && (autosave || new_record? || record_changed?(reflection, record, key)) + + unless reflection.through_reflection + record[reflection.foreign_key] = key + end + + saved = record.save(:validate => !autosave) + raise ActiveRecord::Rollback if !saved && autosave + saved + end end end end - end - # Saves the associated record if it's new or <tt>:autosave</tt> is enabled. - # - # In addition, it will destroy the association if it was marked for destruction. - def save_belongs_to_association(reflection) - association = association_instance_get(reflection.name) - record = association && association.load_target - if record && !record.destroyed? - autosave = reflection.options[:autosave] - - if autosave && record.marked_for_destruction? - self[reflection.foreign_key] = nil - record.destroy - elsif autosave != false - saved = record.save(:validate => !autosave) if record.new_record? || (autosave && record.changed_for_autosave?) - - if association.updated? - association_id = record.send(reflection.options[:primary_key] || :id) - self[reflection.foreign_key] = association_id - association.loaded! - end + # If the record is new or it has changed, returns true. + def record_changed?(reflection, record, key) + record.new_record? || record[reflection.foreign_key] != key || record.attribute_changed?(reflection.foreign_key) + end - saved if autosave + # Saves the associated record if it's new or <tt>:autosave</tt> is enabled. + # + # In addition, it will destroy the association if it was marked for destruction. + def save_belongs_to_association(reflection) + association = association_instance_get(reflection.name) + record = association && association.load_target + if record && !record.destroyed? + autosave = reflection.options[:autosave] + + if autosave && record.marked_for_destruction? + self[reflection.foreign_key] = nil + record.destroy + elsif autosave != false + saved = record.save(:validate => !autosave) if record.new_record? || (autosave && record.changed_for_autosave?) + + if association.updated? + association_id = record.send(reflection.options[:primary_key] || :id) + self[reflection.foreign_key] = association_id + association.loaded! + end + + saved if autosave + end end end - end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b06add096f..04e3dd49e7 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -18,6 +18,7 @@ require 'arel' require 'active_record/errors' require 'active_record/log_subscriber' require 'active_record/explain_subscriber' +require 'active_record/relation/delegation' module ActiveRecord #:nodoc: # = Active Record @@ -290,6 +291,7 @@ module ActiveRecord #:nodoc: extend Translation extend DynamicMatchers extend Explain + extend Delegation::DelegateCache include Persistence include ReadonlyAttributes diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index e4c484d64b..128a9377c1 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -23,11 +23,14 @@ module ActiveRecord # Check out <tt>ActiveRecord::Transactions</tt> for more details about <tt>after_commit</tt> and # <tt>after_rollback</tt>. # + # Additionally, an <tt>after_touch</tt> callback is triggered whenever an + # object is touched. + # # Lastly an <tt>after_find</tt> and <tt>after_initialize</tt> callback is triggered for each object that # is found and instantiated by a finder, with <tt>after_initialize</tt> being triggered after new objects # are instantiated as well. # - # That's a total of twelve callbacks, which gives you immense power to react and prepare for each state in the + # There are nineteen callbacks in total, which give you immense power to react and prepare for each state in the # Active Record life cycle. The sequence for calling <tt>Base#save</tt> for an existing record is similar, # except that each <tt>_create</tt> callback is replaced by the corresponding <tt>_update</tt> callback. # 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 32244b1755..e1f29ea03a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -377,7 +377,7 @@ module ActiveRecord def sql_for_insert(sql, pk, id_value, sequence_name, binds) [sql, binds] end - + def last_inserted_id(result) row = result.rows.first row && row.first diff --git a/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb b/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb new file mode 100644 index 0000000000..25c17ce971 --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb @@ -0,0 +1,21 @@ +module ActiveRecord + module ConnectionAdapters + module Savepoints #:nodoc: + def supports_savepoints? + true + end + + def create_savepoint(name = current_savepoint_name) + execute("SAVEPOINT #{name}") + end + + def rollback_to_savepoint(name = current_savepoint_name) + execute("ROLLBACK TO SAVEPOINT #{name}") + end + + def release_savepoint(name = current_savepoint_name) + execute("RELEASE SAVEPOINT #{name}") + end + end + end +end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index ba1cb05d2c..cbe563676b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -33,6 +33,7 @@ module ActiveRecord autoload :Quoting autoload :ConnectionPool autoload :QueryCache + autoload :Savepoints end autoload_at 'active_record/connection_adapters/abstract/transaction' do @@ -97,6 +98,7 @@ module ActiveRecord @pool = pool @schema_cache = SchemaCache.new self @visitor = nil + @prepared_statements = false end def valid_type?(type) @@ -208,10 +210,11 @@ module ActiveRecord end def unprepared_statement - old, @visitor = @visitor, unprepared_visitor + old_prepared_statements, @prepared_statements = @prepared_statements, false + old_visitor, @visitor = @visitor, unprepared_visitor yield ensure - @visitor = old + @visitor, @prepared_statements = old_visitor, old_prepared_statements end # Returns the human-readable name of the adapter. Use mixed case - one @@ -393,13 +396,13 @@ module ActiveRecord @transaction.number end - def create_savepoint + def create_savepoint(name = nil) end - def rollback_to_savepoint + def rollback_to_savepoint(name = nil) end - def release_savepoint + def release_savepoint(name = nil) end def case_sensitive_modifier(node) @@ -421,13 +424,14 @@ module ActiveRecord protected - def log(sql, name = "SQL", binds = []) + def log(sql, name = "SQL", binds = [], statement_name = nil) @instrumenter.instrument( "sql.active_record", - :sql => sql, - :name => name, - :connection_id => object_id, - :binds => binds) { yield } + :sql => sql, + :name => name, + :connection_id => object_id, + :statement_name => statement_name, + :binds => binds) { yield } rescue => e message = "#{e.class.name}: #{e.message}: #{sql}" @logger.error message if @logger @@ -440,6 +444,10 @@ module ActiveRecord # override in derived class ActiveRecord::StatementInvalid.new(message, exception) end + + def without_prepared_statement?(binds) + !@prepared_statements || binds.empty? + end end end end 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 4b11ea795c..138ab811dc 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -3,6 +3,8 @@ require 'arel/visitors/bind_visitor' module ActiveRecord module ConnectionAdapters class AbstractMysqlAdapter < AbstractAdapter + include Savepoints + class SchemaCreation < AbstractAdapter::SchemaCreation def visit_AddColumn(o) @@ -174,6 +176,7 @@ module ActiveRecord @quoted_column_names, @quoted_table_names = {}, {} if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) + @prepared_statements = true @visitor = Arel::Visitors::MySQL.new self else @visitor = unprepared_visitor @@ -193,11 +196,6 @@ module ActiveRecord true end - # Returns true, since this connection adapter supports savepoints. - def supports_savepoints? - true - end - def supports_bulk_alter? #:nodoc: true end @@ -339,18 +337,6 @@ module ActiveRecord # Transactions aren't supported end - def create_savepoint - execute("SAVEPOINT #{current_savepoint_name}") - end - - def rollback_to_savepoint - execute("ROLLBACK TO SAVEPOINT #{current_savepoint_name}") - end - - def release_savepoint - execute("RELEASE SAVEPOINT #{current_savepoint_name}") - end - # In the simple case, MySQL allows us to place JOINs directly into the UPDATE # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support # these, we must use a subquery. diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index fb53090edc..f2fbd5a8f2 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -13,7 +13,7 @@ module ActiveRecord ISO_DATETIME = /\A(\d{4})-(\d\d)-(\d\d) (\d\d):(\d\d):(\d\d)(\.\d+)?\z/ end - attr_reader :name, :default, :type, :limit, :null, :sql_type, :precision, :scale + attr_reader :name, :default, :type, :limit, :null, :sql_type, :precision, :scale, :default_function attr_accessor :primary, :coder alias :encoded? :coder @@ -27,16 +27,17 @@ module ActiveRecord # It will be mapped to one of the standard Rails SQL types in the <tt>type</tt> attribute. # +null+ determines if this column allows +NULL+ values. def initialize(name, default, sql_type = nil, null = true) - @name = name - @sql_type = sql_type - @null = null - @limit = extract_limit(sql_type) - @precision = extract_precision(sql_type) - @scale = extract_scale(sql_type) - @type = simplified_type(sql_type) - @default = extract_default(default) - @primary = nil - @coder = nil + @name = name + @sql_type = sql_type + @null = null + @limit = extract_limit(sql_type) + @precision = extract_precision(sql_type) + @scale = extract_scale(sql_type) + @type = simplified_type(sql_type) + @default = extract_default(default) + @default_function = nil + @primary = nil + @coder = nil end # Returns +true+ if the column is either of type string or text. @@ -203,11 +204,19 @@ module ActiveRecord end end - def new_time(year, mon, mday, hour, min, sec, microsec) + def new_time(year, mon, mday, hour, min, sec, microsec, offset = nil) # Treat 0000-00-00 00:00:00 as nil. return nil if year.nil? || (year == 0 && mon == 0 && mday == 0) - Time.send(Base.default_timezone, year, mon, mday, hour, min, sec, microsec) rescue nil + if offset + time = Time.utc(year, mon, mday, hour, min, sec, microsec) rescue nil + return nil unless time + + time -= offset + Base.default_timezone == :utc ? time : time.getlocal + else + Time.public_send(Base.default_timezone, year, mon, mday, hour, min, sec, microsec) rescue nil + end end def fast_string_to_date(string) @@ -232,7 +241,7 @@ module ActiveRecord time_hash = Date._parse(string) time_hash[:sec_fraction] = microseconds(time_hash) - new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction)) + new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction, :offset)) end end diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 8bad7d0cf5..64fc9e95d8 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -55,7 +55,7 @@ module ActiveRecord begin require path_to_adapter rescue Gem::LoadError => e - raise Gem::LoadError, "Specified '#{spec[:adapter]}' for database adapter, but the gem is not loaded. Add `gem '#{e.name}'` to your Gemfile." + raise Gem::LoadError, "Specified '#{spec[:adapter]}' for database adapter, but the gem is not loaded. Add `gem '#{e.name}'` to your Gemfile (and ensure its version is at the minimum required by ActiveRecord)." rescue LoadError => e raise LoadError, "Could not load '#{path_to_adapter}'. Make sure that the adapter in config/database.yml is valid. If you use an adapter other than 'mysql', 'mysql2', 'postgresql' or 'sqlite3' add the necessary adapter gem to the Gemfile.", e.backtrace end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 92796c996e..e790f731ea 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -229,7 +229,7 @@ module ActiveRecord alias exec_without_stmt exec_query - # Returns an ActiveRecord::Result instance. + # Returns an ActiveRecord::Result instance. def select(sql, name = nil, binds = []) exec_query(sql, name) end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 15b5452850..41a47183e0 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -279,11 +279,7 @@ module ActiveRecord end def exec_query(sql, name = 'SQL', binds = []) - # If the configuration sets prepared_statements:false, binds will - # always be empty, since the bind variables will have been already - # substituted and removed from binds by BindVisitor, so this will - # effectively disable prepared statement usage completely. - if binds.empty? + if without_prepared_statement?(binds) result_set, affected_rows = exec_without_stmt(sql, name) else result_set, affected_rows = exec_stmt(sql, name, binds) 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 527d13b9b0..fa173d13a2 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -134,35 +134,31 @@ module ActiveRecord end def exec_query(sql, name = 'SQL', binds = []) - log(sql, name, binds) do - result = binds.empty? ? exec_no_cache(sql, binds) : - exec_cache(sql, binds) - - types = {} - fields = result.fields - fields.each_with_index do |fname, i| - ftype = result.ftype i - fmod = result.fmod i - types[fname] = OID::TYPE_MAP.fetch(ftype, fmod) { |oid, mod| - warn "unknown OID: #{fname}(#{oid}) (#{sql})" - OID::Identity.new - } - end - - ret = ActiveRecord::Result.new(fields, result.values, types) - result.clear - return ret + result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : + exec_cache(sql, name, binds) + + types = {} + fields = result.fields + fields.each_with_index do |fname, i| + ftype = result.ftype i + fmod = result.fmod i + types[fname] = OID::TYPE_MAP.fetch(ftype, fmod) { |oid, mod| + warn "unknown OID: #{fname}(#{oid}) (#{sql})" + OID::Identity.new + } end + + ret = ActiveRecord::Result.new(fields, result.values, types) + result.clear + return ret end def exec_delete(sql, name = 'SQL', binds = []) - log(sql, name, binds) do - result = binds.empty? ? exec_no_cache(sql, binds) : - exec_cache(sql, binds) - affected = result.cmd_tuples - result.clear - affected - end + result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : + exec_cache(sql, name, binds) + affected = result.cmd_tuples + result.clear + affected end alias :exec_update :exec_delete @@ -218,18 +214,6 @@ module ActiveRecord def rollback_db_transaction execute "ROLLBACK" end - - def create_savepoint - execute("SAVEPOINT #{current_savepoint_name}") - end - - def rollback_to_savepoint - execute("ROLLBACK TO SAVEPOINT #{current_savepoint_name}") - end - - def release_savepoint - execute("RELEASE SAVEPOINT #{current_savepoint_name}") - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb index dab876af14..6c5792954f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb @@ -234,6 +234,10 @@ module ActiveRecord ConnectionAdapters::PostgreSQLColumn.string_to_hstore value end + + def accessor + ActiveRecord::Store::StringKeyedHashAccessor + end end class Cidr < Type @@ -245,11 +249,19 @@ module ActiveRecord end class Json < Type + def type_cast_for_write(value) + ConnectionAdapters::PostgreSQLColumn.json_to_string value + end + def type_cast(value) return if value.nil? ConnectionAdapters::PostgreSQLColumn.string_to_json value end + + def accessor + ActiveRecord::Store::StringKeyedHashAccessor + end end class TypeMap diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 5b9453a579..3668aecd4b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -49,13 +49,17 @@ module ActiveRecord # Instantiates a new PostgreSQL column definition in a table. def initialize(name, default, oid_type, sql_type = nil, null = true) @oid_type = oid_type + default_value = self.class.extract_value_from_default(default) + if sql_type =~ /\[\]$/ @array = true - super(name, self.class.extract_value_from_default(default), sql_type[0..sql_type.length - 3], null) + super(name, default_value, sql_type[0..sql_type.length - 3], null) else @array = false - super(name, self.class.extract_value_from_default(default), sql_type, null) + super(name, default_value, sql_type, null) end + + @default_function = default if has_default_function?(default_value, default) end # :stopdoc: @@ -144,8 +148,16 @@ module ActiveRecord @oid_type.type_cast value end + def accessor + @oid_type.accessor + end + private + def has_default_function?(default_value, default) + !default_value && (%r{\w+\(.*\)} === default) + end + def extract_limit(sql_type) case sql_type when /^bigint/i; 8 @@ -428,6 +440,7 @@ module ActiveRecord include ReferentialIntegrity include SchemaStatements include DatabaseStatements + include Savepoints # Returns 'PostgreSQL' as adapter name for identification purposes. def adapter_name @@ -439,6 +452,7 @@ module ActiveRecord def prepare_column_options(column, types) spec = super spec[:array] = 'true' if column.respond_to?(:array) && column.array + spec[:default] = "\"#{column.default_function}\"" if column.default_function spec end @@ -531,6 +545,7 @@ module ActiveRecord super(connection, logger) if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) + @prepared_statements = true @visitor = Arel::Visitors::PostgreSQL.new self else @visitor = unprepared_visitor @@ -616,11 +631,6 @@ module ActiveRecord true end - # Returns true, since this connection adapter supports savepoints. - def supports_savepoints? - true - end - # Returns true. def supports_explain? true @@ -767,27 +777,29 @@ module ActiveRecord FEATURE_NOT_SUPPORTED = "0A000" # :nodoc: - def exec_no_cache(sql, binds) - @connection.async_exec(sql) + def exec_no_cache(sql, name, binds) + log(sql, name, binds) { @connection.async_exec(sql) } end - def exec_cache(sql, binds) + def exec_cache(sql, name, binds) stmt_key = prepare_statement(sql) - # Clear the queue - @connection.get_last_result - @connection.send_query_prepared(stmt_key, binds.map { |col, val| - type_cast(val, col) - }) - @connection.block - @connection.get_last_result - rescue PGError => e + log(sql, name, binds, stmt_key) do + @connection.send_query_prepared(stmt_key, binds.map { |col, val| + type_cast(val, col) + }) + @connection.block + @connection.get_last_result + end + rescue ActiveRecord::StatementInvalid => e + pgerror = e.original_exception + # Get the PG code for the failure. Annoyingly, the code for # prepared statements whose return value may have changed is # FEATURE_NOT_SUPPORTED. Check here for more details: # http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/cache/plancache.c#l573 begin - code = e.result.result_error_field(PGresult::PG_DIAG_SQLSTATE) + code = pgerror.result.result_error_field(PGresult::PG_DIAG_SQLSTATE) rescue raise e end @@ -812,6 +824,8 @@ module ActiveRecord unless @statements.key? sql_key nextkey = @statements.next_key @connection.prepare nextkey, sql + # Clear the queue + @connection.get_last_result @statements[sql_key] = nextkey end @statements[sql_key] diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index df489a5b1f..e5ad08b6b0 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -53,6 +53,23 @@ module ActiveRecord # # * <tt>:database</tt> - Path to the database file. class SQLite3Adapter < AbstractAdapter + include Savepoints + + NATIVE_DATABASE_TYPES = { + primary_key: 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL', + string: { name: "varchar", limit: 255 }, + text: { name: "text" }, + integer: { name: "integer" }, + float: { name: "float" }, + decimal: { name: "decimal" }, + datetime: { name: "datetime" }, + timestamp: { name: "datetime" }, + time: { name: "time" }, + date: { name: "date" }, + binary: { name: "blob" }, + boolean: { name: "boolean" } + } + class Version include Comparable @@ -113,6 +130,7 @@ module ActiveRecord @config = config if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) + @prepared_statements = true @visitor = Arel::Visitors::SQLite.new self else @visitor = unprepared_visitor @@ -180,11 +198,6 @@ module ActiveRecord true end - # Returns true - def supports_autoincrement? #:nodoc: - true - end - def supports_index_sort_order? true end @@ -197,20 +210,7 @@ module ActiveRecord end def native_database_types #:nodoc: - { - :primary_key => default_primary_key_type, - :string => { :name => "varchar", :limit => 255 }, - :text => { :name => "text" }, - :integer => { :name => "integer" }, - :float => { :name => "float" }, - :decimal => { :name => "decimal" }, - :datetime => { :name => "datetime" }, - :timestamp => { :name => "datetime" }, - :time => { :name => "time" }, - :date => { :name => "date" }, - :binary => { :name => "blob" }, - :boolean => { :name => "boolean" } - } + NATIVE_DATABASE_TYPES end # Returns the current database encoding format as a string, eg: 'UTF-8' @@ -293,8 +293,8 @@ module ActiveRecord def exec_query(sql, name = nil, binds = []) log(sql, name, binds) do - # Don't cache statements without bind values - if binds.empty? + # Don't cache statements if they are not prepared + if without_prepared_statement?(binds) stmt = @connection.prepare(sql) cols = stmt.columns records = stmt.to_a @@ -350,18 +350,6 @@ module ActiveRecord exec_query(sql, name).rows end - def create_savepoint - execute("SAVEPOINT #{current_savepoint_name}") - end - - def rollback_to_savepoint - execute("ROLLBACK TO SAVEPOINT #{current_savepoint_name}") - end - - def release_savepoint - execute("RELEASE SAVEPOINT #{current_savepoint_name}") - end - def begin_db_transaction #:nodoc: log('begin transaction',nil) { @connection.transaction } end @@ -605,14 +593,6 @@ module ActiveRecord @sqlite_version ||= SQLite3Adapter::Version.new(select_value('select sqlite_version(*)')) end - def default_primary_key_type - if supports_autoincrement? - 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL' - else - 'INTEGER PRIMARY KEY NOT NULL' - end - end - def translate_exception(exception, message) case exception.message when /column(s)? .* (is|are) not unique/ diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 2b1e997ef4..366ebde418 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -163,19 +163,22 @@ module ActiveRecord # ==== Example: # # Instantiates a single new object # User.new(first_name: 'Jamie') - def initialize(attributes = nil) + def initialize(attributes = nil, options = {}) defaults = self.class.column_defaults.dup defaults.each { |k, v| defaults[k] = v.dup if v.duplicable? } @attributes = self.class.initialize_attributes(defaults) - @columns_hash = self.class.column_types.dup + @column_types_override = nil + @column_types = self.class.column_types init_internals init_changed_attributes ensure_proper_type populate_with_current_scope_attributes - assign_attributes(attributes) if attributes + # +options+ argument is only needed to make protected_attributes gem easier to hook. + # Remove it when we drop support to this gem. + init_attributes(attributes, options) if attributes yield self if block_given? run_callbacks :initialize unless _initialize_callbacks.empty? @@ -193,7 +196,8 @@ module ActiveRecord # post.title # => 'hello world' def init_with(coder) @attributes = self.class.initialize_attributes(coder['attributes']) - @columns_hash = self.class.column_types.merge(coder['column_types'] || {}) + @column_types_override = coder['column_types'] + @column_types = self.class.column_types init_internals @@ -282,7 +286,7 @@ module ActiveRecord def ==(comparison_object) super || comparison_object.instance_of?(self.class) && - id.present? && + id && comparison_object.id == id end alias :eql? :== @@ -306,13 +310,6 @@ module ActiveRecord @attributes.frozen? end - # Allows sort on objects - def <=>(other_object) - if other_object.is_a?(self.class) - self.to_key <=> other_object.to_key - end - end - # Returns +true+ if the record is read only. Records loaded through joins with piggy-back # attributes will be marked as read only since they cannot be saved. def readonly? @@ -416,8 +413,6 @@ module ActiveRecord @aggregation_cache = {} @association_cache = {} @attributes_cache = {} - @previously_changed = {} - @changed_attributes = {} @readonly = false @destroyed = false @marked_for_destruction = false @@ -434,8 +429,14 @@ module ActiveRecord # optimistic locking) won't get written unless they get marked as changed self.class.columns.each do |c| attr, orig_value = c.name, c.default - @changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr]) + changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr]) end end + + # This method is needed to make protected_attributes gem easier to hook. + # Remove it when we drop support to this gem. + def init_attributes(attributes, options) + assign_attributes(attributes) + end end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 40263b2a70..3bb3131bd1 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -436,9 +436,47 @@ module ActiveRecord cattr_accessor :all_loaded_fixtures self.all_loaded_fixtures = {} + class ClassCache + def initialize(class_names, config) + @class_names = class_names.stringify_keys + @config = config + + # Remove string values that aren't constants or subclasses of AR + @class_names.delete_if { |k,klass| + unless klass.is_a? Class + klass = klass.safe_constantize + ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name will be removed in Rails 4.2, consider using the class itself instead.") + end + !insert_class(@class_names, k, klass) + } + end + + def [](fs_name) + @class_names.fetch(fs_name) { + klass = default_fixture_model(fs_name, @config).safe_constantize + insert_class(@class_names, fs_name, klass) + } + end + + private + + def insert_class(class_names, name, klass) + # We only want to deal with AR objects. + if klass && klass < ActiveRecord::Base + class_names[name] = klass + else + class_names[name] = nil + end + end + + def default_fixture_model(fs_name, config) + ActiveRecord::FixtureSet.default_fixture_model_name(fs_name, config) + end + end + def self.create_fixtures(fixtures_directory, fixture_set_names, class_names = {}, config = ActiveRecord::Base) fixture_set_names = Array(fixture_set_names).map(&:to_s) - class_names = class_names.stringify_keys + class_names = ClassCache.new class_names, config # FIXME: Apparently JK uses this. connection = block_given? ? yield : ActiveRecord::Base.connection @@ -452,10 +490,12 @@ module ActiveRecord fixtures_map = {} fixture_sets = files_to_read.map do |fs_name| + klass = class_names[fs_name] + conn = klass ? klass.connection : connection fixtures_map[fs_name] = new( # ActiveRecord::FixtureSet.new - connection, + conn, fs_name, - class_names[fs_name] || (default_fixture_model_name(fs_name, config).safe_constantize), + klass, ::File.join(fixtures_directory, fs_name)) end @@ -500,10 +540,10 @@ module ActiveRecord attr_reader :table_name, :name, :fixtures, :model_class, :config def initialize(connection, name, class_name, path, config = ActiveRecord::Base) - @fixtures = {} # Ordered hash @name = name @path = path @config = config + @model_class = nil if class_name.is_a?(String) ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name will be removed in Rails 4.2, consider using the class itself instead.") @@ -515,14 +555,13 @@ module ActiveRecord @model_class = class_name.safe_constantize if class_name end - @connection = ( model_class.respond_to?(:connection) ? - model_class.connection : connection ) + @connection = connection @table_name = ( model_class.respond_to?(:table_name) ? model_class.table_name : self.class.default_fixture_table_name(name, config) ) - read_fixture_files + @fixtures = read_fixture_files path, @model_class end def [](x) @@ -556,7 +595,7 @@ module ActiveRecord rows[table_name] = fixtures.map do |label, fixture| row = fixture.to_hash - if model_class && model_class < ActiveRecord::Base + if model_class # fill in timestamp columns if they aren't specified and the model is set to record_timestamps if model_class.record_timestamps timestamp_column_names.each do |c_name| @@ -596,8 +635,10 @@ module ActiveRecord row[fk_name] = ActiveRecord::FixtureSet.identify(value) end - when :has_and_belongs_to_many - handle_habtm(rows, row, association) + when :has_many + if association.options[:through] + add_join_records(rows, row, HasManyThroughProxy.new(association)) + end end end end @@ -607,18 +648,46 @@ module ActiveRecord rows end + class ReflectionProxy # :nodoc: + def initialize(association) + @association = association + end + + def join_table + @association.join_table + end + + def name + @association.name + end + end + + class HasManyThroughProxy < ReflectionProxy # :nodoc: + def rhs_key + @association.foreign_key + end + + def lhs_key + @association.through_reflection.foreign_key + end + end + private def primary_key_name @primary_key_name ||= model_class && model_class.primary_key end - def handle_habtm(rows, row, association) + def add_join_records(rows, row, association) + # This is the case when the join table has no fixtures file if (targets = row.delete(association.name.to_s)) - targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/) table_name = association.join_table + lhs_key = association.lhs_key + rhs_key = association.rhs_key + + targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/) rows[table_name].concat targets.map { |target| - { association.foreign_key => row[primary_key_name], - association.association_foreign_key => ActiveRecord::FixtureSet.identify(target) } + { lhs_key => row[primary_key_name], + rhs_key => ActiveRecord::FixtureSet.identify(target) } } end end @@ -641,12 +710,12 @@ module ActiveRecord @column_names ||= @connection.columns(@table_name).collect { |c| c.name } end - def read_fixture_files - yaml_files = Dir["#{@path}/{**,*}/*.yml"].select { |f| + def read_fixture_files(path, model_class) + yaml_files = Dir["#{path}/{**,*}/*.yml"].select { |f| ::File.file?(f) - } + [yaml_file_path] + } + [yaml_file_path(path)] - yaml_files.each do |file| + yaml_files.each_with_object({}) do |file, fixtures| FixtureSet::File.open(file) do |fh| fh.each do |fixture_name, row| fixtures[fixture_name] = ActiveRecord::Fixture.new(row, model_class) @@ -655,8 +724,8 @@ module ActiveRecord end end - def yaml_file_path - "#{@path}.yml" + def yaml_file_path(path) + "#{path}.yml" end end diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index e826762def..7e1e120288 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -160,7 +160,7 @@ module ActiveRecord end def type_condition(table = arel_table) - sti_column = table[inheritance_column.to_sym] + sti_column = table[inheritance_column] sti_names = ([self] + descendants).map { |model| model.sti_name } sti_column.in(sti_names) diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 626fe40103..55776a91c0 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -150,6 +150,7 @@ module ActiveRecord # Quote the column name used for optimistic locking. def quoted_locking_column + ActiveSupport::Deprecation.warn "ActiveRecord::Base.quoted_locking_column is deprecated and will be removed in Rails 4.2 or later." connection.quote_column_name(locking_column) end diff --git a/activerecord/lib/active_record/locking/pessimistic.rb b/activerecord/lib/active_record/locking/pessimistic.rb index ddf2afca0c..ff7102d35b 100644 --- a/activerecord/lib/active_record/locking/pessimistic.rb +++ b/activerecord/lib/active_record/locking/pessimistic.rb @@ -3,12 +3,12 @@ module ActiveRecord # Locking::Pessimistic provides support for row-level locking using # SELECT ... FOR UPDATE and other lock types. # - # Pass <tt>lock: true</tt> to <tt>ActiveRecord::Base.find</tt> to obtain an exclusive + # Chain <tt>ActiveRecord::Base#find</tt> to <tt>ActiveRecord::QueryMethods#lock</tt> to obtain an exclusive # lock on the selected rows: # # select * from accounts where id=1 for update - # Account.find(1, lock: true) + # Account.lock.find(1) # - # Pass <tt>lock: 'some locking clause'</tt> to give a database-specific locking clause + # Call <tt>lock('some locking clause')</tt> to use a database-specific locking clause # of your own such as 'LOCK IN SHARE MODE' or 'FOR UPDATE NOWAIT'. Example: # # Account.transaction do diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 0358a36b14..927fbab8f0 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -17,7 +17,7 @@ module ActiveRecord def initialize super - @odd_or_even = false + @odd = false end def render_bind(column, value) @@ -60,17 +60,8 @@ module ActiveRecord debug " #{name} #{sql}#{binds}" end - def identity(event) - return unless logger.debug? - - name = color(event.payload[:name], odd? ? CYAN : MAGENTA, true) - line = odd? ? color(event.payload[:line], nil, true) : event.payload[:line] - - debug " #{name} #{line}" - end - def odd? - @odd_or_even = !@odd_or_even + @odd = !@odd end def logger diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index a1ad4f6255..27d398ad07 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -120,8 +120,8 @@ module ActiveRecord # a column but keeps the type and content. # * <tt>change_column(table_name, column_name, type, options)</tt>: Changes # the column to a different type using the same parameters as add_column. - # * <tt>remove_column(table_name, column_names)</tt>: Removes the column listed in - # +column_names+ from the table called +table_name+. + # * <tt>remove_column(table_name, column_name, type, options)</tt>: Removes the column + # named +column_name+ from the table called +table_name+. # * <tt>add_index(table_name, column_names, options)</tt>: Adds a new index # with the name of the column. Other options include # <tt>:name</tt>, <tt>:unique</tt> (e.g. diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 75c0c1bda8..dc5ff02882 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -34,6 +34,12 @@ module ActiveRecord ## # :singleton-method: + # Accessor for the name of the schema migrations table. By default, the value is "schema_migrations" + class_attribute :schema_migrations_table_name, instance_accessor: false + self.schema_migrations_table_name = "schema_migrations" + + ## + # :singleton-method: # Indicates whether table names should be the pluralized versions of the corresponding class names. # If true, the default table name for a Product class will be +products+. If false, it would just be +product+. # See table_name for the full rules on table/class naming. This is true, by default. diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index d166f0dd66..080b20134d 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -6,7 +6,7 @@ module ActiveRecord @records = [] end - def pluck(_column_name) + def pluck(*column_names) [] end @@ -42,10 +42,6 @@ module ActiveRecord "" end - def where_values_hash - {} - end - def count(*) 0 end @@ -54,7 +50,7 @@ module ActiveRecord 0 end - def calculate(_operation, _column_name, _options = {}) + def calculate(_operation, _column_name) if _operation == :count 0 else diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 582006ea7d..a73a140ef1 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -335,8 +335,18 @@ module ActiveRecord # Reloads the record from the database. # - # This method modifies the receiver in-place. Attributes are updated, and - # caches busted, in particular the associations cache. + # This method finds record by its primary key (which could be assigned manually) and + # modifies the receiver in-place: + # + # account = Account.new + # # => #<Account id: nil, email: nil> + # account.id = 1 + # account.reload + # # Account Load (1.2ms) SELECT "accounts".* FROM "accounts" WHERE "accounts"."id" = $1 LIMIT 1 [["id", 1]] + # # => #<Account id: 1, email: 'account@example.com'> + # + # Attributes are reloaded from the database, and caches busted, in + # particular the associations cache. # # If the record no longer exists in the database <tt>ActiveRecord::RecordNotFound</tt> # is raised. Otherwise, in addition to the in-place modification the method @@ -383,14 +393,16 @@ module ActiveRecord end @attributes.update(fresh_object.instance_variable_get('@attributes')) - @columns_hash = fresh_object.instance_variable_get('@columns_hash') - @attributes_cache = {} + @column_types = self.class.column_types + @column_types_override = fresh_object.instance_variable_get('@column_types_override') + @attributes_cache = {} self end # Saves the record with the updated_at/on attributes set to the current time. - # Please note that no validation is performed and no callbacks are executed. + # Please note that no validation is performed and only the +after_touch+ + # callback is executed. # If an attribute name is passed, that attribute is updated along with # updated_at/on attributes. # @@ -433,7 +445,7 @@ module ActiveRecord changes[self.class.locking_column] = increment_lock if locking_enabled? - @changed_attributes.except!(*changes.keys) + changed_attributes.except!(*changes.keys) primary_key = self.class.primary_key self.class.unscoped.where(primary_key => self[primary_key]).update_all(changes) == 1 end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 269f9f975b..eef08aea88 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -46,6 +46,7 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.database_configuration = Rails.application.config.database_configuration ActiveRecord::Tasks::DatabaseTasks.migrations_paths = Rails.application.paths['db/migrate'].to_a ActiveRecord::Tasks::DatabaseTasks.fixtures_path = File.join Rails.root, 'test', 'fixtures' + ActiveRecord::Tasks::DatabaseTasks.root = Rails.root if defined?(ENGINE_PATH) && engine = Rails::Engine.find(ENGINE_PATH) if engine.paths['db/migrate'].existent diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index daccab762f..ecadb95a5d 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -287,6 +287,7 @@ db_namespace = namespace :db do if ActiveRecord::Base.connection.supports_migrations? File.open(filename, "a") do |f| f.puts ActiveRecord::Base.connection.dump_schema_information + f.print "\n" end end db_namespace['structure:dump'].reenable diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 5245fc130a..bce7766501 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -12,8 +12,6 @@ module ActiveRecord def self.create(macro, name, scope, options, ar) case macro - when :has_and_belongs_to_many - klass = AssociationReflection when :has_many, :belongs_to, :has_one klass = options[:through] ? ThroughReflection : AssociationReflection when :composed_of @@ -24,11 +22,11 @@ module ActiveRecord end def self.add_reflection(ar, name, reflection) - if reflection.class == AggregateReflection - ar.aggregate_reflections = ar.aggregate_reflections.merge(name => reflection) - else - ar.reflections = ar.reflections.merge(name => reflection) - end + ar.reflections = ar.reflections.merge(name => reflection) + end + + def self.add_aggregate_reflection(ar, name, reflection) + ar.aggregate_reflections = ar.aggregate_reflections.merge(name => reflection) end # \Reflection enables to interrogate Active Record classes and objects @@ -121,6 +119,7 @@ module ActiveRecord @scope = scope @options = options @active_record = active_record + @klass = options[:class] @plural_name = active_record.pluralize_table_names ? name.to_s.pluralize : name.to_s end @@ -195,10 +194,11 @@ module ActiveRecord def initialize(macro, name, scope, options, active_record) super - @collection = [:has_many, :has_and_belongs_to_many].include?(macro) + @collection = :has_many == macro @automatic_inverse_of = nil @type = options[:as] && "#{options[:as]}_type" @foreign_type = options[:foreign_type] || "#{name}_type" + @constructable = calculate_constructable(macro, options) end # Returns a new, unsaved instance of the associated class. +attributes+ will @@ -207,6 +207,10 @@ module ActiveRecord klass.new(attributes, &block) end + def constructable? # :nodoc: + @constructable + end + def table_name klass.table_name end @@ -250,10 +254,6 @@ module ActiveRecord def check_validity! check_validity_of_inverse! - - if has_and_belongs_to_many? && association_foreign_key == foreign_key - raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(self) - end end def check_validity_of_inverse! @@ -335,10 +335,6 @@ module ActiveRecord macro == :belongs_to end - def has_and_belongs_to_many? - macro == :has_and_belongs_to_many - end - def association_class case macro when :belongs_to @@ -347,8 +343,6 @@ module ActiveRecord else Associations::BelongsToAssociation end - when :has_and_belongs_to_many - Associations::HasAndBelongsToManyAssociation when :has_many if options[:through] Associations::HasManyThroughAssociation @@ -373,11 +367,23 @@ module ActiveRecord protected - def actual_source_reflection # FIXME: this is a horrible name - self - end + def actual_source_reflection # FIXME: this is a horrible name + self + end private + + def calculate_constructable(macro, options) + case macro + when :belongs_to + !options[:polymorphic] + when :has_one + !options[:through] + else + true + end + end + # Attempts to find the inverse association name automatically. # If it cannot find a suitable inverse association name, it returns # nil. @@ -568,7 +574,7 @@ module ActiveRecord # Add to it the scope from this reflection (if any) scope_chain.first << scope if scope - through_scope_chain = through_reflection.scope_chain + through_scope_chain = through_reflection.scope_chain.map(&:dup) if options[:source_type] through_scope_chain.first << @@ -587,7 +593,7 @@ module ActiveRecord # A through association is nested if there would be more than one join table def nested? - chain.length > 2 || through_reflection.has_and_belongs_to_many? + chain.length > 2 end # We want to use the klass from this reflection, rather than just delegate straight to diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4e86e905ed..60f2726a6e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -244,8 +244,7 @@ module ActiveRecord def empty? return @records.empty? if loaded? - c = count(:all) - c.respond_to?(:zero?) ? c.zero? : c.empty? + limit_value == 0 ? true : !exists? end # Returns true if there are any records. @@ -507,8 +506,7 @@ module ActiveRecord visitor = connection.visitor if eager_loading? - join_dependency = construct_join_dependency - relation = construct_relation_for_association_find(join_dependency) + find_with_associations { |rel| relation = rel } end ast = relation.arel.ast @@ -599,8 +597,9 @@ module ActiveRecord preload = preload_values preload += includes_values unless eager_loading? + preloader = ActiveRecord::Associations::Preloader.new preload.each do |associations| - ActiveRecord::Associations::Preloader.new(@records, associations).run + preloader.preload @records, associations end @records.each { |record| record.readonly! } if readonly_value diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index fd8496442e..49b01909c6 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -34,7 +34,7 @@ module ActiveRecord # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond # (by setting the +:start+ option on that worker). # - # # Let's process for a batch of 2000 records, skiping the first 2000 rows + # # Let's process for a batch of 2000 records, skipping the first 2000 rows # Person.find_each(start: 2000, batch_size: 2000) do |person| # person.party_all_night! # end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 52a538e5b5..2d267183ce 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -19,17 +19,16 @@ module ActiveRecord # # Person.group(:city).count # # => { 'Rome' => 5, 'Paris' => 3 } - def count(column_name = nil, options = {}) - column_name, options = nil, column_name if column_name.is_a?(Hash) - calculate(:count, column_name, options) + def count(column_name = nil) + calculate(:count, column_name) end # Calculates the average value on a given column. Returns +nil+ if there's # no row. See +calculate+ for examples with options. # # Person.average(:age) # => 35.8 - def average(column_name, options = {}) - calculate(:average, column_name, options) + def average(column_name) + calculate(:average, column_name) end # Calculates the minimum value on a given column. The value is returned @@ -37,8 +36,8 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.minimum(:age) # => 7 - def minimum(column_name, options = {}) - calculate(:minimum, column_name, options) + def minimum(column_name) + calculate(:minimum, column_name) end # Calculates the maximum value on a given column. The value is returned @@ -46,8 +45,8 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.maximum(:age) # => 93 - def maximum(column_name, options = {}) - calculate(:maximum, column_name, options) + def maximum(column_name) + calculate(:maximum, column_name) end # Calculates the sum of values on a given column. The value is returned @@ -90,15 +89,15 @@ module ActiveRecord # Person.group(:last_name).having("min(age) > 17").minimum(:age) # # Person.sum("2 * age") - def calculate(operation, column_name, options = {}) + def calculate(operation, column_name) if column_name.is_a?(Symbol) && attribute_alias?(column_name) column_name = attribute_alias(column_name) end if has_include?(column_name) - construct_relation_for_association_calculations.calculate(operation, column_name, options) + construct_relation_for_association_calculations.calculate(operation, column_name) else - perform_calculation(operation, column_name, options) + perform_calculation(operation, column_name) end end @@ -161,8 +160,7 @@ module ActiveRecord result = result.map do |attributes| values = klass.initialize_attributes(attributes).values - iter = columns.each - values.map { |value| iter.next.type_cast value } + columns.zip(values).map { |column, value| column.type_cast value } end columns.one? ? result.map!(&:first) : result end @@ -182,7 +180,7 @@ module ActiveRecord eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) end - def perform_calculation(operation, column_name, options = {}) + def perform_calculation(operation, column_name) operation = operation.to_s.downcase # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count) diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index b6f80ac5c7..1e15bddcdf 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,8 +1,34 @@ -require 'thread' -require 'thread_safe' +require 'active_support/concern' +require 'active_support/deprecation' module ActiveRecord module Delegation # :nodoc: + module DelegateCache + def relation_delegate_class(klass) # :nodoc: + @relation_delegate_cache[klass] + end + + def initialize_relation_delegate_cache # :nodoc: + @relation_delegate_cache = cache = {} + [ + ActiveRecord::Relation, + ActiveRecord::Associations::CollectionProxy, + ActiveRecord::AssociationRelation + ].each do |klass| + delegate = Class.new(klass) { + include ClassSpecificRelation + } + const_set klass.name.gsub('::', '_'), delegate + cache[klass] = delegate + end + end + + def inherited(child_class) + child_class.initialize_relation_delegate_cache + super + end + end + extend ActiveSupport::Concern # This module creates compiled delegation methods dynamically at runtime, which makes @@ -58,7 +84,7 @@ module ActiveRecord if @klass.respond_to?(method) self.class.delegate_to_scoped_klass(method) scoping { @klass.send(method, *args, &block) } - elsif Array.method_defined?(method) + elsif array_delegable?(method) self.class.delegate method, :to => :to_a to_a.send(method, *args, &block) elsif arel.respond_to?(method) @@ -71,47 +97,39 @@ module ActiveRecord end module ClassMethods # :nodoc: - @@subclasses = ThreadSafe::Cache.new(:initial_capacity => 2) - def create(klass, *args) relation_class_for(klass).new(klass, *args) end private - # Cache the constants in @@subclasses because looking them up via const_get - # make instantiation significantly slower. + def relation_class_for(klass) - if klass && (klass_name = klass.name) - my_cache = @@subclasses.compute_if_absent(self) { ThreadSafe::Cache.new } - # This hash is keyed by klass.name to avoid memory leaks in development mode - my_cache.compute_if_absent(klass_name) do - # Cache#compute_if_absent guarantees that the block will only executed once for the given klass_name - subclass_name = "#{name.gsub('::', '_')}_#{klass_name.gsub('::', '_')}" - - if const_defined?(subclass_name) - const_get(subclass_name) - else - const_set(subclass_name, Class.new(self) { include ClassSpecificRelation }) - end - end - else - ActiveRecord::Relation - end + klass.relation_delegate_class(self) end end def respond_to?(method, include_private = false) - super || Array.method_defined?(method) || + super || array_delegable?(method) || @klass.respond_to?(method, include_private) || arel.respond_to?(method, include_private) end protected + def array_delegable?(method) + defined = Array.method_defined?(method) + if defined && method.to_s.ends_with?('!') + ActiveSupport::Deprecation.warn( + "Association will no longer delegate #{method} to #to_a as of Rails 4.2. You instead must first call #to_a on the association to expose the array to be acted on." + ) + end + defined + end + def method_missing(method, *args, &block) if @klass.respond_to?(method) scoping { @klass.send(method, *args, &block) } - elsif Array.method_defined?(method) + elsif array_delegable?(method) to_a.send(method, *args, &block) elsif arel.respond_to?(method) arel.send(method, *args, &block) diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 0132a02f83..3a02bf90e9 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -201,7 +201,7 @@ module ActiveRecord conditions = conditions.id if Base === conditions return false if !conditions - relation = construct_relation_for_association_find(construct_join_dependency) + relation = apply_join_dependency(self, construct_join_dependency) return false if ActiveRecord::NullRelation === relation relation = relation.except(:select, :order).select(ONE_AS_ONE).limit(1) @@ -242,17 +242,25 @@ module ActiveRecord def find_with_associations join_dependency = construct_join_dependency - relation = construct_relation_for_association_find(join_dependency) - if ActiveRecord::NullRelation === relation - [] + + aliases = join_dependency.aliases + relation = select aliases.columns + relation = apply_join_dependency(relation, join_dependency) + + if block_given? + yield relation else - rows = connection.select_all(relation.arel, 'SQL', relation.bind_values.dup) - join_dependency.instantiate(rows) + if ActiveRecord::NullRelation === relation + [] + else + rows = connection.select_all(relation.arel, 'SQL', relation.bind_values.dup) + join_dependency.instantiate(rows, aliases) + end end end def construct_join_dependency(joins = []) - including = (eager_load_values + includes_values).uniq + including = eager_load_values + includes_values ActiveRecord::Associations::JoinDependency.new(@klass, including, joins) end @@ -260,14 +268,9 @@ module ActiveRecord apply_join_dependency(self, construct_join_dependency(arel.froms.first)) end - def construct_relation_for_association_find(join_dependency) - relation = except(:select).select(join_dependency.columns) - apply_join_dependency(relation, join_dependency) - end - def apply_join_dependency(relation, join_dependency) relation = relation.except(:includes, :eager_load, :preload) - relation = join_dependency.join_relation(relation) + relation = relation.joins join_dependency if using_limitable_reflections?(join_dependency.reflections) relation @@ -297,6 +300,8 @@ module ActiveRecord protected def find_with_ids(*ids) + raise UnknownPrimaryKey.new(@klass) if primary_key.nil? + expects_array = ids.first.kind_of?(Array) return ids.first if expects_array && ids.first.empty? diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index c08158d38b..182b9ed89c 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -58,7 +58,11 @@ module ActiveRecord def merge normal_values.each do |name| value = values[name] - relation.send("#{name}!", *value) unless value.blank? + # The unless clause is here mostly for performance reasons (since the `send` call might be moderately + # expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that + # `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values + # don't fall through the cracks. + relation.send("#{name}!", *value) unless value.nil? || (value.blank? && false != value) end merge_multi_values @@ -90,7 +94,7 @@ module ActiveRecord []) relation.joins! rest - @relation = join_dependency.join_relation(relation) + @relation = relation.joins join_dependency end end @@ -107,11 +111,11 @@ module ActiveRecord bind_values = filter_binds(lhs_binds, removed) + rhs_binds conn = relation.klass.connection - bviter = bind_values.each.with_index + bv_index = 0 where_values.map! do |node| if Arel::Nodes::Equality === node && Arel::Nodes::BindParam === node.right - (column, _), i = bviter.next - substitute = conn.substitute_at column, i + substitute = conn.substitute_at(bind_values[bv_index].first, bv_index) + bv_index += 1 Arel::Nodes::Equality.new(node.left, substitute) else node diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 9f2a039d94..bffd8b5d0f 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -289,17 +289,7 @@ module ActiveRecord end def order!(*args) # :nodoc: - args.flatten! - validate_order_args(args) - - references = args.grep(String) - references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact! - references!(references) if references.any? - - # if a symbol is given we prepend the quoted table name - args.map! do |arg| - arg.is_a?(Symbol) ? Arel::Nodes::Ascending.new(klass.arel_table[arg]) : arg - end + preprocess_order_args(args) self.order_values += args self @@ -320,8 +310,7 @@ module ActiveRecord end def reorder!(*args) # :nodoc: - args.flatten! - validate_order_args(args) + preprocess_order_args(args) self.reordering_value = true self.order_values = args @@ -867,7 +856,7 @@ module ActiveRecord where_values.reject! do |rel| case rel - when Arel::Nodes::In, Arel::Nodes::Equality + when Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual subrelation = (rel.left.kind_of?(Arel::Attributes::Attribute) ? rel.left : rel.right) subrelation.name.to_sym == target_value_sym else @@ -893,19 +882,25 @@ module ActiveRecord end def collapse_wheres(arel, wheres) - equalities = wheres.grep(Arel::Nodes::Equality) - - arel.where(Arel::Nodes::And.new(equalities)) unless equalities.empty? - - (wheres - equalities).each do |where| + predicates = wheres.map do |where| + next where if ::Arel::Nodes::Equality === where where = Arel.sql(where) if String === where - arel.where(Arel::Nodes::Grouping.new(where)) + Arel::Nodes::Grouping.new(where) end + + arel.where(Arel::Nodes::And.new(predicates)) if predicates.present? end def build_where(opts, other = []) case opts when String, Array + #TODO: Remove duplication with: /activerecord/lib/active_record/sanitization.rb:113 + values = Hash === other.first ? other.first.values : other + + values.grep(ActiveRecord::Relation) do |rel| + self.bind_values += rel.bind_values + end + [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] when Hash opts = PredicateBuilder.resolve_column_aliases(klass, opts) @@ -926,6 +921,7 @@ module ActiveRecord case opts when Relation name ||= 'subquery' + self.bind_values = opts.bind_values + self.bind_values opts.arel.as(name.to_s) else opts @@ -939,7 +935,7 @@ module ActiveRecord :string_join when Hash, Symbol, Array :association_join - when ActiveRecord::Associations::JoinDependency::JoinAssociation + when ActiveRecord::Associations::JoinDependency :stashed_join when Arel::Nodes::Join :join_node @@ -961,10 +957,7 @@ module ActiveRecord join_list ) - join_dependency.graft(*stashed_association_joins) - - joins = join_dependency.join_associations.map!(&:join_constraints) - joins.flatten! + joins = join_dependency.join_constraints stashed_association_joins joins.each { |join| manager.from(join) } @@ -1036,6 +1029,20 @@ module ActiveRecord end end + def preprocess_order_args(order_args) + order_args.flatten! + validate_order_args(order_args) + + references = order_args.grep(String) + references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact! + references!(references) if references.any? + + # if a symbol is given we prepend the quoted table name + order_args.map! do |arg| + arg.is_a?(Symbol) ? Arel::Nodes::Ascending.new(klass.arel_table[arg]) : arg + end + end + # Checks to make sure that the arguments are not blank. Note that if some # blank-like object were initially passed into the query method, then this # method will not raise an error. diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index 253368ae5b..1dc3bf3f12 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -46,6 +46,10 @@ module ActiveRecord IDENTITY_TYPE end + def column_type(name) + @column_types[name] || identity_type + end + def each if block_given? hash_rows.each { |row| yield row } @@ -93,7 +97,21 @@ module ActiveRecord # used as keys in ActiveRecord::Base's @attributes hash columns = @columns.map { |c| c.dup.freeze } @rows.map { |row| - Hash[columns.zip(row)] + # In the past we used Hash[columns.zip(row)] + # though elegant, the verbose way is much more efficient + # both time and memory wise cause it avoids a big array allocation + # this method is called a lot and needs to be micro optimised + hash = {} + + index = 0 + length = columns.length + + while index < length + hash[columns[index]] = row[index] + index += 1 + end + + hash } end end diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 0b87ab9926..cab8fd745a 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -127,7 +127,17 @@ module ActiveRecord raise_if_bind_arity_mismatch(statement, statement.count('?'), values.size) bound = values.dup c = connection - statement.gsub('?') { quote_bound_value(bound.shift, c) } + statement.gsub('?') do + replace_bind_variable(bound.shift, c) + end + end + + def replace_bind_variable(value, c = connection) #:nodoc: + if ActiveRecord::Relation === value + value.to_sql + else + quote_bound_value(value, c) + end end def replace_named_bind_variables(statement, bind_vars) #:nodoc: @@ -135,7 +145,7 @@ module ActiveRecord if $1 == ':' # skip postgresql casts $& # return the whole match elsif bind_vars.include?(match = $2.to_sym) - quote_bound_value(bind_vars[match]) + replace_bind_variable(bind_vars[match]) else raise PreparedStatementInvalid, "missing value for :#{match} in #{statement}" end diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 1181cc739e..e055d571ab 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -17,9 +17,19 @@ module ActiveRecord cattr_accessor :ignore_tables @@ignore_tables = [] - def self.dump(connection=ActiveRecord::Base.connection, stream=STDOUT) - new(connection).dump(stream) - stream + class << self + def dump(connection=ActiveRecord::Base.connection, stream=STDOUT, config = ActiveRecord::Base) + new(connection, generate_options(config)).dump(stream) + stream + end + + private + def generate_options(config) + { + table_name_prefix: config.table_name_prefix, + table_name_suffix: config.table_name_suffix + } + end end def dump(stream) @@ -32,10 +42,11 @@ module ActiveRecord private - def initialize(connection) + def initialize(connection, options = {}) @connection = connection @types = @connection.native_database_types @version = Migrator::current_version rescue nil + @options = options end def header(stream) @@ -112,6 +123,7 @@ HEADER tbl.print %Q(, primary_key: "#{pk}") elsif pkcol.sql_type == 'uuid' tbl.print ", id: :uuid" + tbl.print %Q(, default: "#{pkcol.default_function}") if pkcol.default_function end else tbl.print ", id: false" @@ -201,7 +213,7 @@ HEADER end def remove_prefix_and_suffix(table) - table.gsub(/^(#{ActiveRecord::Base.table_name_prefix})(.+)(#{ActiveRecord::Base.table_name_suffix})$/, "\\2") + table.gsub(/^(#{@options[:table_name_prefix]})(.+)(#{@options[:table_name_suffix]})$/, "\\2") end end end diff --git a/activerecord/lib/active_record/schema_migration.rb b/activerecord/lib/active_record/schema_migration.rb index fee19b1096..a9d164e366 100644 --- a/activerecord/lib/active_record/schema_migration.rb +++ b/activerecord/lib/active_record/schema_migration.rb @@ -7,11 +7,11 @@ module ActiveRecord class << self def table_name - "#{table_name_prefix}schema_migrations#{table_name_suffix}" + "#{table_name_prefix}#{ActiveRecord::Base.schema_migrations_table_name}#{table_name_suffix}" end def index_name - "#{table_name_prefix}unique_schema_migrations#{table_name_suffix}" + "#{table_name_prefix}unique_#{ActiveRecord::Base.schema_migrations_table_name}#{table_name_suffix}" end def table_exists? diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index a5d6aad3f0..01fec31544 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -100,11 +100,7 @@ module ActiveRecord elsif default_scopes.any? evaluate_default_scope do default_scopes.inject(relation) do |default_scope, scope| - if !scope.is_a?(Relation) && scope.respond_to?(:call) - default_scope.merge(unscoped { scope.call }) - else - default_scope.merge(scope) - end + default_scope.merge(unscoped { scope.call }) end end end diff --git a/activerecord/lib/active_record/store.rb b/activerecord/lib/active_record/store.rb index a610f479f2..b841b977fc 100644 --- a/activerecord/lib/active_record/store.rb +++ b/activerecord/lib/active_record/store.rb @@ -86,6 +86,9 @@ module ActiveRecord end end + # assign new store attribute and create new hash to ensure that each class in the hierarchy + # has its own hash of stored attributes. + self.stored_attributes = {} if self.stored_attributes.blank? self.stored_attributes[store_attribute] ||= [] self.stored_attributes[store_attribute] |= keys end @@ -101,26 +104,58 @@ module ActiveRecord protected def read_store_attribute(store_attribute, key) - attribute = initialize_store_attribute(store_attribute) - attribute[key] + accessor = store_accessor_for(store_attribute) + accessor.read(self, store_attribute, key) end def write_store_attribute(store_attribute, key, value) - attribute = initialize_store_attribute(store_attribute) - if value != attribute[key] - send :"#{store_attribute}_will_change!" - attribute[key] = value - end + accessor = store_accessor_for(store_attribute) + accessor.write(self, store_attribute, key, value) end private - def initialize_store_attribute(store_attribute) - attribute = send(store_attribute) - unless attribute.is_a?(ActiveSupport::HashWithIndifferentAccess) - attribute = IndifferentCoder.as_indifferent_hash(attribute) - send :"#{store_attribute}=", attribute + def store_accessor_for(store_attribute) + @column_types[store_attribute.to_s].accessor + end + + class HashAccessor + def self.read(object, attribute, key) + prepare(object, attribute) + object.public_send(attribute)[key] + end + + def self.write(object, attribute, key, value) + prepare(object, attribute) + if value != read(object, attribute, key) + object.public_send :"#{attribute}_will_change!" + object.public_send(attribute)[key] = value + end + end + + def self.prepare(object, attribute) + object.public_send :"#{attribute}=", {} unless object.send(attribute) + end + end + + class StringKeyedHashAccessor < HashAccessor + def self.read(object, attribute, key) + super object, attribute, key.to_s + end + + def self.write(object, attribute, key, value) + super object, attribute, key.to_s, value + end + end + + class IndifferentHashAccessor < ActiveRecord::Store::HashAccessor + def self.prepare(object, store_attribute) + attribute = object.send(store_attribute) + unless attribute.is_a?(ActiveSupport::HashWithIndifferentAccess) + attribute = IndifferentCoder.as_indifferent_hash(attribute) + object.send :"#{store_attribute}=", attribute + end + attribute end - attribute end class IndifferentCoder # :nodoc: @@ -138,7 +173,7 @@ module ActiveRecord end def load(yaml) - self.class.as_indifferent_hash @coder.load(yaml) + self.class.as_indifferent_hash(@coder.load(yaml)) end def self.as_indifferent_hash(obj) diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 5ff594fdca..b91bbeb412 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -23,6 +23,7 @@ module ActiveRecord # * +fixtures_path+: a path to fixtures directory. # * +migrations_paths+: a list of paths to directories with migrations. # * +seed_loader+: an object which will load seeds, it needs to respond to the +load_seed+ method. + # * +root+: a path to the root of the application. # # Example usage of +DatabaseTasks+ outside Rails could look as such: # @@ -37,7 +38,7 @@ module ActiveRecord attr_writer :current_config attr_accessor :database_configuration, :migrations_paths, :seed_loader, :db_dir, - :fixtures_path, :env + :fixtures_path, :env, :root LOCAL_HOSTS = ['127.0.0.1', 'localhost'] diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 50569d2462..c755831e6d 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -134,8 +134,9 @@ IDENTIFIED BY '#{configuration['password']}' WITH GRANT OPTION; args << "--password=#{configuration['password']}" if configuration['password'] args.concat(['--default-character-set', configuration['encoding']]) if configuration['encoding'] configuration.slice('host', 'port', 'socket').each do |k, v| - args.concat([ "--#{k}", v ]) if v + args.concat([ "--#{k}", v.to_s ]) if v end + args end end diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index 4413330fab..3d02ee07d0 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -59,7 +59,7 @@ module ActiveRecord def structure_load(filename) set_psql_env - Kernel.system("psql -q -f #{filename} #{configuration['database']}") + Kernel.system("psql -q -f #{Shellwords.escape(filename)} #{configuration['database']}") end private diff --git a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb index de8b16627e..5688931db2 100644 --- a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb @@ -3,7 +3,7 @@ module ActiveRecord class SQLiteDatabaseTasks # :nodoc: delegate :connection, :establish_connection, to: ActiveRecord::Base - def initialize(configuration, root = Rails.root) + def initialize(configuration, root = ActiveRecord::Tasks::DatabaseTasks.root) @configuration, @root = configuration, root end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index dcbf38a89f..45313b5e75 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -220,8 +220,8 @@ module ActiveRecord # after_commit :do_bar, on: :update # after_commit :do_baz, on: :destroy # - # after_commit :do_foo_bar, :on [:create, :update] - # after_commit :do_bar_baz, :on [:update, :destroy] + # after_commit :do_foo_bar, on: [:create, :update] + # after_commit :do_bar_baz, on: [:update, :destroy] # # Note that transactional fixtures do not play well with this feature. Please # use the +test_after_commit+ gem to have these hooks fired in tests. diff --git a/activerecord/lib/rails/generators/active_record.rb b/activerecord/lib/rails/generators/active_record.rb index c8aa37f275..dc29213235 100644 --- a/activerecord/lib/rails/generators/active_record.rb +++ b/activerecord/lib/rails/generators/active_record.rb @@ -1,23 +1,17 @@ require 'rails/generators/named_base' -require 'rails/generators/migration' require 'rails/generators/active_model' +require 'rails/generators/active_record/migration' require 'active_record' module ActiveRecord module Generators # :nodoc: class Base < Rails::Generators::NamedBase # :nodoc: - include Rails::Generators::Migration + include ActiveRecord::Generators::Migration # Set the current directory as base for the inherited generators. def self.base_root File.dirname(__FILE__) end - - # Implement the required interface for Rails::Generators::Migration. - def self.next_migration_number(dirname) - next_migration_number = current_migration_number(dirname) + 1 - ActiveRecord::Migration.next_migration_number(next_migration_number) - end end end end diff --git a/activerecord/lib/rails/generators/active_record/migration.rb b/activerecord/lib/rails/generators/active_record/migration.rb new file mode 100644 index 0000000000..b7418cf42f --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/migration.rb @@ -0,0 +1,18 @@ +require 'rails/generators/migration' + +module ActiveRecord + module Generators # :nodoc: + module Migration + extend ActiveSupport::Concern + include Rails::Generators::Migration + + module ClassMethods + # Implement the required interface for Rails::Generators::Migration. + def next_migration_number(dirname) + next_migration_number = current_migration_number(dirname) + 1 + ActiveRecord::Migration.next_migration_number(next_migration_number) + end + end + end + end +end diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index fedd9f603c..679c515e8c 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -3,14 +3,14 @@ require "cases/helper" class MysqlConnectionTest < ActiveRecord::TestCase def setup super + @subscriber = SQLSubscriber.new + ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) @connection = ActiveRecord::Base.connection - @connection.extend(LogIntercepter) - @connection.intercepted = true end def teardown - @connection.intercepted = false - @connection.logged = [] + ActiveSupport::Notifications.unsubscribe(@subscriber) + super end def test_no_automatic_reconnection_after_timeout @@ -72,14 +72,14 @@ class MysqlConnectionTest < ActiveRecord::TestCase def test_logs_name_show_variable @connection.show_variable 'foo' - assert_equal "SCHEMA", @connection.logged[0][1] + assert_equal "SCHEMA", @subscriber.logged[0][1] end def test_logs_name_rename_column_sql @connection.execute "CREATE TABLE `bar_baz` (`foo` varchar(255))" - @connection.logged = [] + @subscriber.logged.clear @connection.send(:rename_column_sql, 'bar_baz', 'foo', 'foo2') - assert_equal "SCHEMA", @connection.logged[0][1] + assert_equal "SCHEMA", @subscriber.logged[0][1] ensure @connection.execute "DROP TABLE `bar_baz`" end diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index 6b726ce875..81aa977c59 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -7,14 +7,14 @@ module ActiveRecord def setup super + @subscriber = SQLSubscriber.new + ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) @connection = ActiveRecord::Base.connection - @connection.extend(LogIntercepter) - @connection.intercepted = true end def teardown - @connection.intercepted = false - @connection.logged = [] + ActiveSupport::Notifications.unsubscribe(@subscriber) + super end def test_encoding @@ -47,38 +47,48 @@ module ActiveRecord def test_tables_logs_name @connection.tables('hello') - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_indexes_logs_name @connection.indexes('items', 'hello') - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_table_exists_logs_name @connection.table_exists?('items') - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_table_alias_length_logs_name @connection.instance_variable_set("@table_alias_length", nil) @connection.table_alias_length - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_current_database_logs_name @connection.current_database - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_encoding_logs_name @connection.encoding - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_schema_names_logs_name @connection.schema_names - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] + end + + def test_statement_key_is_logged + bindval = 1 + @connection.exec_query('SELECT $1::integer', 'SQL', [[nil, bindval]]) + name = @subscriber.payloads.last[:statement_name] + assert name + res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(#{bindval})") + plan = res.column_types['QUERY PLAN'].type_cast res.rows.first.first + assert_operator plan.length, :>, 0 end # Must have with_manual_interventions set to true for this diff --git a/activerecord/test/cases/adapters/postgresql/datatype_test.rb b/activerecord/test/cases/adapters/postgresql/datatype_test.rb index 3dbab08a99..c5ff8cb609 100644 --- a/activerecord/test/cases/adapters/postgresql/datatype_test.rb +++ b/activerecord/test/cases/adapters/postgresql/datatype_test.rb @@ -589,38 +589,28 @@ _SQL end def test_timestamp_with_zone_values_with_rails_time_zone_support - old_tz = ActiveRecord::Base.time_zone_aware_attributes - old_default_tz = ActiveRecord::Base.default_timezone + with_timezone_config default: :utc, aware_attributes: true do + @connection.reconnect! - ActiveRecord::Base.time_zone_aware_attributes = true - ActiveRecord::Base.default_timezone = :utc - - @connection.reconnect! - - @first_timestamp_with_zone = PostgresqlTimestampWithZone.find(1) - assert_equal Time.utc(2010,1,1, 11,0,0), @first_timestamp_with_zone.time - assert_instance_of Time, @first_timestamp_with_zone.time + @first_timestamp_with_zone = PostgresqlTimestampWithZone.find(1) + assert_equal Time.utc(2010,1,1, 11,0,0), @first_timestamp_with_zone.time + assert_instance_of Time, @first_timestamp_with_zone.time + end ensure - ActiveRecord::Base.default_timezone = old_default_tz - ActiveRecord::Base.time_zone_aware_attributes = old_tz @connection.reconnect! end def test_timestamp_with_zone_values_without_rails_time_zone_support - old_tz = ActiveRecord::Base.time_zone_aware_attributes - old_default_tz = ActiveRecord::Base.default_timezone - - ActiveRecord::Base.time_zone_aware_attributes = false - ActiveRecord::Base.default_timezone = :local - - @connection.reconnect! - - @first_timestamp_with_zone = PostgresqlTimestampWithZone.find(1) - assert_equal Time.local(2010,1,1, 11,0,0), @first_timestamp_with_zone.time - assert_instance_of Time, @first_timestamp_with_zone.time + with_timezone_config default: :local, aware_attributes: false do + @connection.reconnect! + # make sure to use a non-UTC time zone + @connection.execute("SET time zone 'America/Jamaica'", 'SCHEMA') + + @first_timestamp_with_zone = PostgresqlTimestampWithZone.find(1) + assert_equal Time.utc(2010,1,1, 11,0,0), @first_timestamp_with_zone.time + assert_instance_of Time, @first_timestamp_with_zone.time + end ensure - ActiveRecord::Base.default_timezone = old_default_tz - ActiveRecord::Base.time_zone_aware_attributes = old_tz @connection.reconnect! end end diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index f61f196c71..de724486c2 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -7,6 +7,8 @@ require 'active_record/connection_adapters/postgresql_adapter' class PostgresqlHstoreTest < ActiveRecord::TestCase class Hstore < ActiveRecord::Base self.table_name = 'hstores' + + store_accessor :settings, :language, :timezone end def setup @@ -26,6 +28,7 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase @connection.transaction do @connection.create_table('hstores') do |t| t.hstore 'tags', :default => '' + t.hstore 'settings' end end @column = Hstore.columns.find { |c| c.name == 'tags' } @@ -90,6 +93,24 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase assert_equal({'c'=>'}','"a"'=>'b "a b'}, @column.type_cast(%q(c=>"}", "\"a\""=>"b \"a b"))) end + def test_with_store_accessors + x = Hstore.new(language: "fr", timezone: "GMT") + assert_equal "fr", x.language + assert_equal "GMT", x.timezone + + x.save! + x = Hstore.first + assert_equal "fr", x.language + assert_equal "GMT", x.timezone + + x.language = "de" + x.save! + + x = Hstore.first + assert_equal "de", x.language + assert_equal "GMT", x.timezone + end + def test_gen1 assert_equal(%q(" "=>""), @column.class.hstore_to_string({' '=>''})) end diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index adac1d3c13..c33c7ef968 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -7,6 +7,8 @@ require 'active_record/connection_adapters/postgresql_adapter' class PostgresqlJSONTest < ActiveRecord::TestCase class JsonDataType < ActiveRecord::Base self.table_name = 'json_data_type' + + store_accessor :settings, :resolution end def setup @@ -15,6 +17,7 @@ class PostgresqlJSONTest < ActiveRecord::TestCase @connection.transaction do @connection.create_table('json_data_type') do |t| t.json 'payload', :default => {} + t.json 'settings' end end rescue ActiveRecord::StatementInvalid @@ -46,6 +49,13 @@ class PostgresqlJSONTest < ActiveRecord::TestCase JsonDataType.reset_column_information end + def test_cast_value_on_write + x = JsonDataType.new payload: {"string" => "foo", :symbol => :bar} + assert_equal({"string" => "foo", "symbol" => "bar"}, x.payload) + x.save + assert_equal({"string" => "foo", "symbol" => "bar"}, x.reload.payload) + end + def test_type_cast_json assert @column @@ -96,4 +106,19 @@ class PostgresqlJSONTest < ActiveRecord::TestCase x.payload = ['v1', {'k2' => 'v2'}, 'v3'] assert x.save! end + + def test_with_store_accessors + x = JsonDataType.new(resolution: "320×480") + assert_equal "320×480", x.resolution + + x.save! + x = JsonDataType.first + assert_equal "320×480", x.resolution + + x.resolution = "640×1136" + x.save! + + x = JsonDataType.first + assert_equal "640×1136", x.resolution + end end diff --git a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb index f1c4b85126..c5fd40accc 100644 --- a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb +++ b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb @@ -1,38 +1,40 @@ require 'cases/helper' -module ActiveRecord::ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter - class InactivePGconn - def query(*args) - raise PGError - end +module ActiveRecord + module ConnectionAdapters + class PostgreSQLAdapter < AbstractAdapter + class InactivePGconn + def query(*args) + raise PGError + end - def status - PGconn::CONNECTION_BAD + def status + PGconn::CONNECTION_BAD + end end - end - class StatementPoolTest < ActiveRecord::TestCase - def test_cache_is_per_pid - return skip('must support fork') unless Process.respond_to?(:fork) + class StatementPoolTest < ActiveRecord::TestCase + def test_cache_is_per_pid + return skip('must support fork') unless Process.respond_to?(:fork) - cache = StatementPool.new nil, 10 - cache['foo'] = 'bar' - assert_equal 'bar', cache['foo'] + cache = StatementPool.new nil, 10 + cache['foo'] = 'bar' + assert_equal 'bar', cache['foo'] - pid = fork { - lookup = cache['foo']; - exit!(!lookup) - } + pid = fork { + lookup = cache['foo']; + exit!(!lookup) + } - Process.waitpid pid - assert $?.success?, 'process should exit successfully' - end + Process.waitpid pid + assert $?.success?, 'process should exit successfully' + end - def test_dealloc_does_not_raise_on_inactive_connection - cache = StatementPool.new InactivePGconn.new, 10 - cache['foo'] = 'bar' - assert_nothing_raised { cache.clear } + def test_dealloc_does_not_raise_on_inactive_connection + cache = StatementPool.new InactivePGconn.new, 10 + cache['foo'] = 'bar' + assert_nothing_raised { cache.clear } + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index b573d48807..a753a23c09 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -24,7 +24,7 @@ class PostgresqlUUIDTest < ActiveRecord::TestCase @connection.reconnect! @connection.transaction do - @connection.create_table('pg_uuids', id: :uuid) do |t| + @connection.create_table('pg_uuids', id: :uuid, default: 'uuid_generate_v1()') do |t| t.string 'name' t.uuid 'other_uuid', default: 'uuid_generate_v4()' end @@ -60,7 +60,8 @@ class PostgresqlUUIDTest < ActiveRecord::TestCase def test_schema_dumper_for_uuid_primary_key schema = StringIO.new ActiveRecord::SchemaDumper.dump(@connection, schema) - assert_match(/\bcreate_table "pg_uuids", id: :uuid\b/, schema.string) + assert_match(/\bcreate_table "pg_uuids", id: :uuid, default: "uuid_generate_v1\(\)"/, schema.string) + assert_match(/t\.uuid "other_uuid", default: "uuid_generate_v4\(\)"/, schema.string) end end @@ -93,3 +94,43 @@ class PostgresqlUUIDTestNilDefault < ActiveRecord::TestCase assert_nil col_desc["default"] end end + +class PostgresqlUUIDTestInverseOf < ActiveRecord::TestCase + class UuidPost < ActiveRecord::Base + self.table_name = 'pg_uuid_posts' + has_many :uuid_comments, inverse_of: :uuid_post + end + + class UuidComment < ActiveRecord::Base + self.table_name = 'pg_uuid_comments' + belongs_to :uuid_post + end + + def setup + @connection = ActiveRecord::Base.connection + @connection.reconnect! + + @connection.transaction do + @connection.create_table('pg_uuid_posts', id: :uuid) do |t| + t.string 'title' + end + @connection.create_table('pg_uuid_comments', id: :uuid) do |t| + t.uuid :uuid_post_id, default: 'uuid_generate_v4()' + t.string 'content' + end + end + end + + def teardown + @connection.transaction do + @connection.execute 'drop table if exists pg_uuid_comments' + @connection.execute 'drop table if exists pg_uuid_posts' + end + end + + def test_collection_association_with_uuid + post = UuidPost.create! + comment = post.uuid_comments.create! + assert post.uuid_comments.find(comment.id) + end +end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 6ba6518eaa..ce7c869eec 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -21,8 +21,8 @@ module ActiveRecord ) eosql - @conn.extend(LogIntercepter) - @conn.intercepted = true + @subscriber = SQLSubscriber.new + ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) end def test_valid_column @@ -31,16 +31,16 @@ module ActiveRecord end # sqlite databases should be able to support any type and not - # just the ones mentioned in the native_database_types. - # Therefore test_invalid column should always return true + # just the ones mentioned in the native_database_types. + # Therefore test_invalid column should always return true # even if the type is not valid. def test_invalid_column assert @conn.valid_type?(:foobar) end def teardown - @conn.intercepted = false - @conn.logged = [] + ActiveSupport::Notifications.unsubscribe(@subscriber) + super end def test_column_types @@ -256,7 +256,7 @@ module ActiveRecord def test_tables_logs_name assert_logged [['SCHEMA', []]] do @conn.tables('hello') - assert_not_nil @conn.logged.first.shift + assert_not_nil @subscriber.logged.first.shift end end @@ -268,7 +268,7 @@ module ActiveRecord def test_table_exists_logs_name assert @conn.table_exists?('items') - assert_equal 'SCHEMA', @conn.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_columns @@ -306,10 +306,10 @@ module ActiveRecord end def test_indexes_logs - assert_difference('@conn.logged.length') do + assert_difference('@subscriber.logged.length') do @conn.indexes('items') end - assert_match(/items/, @conn.logged.last.first) + assert_match(/items/, @subscriber.logged.last.first) end def test_no_indexes @@ -370,7 +370,7 @@ module ActiveRecord def assert_logged logs yield - assert_equal logs, @conn.logged + assert_equal logs, @subscriber.logged end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 28bf48f4fd..874ae77ff7 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -747,6 +747,8 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_default_scope_as_block + # warm up the habtm cache + EagerDeveloperWithBlockDefaultScope.where(:name => 'David').first.projects developer = EagerDeveloperWithBlockDefaultScope.where(:name => 'David').first projects = Project.order(:id).to_a assert_no_queries do @@ -1136,6 +1138,10 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_deep_including_through_habtm + # warm up habtm cache + posts = Post.all.merge!(:includes => {:categories => :categorizations}, :order => "posts.id").to_a + posts[0].categories[0].categorizations.length + posts = Post.all.merge!(:includes => {:categories => :categorizations}, :order => "posts.id").to_a assert_no_queries { assert_equal 2, posts[0].categories[0].categorizations.length } assert_no_queries { assert_equal 1, posts[0].categories[1].categorizations.length } @@ -1172,8 +1178,11 @@ class EagerAssociationTest < ActiveRecord::TestCase } end - test "works in combination with order(:symbol)" do - author = Author.includes(:posts).references(:posts).order(:name).where('posts.title IS NOT NULL').first + test "works in combination with order(:symbol) and reorder(:symbol)" do + author = Author.includes(:posts).references(:posts).order(:name).find_by('posts.title IS NOT NULL') + assert_equal authors(:bob), author + + author = Author.includes(:posts).references(:posts).reorder(:name).find_by('posts.title IS NOT NULL') assert_equal authors(:bob), author end end diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb index 47dff7d0ea..f8f2832ab1 100644 --- a/activerecord/test/cases/associations/extension_test.rb +++ b/activerecord/test/cases/associations/extension_test.rb @@ -75,7 +75,6 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase private def extend!(model) - builder = ActiveRecord::Associations::Builder::HasMany.new(:association_name, nil, {}) { } - builder.define_extensions(model) + ActiveRecord::Associations::Builder::HasMany.define_extensions(model, :association_name) { } end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 712a770133..be928ec8ee 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -605,16 +605,24 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_join_table_alias + # FIXME: `references` has no impact on the aliases generated for the join + # query. The fact that we pass `:developers_projects_join` to `references` + # and that the SQL string contains `developers_projects_join` is merely a + # coincidence. assert_equal( 3, Developer.references(:developers_projects_join).merge( :includes => {:projects => :developers}, - :where => 'developers_projects_join.joined_on IS NOT NULL' + :where => 'projects_developers_projects_join.joined_on IS NOT NULL' ).to_a.size ) end def test_join_with_group + # FIXME: `references` has no impact on the aliases generated for the join + # query. The fact that we pass `:developers_projects_join` to `references` + # and that the SQL string contains `developers_projects_join` is merely a + # coincidence. group = Developer.columns.inject([]) do |g, c| g << "developers.#{c.name}" g << "developers_projects_2.#{c.name}" @@ -624,7 +632,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal( 3, Developer.references(:developers_projects_join).merge( - :includes => {:projects => :developers}, :where => 'developers_projects_join.joined_on IS NOT NULL', + :includes => {:projects => :developers}, :where => 'projects_developers_projects_join.joined_on IS NOT NULL', :group => group.join(",") ).to_a.size ) @@ -638,12 +646,12 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_find_scoped_grouped - assert_equal 5, categories(:general).posts_grouped_by_title.size - assert_equal 1, categories(:technology).posts_grouped_by_title.size + assert_equal 5, categories(:general).posts_grouped_by_title.to_a.size + assert_equal 1, categories(:technology).posts_grouped_by_title.to_a.size end def test_find_scoped_grouped_having - assert_equal 2, projects(:active_record).well_payed_salary_groups.size + assert_equal 2, projects(:active_record).well_payed_salary_groups.to_a.size assert projects(:active_record).well_payed_salary_groups.all? { |g| g.salary > 10000 } end @@ -710,12 +718,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal project, developer.projects.first end - def test_self_referential_habtm_without_foreign_key_set_should_raise_exception - assert_raise(ActiveRecord::HasAndBelongsToManyAssociationForeignKeyNeeded) { - SelfMember.new.friends - } - end - def test_dynamic_find_should_respect_association_include # SQL error in sort clause if :include is not included # due to Unknown column 'authors.id' diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ce51853bf3..caa916346a 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -45,6 +45,24 @@ class HasManyAssociationsTest < ActiveRecord::TestCase Client.destroyed_client_ids.clear end + def test_anonymous_has_many + developer = Class.new(ActiveRecord::Base) { + self.table_name = 'developers' + dev = self + + developer_project = Class.new(ActiveRecord::Base) { + self.table_name = 'developers_projects' + belongs_to :developer, :class => dev + } + has_many :developer_projects, :class => developer_project, :foreign_key => 'developer_id' + } + dev = developer.first + named = Developer.find(dev.id) + assert_operator dev.developer_projects.count, :>, 0 + assert_equal named.projects.map(&:id).sort, + dev.developer_projects.map(&:project_id).sort + end + def test_create_from_association_should_respect_default_scope car = Car.create(:name => 'honda') assert_equal 'honda', car.name @@ -62,6 +80,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 'exotic', bulb.name end + def test_build_from_association_should_respect_scope + author = Author.new + + post = author.thinking_posts.build + assert_equal 'So I was thinking', post.title + end + def test_create_from_association_with_nil_values_should_work car = Car.create(:name => 'honda') @@ -319,6 +344,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) } end + def test_find_ids_and_inverse_of + force_signal37_to_load_all_clients_of_firm + + firm = companies(:first_firm) + client = firm.clients_of_firm.find(3) + assert_kind_of Client, client + + client_ary = firm.clients_of_firm.find([3]) + assert_kind_of Array, client_ary + assert_equal client, client_ary.first + end + def test_find_all firm = Firm.all.merge!(:order => "id").first assert_equal 2, firm.clients.where("#{QUOTED_TYPE} = 'Client'").to_a.length @@ -500,6 +537,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end + def test_inverse_on_before_validate + firm = companies(:first_firm) + assert_queries(1) do + firm.clients_of_firm << Client.new("name" => "Natural Company") + end + end + def test_new_aliased_to_build company = companies(:first_firm) new_client = assert_no_queries { company.clients_of_firm.new("name" => "Another Client") } @@ -1396,15 +1440,17 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end - def test_calling_first_or_last_with_integer_on_association_should_load_association + def test_calling_first_or_last_with_integer_on_association_should_not_load_association firm = companies(:first_firm) + firm.clients.create(:name => 'Foo') + assert !firm.clients.loaded? - assert_queries 1 do + assert_queries 2 do firm.clients.first(2) firm.clients.last(2) end - assert firm.clients.loaded? + assert !firm.clients.loaded? end def test_calling_many_should_count_instead_of_loading_association @@ -1590,6 +1636,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal car.id, bulb.attributes_after_initialize['car_id'] end + def test_attributes_are_set_when_initialized_from_has_many_null_relationship + car = Car.new name: 'honda' + bulb = car.bulbs.where(name: 'headlight').first_or_initialize + assert_equal 'headlight', bulb.name + end + + def test_attributes_are_set_when_initialized_from_polymorphic_has_many_null_relationship + post = Post.new title: 'title', body: 'bar' + tag = Tag.create!(name: 'foo') + + tagging = post.taggings.where(tag: tag).first_or_initialize + + assert_equal tag.id, tagging.tag_id + assert_equal 'Post', tagging.taggable_type + end + def test_replace car = Car.create(:name => 'honda') bulb1 = car.bulbs.create diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 45087fdf8a..c450b1beb5 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -28,7 +28,8 @@ require 'models/club' class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors, :categories, :taggings, :tags, :owners, :pets, :toys, :jobs, :references, :companies, :members, :author_addresses, - :subscribers, :books, :subscriptions, :developers, :categorizations, :essays + :subscribers, :books, :subscriptions, :developers, :categorizations, :essays, + :categories_posts, :clubs, :memberships # Dummies to force column loads so query counts are clean. def setup @@ -36,6 +37,128 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase Reader.create :person_id => 0, :post_id => 0 end + def test_preload_sti_rhs_class + developers = Developer.includes(:firms).all.to_a + assert_no_queries do + developers.each { |d| d.firms } + end + end + + def test_preload_sti_middle_relation + club = Club.create!(name: 'Aaron cool banana club') + member1 = Member.create!(name: 'Aaron') + member2 = Member.create!(name: 'Cat') + + SuperMembership.create! club: club, member: member1 + CurrentMembership.create! club: club, member: member2 + + club1 = Club.includes(:members).find_by_id club.id + assert_equal [member1, member2].sort_by(&:id), + club1.members.sort_by(&:id) + end + + def make_model(name) + Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } } + end + + def test_ordered_habtm + person_prime = Class.new(ActiveRecord::Base) do + def self.name; 'Person'; end + + has_many :readers + has_many :posts, -> { order('posts.id DESC') }, :through => :readers + end + posts = person_prime.includes(:posts).first.posts + + assert_operator posts.length, :>, 1 + posts.each_cons(2) do |left,right| + assert_operator left.id, :>, right.id + end + end + + def test_singleton_has_many_through + book = make_model "Book" + subscription = make_model "Subscription" + subscriber = make_model "Subscriber" + + subscriber.primary_key = 'nick' + subscription.belongs_to :book, class: book + subscription.belongs_to :subscriber, class: subscriber + + book.has_many :subscriptions, class: subscription + book.has_many :subscribers, through: :subscriptions, class: subscriber + + anonbook = book.first + namebook = Book.find anonbook.id + + assert_operator anonbook.subscribers.count, :>, 0 + anonbook.subscribers.each do |s| + assert_instance_of subscriber, s + end + assert_equal namebook.subscribers.map(&:id).sort, + anonbook.subscribers.map(&:id).sort + end + + def test_no_pk_join_table_append + lesson, _, student = make_no_pk_hm_t + + sicp = lesson.new(:name => "SICP") + ben = student.new(:name => "Ben Bitdiddle") + sicp.students << ben + assert sicp.save! + end + + def test_no_pk_join_table_delete + lesson, lesson_student, student = make_no_pk_hm_t + + sicp = lesson.new(:name => "SICP") + ben = student.new(:name => "Ben Bitdiddle") + louis = student.new(:name => "Louis Reasoner") + sicp.students << ben + sicp.students << louis + assert sicp.save! + + sicp.students.reload + assert_operator lesson_student.count, :>=, 2 + assert_no_difference('student.count') do + assert_difference('lesson_student.count', -2) do + sicp.students.destroy(*student.all.to_a) + end + end + end + + def test_no_pk_join_model_callbacks + lesson, lesson_student, student = make_no_pk_hm_t + + after_destroy_called = false + lesson_student.after_destroy do + after_destroy_called = true + end + + sicp = lesson.new(:name => "SICP") + ben = student.new(:name => "Ben Bitdiddle") + sicp.students << ben + assert sicp.save! + + sicp.students.reload + sicp.students.destroy(*student.all.to_a) + assert after_destroy_called, "after destroy should be called" + end + + def make_no_pk_hm_t + lesson = make_model 'Lesson' + student = make_model 'Student' + + lesson_student = make_model 'LessonStudent' + lesson_student.table_name = 'lessons_students' + + lesson_student.belongs_to :lesson, :class => lesson + lesson_student.belongs_to :student, :class => student + lesson.has_many :lesson_students, :class => lesson_student + lesson.has_many :students, :through => :lesson_students, :class => student + [lesson, lesson_student, student] + end + def test_pk_is_not_required_for_join post = Post.includes(:scategories).first post2 = Post.includes(:categories).first @@ -662,7 +785,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase sarah = Person.create!(:first_name => 'Sarah', :primary_contact_id => people(:susan).id, :gender => 'F', :number1_fan_id => 1) john = Person.create!(:first_name => 'John', :primary_contact_id => sarah.id, :gender => 'M', :number1_fan_id => 1) assert_equal sarah.agents, [john] - assert_equal people(:susan).agents.map(&:agents).flatten, people(:susan).agents_of_agents + assert_equal people(:susan).agents.flat_map(&:agents), people(:susan).agents_of_agents end def test_associate_existing_with_nonstandard_primary_key_on_belongs_to @@ -962,4 +1085,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase readers(:michael_authorless).update(first_post_id: 1) assert_equal [posts(:thinking)], person.reload.first_posts end + + def test_has_many_through_with_includes_in_through_association_scope + assert_not_empty posts(:welcome).author_address_extra_with_address + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 4fdf9a9643..cdd386187b 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -505,6 +505,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_no_queries { company.account = nil } account = Account.find(2) assert_queries { company.account = account } + + assert_no_queries { Firm.new.account = account } end def test_has_one_assignment_triggers_save_on_change @@ -522,4 +524,15 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal 'new name', pirate.ship.reload.name end + def test_has_one_autosave_with_primary_key_manually_set + post = Post.create(id: 1234, title: "Some title", body: 'Some content') + author = Author.new(id: 33, name: 'Hank Moody') + + author.post = post + author.save + author.reload + + assert_not_nil author.post + assert_equal author.post, post + end end diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index de47a576c6..9fe5ff50d9 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -41,6 +41,11 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase assert_no_match(/WHERE/i, sql) end + def test_join_association_conditions_support_string_and_arel_expressions + assert_equal 0, Author.joins(:welcome_posts_with_comment).count + assert_equal 1, Author.joins(:welcome_posts_with_comments).count + end + def test_join_conditions_allow_nil_associations authors = Author.includes(:essays).where(:essays => {:id => nil}) assert_equal 2, authors.count diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 2477e60e51..893030345f 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -446,6 +446,19 @@ class InverseHasManyTests < ActiveRecord::TestCase def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.first.secret_interests } end + + def test_child_instance_should_point_to_parent_without_saving + man = Man.new + i = Interest.create(:topic => 'Industrial Revolution Re-enactment') + + man.interests << i + assert_not_nil i.man + + i.man.name = "Charles" + assert_equal i.man.name, man.name + + assert !man.persisted? + end end class InverseBelongsToTests < ActiveRecord::TestCase @@ -590,6 +603,18 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance" end + def test_inversed_instance_should_not_be_reloaded_after_stale_state_changed + new_man = Man.new + face = Face.new + new_man.face = face + + old_inversed_man = face.man + new_man.save! + new_inversed_man = face.man + + assert_equal old_inversed_man.object_id, new_inversed_man.object_id + end + def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many i = interests(:llama_wrangling) m = i.polymorphic_man diff --git a/activerecord/test/cases/associations/join_dependency_test.rb b/activerecord/test/cases/associations/join_dependency_test.rb deleted file mode 100644 index 08c166dc33..0000000000 --- a/activerecord/test/cases/associations/join_dependency_test.rb +++ /dev/null @@ -1,8 +0,0 @@ -require "cases/helper" -require 'models/edge' - -class JoinDependencyTest < ActiveRecord::TestCase - def test_column_names_with_alias_handles_nil_primary_key - assert_equal Edge.column_names, ActiveRecord::Associations::JoinDependency::JoinBase.new(Edge).column_names_with_alias.map(&:first) - end -end
\ No newline at end of file diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index 9b1abc3b81..8ef351cda8 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -214,7 +214,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_has_many_with_has_and_belongs_to_many_source_reflection_preload - authors = assert_queries(3) { Author.includes(:post_categories).to_a.sort_by(&:id) } + authors = assert_queries(4) { Author.includes(:post_categories).to_a.sort_by(&:id) } general, cooking = categories(:general), categories(:cooking) assert_no_queries do @@ -242,7 +242,8 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection_preload - categories = assert_queries(3) { Category.includes(:post_comments).to_a.sort_by(&:id) } + Category.includes(:post_comments).to_a # preheat cache + categories = assert_queries(4) { Category.includes(:post_comments).to_a.sort_by(&:id) } greetings, more = comments(:greetings), comments(:more_greetings) assert_no_queries do @@ -270,7 +271,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection_preload - authors = assert_queries(5) { Author.includes(:category_post_comments).to_a.sort_by(&:id) } + authors = assert_queries(6) { Author.includes(:category_post_comments).to_a.sort_by(&:id) } greetings, more = comments(:greetings), comments(:more_greetings) assert_no_queries do @@ -371,7 +372,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase prev_default_scope = Club.default_scopes [:includes, :preload, :joins, :eager_load].each do |q| - Club.default_scopes = [Club.send(q, :category)] + Club.default_scopes = [proc { Club.send(q, :category) }] assert_equal categories(:general), members(:groucho).reload.club_category end ensure diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index baba55ea43..9c66ed354e 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -92,7 +92,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_set_attributes_without_hash topic = Topic.new - assert_nothing_raised { topic.attributes = '' } + assert_raise(ArgumentError) { topic.attributes = '' } end def test_integers_as_nil diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 635278abc1..517d2674a7 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -1440,10 +1440,6 @@ class TestAutosaveAssociationValidationMethodsGeneration < ActiveRecord::TestCas test "should generate validation methods for HABTM associations with :validate => true" do assert_respond_to @pirate, :validate_associated_records_for_parrots end - - test "should not generate validation methods for HABTM associations without :validate => true" do - assert !@pirate.respond_to?(:validate_associated_records_for_non_validated_parrots) - end end class TestAutosaveAssociationWithTouch < ActiveRecord::TestCase diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index c91cf89f6d..4de92b71be 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -78,12 +78,6 @@ end class BasicsTest < ActiveRecord::TestCase fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts - def setup - ActiveRecord::Base.time_zone_aware_attributes = false - ActiveRecord::Base.default_timezone = :local - Time.zone = nil - end - def test_generated_methods_modules modules = Computer.ancestors assert modules.include?(Computer::GeneratedFeatureMethods) @@ -234,7 +228,7 @@ class BasicsTest < ActiveRecord::TestCase def test_preserving_time_objects_with_local_time_conversion_to_default_timezone_utc with_env_tz 'America/New_York' do - with_active_record_default_timezone :utc do + with_timezone_config default: :utc do time = Time.local(2000) topic = Topic.create('written_on' => time) saved_time = Topic.find(topic.id).reload.written_on @@ -247,7 +241,7 @@ class BasicsTest < ActiveRecord::TestCase def test_preserving_time_objects_with_time_with_zone_conversion_to_default_timezone_utc with_env_tz 'America/New_York' do - with_active_record_default_timezone :utc do + with_timezone_config default: :utc do Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) topic = Topic.create('written_on' => time) @@ -262,18 +256,20 @@ class BasicsTest < ActiveRecord::TestCase def test_preserving_time_objects_with_utc_time_conversion_to_default_timezone_local with_env_tz 'America/New_York' do - time = Time.utc(2000) - topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).reload.written_on - assert_equal time, saved_time - assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "UTC"], time.to_a - assert_equal [0, 0, 19, 31, 12, 1999, 5, 365, false, "EST"], saved_time.to_a + with_timezone_config default: :local do + time = Time.utc(2000) + topic = Topic.create('written_on' => time) + saved_time = Topic.find(topic.id).reload.written_on + assert_equal time, saved_time + assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "UTC"], time.to_a + assert_equal [0, 0, 19, 31, 12, 1999, 5, 365, false, "EST"], saved_time.to_a + end end end def test_preserving_time_objects_with_time_with_zone_conversion_to_default_timezone_local with_env_tz 'America/New_York' do - with_active_record_default_timezone :local do + with_timezone_config default: :local do Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) topic = Topic.create('written_on' => time) @@ -493,25 +489,25 @@ class BasicsTest < ActiveRecord::TestCase # Oracle, and Sybase do not have a TIME datatype. unless current_adapter?(:OracleAdapter, :SybaseAdapter) def test_utc_as_time_zone - Topic.default_timezone = :utc - attributes = { "bonus_time" => "5:42:00AM" } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.utc(2000, 1, 1, 5, 42, 0), topic.bonus_time - Topic.default_timezone = :local + with_timezone_config default: :utc do + attributes = { "bonus_time" => "5:42:00AM" } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.utc(2000, 1, 1, 5, 42, 0), topic.bonus_time + end end def test_utc_as_time_zone_and_new - Topic.default_timezone = :utc - attributes = { "bonus_time(1i)"=>"2000", - "bonus_time(2i)"=>"1", - "bonus_time(3i)"=>"1", - "bonus_time(4i)"=>"10", - "bonus_time(5i)"=>"35", - "bonus_time(6i)"=>"50" } - topic = Topic.new(attributes) - assert_equal Time.utc(2000, 1, 1, 10, 35, 50), topic.bonus_time - Topic.default_timezone = :local + with_timezone_config default: :utc do + attributes = { "bonus_time(1i)"=>"2000", + "bonus_time(2i)"=>"1", + "bonus_time(3i)"=>"1", + "bonus_time(4i)"=>"10", + "bonus_time(5i)"=>"35", + "bonus_time(6i)"=>"50" } + topic = Topic.new(attributes) + assert_equal Time.utc(2000, 1, 1, 10, 35, 50), topic.bonus_time + end end end @@ -558,11 +554,27 @@ class BasicsTest < ActiveRecord::TestCase assert_equal [ Topic.find(1) ], [ Topic.find(2).topic ] & [ Topic.find(1) ] end - def test_comparison - topic_1 = Topic.create! - topic_2 = Topic.create! + def test_create_without_prepared_statement + topic = Topic.connection.unprepared_statement do + Topic.create(:title => 'foo') + end + + assert_equal topic, Topic.find(topic.id) + end - assert_equal [topic_2, topic_1].sort, [topic_1, topic_2] + def test_destroy_without_prepared_statement + topic = Topic.create(title: 'foo') + Topic.connection.unprepared_statement do + Topic.find(topic.id).destroy + end + + assert_equal nil, Topic.find_by_id(topic.id) + end + + def test_blank_ids + one = Subscriber.new(:id => '') + two = Subscriber.new(:id => '') + assert_equal one, two end def test_comparison_with_different_objects @@ -571,6 +583,13 @@ class BasicsTest < ActiveRecord::TestCase assert_nil topic <=> category end + def test_comparison_with_different_objects_in_array + topic = Topic.create + assert_raises(ArgumentError) do + [1, topic].sort + end + end + def test_readonly_attributes assert_equal Set.new([ 'title' , 'comments_count' ]), ReadonlyTitlePost.readonly_attributes @@ -611,12 +630,14 @@ class BasicsTest < ActiveRecord::TestCase # Oracle, and Sybase do not have a TIME datatype. return true if current_adapter?(:OracleAdapter, :SybaseAdapter) - attributes = { - "bonus_time" => "5:42:00AM" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.local(2000, 1, 1, 5, 42, 0), topic.bonus_time + with_timezone_config default: :local do + attributes = { + "bonus_time" => "5:42:00AM" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.local(2000, 1, 1, 5, 42, 0), topic.bonus_time + end end def test_attributes_on_dummy_time_with_invalid_time @@ -804,19 +825,18 @@ class BasicsTest < ActiveRecord::TestCase # TODO: extend defaults tests to other databases! if current_adapter?(:PostgreSQLAdapter) def test_default - tz = Default.default_timezone - Default.default_timezone = :local - default = Default.new - Default.default_timezone = tz - - # fixed dates / times - assert_equal Date.new(2004, 1, 1), default.fixed_date - assert_equal Time.local(2004, 1,1,0,0,0,0), default.fixed_time - - # char types - assert_equal 'Y', default.char1 - assert_equal 'a varchar field', default.char2 - assert_equal 'a text field', default.char3 + with_timezone_config default: :local do + default = Default.new + + # fixed dates / times + assert_equal Date.new(2004, 1, 1), default.fixed_date + assert_equal Time.local(2004, 1,1,0,0,0,0), default.fixed_time + + # char types + assert_equal 'Y', default.char1 + assert_equal 'a varchar field', default.char2 + assert_equal 'a text field', default.char3 + end end class Geometric < ActiveRecord::Base; end @@ -1324,6 +1344,36 @@ class BasicsTest < ActiveRecord::TestCase assert_equal 1, post.comments.length end + def test_marshal_between_processes + skip "can't marshal between processes when using an in-memory db" if in_memory_db? + skip "fork isn't supported" unless Process.respond_to?(:fork) + + # Define a new model to ensure there are no caches + if self.class.const_defined?("Post", false) + flunk "there should be no post constant" + end + + self.class.const_set("Post", Class.new(ActiveRecord::Base) { + has_many :comments + }) + + rd, wr = IO.pipe + + ActiveRecord::Base.connection_handler.clear_all_connections! + + fork do + rd.close + post = Post.new + post.comments.build + wr.write Marshal.dump(post) + wr.close + end + + wr.close + assert Marshal.load rd.read + rd.close + end + def test_marshalling_new_record_round_trip_with_associations post = Post.new post.comments.build diff --git a/activerecord/test/cases/column_test.rb b/activerecord/test/cases/column_test.rb index 3a4f414ae8..2a6d8cc2ab 100644 --- a/activerecord/test/cases/column_test.rb +++ b/activerecord/test/cases/column_test.rb @@ -110,6 +110,14 @@ module ActiveRecord assert_equal 1800, column.type_cast(30.minutes) assert_equal 7200, column.type_cast(2.hours) end + + def test_string_to_time_with_timezone + [:utc, :local].each do |zone| + with_timezone_config default: zone do + assert_equal Time.utc(2013, 9, 4, 0, 0, 0), Column.string_to_time("Wed, 04 Sep 2013 03:00:00 EAT") + end + end + end end end end diff --git a/activerecord/test/cases/date_time_test.rb b/activerecord/test/cases/date_time_test.rb index 427076bd80..c0491bbee5 100644 --- a/activerecord/test/cases/date_time_test.rb +++ b/activerecord/test/cases/date_time_test.rb @@ -5,7 +5,7 @@ require 'models/task' class DateTimeTest < ActiveRecord::TestCase def test_saves_both_date_and_time with_env_tz 'America/New_York' do - with_active_record_default_timezone :utc do + with_timezone_config default: :utc do time_values = [1807, 2, 10, 15, 30, 45] # create DateTime value with local time zone offset local_offset = Rational(Time.local(*time_values).utc_offset, 86400) diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index b277ef0317..9d7f57bf85 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -125,30 +125,30 @@ class DirtyTest < ActiveRecord::TestCase end def test_time_attributes_changes_without_time_zone - target = Class.new(ActiveRecord::Base) - target.table_name = 'pirates' + with_timezone_config aware_attributes: false do + target = Class.new(ActiveRecord::Base) + target.table_name = 'pirates' - target.time_zone_aware_attributes = false + # New record - no changes. + pirate = target.new + assert !pirate.created_on_changed? + assert_nil pirate.created_on_change - # New record - no changes. - pirate = target.new - assert !pirate.created_on_changed? - assert_nil pirate.created_on_change + # Saved - no changes. + pirate.catchphrase = 'arrrr, time zone!!' + pirate.save! + assert !pirate.created_on_changed? + assert_nil pirate.created_on_change - # Saved - no changes. - pirate.catchphrase = 'arrrr, time zone!!' - pirate.save! - assert !pirate.created_on_changed? - assert_nil pirate.created_on_change - - # Change created_on. - old_created_on = pirate.created_on - pirate.created_on = Time.now + 1.day - assert pirate.created_on_changed? - # kind_of does not work because - # ActiveSupport::TimeWithZone.name == 'Time' - assert_instance_of Time, pirate.created_on_was - assert_equal old_created_on, pirate.created_on_was + # Change created_on. + old_created_on = pirate.created_on + pirate.created_on = Time.now + 1.day + assert pirate.created_on_changed? + # kind_of does not work because + # ActiveSupport::TimeWithZone.name == 'Time' + assert_instance_of Time, pirate.created_on_was + assert_equal old_created_on, pirate.created_on_was + end end diff --git a/activerecord/test/cases/dup_test.rb b/activerecord/test/cases/dup_test.rb index f73e449610..1e6ccecfab 100644 --- a/activerecord/test/cases/dup_test.rb +++ b/activerecord/test/cases/dup_test.rb @@ -126,7 +126,7 @@ module ActiveRecord def test_dup_with_default_scope prev_default_scopes = Topic.default_scopes - Topic.default_scopes = [Topic.where(:approved => true)] + Topic.default_scopes = [proc { Topic.where(:approved => true) }] topic = Topic.new(:approved => false) assert !topic.dup.approved?, "should not be overridden by default scopes" ensure diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 949e994f8d..4188b32731 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -11,6 +11,7 @@ require 'models/project' require 'models/developer' require 'models/customer' require 'models/toy' +require 'models/matey' class FinderTest < ActiveRecord::TestCase fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations @@ -51,6 +52,7 @@ class FinderTest < ActiveRecord::TestCase assert_equal true, Topic.exists?(heading: "The First Topic") assert_equal true, Topic.exists?(:author_name => "Mary", :approved => true) assert_equal true, Topic.exists?(["parent_id = ?", 1]) + assert_equal true, Topic.exists?(id: [1, 9999]) assert_equal false, Topic.exists?(45) assert_equal false, Topic.exists?(Topic.new) @@ -477,7 +479,7 @@ class FinderTest < ActiveRecord::TestCase def test_condition_utc_time_interpolation_with_default_timezone_local with_env_tz 'America/New_York' do - with_active_record_default_timezone :local do + with_timezone_config default: :local do topic = Topic.first assert_equal topic, Topic.all.merge!(:where => ['written_on = ?', topic.written_on.getutc]).first end @@ -486,7 +488,7 @@ class FinderTest < ActiveRecord::TestCase def test_hash_condition_utc_time_interpolation_with_default_timezone_local with_env_tz 'America/New_York' do - with_active_record_default_timezone :local do + with_timezone_config default: :local do topic = Topic.first assert_equal topic, Topic.all.merge!(:where => {:written_on => topic.written_on.getutc}).first end @@ -495,7 +497,7 @@ class FinderTest < ActiveRecord::TestCase def test_condition_local_time_interpolation_with_default_timezone_utc with_env_tz 'America/New_York' do - with_active_record_default_timezone :utc do + with_timezone_config default: :utc do topic = Topic.first assert_equal topic, Topic.all.merge!(:where => ['written_on = ?', topic.written_on.getlocal]).first end @@ -504,7 +506,7 @@ class FinderTest < ActiveRecord::TestCase def test_hash_condition_local_time_interpolation_with_default_timezone_utc with_env_tz 'America/New_York' do - with_active_record_default_timezone :utc do + with_timezone_config default: :utc do topic = Topic.first assert_equal topic, Topic.all.merge!(:where => {:written_on => topic.written_on.getlocal}).first end @@ -859,6 +861,12 @@ class FinderTest < ActiveRecord::TestCase Toy.reset_primary_key end + def test_find_without_primary_key + assert_raises(ActiveRecord::UnknownPrimaryKey) do + Matey.find(1) + end + end + def test_finder_with_offset_string assert_nothing_raised(ActiveRecord::StatementInvalid) { Topic.all.merge!(:offset => "3").to_a } end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 4e877aed2e..bffff07089 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -84,6 +84,12 @@ class FixturesTest < ActiveRecord::TestCase assert fixtures.detect { |f| f.name == 'collections' }, "no fixtures named 'collections' in #{fixtures.map(&:name).inspect}" end + def test_create_symbol_fixtures_is_deprecated + assert_deprecated do + ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, :collections, :collections => 'Course') { Course.connection } + end + end + def test_attributes topics = create_fixtures("topics").first assert_equal("The First Topic", topics["first"]["title"]) @@ -247,7 +253,8 @@ class FixturesTest < ActiveRecord::TestCase end def test_fixtures_are_set_up_with_database_env_variable - ENV.stubs(:[]).with("DATABASE_URL").returns("sqlite3:///:memory:") + db_url_tmp = ENV['DATABASE_URL'] + ENV['DATABASE_URL'] = "sqlite3:///:memory:" ActiveRecord::Base.stubs(:configurations).returns({}) test_case = Class.new(ActiveRecord::TestCase) do fixtures :accounts @@ -260,6 +267,43 @@ class FixturesTest < ActiveRecord::TestCase result = test_case.new(:test_fixtures).run assert result.passed?, "Expected #{result.name} to pass:\n#{result}" + ensure + ENV['DATABASE_URL'] = db_url_tmp + end +end + +class HasManyThroughFixture < ActiveSupport::TestCase + def make_model(name) + Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } } + end + + def test_has_many_through + pt = make_model "ParrotTreasure" + parrot = make_model "Parrot" + treasure = make_model "Treasure" + + pt.table_name = "parrots_treasures" + pt.belongs_to :parrot, :class => parrot + pt.belongs_to :treasure, :class => treasure + + parrot.has_many :parrot_treasures, :class => pt + parrot.has_many :treasures, :through => :parrot_treasures + + parrots = File.join FIXTURES_ROOT, 'parrots' + + fs = ActiveRecord::FixtureSet.new parrot.connection, "parrots", parrot, parrots + rows = fs.table_rows + assert_equal load_has_and_belongs_to_many['parrots_treasures'], rows['parrots_treasures'] + end + + def load_has_and_belongs_to_many + parrot = make_model "Parrot" + parrot.has_and_belongs_to_many :treasures + + parrots = File.join FIXTURES_ROOT, 'parrots' + + fs = ActiveRecord::FixtureSet.new parrot.connection, "parrots", parrot, parrots + fs.table_rows end end diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index 490b599fb6..981a75faf6 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -61,4 +61,9 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase assert_equal 'Guille', person.first_name assert_equal 'm', person.gender end + + def test_blank_attributes_should_not_raise + person = Person.new + assert_nil person.assign_attributes(ProtectedParams.new({})) + end end diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index f96978aff8..2af647e2d7 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -49,11 +49,58 @@ ensure old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ') end -def with_active_record_default_timezone(zone) - old_zone, ActiveRecord::Base.default_timezone = ActiveRecord::Base.default_timezone, zone +def with_timezone_config(cfg) + verify_default_timezone_config + + old_default_zone = ActiveRecord::Base.default_timezone + old_awareness = ActiveRecord::Base.time_zone_aware_attributes + old_zone = Time.zone + + if cfg.has_key?(:default) + ActiveRecord::Base.default_timezone = cfg[:default] + end + if cfg.has_key?(:aware_attributes) + ActiveRecord::Base.time_zone_aware_attributes = cfg[:aware_attributes] + end + if cfg.has_key?(:zone) + Time.zone = cfg[:zone] + end yield ensure - ActiveRecord::Base.default_timezone = old_zone + ActiveRecord::Base.default_timezone = old_default_zone + ActiveRecord::Base.time_zone_aware_attributes = old_awareness + Time.zone = old_zone +end + +# This method makes sure that tests don't leak global state related to time zones. +EXPECTED_ZONE = nil +EXPECTED_DEFAULT_TIMEZONE = :utc +EXPECTED_TIME_ZONE_AWARE_ATTRIBUTES = false +def verify_default_timezone_config + if Time.zone != EXPECTED_ZONE + $stderr.puts <<-MSG +\n#{self.to_s} + Global state `Time.zone` was leaked. + Expected: #{EXPECTED_ZONE} + Got: #{Time.zone} + MSG + end + if ActiveRecord::Base.default_timezone != EXPECTED_DEFAULT_TIMEZONE + $stderr.puts <<-MSG +\n#{self.to_s} + Global state `ActiveRecord::Base.default_timezone` was leaked. + Expected: #{EXPECTED_DEFAULT_TIMEZONE} + Got: #{ActiveRecord::Base.default_timezone} + MSG + end + if ActiveRecord::Base.time_zone_aware_attributes != EXPECTED_TIME_ZONE_AWARE_ATTRIBUTES + $stderr.puts <<-MSG +\n#{self.to_s} + Global state `ActiveRecord::Base.time_zone_aware_attributes` was leaked. + Expected: #{EXPECTED_TIME_ZONE_AWARE_ATTRIBUTES} + Got: #{ActiveRecord::Base.time_zone_aware_attributes} + MSG + end end unless ENV['FIXTURE_DEBUG'] @@ -119,21 +166,24 @@ class << Time end end -module LogIntercepter - attr_accessor :logged, :intercepted - def self.extended(base) - base.logged = [] - end - def log(sql, name = 'SQL', binds = [], &block) - if @intercepted - @logged << [sql, name, binds] - yield - else - super(sql, name,binds, &block) - end +class SQLSubscriber + attr_reader :logged + attr_reader :payloads + + def initialize + @logged = [] + @payloads = [] end + + def start(name, id, payload) + @payloads << payload + @logged << [payload[:sql], payload[:name], payload[:binds]] + end + + def finish(name, id, payload); end end + module InTimeZone private diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index a9be132801..73cf99a5d7 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -313,8 +313,12 @@ class InheritanceTest < ActiveRecord::TestCase assert_kind_of SpecialSubscriber, SpecialSubscriber.find("webster132") assert_nothing_raised { s = SpecialSubscriber.new("name" => "And breaaaaathe!"); s.id = 'roger'; s.save } end -end + def test_scope_inherited_properly + assert_nothing_raised { Company.of_first_firm } + assert_nothing_raised { Client.of_first_firm } + end +end class InheritanceComputeTypeTest < ActiveRecord::TestCase fixtures :companies diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index f5daca2fa8..406aacb056 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -23,17 +23,12 @@ class IntegrationTest < ActiveRecord::TestCase end def test_cache_key_for_existing_record_is_not_timezone_dependent - ActiveRecord::Base.time_zone_aware_attributes = true - - Time.zone = 'UTC' utc_key = Developer.first.cache_key - Time.zone = 'EST' - est_key = Developer.first.cache_key - - assert_equal utc_key, est_key - ensure - Time.zone = 'UTC' + with_timezone_config zone: "EST" do + est_key = Developer.first.cache_key + assert_equal utc_key, est_key + end end def test_cache_key_format_for_existing_record_with_updated_at diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index dfa12cb97c..a16ed963fe 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -272,6 +272,10 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert p.treasures.empty? assert RichPerson.connection.select_all("SELECT * FROM peoples_treasures WHERE rich_person_id = 1").empty? end + + def test_quoted_locking_column_is_deprecated + assert_deprecated { ActiveRecord::Base.quoted_locking_column } + end end class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 931caff727..ed32d68530 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -311,14 +311,23 @@ class MigrationTest < ActiveRecord::TestCase end def test_schema_migrations_table_name + original_schema_migrations_table_name = ActiveRecord::Migrator.schema_migrations_table_name + + assert_equal "schema_migrations", ActiveRecord::Migrator.schema_migrations_table_name ActiveRecord::Base.table_name_prefix = "prefix_" ActiveRecord::Base.table_name_suffix = "_suffix" Reminder.reset_table_name assert_equal "prefix_schema_migrations_suffix", ActiveRecord::Migrator.schema_migrations_table_name + ActiveRecord::Base.schema_migrations_table_name = "changed" + Reminder.reset_table_name + assert_equal "prefix_changed_suffix", ActiveRecord::Migrator.schema_migrations_table_name ActiveRecord::Base.table_name_prefix = "" ActiveRecord::Base.table_name_suffix = "" Reminder.reset_table_name - assert_equal "schema_migrations", ActiveRecord::Migrator.schema_migrations_table_name + assert_equal "changed", ActiveRecord::Migrator.schema_migrations_table_name + ensure + ActiveRecord::Base.schema_migrations_table_name = original_schema_migrations_table_name + Reminder.reset_table_name end def test_proper_table_name_on_migrator diff --git a/activerecord/test/cases/modules_test.rb b/activerecord/test/cases/modules_test.rb index 08b3408665..9124105e6d 100644 --- a/activerecord/test/cases/modules_test.rb +++ b/activerecord/test/cases/modules_test.rb @@ -1,6 +1,7 @@ require "cases/helper" require 'models/company_in_module' require 'models/shop' +require 'models/developer' class ModulesTest < ActiveRecord::TestCase fixtures :accounts, :companies, :projects, :developers, :collections, :products, :variants diff --git a/activerecord/test/cases/multiparameter_attributes_test.rb b/activerecord/test/cases/multiparameter_attributes_test.rb index ce21760645..b82409bfbe 100644 --- a/activerecord/test/cases/multiparameter_attributes_test.rb +++ b/activerecord/test/cases/multiparameter_attributes_test.rb @@ -5,16 +5,6 @@ require 'models/customer' class MultiParameterAttributeTest < ActiveRecord::TestCase fixtures :topics - def setup - ActiveRecord::Base.time_zone_aware_attributes = false - ActiveRecord::Base.default_timezone = :local - Time.zone = nil - end - - def teardown - ActiveRecord::Base.default_timezone = :utc - end - def test_multiparameter_attributes_on_date attributes = { "last_read(1i)" => "2004", "last_read(2i)" => "6", "last_read(3i)" => "24" } topic = Topic.find(1) @@ -86,13 +76,15 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase end def test_multiparameter_attributes_on_time - attributes = { - "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", - "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on + with_timezone_config default: :local do + attributes = { + "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", + "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on + end end def test_multiparameter_attributes_on_time_with_no_date @@ -152,13 +144,15 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase end def test_multiparameter_attributes_on_time_will_ignore_hour_if_missing - attributes = { - "written_on(1i)" => "2004", "written_on(2i)" => "12", "written_on(3i)" => "12", - "written_on(5i)" => "12", "written_on(6i)" => "02" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.local(2004, 12, 12, 0, 12, 2), topic.written_on + with_timezone_config default: :local do + attributes = { + "written_on(1i)" => "2004", "written_on(2i)" => "12", "written_on(3i)" => "12", + "written_on(5i)" => "12", "written_on(6i)" => "02" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.local(2004, 12, 12, 0, 12, 2), topic.written_on + end end def test_multiparameter_attributes_on_time_will_ignore_hour_if_blank @@ -180,6 +174,7 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase topic.attributes = attributes assert_nil topic.written_on end + def test_multiparameter_attributes_on_time_with_seconds_will_ignore_date_if_empty attributes = { "written_on(1i)" => "", "written_on(2i)" => "", "written_on(3i)" => "", @@ -191,56 +186,56 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase end def test_multiparameter_attributes_on_time_with_utc - ActiveRecord::Base.default_timezone = :utc - attributes = { - "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", - "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on + with_timezone_config default: :utc do + attributes = { + "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", + "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on + end end def test_multiparameter_attributes_on_time_with_time_zone_aware_attributes - ActiveRecord::Base.time_zone_aware_attributes = true - ActiveRecord::Base.default_timezone = :utc - Time.zone = ActiveSupport::TimeZone[-28800] - attributes = { - "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", - "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.utc(2004, 6, 24, 23, 24, 0), topic.written_on - assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on.time - assert_equal Time.zone, topic.written_on.time_zone + with_timezone_config default: :utc, aware_attributes: true, zone: -28800 do + attributes = { + "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", + "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.utc(2004, 6, 24, 23, 24, 0), topic.written_on + assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on.time + assert_equal Time.zone, topic.written_on.time_zone + end end def test_multiparameter_attributes_on_time_with_time_zone_aware_attributes_false - Time.zone = ActiveSupport::TimeZone[-28800] - attributes = { - "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", - "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on - assert_equal false, topic.written_on.respond_to?(:time_zone) + with_timezone_config default: :local, aware_attributes: false, zone: -28800 do + attributes = { + "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", + "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on + assert_equal false, topic.written_on.respond_to?(:time_zone) + end end def test_multiparameter_attributes_on_time_with_skip_time_zone_conversion_for_attributes - ActiveRecord::Base.time_zone_aware_attributes = true - ActiveRecord::Base.default_timezone = :utc - Time.zone = ActiveSupport::TimeZone[-28800] - Topic.skip_time_zone_conversion_for_attributes = [:written_on] - attributes = { - "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", - "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on - assert_equal false, topic.written_on.respond_to?(:time_zone) + with_timezone_config default: :utc, aware_attributes: true, zone: -28800 do + Topic.skip_time_zone_conversion_for_attributes = [:written_on] + attributes = { + "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", + "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on + assert_equal false, topic.written_on.respond_to?(:time_zone) + end ensure Topic.skip_time_zone_conversion_for_attributes = [] end @@ -248,30 +243,31 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase # Oracle, and Sybase do not have a TIME datatype. unless current_adapter?(:OracleAdapter, :SybaseAdapter) def test_multiparameter_attributes_on_time_only_column_with_time_zone_aware_attributes_does_not_do_time_zone_conversion - ActiveRecord::Base.time_zone_aware_attributes = true - ActiveRecord::Base.default_timezone = :utc - Time.zone = ActiveSupport::TimeZone[-28800] + with_timezone_config default: :utc, aware_attributes: true, zone: -28800 do + attributes = { + "bonus_time(1i)" => "2000", "bonus_time(2i)" => "1", "bonus_time(3i)" => "1", + "bonus_time(4i)" => "16", "bonus_time(5i)" => "24" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_equal Time.utc(2000, 1, 1, 16, 24, 0), topic.bonus_time + assert topic.bonus_time.utc? + end + end + end + + def test_multiparameter_attributes_on_time_with_empty_seconds + with_timezone_config default: :local do attributes = { - "bonus_time(1i)" => "2000", "bonus_time(2i)" => "1", "bonus_time(3i)" => "1", - "bonus_time(4i)" => "16", "bonus_time(5i)" => "24" + "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", + "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "" } topic = Topic.find(1) topic.attributes = attributes - assert_equal Time.utc(2000, 1, 1, 16, 24, 0), topic.bonus_time - assert topic.bonus_time.utc? + assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on end end - def test_multiparameter_attributes_on_time_with_empty_seconds - attributes = { - "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", - "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "" - } - topic = Topic.find(1) - topic.attributes = attributes - assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on - end - def test_multiparameter_attributes_setting_time_attribute return skip "Oracle does not have TIME data type" if current_adapter? :OracleAdapter diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 30dc2a34c6..6cd3e2154e 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -419,10 +419,6 @@ class PersistenceTest < ActiveRecord::TestCase assert !Topic.find(1).approved? end - def test_update_attribute_does_not_choke_on_nil - assert Topic.find(1).update(nil) - end - def test_update_attribute_for_readonly_attribute minivan = Minivan.find('m1') assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } @@ -701,6 +697,17 @@ class PersistenceTest < ActiveRecord::TestCase assert_equal topic.title, Topic.find(1234).title end + def test_update_attributes_parameters + topic = Topic.find(1) + assert_nothing_raised do + topic.update_attributes({}) + end + + assert_raises(ArgumentError) do + topic.update_attributes(nil) + end + end + def test_update! Reply.validates_presence_of(:title) reply = Reply.find(2) diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index 44b2064110..e2439b9a24 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -53,28 +53,28 @@ module ActiveRecord end def test_quoted_time_utc - with_active_record_default_timezone :utc do + with_timezone_config default: :utc do t = Time.now assert_equal t.getutc.to_s(:db), @quoter.quoted_date(t) end end def test_quoted_time_local - with_active_record_default_timezone :local do + with_timezone_config default: :local do t = Time.now assert_equal t.getlocal.to_s(:db), @quoter.quoted_date(t) end end def test_quoted_time_crazy - with_active_record_default_timezone :asdfasdf do + with_timezone_config default: :asdfasdf do t = Time.now assert_equal t.getlocal.to_s(:db), @quoter.quoted_date(t) end end def test_quoted_datetime_utc - with_active_record_default_timezone :utc do + with_timezone_config default: :utc do t = DateTime.now assert_equal t.getutc.to_s(:db), @quoter.quoted_date(t) end @@ -83,7 +83,7 @@ module ActiveRecord ### # DateTime doesn't define getlocal, so make sure it does nothing def test_quoted_datetime_local - with_active_record_default_timezone :local do + with_timezone_config default: :local do t = DateTime.now assert_equal t.to_s(:db), @quoter.quoted_date(t) end diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 0e5c7df2cc..d7ad5ed29f 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -18,6 +18,11 @@ require 'models/subscription' require 'models/tag' require 'models/sponsor' require 'models/edge' +require 'models/hotel' +require 'models/chef' +require 'models/department' +require 'models/cake_designer' +require 'models/drink_designer' class ReflectionTest < ActiveRecord::TestCase include ActiveRecord::Reflection @@ -227,6 +232,17 @@ class ReflectionTest < ActiveRecord::TestCase assert_equal expected, actual end + def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case + @hotel = Hotel.create! + @department = @hotel.departments.create! + @department.chefs.create!(employable: CakeDesigner.create!) + @department.chefs.create!(employable: DrinkDesigner.create!) + + assert_equal 1, @hotel.cake_designers.size + assert_equal 1, @hotel.drink_designers.size + assert_equal 2, @hotel.chefs.size + end + def test_nested? assert !Author.reflect_on_association(:comments).nested? assert Author.reflect_on_association(:tags).nested? diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb new file mode 100644 index 0000000000..c171c5e14e --- /dev/null +++ b/activerecord/test/cases/relation/delegation_test.rb @@ -0,0 +1,98 @@ +require 'cases/helper' +require 'models/post' +require 'models/comment' + +module ActiveRecord + class DelegationTest < ActiveRecord::TestCase + fixtures :posts + + def assert_responds(target, method) + assert target.respond_to?(method) + assert_nothing_raised do + method_arity = target.to_a.method(method).arity + + if method_arity.zero? + target.send(method) + elsif method_arity < 0 + if method == :shuffle! + target.send(method) + else + target.send(method, 1) + end + else + raise NotImplementedError + end + end + end + end + + class DelegationAssociationTest < DelegationTest + def target + Post.first.comments + end + + [:map, :collect].each do |method| + test "##{method} is delgated" do + assert_responds(target, method) + assert_equal(target.pluck(:body), target.send(method) {|post| post.body }) + end + + test "##{method}! is not delgated" do + assert_deprecated do + assert_responds(target, "#{method}!") + end + end + end + + [:compact!, :flatten!, :reject!, :reverse!, :rotate!, + :shuffle!, :slice!, :sort!, :sort_by!].each do |method| + test "##{method} delegation is deprecated" do + assert_deprecated do + assert_responds(target, method) + end + end + end + + [:select!, :uniq!].each do |method| + test "##{method} is implemented" do + assert_responds(target, method) + end + end + end + + class DelegationRelationTest < DelegationTest + def target + Comment.where.not(body: nil) + end + + [:map, :collect].each do |method| + test "##{method} is delgated" do + assert_responds(target, method) + assert_equal(target.pluck(:body), target.send(method) {|post| post.body }) + end + + test "##{method}! is not delgated" do + assert_deprecated do + assert_responds(target, "#{method}!") + end + end + end + + [:compact!, :flatten!, :reject!, :reverse!, :rotate!, + :shuffle!, :slice!, :sort!, :sort_by!].each do |method| + test "##{method} delegation is deprecated" do + assert_deprecated do + assert_responds(target, method) + end + end + end + + [:select!, :uniq!].each do |method| + test "##{method} is triggers an immutable error" do + assert_raises ActiveRecord::ImmutableRelation do + assert_responds(target, method) + end + end + end + end +end diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb new file mode 100644 index 0000000000..020fb24afa --- /dev/null +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -0,0 +1,148 @@ +require 'cases/helper' +require 'models/post' + +module ActiveRecord + class RelationMutationTest < ActiveSupport::TestCase + class FakeKlass < Struct.new(:table_name, :name) + extend ActiveRecord::Delegation::DelegateCache + inherited self + + def arel_table + Post.arel_table + end + + def connection + Post.connection + end + + def relation_delegate_class(klass) + self.class.relation_delegate_class(klass) + end + end + + def relation + @relation ||= Relation.new FakeKlass.new('posts'), :b + end + + (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order]).each do |method| + test "##{method}!" do + assert relation.public_send("#{method}!", :foo).equal?(relation) + assert_equal [:foo], relation.public_send("#{method}_values") + end + end + + test '#order!' do + assert relation.order!('name ASC').equal?(relation) + assert_equal ['name ASC'], relation.order_values + end + + test '#order! with symbol prepends the table name' do + assert relation.order!(:name).equal?(relation) + node = relation.order_values.first + assert node.ascending? + assert_equal :name, node.expr.name + assert_equal "posts", node.expr.relation.name + end + + test '#order! on non-string does not attempt regexp match for references' do + obj = Object.new + obj.expects(:=~).never + assert relation.order!(obj) + assert_equal [obj], relation.order_values + end + + test '#references!' do + assert relation.references!(:foo).equal?(relation) + assert relation.references_values.include?('foo') + end + + test 'extending!' do + mod, mod2 = Module.new, Module.new + + assert relation.extending!(mod).equal?(relation) + assert_equal [mod], relation.extending_values + assert relation.is_a?(mod) + + relation.extending!(mod2) + assert_equal [mod, mod2], relation.extending_values + end + + test 'extending! with empty args' do + relation.extending! + assert_equal [], relation.extending_values + end + + (Relation::SINGLE_VALUE_METHODS - [:from, :lock, :reordering, :reverse_order, :create_with]).each do |method| + test "##{method}!" do + assert relation.public_send("#{method}!", :foo).equal?(relation) + assert_equal :foo, relation.public_send("#{method}_value") + end + end + + test '#from!' do + assert relation.from!('foo').equal?(relation) + assert_equal ['foo', nil], relation.from_value + end + + test '#lock!' do + assert relation.lock!('foo').equal?(relation) + assert_equal 'foo', relation.lock_value + end + + test '#reorder!' do + relation = self.relation.order('foo') + + assert relation.reorder!('bar').equal?(relation) + assert_equal ['bar'], relation.order_values + assert relation.reordering_value + end + + test '#reorder! with symbol prepends the table name' do + assert relation.reorder!(:name).equal?(relation) + node = relation.order_values.first + + assert node.ascending? + assert_equal :name, node.expr.name + assert_equal "posts", node.expr.relation.name + end + + test 'reverse_order!' do + assert relation.reverse_order!.equal?(relation) + assert relation.reverse_order_value + relation.reverse_order! + assert !relation.reverse_order_value + end + + test 'create_with!' do + assert relation.create_with!(foo: 'bar').equal?(relation) + assert_equal({foo: 'bar'}, relation.create_with_value) + end + + test 'test_merge!' do + assert relation.merge!(where: :foo).equal?(relation) + assert_equal [:foo], relation.where_values + end + + test 'merge with a proc' do + assert_equal [:foo], relation.merge(-> { where(:foo) }).where_values + end + + test 'none!' do + assert relation.none!.equal?(relation) + assert_equal [NullRelation], relation.extending_values + assert relation.is_a?(NullRelation) + end + + test 'distinct!' do + relation.distinct! :foo + assert_equal :foo, relation.distinct_value + assert_equal :foo, relation.uniq_value # deprecated access + end + + test 'uniq! was replaced by distinct!' do + relation.uniq! :foo + assert_equal :foo, relation.distinct_value + assert_equal :foo, relation.uniq_value # deprecated access + end + end +end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index f99801c437..70d113fb39 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -9,6 +9,10 @@ module ActiveRecord fixtures :posts, :comments, :authors class FakeKlass < Struct.new(:table_name, :name) + extend ActiveRecord::Delegation::DelegateCache + + inherited self + def self.connection Post.connection end @@ -76,8 +80,8 @@ module ActiveRecord end def test_table_name_delegates_to_klass - relation = Relation.new FakeKlass.new('foo'), :b - assert_equal 'foo', relation.table_name + relation = Relation.new FakeKlass.new('posts'), :b + assert_equal 'posts', relation.table_name end def test_scope_for_create @@ -177,14 +181,26 @@ module ActiveRecord end test 'merging a hash interpolates conditions' do - klass = stub_everything - klass.stubs(:sanitize_sql).with(['foo = ?', 'bar']).returns('foo = bar') + klass = Class.new(FakeKlass) do + def self.sanitize_sql(args) + raise unless args == ['foo = ?', 'bar'] + 'foo = bar' + end + end relation = Relation.new(klass, :b) relation.merge!(where: ['foo = ?', 'bar']) assert_equal ['foo = bar'], relation.where_values end + def test_merging_readonly_false + relation = Relation.new FakeKlass, :b + readonly_false_relation = relation.readonly(false) + # test merging in both directions + assert_equal false, relation.merge(readonly_false_relation).readonly_value + assert_equal false, readonly_false_relation.merge(relation).readonly_value + end + def test_relation_merging_with_merged_joins_as_symbols special_comments_with_ratings = SpecialComment.joins(:ratings) posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings) @@ -207,132 +223,4 @@ module ActiveRecord end end - - class RelationMutationTest < ActiveSupport::TestCase - class FakeKlass < Struct.new(:table_name, :name) - def arel_table - Post.arel_table - end - - def connection - Post.connection - end - end - - def relation - @relation ||= Relation.new FakeKlass.new('posts'), :b - end - - (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order]).each do |method| - test "##{method}!" do - assert relation.public_send("#{method}!", :foo).equal?(relation) - assert_equal [:foo], relation.public_send("#{method}_values") - end - end - - test "#order!" do - assert relation.order!('name ASC').equal?(relation) - assert_equal ['name ASC'], relation.order_values - end - - test "#order! with symbol prepends the table name" do - assert relation.order!(:name).equal?(relation) - node = relation.order_values.first - assert node.ascending? - assert_equal :name, node.expr.name - assert_equal "posts", node.expr.relation.name - end - - test "#order! on non-string does not attempt regexp match for references" do - obj = Object.new - obj.expects(:=~).never - assert relation.order!(obj) - assert_equal [obj], relation.order_values - end - - test '#references!' do - assert relation.references!(:foo).equal?(relation) - assert relation.references_values.include?('foo') - end - - test 'extending!' do - mod, mod2 = Module.new, Module.new - - assert relation.extending!(mod).equal?(relation) - assert_equal [mod], relation.extending_values - assert relation.is_a?(mod) - - relation.extending!(mod2) - assert_equal [mod, mod2], relation.extending_values - end - - test 'extending! with empty args' do - relation.extending! - assert_equal [], relation.extending_values - end - - (Relation::SINGLE_VALUE_METHODS - [:from, :lock, :reordering, :reverse_order, :create_with]).each do |method| - test "##{method}!" do - assert relation.public_send("#{method}!", :foo).equal?(relation) - assert_equal :foo, relation.public_send("#{method}_value") - end - end - - test '#from!' do - assert relation.from!('foo').equal?(relation) - assert_equal ['foo', nil], relation.from_value - end - - test '#lock!' do - assert relation.lock!('foo').equal?(relation) - assert_equal 'foo', relation.lock_value - end - - test '#reorder!' do - relation = self.relation.order('foo') - - assert relation.reorder!('bar').equal?(relation) - assert_equal ['bar'], relation.order_values - assert relation.reordering_value - end - - test 'reverse_order!' do - assert relation.reverse_order!.equal?(relation) - assert relation.reverse_order_value - relation.reverse_order! - assert !relation.reverse_order_value - end - - test 'create_with!' do - assert relation.create_with!(foo: 'bar').equal?(relation) - assert_equal({foo: 'bar'}, relation.create_with_value) - end - - def test_merge! - assert relation.merge!(where: :foo).equal?(relation) - assert_equal [:foo], relation.where_values - end - - test 'merge with a proc' do - assert_equal [:foo], relation.merge(-> { where(:foo) }).where_values - end - - test 'none!' do - assert relation.none!.equal?(relation) - assert_equal [NullRelation], relation.extending_values - assert relation.is_a?(NullRelation) - end - - test "distinct!" do - relation.distinct! :foo - assert_equal :foo, relation.distinct_value - assert_equal :foo, relation.uniq_value # deprecated access - end - - test "uniq! was replaced by distinct!" do - relation.uniq! :foo - assert_equal :foo, relation.distinct_value - assert_equal :foo, relation.uniq_value # deprecated access - end - end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index e1a760d240..c9c7ac04b3 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -42,6 +42,11 @@ class RelationTest < ActiveRecord::TestCase end def test_two_scopes_with_includes_should_not_drop_any_include + # heat habtm cache + car = Car.incl_engines.incl_tyres.first + car.tyres.length + car.engines.length + car = Car.incl_engines.incl_tyres.first assert_no_queries { car.tyres.length } assert_no_queries { car.engines.length } @@ -139,6 +144,13 @@ class RelationTest < ActiveRecord::TestCase assert_equal relation.to_a, Topic.select('a.*').from(relation, :a).to_a end + def test_finding_with_subquery_with_binds + relation = Post.first.comments + assert_equal relation.to_a, Comment.select('*').from(relation).to_a + assert_equal relation.to_a, Comment.select('subquery.*').from(relation).to_a + assert_equal relation.to_a, Comment.select('a.*').from(relation, :a).to_a + end + def test_finding_with_conditions assert_equal ["David"], Author.where(:name => 'David').map(&:name) assert_equal ['Mary'], Author.where(["name = ?", 'Mary']).map(&:name) @@ -262,7 +274,7 @@ class RelationTest < ActiveRecord::TestCase def test_none_chained_to_methods_firing_queries_straight_to_db assert_no_queries do - assert_equal [], Developer.none.pluck(:id) # => uses select_all + assert_equal [], Developer.none.pluck(:id, :name) assert_equal 0, Developer.none.delete_all assert_equal 0, Developer.none.update_all(:name => 'David') assert_equal 0, Developer.none.delete(1) @@ -283,7 +295,7 @@ class RelationTest < ActiveRecord::TestCase def test_null_relation_calculations_methods assert_no_queries do assert_equal 0, Developer.none.count - assert_equal 0, Developer.none.calculate(:count, nil, {}) + assert_equal 0, Developer.none.calculate(:count, nil) assert_equal nil, Developer.none.calculate(:average, 'salary') end end @@ -293,6 +305,10 @@ class RelationTest < ActiveRecord::TestCase assert_equal({}, Developer.none.where_values_hash) end + def test_null_relation_where_values_hash + assert_equal({ 'salary' => 100_000 }, Developer.none.where(salary: 100_000).where_values_hash) + end + def test_joins_with_nil_argument assert_nothing_raised { DependentFirm.joins(nil).first } end @@ -470,6 +486,14 @@ class RelationTest < ActiveRecord::TestCase assert_equal Developer.where(name: 'David').map(&:id).sort, developers end + def test_includes_with_select + query = Post.select('comments_count AS ranking').order('ranking').includes(:comments) + .where(comments: { id: 1 }) + + assert_equal ['comments_count AS ranking'], query.select_values + assert_equal 1, query.to_a.size + end + def test_loading_with_one_association posts = Post.preload(:comments) post = posts.find { |p| p.id == 1 } @@ -610,6 +634,36 @@ class RelationTest < ActiveRecord::TestCase relation = Author.where(:id => Author.where(:id => david.id)) assert_equal [david], relation.to_a } + + assert_queries(1) { + relation = Author.where('id in (?)', Author.where(id: david).select(:id)) + assert_equal [david], relation.to_a + } + + assert_queries(1) do + relation = Author.where('id in (:author_ids)', author_ids: Author.where(id: david).select(:id)) + assert_equal [david], relation.to_a + end + end + + def test_find_all_using_where_with_relation_with_bound_values + david = authors(:david) + davids_posts = david.posts.order(:id).to_a + + assert_queries(1) do + relation = Post.where(id: david.posts.select(:id)) + assert_equal davids_posts, relation.order(:id).to_a + end + + assert_queries(1) do + relation = Post.where('id in (?)', david.posts.select(:id)) + assert_equal davids_posts, relation.order(:id).to_a, 'should process Relation as bind variables' + end + + assert_queries(1) do + relation = Post.where('id in (:post_ids)', post_ids: david.posts.select(:id)) + assert_equal davids_posts, relation.order(:id).to_a, 'should process Relation as named bind variables' + end end def test_find_all_using_where_with_relation_and_alternate_primary_key @@ -1344,6 +1398,24 @@ class RelationTest < ActiveRecord::TestCase assert_equal [], scope.references_values end + def test_automatically_added_reorder_references + scope = Post.reorder('comments.body') + assert_equal %w(comments), scope.references_values + + scope = Post.reorder('comments.body', 'yaks.body') + assert_equal %w(comments yaks), scope.references_values + + # Don't infer yaks, let's not go down that road again... + scope = Post.reorder('comments.body, yaks.body') + assert_equal %w(comments), scope.references_values + + scope = Post.reorder('comments.body asc') + assert_equal %w(comments), scope.references_values + + scope = Post.reorder('foo(comments.body)') + assert_equal [], scope.references_values + end + def test_presence topics = Topic.all diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index 082570c55b..766b2ff2ef 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -1,5 +1,7 @@ require "cases/helper" require 'models/binary' +require 'models/author' +require 'models/post' class SanitizeTest < ActiveRecord::TestCase def setup @@ -9,7 +11,7 @@ class SanitizeTest < ActiveRecord::TestCase quoted_bambi = ActiveRecord::Base.connection.quote("Bambi") quoted_column_name = ActiveRecord::Base.connection.quote_column_name("name") quoted_table_name = ActiveRecord::Base.connection.quote_table_name("adorable_animals") - expected_value = "#{quoted_table_name}.#{quoted_column_name} = #{quoted_bambi}" + expected_value = "#{quoted_table_name}.#{quoted_column_name} = #{quoted_bambi}" assert_equal expected_value, Binary.send(:sanitize_sql_hash, {adorable_animals: {name: 'Bambi'}}) end @@ -31,4 +33,17 @@ class SanitizeTest < ActiveRecord::TestCase assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper"]) assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper".mb_chars]) end + + def test_sanitize_sql_array_handles_relations + david = Author.create!(name: 'David') + david_posts = david.posts.select(:id) + + sub_query_pattern = /\(\bselect\b.*?\bwhere\b.*?\)/i + + select_author_sql = Post.send(:sanitize_sql_array, ['id in (?)', david_posts]) + assert_match(sub_query_pattern, select_author_sql, 'should sanitize `Relation` as subquery for bind variables') + + select_author_sql = Post.send(:sanitize_sql_array, ['id in (:post_ids)', post_ids: david_posts]) + assert_match(sub_query_pattern, select_author_sql, 'should sanitize `Relation` as subquery for named bind variables') + end end diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index a48ae1036f..1ee8e60924 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -202,6 +202,11 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_match %r(primary_key: "movieid"), match[1], "non-standard primary key not preserved" end + def test_schema_dump_should_use_false_as_default + output = standard_dump + assert_match %r{t\.boolean\s+"has_fun",.+default: false}, output + end + if current_adapter?(:MysqlAdapter, :Mysql2Adapter) def test_schema_dump_should_not_add_default_value_for_mysql_text_field output = standard_dump @@ -299,7 +304,7 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dump_includes_uuid_shorthand_definition output = standard_dump - if %r{create_table "poistgresql_uuids"} =~ output + if %r{create_table "postgresql_uuids"} =~ output assert_match %r{t.uuid "guid"}, output end end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index cd7d91ff85..76f395ba83 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -122,17 +122,25 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_unscope_with_where_attributes - expected = Developer.order('salary DESC').collect { |dev| dev.name } - received = DeveloperOrderedBySalary.where(name: 'David').unscope(where: :name).collect { |dev| dev.name } + expected = Developer.order('salary DESC').collect(&:name) + received = DeveloperOrderedBySalary.where(name: 'David').unscope(where: :name).collect(&:name) assert_equal expected, received - expected_2 = Developer.order('salary DESC').collect { |dev| dev.name } - received_2 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope({:where => :name}, :select).collect { |dev| dev.name } + expected_2 = Developer.order('salary DESC').collect(&:name) + received_2 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope({:where => :name}, :select).collect(&:name) assert_equal expected_2, received_2 - expected_3 = Developer.order('salary DESC').collect { |dev| dev.name } - received_3 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope(:select, :where).collect { |dev| dev.name } + expected_3 = Developer.order('salary DESC').collect(&:name) + received_3 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope(:select, :where).collect(&:name) assert_equal expected_3, received_3 + + expected_4 = Developer.order('salary DESC').collect(&:name) + received_4 = DeveloperOrderedBySalary.where.not("name" => "Jamis").unscope(where: :name).collect(&:name) + assert_equal expected_4, received_4 + + expected_5 = Developer.order('salary DESC').collect(&:name) + received_5 = DeveloperOrderedBySalary.where.not("name" => ["Jamis", "David"]).unscope(where: :name).collect(&:name) + assert_equal expected_5, received_5 end def test_unscope_multiple_where_clauses diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index b49c27bc78..bc67da8d27 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -211,16 +211,15 @@ class SerializedAttributeTest < ActiveRecord::TestCase end def test_serialize_attribute_via_select_method_when_time_zone_available - ActiveRecord::Base.time_zone_aware_attributes = true - Topic.serialize(:content, MyObject) + with_timezone_config aware_attributes: true do + Topic.serialize(:content, MyObject) - myobj = MyObject.new('value1', 'value2') - topic = Topic.create(content: myobj) + myobj = MyObject.new('value1', 'value2') + topic = Topic.create(content: myobj) - assert_equal(myobj, Topic.select(:content).find(topic.id).content) - assert_raise(ActiveModel::MissingAttributeError) { Topic.select(:id).find(topic.id).content } - ensure - ActiveRecord::Base.time_zone_aware_attributes = false + assert_equal(myobj, Topic.select(:content).find(topic.id).content) + assert_raise(ActiveModel::MissingAttributeError) { Topic.select(:id).find(topic.id).content } + end end def test_serialize_attribute_can_be_serialized_in_an_integer_column @@ -243,10 +242,7 @@ class SerializedAttributeTest < ActiveRecord::TestCase myobj = MyObject.new('value1', 'value2') Topic.create(content: myobj) Topic.create(content: myobj) - - Topic.all.each do |topic| - type = topic.instance_variable_get("@columns_hash")["content"] - assert !type.instance_variable_get("@column").is_a?(ActiveRecord::AttributeMethods::Serialization::Type) - end + type = Topic.column_types["content"] + assert !type.instance_variable_get("@column").is_a?(ActiveRecord::AttributeMethods::Serialization::Type) end end diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb index c2c56abacd..0c9f7ccd55 100644 --- a/activerecord/test/cases/store_test.rb +++ b/activerecord/test/cases/store_test.rb @@ -150,4 +150,16 @@ class StoreTest < ActiveRecord::TestCase test "all stored attributes are returned" do assert_equal [:color, :homepage, :favorite_food], Admin::User.stored_attributes[:settings] end + + test "stored_attributes are tracked per class" do + first_model = Class.new(ActiveRecord::Base) do + store_accessor :data, :color + end + second_model = Class.new(ActiveRecord::Base) do + store_accessor :data, :width, :height + end + + assert_equal [:color], first_model.stored_attributes[:data] + assert_equal [:width, :height], second_model.stored_attributes[:data] + end end diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 816bd62751..bdcf31043a 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -280,6 +280,15 @@ module ActiveRecord assert_match(/Could not dump the database structure/, warnings) end + + def test_structure_dump_with_port_number + filename = "awesome-file.sql" + Kernel.expects(:system).with("mysqldump", "--port", "10000", "--result-file", filename, "--no-data", "test-db").returns(true) + + ActiveRecord::Tasks::DatabaseTasks.structure_dump( + @configuration.merge('port' => 10000), + filename) + end end class MySQLStructureLoadTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index f31896bc7f..90dac6399d 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -231,6 +231,13 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename) end + + def test_structure_load_accepts_path_with_spaces + filename = "awesome file.sql" + Kernel.expects(:system).with("psql -q -f awesome\\ file.sql my-app-db") + + ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename) + end end end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index a5cb22aaf6..980981903a 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -375,6 +375,36 @@ class TransactionTest < ActiveRecord::TestCase assert_equal "Three", @three end if Topic.connection.supports_savepoints? + def test_using_named_savepoints + Topic.transaction do + @first.approved = true + @first.save! + Topic.connection.create_savepoint("first") + + @first.approved = false + @first.save! + Topic.connection.rollback_to_savepoint("first") + assert @first.reload.approved? + + @first.approved = false + @first.save! + Topic.connection.release_savepoint("first") + assert_not @first.reload.approved? + end + end if Topic.connection.supports_savepoints? + + def test_releasing_named_savepoints + Topic.transaction do + Topic.connection.create_savepoint("another") + Topic.connection.release_savepoint("another") + + # The savepoint is now gone and we can't remove it again. + assert_raises(ActiveRecord::StatementInvalid) do + Topic.connection.release_savepoint("another") + end + end + end + def test_rollback_when_commit_raises Topic.connection.expects(:begin_db_transaction) Topic.connection.expects(:commit_db_transaction).raises('OH NOES') @@ -391,7 +421,9 @@ class TransactionTest < ActiveRecord::TestCase topic = Topic.new(:title => 'test') topic.freeze e = assert_raise(RuntimeError) { topic.save } - assert_equal "can't modify frozen Hash", e.message + assert_match(/frozen/i, e.message) # Not good enough, but we can't do much + # about it since there is no specific error + # for frozen objects. assert !topic.persisted?, 'not persisted' assert_nil topic.id assert topic.frozen?, 'not frozen' diff --git a/activerecord/test/cases/xml_serialization_test.rb b/activerecord/test/cases/xml_serialization_test.rb index 68fa15de50..78fa2f935a 100644 --- a/activerecord/test/cases/xml_serialization_test.rb +++ b/activerecord/test/cases/xml_serialization_test.rb @@ -161,21 +161,17 @@ end class DefaultXmlSerializationTimezoneTest < ActiveRecord::TestCase def test_should_serialize_datetime_with_timezone - timezone, Time.zone = Time.zone, "Pacific Time (US & Canada)" - - toy = Toy.create(:name => 'Mickey', :updated_at => Time.utc(2006, 8, 1)) - assert_match %r{<updated-at type=\"dateTime\">2006-07-31T17:00:00-07:00</updated-at>}, toy.to_xml - ensure - Time.zone = timezone + with_timezone_config zone: "Pacific Time (US & Canada)" do + toy = Toy.create(:name => 'Mickey', :updated_at => Time.utc(2006, 8, 1)) + assert_match %r{<updated-at type=\"dateTime\">2006-07-31T17:00:00-07:00</updated-at>}, toy.to_xml + end end def test_should_serialize_datetime_with_timezone_reloaded - timezone, Time.zone = Time.zone, "Pacific Time (US & Canada)" - - toy = Toy.create(:name => 'Minnie', :updated_at => Time.utc(2006, 8, 1)).reload - assert_match %r{<updated-at type=\"dateTime\">2006-07-31T17:00:00-07:00</updated-at>}, toy.to_xml - ensure - Time.zone = timezone + with_timezone_config zone: "Pacific Time (US & Canada)" do + toy = Toy.create(:name => 'Minnie', :updated_at => Time.utc(2006, 8, 1)).reload + assert_match %r{<updated-at type=\"dateTime\">2006-07-31T17:00:00-07:00</updated-at>}, toy.to_xml + end end end diff --git a/activerecord/test/cases/yaml_serialization_test.rb b/activerecord/test/cases/yaml_serialization_test.rb index 302913e095..83a710b1b7 100644 --- a/activerecord/test/cases/yaml_serialization_test.rb +++ b/activerecord/test/cases/yaml_serialization_test.rb @@ -5,16 +5,10 @@ class YamlSerializationTest < ActiveRecord::TestCase fixtures :topics def test_to_yaml_with_time_with_zone_should_not_raise_exception - tz = Time.zone - Time.zone = ActiveSupport::TimeZone["Pacific Time (US & Canada)"] - ActiveRecord::Base.time_zone_aware_attributes = true - - topic = Topic.new(:written_on => DateTime.now) - assert_nothing_raised { topic.to_yaml } - - ensure - Time.zone = tz - ActiveRecord::Base.time_zone_aware_attributes = false + with_timezone_config aware_attributes: true, zone: "Pacific Time (US & Canada)" do + topic = Topic.new(:written_on => DateTime.now) + assert_nothing_raised { topic.to_yaml } + end end def test_roundtrip diff --git a/activerecord/test/fixtures/sponsors.yml b/activerecord/test/fixtures/sponsors.yml index bfc6b238b1..2da541c539 100644 --- a/activerecord/test/fixtures/sponsors.yml +++ b/activerecord/test/fixtures/sponsors.yml @@ -8,5 +8,5 @@ boring_club_sponsor_for_groucho: sponsorable_type: Member crazy_club_sponsor_for_groucho: sponsor_club: crazy_club - sponsorable_id: 2 + sponsorable_id: 3 sponsorable_type: Member diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index feb828de31..794d1af43d 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -8,9 +8,7 @@ class Author < ActiveRecord::Base has_many :posts_sorted_by_id_limited, -> { order('posts.id').limit(1) }, :class_name => "Post" has_many :posts_with_categories, -> { includes(:categories) }, :class_name => "Post" has_many :posts_with_comments_and_categories, -> { includes(:comments, :categories).order("posts.id") }, :class_name => "Post" - has_many :posts_containing_the_letter_a, :class_name => "Post" has_many :posts_with_special_categorizations, :class_name => 'PostWithSpecialCategorization' - has_many :posts_with_extension, :class_name => "Post" has_one :post_about_thinking, -> { where("posts.title like '%thinking%'") }, :class_name => 'Post' has_one :post_about_thinking_with_last_comment, -> { where("posts.title like '%thinking%'").includes(:last_comment) }, :class_name => 'Post' has_many :comments, through: :posts do @@ -31,8 +29,14 @@ class Author < ActiveRecord::Base has_many :thinking_posts, -> { where(:title => 'So I was thinking') }, :dependent => :delete_all, :class_name => 'Post' has_many :welcome_posts, -> { where(:title => 'Welcome to the weblog') }, :class_name => 'Post' + has_many :welcome_posts_with_comment, + -> { where(title: 'Welcome to the weblog').where('comments_count = ?', 1) }, + class_name: 'Post' + has_many :welcome_posts_with_comments, + -> { where(title: 'Welcome to the weblog').where(Post.arel_table[:comments_count].gt(0)) }, + class_name: 'Post' + has_many :comments_desc, -> { order('comments.id DESC') }, :through => :posts, :source => :comments - has_many :limited_comments, -> { limit(1) }, :through => :posts, :source => :comments has_many :funky_comments, :through => :posts, :source => :comments has_many :ordered_uniq_comments, -> { distinct.order('comments.id') }, :through => :posts, :source => :comments has_many :ordered_uniq_comments_desc, -> { distinct.order('comments.id DESC') }, :through => :posts, :source => :comments diff --git a/activerecord/test/models/auto_id.rb b/activerecord/test/models/auto_id.rb index d720e2be5e..82c6544bd5 100644 --- a/activerecord/test/models/auto_id.rb +++ b/activerecord/test/models/auto_id.rb @@ -1,4 +1,4 @@ class AutoId < ActiveRecord::Base - def self.table_name () "auto_id_tests" end - def self.primary_key () "auto_id" end + self.table_name = "auto_id_tests" + self.primary_key = "auto_id" end diff --git a/activerecord/test/models/cake_designer.rb b/activerecord/test/models/cake_designer.rb new file mode 100644 index 0000000000..9c57ef573a --- /dev/null +++ b/activerecord/test/models/cake_designer.rb @@ -0,0 +1,3 @@ +class CakeDesigner < ActiveRecord::Base + has_one :chef, as: :employable +end diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb index a14a9febba..6d257dbe7e 100644 --- a/activerecord/test/models/car.rb +++ b/activerecord/test/models/car.rb @@ -1,12 +1,9 @@ class Car < ActiveRecord::Base - has_many :bulbs has_many :funky_bulbs, class_name: 'FunkyBulb', dependent: :destroy has_many :foo_bulbs, -> { where(:name => 'foo') }, :class_name => "Bulb" - has_many :frickinawesome_bulbs, -> { where :frickinawesome => true }, :class_name => "Bulb" has_one :bulb - has_one :frickinawesome_bulb, -> { where :frickinawesome => true }, :class_name => "Bulb" has_many :tyres has_many :engines, :dependent => :destroy diff --git a/activerecord/test/models/chef.rb b/activerecord/test/models/chef.rb new file mode 100644 index 0000000000..67a4e54f06 --- /dev/null +++ b/activerecord/test/models/chef.rb @@ -0,0 +1,3 @@ +class Chef < ActiveRecord::Base + belongs_to :employable, polymorphic: true +end diff --git a/activerecord/test/models/citation.rb b/activerecord/test/models/citation.rb index 545aa8110d..3d87eb795c 100644 --- a/activerecord/test/models/citation.rb +++ b/activerecord/test/models/citation.rb @@ -1,6 +1,3 @@ class Citation < ActiveRecord::Base belongs_to :reference_of, :class_name => "Book", :foreign_key => :book2_id - - belongs_to :book1, :class_name => "Book", :foreign_key => :book1_id - belongs_to :book2, :class_name => "Book", :foreign_key => :book2_id end diff --git a/activerecord/test/models/club.rb b/activerecord/test/models/club.rb index 816c5e6937..566e0873f1 100644 --- a/activerecord/test/models/club.rb +++ b/activerecord/test/models/club.rb @@ -2,7 +2,6 @@ class Club < ActiveRecord::Base has_one :membership has_many :memberships, :inverse_of => false has_many :members, :through => :memberships - has_many :current_memberships has_one :sponsor has_one :sponsored_member, :through => :sponsor, :source => :sponsorable, :source_type => "Member" belongs_to :category diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index c5d4ec0833..0b0b304121 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -11,6 +11,11 @@ class Company < AbstractCompany has_many :contracts has_many :developers, :through => :contracts + scope :of_first_firm, lambda { + joins(:account => :firm). + where('firms.id' => 1) + } + def arbitrary_method "I am Jack's profound disappointment" end @@ -39,7 +44,7 @@ class Firm < Company has_many :unsorted_clients, :class_name => "Client" has_many :unsorted_clients_with_symbol, :class_name => :Client has_many :clients_sorted_desc, -> { order "id DESC" }, :class_name => "Client" - has_many :clients_of_firm, -> { order "id" }, :foreign_key => "client_of", :class_name => "Client" + has_many :clients_of_firm, -> { order "id" }, :foreign_key => "client_of", :class_name => "Client", :inverse_of => :firm has_many :clients_ordered_by_name, -> { order "name" }, :class_name => "Client" has_many :unvalidated_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :validate => false has_many :dependent_clients_of_firm, -> { order "id" }, :foreign_key => "client_of", :class_name => "Client", :dependent => :destroy @@ -49,7 +54,6 @@ class Firm < Company has_many :clients_like_ms, -> { where("name = 'Microsoft'").order("id") }, :class_name => "Client" has_many :clients_like_ms_with_hash_conditions, -> { where(:name => 'Microsoft').order("id") }, :class_name => "Client" has_many :plain_clients, :class_name => 'Client' - has_many :readonly_clients, -> { readonly }, :class_name => 'Client' has_many :clients_using_primary_key, :class_name => 'Client', :primary_key => 'name', :foreign_key => 'firm_name' has_many :clients_using_primary_key_with_delete_all, :class_name => 'Client', @@ -118,6 +122,10 @@ class Client < Company has_many :accounts, :through => :firm, :source => :accounts belongs_to :account + validate do + firm + end + class RaisedOnSave < RuntimeError; end attr_accessor :raise_on_save before_save do @@ -167,7 +175,6 @@ class ExclusivelyDependentFirm < Company has_one :account, :foreign_key => "firm_id", :dependent => :delete has_many :dependent_sanitized_conditional_clients_of_firm, -> { order("id").where("name = 'BigShot Inc.'") }, :foreign_key => "client_of", :class_name => "Client", :dependent => :delete_all has_many :dependent_conditional_clients_of_firm, -> { order("id").where("name = ?", 'BigShot Inc.') }, :foreign_key => "client_of", :class_name => "Client", :dependent => :delete_all - has_many :dependent_hash_conditional_clients_of_firm, -> { order("id").where(:name => 'BigShot Inc.') }, :foreign_key => "client_of", :class_name => "Client", :dependent => :delete_all end class SpecialClient < Client diff --git a/activerecord/test/models/contract.rb b/activerecord/test/models/contract.rb index 2cf5aa7a85..cdf7b267b5 100644 --- a/activerecord/test/models/contract.rb +++ b/activerecord/test/models/contract.rb @@ -1,6 +1,7 @@ class Contract < ActiveRecord::Base belongs_to :company belongs_to :developer + belongs_to :firm, :foreign_key => 'company_id' before_save :hi after_save :bye diff --git a/activerecord/test/models/department.rb b/activerecord/test/models/department.rb new file mode 100644 index 0000000000..08004a0ed3 --- /dev/null +++ b/activerecord/test/models/department.rb @@ -0,0 +1,4 @@ +class Department < ActiveRecord::Base + has_many :chefs + belongs_to :hotel +end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index c8e2be580e..a26de55758 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -38,6 +38,8 @@ class Developer < ActiveRecord::Base has_and_belongs_to_many :special_projects, :join_table => 'developers_projects', :association_foreign_key => 'project_id' has_many :audit_logs + has_many :contracts + has_many :firms, :through => :contracts, :source => :firm scope :jamises, -> { where(:name => 'Jamis') } diff --git a/activerecord/test/models/drink_designer.rb b/activerecord/test/models/drink_designer.rb new file mode 100644 index 0000000000..2db968ef11 --- /dev/null +++ b/activerecord/test/models/drink_designer.rb @@ -0,0 +1,3 @@ +class DrinkDesigner < ActiveRecord::Base + has_one :chef, as: :employable +end diff --git a/activerecord/test/models/hotel.rb b/activerecord/test/models/hotel.rb new file mode 100644 index 0000000000..b352cd22f3 --- /dev/null +++ b/activerecord/test/models/hotel.rb @@ -0,0 +1,6 @@ +class Hotel < ActiveRecord::Base + has_many :departments + has_many :chefs, through: :departments + has_many :cake_designers, source_type: 'CakeDesigner', source: :employable, through: :chefs + has_many :drink_designers, source_type: 'DrinkDesigner', source: :employable, through: :chefs +end diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb index cc47c7bc18..72095f9236 100644 --- a/activerecord/test/models/member.rb +++ b/activerecord/test/models/member.rb @@ -2,7 +2,6 @@ class Member < ActiveRecord::Base has_one :current_membership has_one :selected_membership has_one :membership - has_many :fellow_members, :through => :club, :source => :members has_one :club, :through => :current_membership has_one :selected_club, :through => :selected_membership, :source => :club has_one :favourite_club, -> { where "memberships.favourite = ?", true }, :through => :membership, :source => :club diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb index bcbb7e42c5..df7167ee93 100644 --- a/activerecord/test/models/membership.rb +++ b/activerecord/test/models/membership.rb @@ -8,6 +8,11 @@ class CurrentMembership < Membership belongs_to :club end +class SuperMembership < Membership + belongs_to :member, -> { order('members.id DESC') } + belongs_to :club +end + class SelectedMembership < Membership def self.default_scope select("'1' as foo") diff --git a/activerecord/test/models/movie.rb b/activerecord/test/models/movie.rb index 6384b4c801..c441be2bef 100644 --- a/activerecord/test/models/movie.rb +++ b/activerecord/test/models/movie.rb @@ -1,5 +1,3 @@ class Movie < ActiveRecord::Base - def self.primary_key - "movieid" - end + self.primary_key = "movieid" end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 25fb2cffe2..faf539a562 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -65,6 +65,9 @@ class Post < ActiveRecord::Base has_many :author_favorites, :through => :author has_many :author_categorizations, :through => :author, :source => :categorizations has_many :author_addresses, :through => :author + has_many :author_address_extra_with_address, + through: :author_with_address, + source: :author_address_extra has_many :comments_with_interpolated_conditions, ->(p) { where "#{"#{p.aliased_table_name}." rescue ""}body = ?", 'Thank you for the welcome' }, @@ -130,7 +133,6 @@ class Post < ActiveRecord::Base has_many :secure_readers has_many :readers_with_person, -> { includes(:person) }, :class_name => "Reader" has_many :people, :through => :readers - has_many :secure_people, :through => :secure_readers has_many :single_people, :through => :readers has_many :people_with_callbacks, :source=>:person, :through => :readers, :before_add => lambda {|owner, reader| log(:added, :before, reader.first_name) }, diff --git a/activerecord/test/models/project.rb b/activerecord/test/models/project.rb index c094b726b4..7f42a4b1f8 100644 --- a/activerecord/test/models/project.rb +++ b/activerecord/test/models/project.rb @@ -1,7 +1,6 @@ class Project < ActiveRecord::Base has_and_belongs_to_many :developers, -> { distinct.order 'developers.name desc, developers.id desc' } has_and_belongs_to_many :readonly_developers, -> { readonly }, :class_name => "Developer" - has_and_belongs_to_many :selected_developers, -> { distinct.select "developers.*" }, :class_name => "Developer" has_and_belongs_to_many :non_unique_developers, -> { order 'developers.name desc, developers.id desc' }, :class_name => 'Developer' has_and_belongs_to_many :limited_developers, -> { limit 1 }, :class_name => "Developer" has_and_belongs_to_many :developers_named_david, -> { where("name = 'David'").distinct }, :class_name => "Developer" diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 17035bf338..40c8e97fc2 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -34,7 +34,6 @@ class Topic < ActiveRecord::Base has_many :replies, :dependent => :destroy, :foreign_key => "parent_id" has_many :approved_replies, -> { approved }, class_name: 'Reply', foreign_key: "parent_id", counter_cache: 'replies_count' - has_many :replies_with_primary_key, :class_name => "Reply", :dependent => :destroy, :primary_key => "title", :foreign_key => "parent_title" has_many :unique_replies, :dependent => :destroy, :foreign_key => "parent_id" has_many :silly_unique_replies, :dependent => :destroy, :foreign_key => "parent_id" diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 75711673a7..88a686d436 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -787,6 +787,22 @@ ActiveRecord::Schema.define do t.string 'from' end + create_table :hotels, force: true do |t| + end + create_table :departments, force: true do |t| + t.integer :hotel_id + end + create_table :cake_designers, force: true do |t| + end + create_table :drink_designers, force: true do |t| + end + create_table :chefs, force: true do |t| + t.integer :employable_id + t.string :employable_type + t.integer :department_id + end + + except 'SQLite' do # fk_test_has_fk should be before fk_test_has_pk create_table :fk_test_has_fk, :force => true do |t| diff --git a/activerecord/test/schema/sqlite_specific_schema.rb b/activerecord/test/schema/sqlite_specific_schema.rb index e9ddeb32cf..b7aff4f47d 100644 --- a/activerecord/test/schema/sqlite_specific_schema.rb +++ b/activerecord/test/schema/sqlite_specific_schema.rb @@ -1,9 +1,6 @@ ActiveRecord::Schema.define do - # For sqlite 3.1.0+, make a table with an autoincrement column - if supports_autoincrement? - create_table :table_with_autoincrement, :force => true do |t| - t.column :name, :string - end + create_table :table_with_autoincrement, :force => true do |t| + t.column :name, :string end execute "DROP TABLE fk_test_has_fk" rescue nil |