diff options
Diffstat (limited to 'activerecord')
43 files changed, 537 insertions, 710 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1a34ed441f..130d0f05d2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,26 @@ ## Rails 4.0.0 (unreleased) ## +* Add STI support to init and building associations. + Allows you to do BaseClass.new(:type => "SubClass") as well as + parent.children.build(:type => "SubClass") or parent.build_child + to initialize an STI subclass. Ensures that the class name is a + valid class and that it is in the ancestors of the super class + that the association is expecting. + + *Jason Rush* + +* Observers was extracted from Active Record as `rails-observers` gem. + + *Rafael Mendonça França* + +* Ensure that associations take a symbol argument. *Steve Klabnik* + +* Fix dirty attribute checks for `TimeZoneConversion` with nil and blank + datetime attributes. Setting a nil datetime to a blank string should not + result in a change being flagged. Fix #8310 + + *Alisdair McDiarmid* + * Prevent mass assignment to the type column of polymorphic associations when using `build` Fix #8265 @@ -452,11 +473,11 @@ *kennyj* -* Use inversed parent for first and last child of has_many association. +* Use inversed parent for first and last child of `has_many` association. *Ravil Bayramgalin* -* Fix Column.microseconds and Column.fast_string_to_date to avoid converting +* Fix `Column.microseconds` and `Column.fast_string_to_time` to avoid converting timestamp seconds to a float, since it occasionally results in inaccuracies with microsecond-precision times. Fixes #7352. diff --git a/activerecord/README.rdoc b/activerecord/README.rdoc index cc8942809c..9fc6785d99 100644 --- a/activerecord/README.rdoc +++ b/activerecord/README.rdoc @@ -80,17 +80,6 @@ A short rundown of some of the major features: {Learn more}[link:classes/ActiveRecord/Callbacks.html] -* Observers that react to changes in a model. - - class CommentObserver < ActiveRecord::Observer - def after_create(comment) # is called just after Comment#save - CommentMailer.new_comment_email('david@loudthinking.com', comment).deliver - end - end - - {Learn more}[link:classes/ActiveRecord/Observer.html] - - * Inheritance hierarchies. class Company < ActiveRecord::Base; end diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 45122539f1..822da84d19 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -45,7 +45,6 @@ module ActiveRecord autoload :Migrator, 'active_record/migration' autoload :ModelSchema autoload :NestedAttributes - autoload :Observer autoload :Persistence autoload :QueryCache autoload :Querying diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 1df876bf62..5c37f42794 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -13,6 +13,8 @@ module ActiveRecord::Associations::Builder end def initialize(model, name, scope, options) + raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) + @model = model @name = name diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 1548e68cea..832b963052 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -34,7 +34,7 @@ module ActiveRecord reload end - CollectionProxy.new(self) + CollectionProxy.new(klass, self) end # Implements the writer method, e.g. foo.items= for Foo.has_many :items diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 07a5d07256..7f9628499c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -30,9 +30,9 @@ module ActiveRecord class CollectionProxy < Relation delegate(*(ActiveRecord::Calculations.public_instance_methods - [:count]), to: :scope) - def initialize(association) #:nodoc: + def initialize(klass, association) #:nodoc: @association = association - super association.klass, association.klass.arel_table + super klass, klass.arel_table merge! association.scope(nullify: false) end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 427c61079a..47a8b576c0 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -34,6 +34,7 @@ module ActiveRecord if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) method_body, line = <<-EOV, __LINE__ + 1 def #{attr_name}=(original_time) + original_time = nil if original_time.blank? time = original_time unless time.acts_like?(:time) time = time.is_a?(String) ? Time.zone.parse(time) : time.to_time rescue time diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 5eacb8f143..aab832c2f7 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -13,6 +13,7 @@ require 'active_support/core_ext/string/behavior' require 'active_support/core_ext/kernel/singleton_class' require 'active_support/core_ext/module/introspection' require 'active_support/core_ext/object/duplicable' +require 'active_support/core_ext/class/subclasses' require 'arel' require 'active_record/errors' require 'active_record/log_subscriber' @@ -320,7 +321,6 @@ module ActiveRecord #:nodoc: # So it's possible to assign a logger to the class through <tt>Base.logger=</tt> which will then be used by all # instances in the current object space. class Base - extend ActiveModel::Observing::ClassMethods extend ActiveModel::Naming extend ActiveSupport::Benchmarkable @@ -348,7 +348,6 @@ module ActiveRecord #:nodoc: include Locking::Pessimistic include AttributeMethods include Callbacks - include ActiveModel::Observing include Timestamp include Associations include ActiveModel::SecurePassword diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index db0db272a6..b5a8011ca4 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -490,6 +490,9 @@ module ActiveRecord # determine the connection pool that they should use. class ConnectionHandler def initialize + # These hashes are keyed by klass.name, NOT klass. Keying them by klass + # alone would lead to memory leaks in development mode as all previous + # instances of the class would stay in memory. @owner_to_pool = Hash.new { |h,k| h[k] = {} } @class_to_pool = Hash.new { |h,k| h[k] = {} } end @@ -508,7 +511,7 @@ module ActiveRecord def establish_connection(owner, spec) @class_to_pool.clear - owner_to_pool[owner] = ConnectionAdapters::ConnectionPool.new(spec) + owner_to_pool[owner.name] = ConnectionAdapters::ConnectionPool.new(spec) end # Returns true if there are any active connections among the connection @@ -554,7 +557,7 @@ module ActiveRecord # can be used as an argument for establish_connection, for easily # re-establishing the connection. def remove_connection(owner) - if pool = owner_to_pool.delete(owner) + if pool = owner_to_pool.delete(owner.name) @class_to_pool.clear pool.automatic_reconnect = false pool.disconnect! @@ -572,13 +575,13 @@ module ActiveRecord # but that's ok since the nil case is not the common one that we wish to optimise # for. def retrieve_connection_pool(klass) - class_to_pool[klass] ||= begin + class_to_pool[klass.name] ||= begin until pool = pool_for(klass) klass = klass.superclass break unless klass <= Base end - class_to_pool[klass] = pool + class_to_pool[klass.name] = pool end end @@ -593,21 +596,21 @@ module ActiveRecord end def pool_for(owner) - owner_to_pool.fetch(owner) { + owner_to_pool.fetch(owner.name) { if ancestor_pool = pool_from_any_process_for(owner) # A connection was established in an ancestor process that must have # subsequently forked. We can't reuse the connection, but we can copy # the specification and establish a new connection with it. establish_connection owner, ancestor_pool.spec else - owner_to_pool[owner] = nil + owner_to_pool[owner.name] = nil end } end def pool_from_any_process_for(owner) - owner_to_pool = @owner_to_pool.values.find { |v| v[owner] } - owner_to_pool && owner_to_pool[owner] + owner_to_pool = @owner_to_pool.values.find { |v| v[owner.name] } + owner_to_pool && owner_to_pool[owner.name] end end 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 4f3eebce7d..c3d15ca929 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -287,7 +287,7 @@ module ActiveRecord # Inserts the given fixture into the table. Overridden in adapters that require # something beyond a simple insert (eg. Oracle). def insert_fixture(fixture, table_name) - columns = Hash[columns(table_name).map { |c| [c.name, c] }] + columns = schema_cache.columns_hash(table_name) key_list = [] value_list = fixture.map do |name, value| diff --git a/activerecord/lib/active_record/connection_adapters/schema_cache.rb b/activerecord/lib/active_record/connection_adapters/schema_cache.rb index aad1f9a7ef..5839d1d3b4 100644 --- a/activerecord/lib/active_record/connection_adapters/schema_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/schema_cache.rb @@ -1,7 +1,7 @@ module ActiveRecord module ConnectionAdapters class SchemaCache - attr_reader :columns, :columns_hash, :primary_keys, :tables, :version + attr_reader :primary_keys, :tables, :version attr_accessor :connection def initialize(conn) @@ -30,6 +30,25 @@ module ActiveRecord end end + # Get the columns for a table + def columns(table = nil) + if table + @columns[table] + else + @columns + end + end + + # Get the columns for a table as a hash, key is the column name + # value is the column object. + def columns_hash(table = nil) + if table + @columns_hash[table] + else + @columns_hash + end + end + # Clears out internal caches def clear! @columns.clear diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb index af772996f1..70683eb731 100644 --- a/activerecord/lib/active_record/explain.rb +++ b/activerecord/lib/active_record/explain.rb @@ -6,11 +6,12 @@ module ActiveRecord base.mattr_accessor :auto_explain_threshold_in_seconds, instance_accessor: false end - # If auto explain is enabled, this method triggers EXPLAIN logging for the - # queries triggered by the block if it takes more than the threshold as a - # whole. That is, the threshold is not checked against each individual - # query, but against the duration of the entire block. This approach is - # convenient for relations. + # If the database adapter supports explain and auto explain is enabled, + # this method triggers EXPLAIN logging for the queries triggered by the + # block if it takes more than the threshold as a whole. That is, the + # threshold is not checked against each individual query, but against the + # duration of the entire block. This approach is convenient for relations. + # # The available_queries_for_explain thread variable collects the queries # to be explained. If the value is nil, it means queries are not being @@ -21,7 +22,7 @@ module ActiveRecord threshold = auto_explain_threshold_in_seconds current = Thread.current - if threshold && current[:available_queries_for_explain].nil? + if connection.supports_explain? && threshold && current[:available_queries_for_explain].nil? begin queries = current[:available_queries_for_explain] = [] start = Time.now diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 7922bbcfa0..c5ad14722e 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -872,11 +872,7 @@ module ActiveRecord end def teardown_fixtures - return unless defined?(ActiveRecord) && !ActiveRecord::Base.configurations.blank? - - unless run_in_transaction? - ActiveRecord::FixtureSet.reset_cache - end + return if ActiveRecord::Base.configurations.blank? # Rollback changes if a transaction is active. if run_in_transaction? @@ -884,7 +880,10 @@ module ActiveRecord connection.rollback_transaction if connection.transaction_open? end @fixture_connections.clear + else + ActiveRecord::FixtureSet.reset_cache end + ActiveRecord::Base.clear_active_connections! end diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index a448fa1f5c..6ab67fdece 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -9,6 +9,19 @@ module ActiveRecord end module ClassMethods + # Determines if one of the attributes passed in is the inheritance column, + # and if the inheritance column is attr accessible, it initializes an + # instance of the given subclass instead of the base class + def new(*args, &block) + if (attrs = args.first).is_a?(Hash) + if subclass = subclass_from_attrs(attrs) + return subclass.new(*args, &block) + end + end + # Delegate to the original .new + super + end + # True if this isn't a concrete subclass needing a STI type condition. def descends_from_active_record? if self == Base @@ -79,15 +92,6 @@ module ActiveRecord store_full_sti_class ? name : name.demodulize end - # Finder methods must instantiate through this method to work with the - # single-table inheritance model that makes it possible to create - # objects of different types from the same table. - def instantiate(record, column_types = {}) - sti_class = find_sti_class(record[inheritance_column]) - column_types = sti_class.decorate_columns(column_types) - sti_class.allocate.init_with('attributes' => record, 'column_types' => column_types) - end - protected # Returns the class type of the record using the current module as a prefix. So descendants of @@ -119,24 +123,33 @@ module ActiveRecord private + # Called by +instantiate+ to decide which class to use for a new + # record instance. For single-table inheritance, we check the record + # for a +type+ column and return the corresponding class. + def discriminate_class_for_record(record) + if using_single_table_inheritance?(record) + find_sti_class(record[inheritance_column]) + else + super + end + end + + def using_single_table_inheritance?(record) + record[inheritance_column].present? && columns_hash.include?(inheritance_column) + end + def find_sti_class(type_name) - if type_name.blank? || !columns_hash.include?(inheritance_column) - self + if store_full_sti_class + ActiveSupport::Dependencies.constantize(type_name) else - begin - if store_full_sti_class - ActiveSupport::Dependencies.constantize(type_name) - else - compute_type(type_name) - end - rescue NameError - raise SubclassNotFound, - "The single-table inheritance mechanism failed to locate the subclass: '#{type_name}'. " + - "This error is raised because the column '#{inheritance_column}' is reserved for storing the class in case of inheritance. " + - "Please rename this column if you didn't intend it to be used for storing the inheritance class " + - "or overwrite #{name}.inheritance_column to use another column for that information." - end + compute_type(type_name) end + rescue NameError + raise SubclassNotFound, + "The single-table inheritance mechanism failed to locate the subclass: '#{type_name}'. " + + "This error is raised because the column '#{inheritance_column}' is reserved for storing the class in case of inheritance. " + + "Please rename this column if you didn't intend it to be used for storing the inheritance class " + + "or overwrite #{name}.inheritance_column to use another column for that information." end def type_condition(table = arel_table) @@ -145,6 +158,19 @@ module ActiveRecord sti_column.in(sti_names) end + + # Detect the subclass from the inheritance column of attrs. If the inheritance column value + # is not self or a valid subclass, raises ActiveRecord::SubclassNotFound + # If this is a StrongParameters hash, and access to inheritance_column is not permitted, + # this will ignore the inheritance column and return nil + def subclass_from_attrs(attrs) + subclass_name = attrs.with_indifferent_access[inheritance_column] + return nil if subclass_name.blank? || subclass_name == self.name + unless subclass = subclasses.detect { |sub| sub.name == subclass_name } + raise ActiveRecord::SubclassNotFound.new("Invalid single-table inheritance type: #{subclass_name} is not a subclass of #{name}") + end + subclass + end end private diff --git a/activerecord/lib/active_record/observer.rb b/activerecord/lib/active_record/observer.rb deleted file mode 100644 index 6b2f6f98a5..0000000000 --- a/activerecord/lib/active_record/observer.rb +++ /dev/null @@ -1,126 +0,0 @@ - -module ActiveRecord - # = Active Record Observer - # - # Observer classes respond to life cycle callbacks to implement trigger-like - # behavior outside the original class. This is a great way to reduce the - # clutter that normally comes when the model class is burdened with - # functionality that doesn't pertain to the core responsibility of the - # class. Example: - # - # class CommentObserver < ActiveRecord::Observer - # def after_save(comment) - # Notifications.comment("admin@do.com", "New comment was posted", comment).deliver - # end - # end - # - # This Observer sends an email when a Comment#save is finished. - # - # class ContactObserver < ActiveRecord::Observer - # def after_create(contact) - # contact.logger.info('New contact added!') - # end - # - # def after_destroy(contact) - # contact.logger.warn("Contact with an id of #{contact.id} was destroyed!") - # end - # end - # - # This Observer uses logger to log when specific callbacks are triggered. - # - # == Observing a class that can't be inferred - # - # Observers will by default be mapped to the class with which they share a name. So CommentObserver will - # be tied to observing Comment, ProductManagerObserver to ProductManager, and so on. If you want to name your observer - # differently than the class you're interested in observing, you can use the Observer.observe class method which takes - # either the concrete class (Product) or a symbol for that class (:product): - # - # class AuditObserver < ActiveRecord::Observer - # observe :account - # - # def after_update(account) - # AuditTrail.new(account, "UPDATED") - # end - # end - # - # If the audit observer needs to watch more than one kind of object, this can be specified with multiple arguments: - # - # class AuditObserver < ActiveRecord::Observer - # observe :account, :balance - # - # def after_update(record) - # AuditTrail.new(record, "UPDATED") - # end - # end - # - # The AuditObserver will now act on both updates to Account and Balance by treating them both as records. - # - # == Available callback methods - # - # The observer can implement callback methods for each of the methods described in the Callbacks module. - # - # == Storing Observers in Rails - # - # If you're using Active Record within Rails, observer classes are usually stored in app/models with the - # naming convention of app/models/audit_observer.rb. - # - # == Configuration - # - # In order to activate an observer, list it in the <tt>config.active_record.observers</tt> configuration - # setting in your <tt>config/application.rb</tt> file. - # - # config.active_record.observers = :comment_observer, :signup_observer - # - # Observers will not be invoked unless you define these in your application configuration. - # - # If you are using Active Record outside Rails, activate the observers explicitly in a configuration or - # environment file: - # - # ActiveRecord::Base.add_observer CommentObserver.instance - # ActiveRecord::Base.add_observer SignupObserver.instance - # - # == Loading - # - # Observers register themselves in the model class they observe, since it is the class that - # notifies them of events when they occur. As a side-effect, when an observer is loaded its - # corresponding model class is loaded. - # - # Up to (and including) Rails 2.0.2 observers were instantiated between plugins and - # application initializers. Now observers are loaded after application initializers, - # so observed models can make use of extensions. - # - # If by any chance you are using observed models in the initialization you can still - # load their observers by calling <tt>ModelObserver.instance</tt> before. Observers are - # singletons and that call instantiates and registers them. - # - class Observer < ActiveModel::Observer - - protected - - def observed_classes - klasses = super - klasses + klasses.map { |klass| klass.descendants }.flatten - end - - def add_observer!(klass) - super - define_callbacks klass - end - - def define_callbacks(klass) - observer = self - observer_name = observer.class.name.underscore.gsub('/', '__') - - ActiveRecord::Callbacks::CALLBACKS.each do |callback| - next unless respond_to?(callback) - callback_meth = :"_notify_#{observer_name}_for_#{callback}" - unless klass.respond_to?(callback_meth) - klass.send(:define_method, callback_meth) do |&block| - observer.update(callback, self, &block) - end - klass.send(callback, callback_meth) - end - end - end - end -end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index eed49e17b1..94c109e72b 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -38,6 +38,32 @@ module ActiveRecord object end end + + # Given an attributes hash, +instantiate+ returns a new instance of + # the appropriate class. + # + # For example, +Post.all+ may return Comments, Messages, and Emails + # by storing the record's subclass in a +type+ attribute. By calling + # +instantiate+ instead of +new+, finder methods ensure they get new + # instances of the appropriate class for each record. + # + # See +ActiveRecord::Inheritance#discriminate_class_for_record+ to see + # how this "single-table" inheritance mapping is implemented. + def instantiate(record, column_types = {}) + klass = discriminate_class_for_record(record) + column_types = klass.decorate_columns(column_types) + klass.allocate.init_with('attributes' => record, 'column_types' => column_types) + end + + private + # Called by +instantiate+ to decide which class to use for a new + # record instance. + # + # See +ActiveRecord::Inheritance#discriminate_class_for_record+ for + # the single-table inheritance discriminator. + def discriminate_class_for_record(record) + self + end end # Returns true if this object hasn't been saved yet -- that is, a record @@ -102,7 +128,7 @@ module ActiveRecord # record's primary key, and no callbacks are executed. # # To enforce the object's +before_destroy+ and +after_destroy+ - # callbacks, Observer methods, or any <tt>:dependent</tt> association + # callbacks or any <tt>:dependent</tt> association # options, use <tt>#destroy</tt>. def delete self.class.delete(id) if persisted? diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 5464ca6066..1081b82bc6 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -115,6 +115,18 @@ module ActiveRecord See http://edgeguides.rubyonrails.org/security.html#mass-assignment for more information EOF end + + unless app.config.active_record.delete(:observers).nil? + ActiveSupport::Deprecation.warn <<-EOF.strip_heredoc, [] + Active Record Observers has been extracted out of Rails into a gem. + Please use callbacks or add `rails-observers` to your Gemfile to use observers. + + To disable this message remove the `observers` option from your + `config/application.rb` or from your initializers. + + See http://edgeguides.rubyonrails.org/4_0_release_notes.html for more information + EOF + end ensure ActiveSupport::Deprecation.behavior = old_behavior end @@ -136,6 +148,13 @@ module ActiveRecord end end + initializer "active_record.validate_explain_support" do |app| + if app.config.active_record[:auto_explain_threshold_in_seconds] && + !ActiveRecord::Base.connection.supports_explain? + warn "auto_explain_threshold_in_seconds is set but will be ignored because your adapter does not support this feature. Please unset the configuration to avoid this warning." + end + end + # Expose database runtime to controller for logging. initializer "active_record.log_runtime" do |app| require "active_record/railties/controller_runtime" @@ -161,15 +180,5 @@ module ActiveRecord path = app.paths["db"].first config.watchable_files.concat ["#{path}/schema.rb", "#{path}/structure.sql"] end - - config.after_initialize do |app| - ActiveSupport.on_load(:active_record) do - instantiate_observers - - ActionDispatch::Reloader.to_prepare do - ActiveRecord::Base.instantiate_observers - end - end - end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 0103de4cbd..bcfcb061f2 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -179,7 +179,7 @@ module ActiveRecord @collection = [:has_many, :has_and_belongs_to_many].include?(macro) end - # Returns a new, unsaved instance of the associated class. +options+ will + # Returns a new, unsaved instance of the associated class. +attributes+ will # be passed to the class's constructor. def build_association(attributes, &block) klass.new(attributes, &block) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 3ee55c580e..0df895eb67 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -31,6 +31,14 @@ module ActiveRecord @default_scoped = false end + def initialize_copy(other) + # This method is a hot spot, so for now, use Hash[] to dup the hash. + # https://bugs.ruby-lang.org/issues/7166 + @values = Hash[@values] + @values[:bind] = @values[:bind].dup if @values.key? :bind + reset + end + def insert(values) primary_key_value = nil @@ -90,14 +98,6 @@ module ActiveRecord scoping { @klass.new(*args, &block) } end - def initialize_copy(other) - # This method is a hot spot, so for now, use Hash[] to dup the hash. - # https://bugs.ruby-lang.org/issues/7166 - @values = Hash[@values] - @values[:bind] = @values[:bind].dup if @values.key? :bind - reset - end - alias build new # Tries to create a new record with the same scoped attributes @@ -315,11 +315,9 @@ module ActiveRecord # Destroys the records matching +conditions+ by instantiating each # record and calling its +destroy+ method. Each object's callbacks are - # executed (including <tt>:dependent</tt> association options and - # +before_destroy+/+after_destroy+ Observer methods). Returns the + # executed (including <tt>:dependent</tt> association options). Returns the # collection of objects that were destroyed; each will be frozen, to - # reflect that no changes should be made (since they can't be - # persisted). + # reflect that no changes should be made (since they can't be persisted). # # Note: Instantiation, callback execution, and deletion of each # record can be time consuming when you're removing many records at @@ -419,8 +417,7 @@ module ActiveRecord # Deletes the row with a primary key matching the +id+ argument, using a # SQL +DELETE+ statement, and returns the number of rows deleted. Active # Record objects are not instantiated, so the object's callbacks are not - # executed, including any <tt>:dependent</tt> association options or - # Observer methods. + # executed, including any <tt>:dependent</tt> association options. # # You can delete multiple rows at once by passing an Array of <tt>id</tt>s. # diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index dbfa92bbbd..2184625e22 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,27 +1,113 @@ -require 'thread' +require 'active_support/concern' +require 'mutex_m' module ActiveRecord module Delegation # :nodoc: - # Set up common delegations for performance (avoids method_missing) + extend ActiveSupport::Concern + + # This module creates compiled delegation methods dynamically at runtime, which makes + # subsequent calls to that method faster by avoiding method_missing. The delegations + # may vary depending on the klass of a relation, so we create a subclass of Relation + # for each different klass, and the delegations are compiled into that subclass only. + delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :to => :to_a delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :auto_explain_threshold_in_seconds, :to => :klass - @@delegation_mutex = Mutex.new + module ClassSpecificRelation + extend ActiveSupport::Concern - def self.delegate_to_scoped_klass(method) - if method.to_s =~ /\A[a-zA-Z_]\w*[!?]?\z/ - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - scoping { @klass.#{method}(*args, &block) } + included do + @delegation_mutex = Mutex.new + end + + module ClassMethods + def name + superclass.name + end + + def delegate_to_scoped_klass(method) + @delegation_mutex.synchronize do + return if method_defined?(method) + + if method.to_s =~ /\A[a-zA-Z_]\w*[!?]?\z/ + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{method}(*args, &block) + scoping { @klass.#{method}(*args, &block) } + end + RUBY + else + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{method}(*args, &block) + scoping { @klass.send(#{method.inspect}, *args, &block) } + end + RUBY + end end - RUBY - else - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - scoping { @klass.send(#{method.inspect}, *args, &block) } + end + + def delegate(method, opts = {}) + @delegation_mutex.synchronize do + return if method_defined?(method) + super end - RUBY + end + end + + protected + + def method_missing(method, *args, &block) + if @klass.respond_to?(method) + self.class.delegate_to_scoped_klass(method) + scoping { @klass.send(method, *args, &block) } + elsif Array.method_defined?(method) + self.class.delegate method, :to => :to_a + to_a.send(method, *args, &block) + elsif arel.respond_to?(method) + self.class.delegate method, :to => :arel + arel.send(method, *args, &block) + else + super + end + end + end + + module ClassMethods + # This hash is keyed by klass.name to avoid memory leaks in development mode + @@subclasses = Hash.new { |h, k| h[k] = {} }.extend(Mutex_m) + + def new(klass, *args) + relation = relation_class_for(klass).allocate + relation.__send__(:initialize, klass, *args) + relation + end + + # 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 + if subclass = @@subclasses.synchronize { @@subclasses[self][klass.name] } + subclass + else + subclass = const_get("#{name.gsub('::', '_')}_#{klass.name.gsub('::', '_')}", false) + @@subclasses.synchronize { @@subclasses[self][klass.name] = subclass } + subclass + end + else + ActiveRecord::Relation + end + end + + # Check const_defined? in case another thread has already defined the constant. + # I am not sure whether this is strictly necessary. + def const_missing(name) + @@subclasses.synchronize { + if const_defined?(name) + const_get(name) + else + const_set(name, Class.new(self) { include ClassSpecificRelation }) + end + } end end @@ -35,28 +121,10 @@ module ActiveRecord def method_missing(method, *args, &block) if @klass.respond_to?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate_to_scoped_klass(method) - end - end - scoping { @klass.send(method, *args, &block) } elsif Array.method_defined?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate method, :to => :to_a - end - end - to_a.send(method, *args, &block) elsif arel.respond_to?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate method, :to => :arel - end - end - arel.send(method, *args, &block) else super diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index b3712b4ad6..a480ddec9e 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -66,8 +66,7 @@ module ActiveRecord args.empty? ? self : spawn.includes!(*args) end - # Like #includes, but modifies the relation in place. - def includes!(*args) + def includes!(*args) # :nodoc: args.reject! {|a| a.blank? } self.includes_values = (includes_values + args).flatten.uniq @@ -84,8 +83,7 @@ module ActiveRecord args.blank? ? self : spawn.eager_load!(*args) end - # Like #eager_load, but modifies relation in place. - def eager_load!(*args) + def eager_load!(*args) # :nodoc: self.eager_load_values += args self end @@ -98,8 +96,7 @@ module ActiveRecord args.blank? ? self : spawn.preload!(*args) end - # Like #preload, but modifies relation in place. - def preload!(*args) + def preload!(*args) # :nodoc: self.preload_values += args self end @@ -116,8 +113,7 @@ module ActiveRecord args.blank? ? self : spawn.references!(*args) end - # Like #references, but modifies relation in place. - def references!(*args) + def references!(*args) # :nodoc: args.flatten! self.references_values = (references_values + args.map!(&:to_s)).uniq @@ -162,8 +158,7 @@ module ActiveRecord end end - # Like #select, but modifies relation in place. - def select!(*fields) + def select!(*fields) # :nodoc: self.select_values += fields.flatten self end @@ -184,8 +179,7 @@ module ActiveRecord args.blank? ? self : spawn.group!(*args) end - # Like #group, but modifies relation in place. - def group!(*args) + def group!(*args) # :nodoc: args.flatten! self.group_values += args @@ -215,8 +209,7 @@ module ActiveRecord args.blank? ? self : spawn.order!(*args) end - # Like #order, but modifies relation in place. - def order!(*args) + def order!(*args) # :nodoc: args.flatten! validate_order_args args @@ -241,8 +234,7 @@ module ActiveRecord args.blank? ? self : spawn.reorder!(*args) end - # Like #reorder, but modifies relation in place. - def reorder!(*args) + def reorder!(*args) # :nodoc: args.flatten! validate_order_args args @@ -259,8 +251,7 @@ module ActiveRecord args.compact.blank? ? self : spawn.joins!(*args.flatten) end - # Like #joins, but modifies relation in place. - def joins!(*args) + def joins!(*args) # :nodoc: self.joins_values += args self end @@ -269,7 +260,7 @@ module ActiveRecord spawn.bind!(value) end - def bind!(value) + def bind!(value) # :nodoc: self.bind_values += [value] self end @@ -386,9 +377,7 @@ module ActiveRecord opts.blank? ? self : spawn.where!(opts, *rest) end - # #where! is identical to #where, except that instead of returning a new relation, it adds - # the condition to the existing relation. - def where!(opts, *rest) + def where!(opts, *rest) # :nodoc: references!(PredicateBuilder.references(opts)) if Hash === opts self.where_values += build_where(opts, rest) @@ -403,8 +392,7 @@ module ActiveRecord opts.blank? ? self : spawn.having!(opts, *rest) end - # Like #having, but modifies relation in place. - def having!(opts, *rest) + def having!(opts, *rest) # :nodoc: references!(PredicateBuilder.references(opts)) if Hash === opts self.having_values += build_where(opts, rest) @@ -420,8 +408,7 @@ module ActiveRecord spawn.limit!(value) end - # Like #limit, but modifies relation in place. - def limit!(value) + def limit!(value) # :nodoc: self.limit_value = value self end @@ -437,8 +424,7 @@ module ActiveRecord spawn.offset!(value) end - # Like #offset, but modifies relation in place. - def offset!(value) + def offset!(value) # :nodoc: self.offset_value = value self end @@ -449,8 +435,7 @@ module ActiveRecord spawn.lock!(locks) end - # Like #lock, but modifies relation in place. - def lock!(locks = true) + def lock!(locks = true) # :nodoc: case locks when String, TrueClass, NilClass self.lock_value = locks || true @@ -494,8 +479,7 @@ module ActiveRecord extending(NullRelation) end - # Like #none, but modifies relation in place. - def none! + def none! # :nodoc: extending!(NullRelation) end @@ -509,8 +493,7 @@ module ActiveRecord spawn.readonly!(value) end - # Like #readonly, but modifies relation in place. - def readonly!(value = true) + def readonly!(value = true) # :nodoc: self.readonly_value = value self end @@ -532,12 +515,7 @@ module ActiveRecord spawn.create_with!(value) end - # Like #create_with but modifies the relation in place. Raises - # +ImmutableRelation+ if the relation has already been loaded. - # - # users = User.all.create_with!(name: 'Oscar') - # users.new.name # => 'Oscar' - def create_with!(value) + def create_with!(value) # :nodoc: self.create_with_value = value ? create_with_value.merge(value) : {} self end @@ -560,7 +538,7 @@ module ActiveRecord end # Like #from, but modifies relation in place. - def from!(value, subquery_name = nil) + def from!(value, subquery_name = nil) # :nodoc: self.from_value = [value, subquery_name] self end @@ -580,7 +558,7 @@ module ActiveRecord end # Like #uniq, but modifies relation in place. - def uniq!(value = true) + def uniq!(value = true) # :nodoc: self.uniq_value = value self end @@ -629,8 +607,7 @@ module ActiveRecord end end - # Like #extending, but modifies relation in place. - def extending!(*modules, &block) + def extending!(*modules, &block) # :nodoc: modules << Module.new(&block) if block_given? self.extending_values += modules.flatten @@ -646,8 +623,7 @@ module ActiveRecord spawn.reverse_order! end - # Like #reverse_order, but modifies relation in place. - def reverse_order! + def reverse_order! # :nodoc: self.reverse_order_value = !reverse_order_value self end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 62dda542ab..352dee3826 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -40,8 +40,7 @@ module ActiveRecord end end - # Like #merge, but applies changes in place. - def merge!(other) + def merge!(other) # :nodoc: if !other.is_a?(Relation) && other.respond_to?(:to_proc) instance_exec(&other) else diff --git a/activerecord/lib/rails/generators/active_record/observer/observer_generator.rb b/activerecord/lib/rails/generators/active_record/observer/observer_generator.rb deleted file mode 100644 index e7445d03a2..0000000000 --- a/activerecord/lib/rails/generators/active_record/observer/observer_generator.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'rails/generators/active_record' - -module ActiveRecord - module Generators # :nodoc: - class ObserverGenerator < Base # :nodoc: - check_class_collision :suffix => "Observer" - - def create_observer_file - template 'observer.rb', File.join('app/models', class_path, "#{file_name}_observer.rb") - end - - hook_for :test_framework - end - end -end diff --git a/activerecord/lib/rails/generators/active_record/observer/templates/observer.rb b/activerecord/lib/rails/generators/active_record/observer/templates/observer.rb deleted file mode 100644 index eaa256a9bd..0000000000 --- a/activerecord/lib/rails/generators/active_record/observer/templates/observer.rb +++ /dev/null @@ -1,4 +0,0 @@ -<% module_namespacing do -%> -class <%= class_name %>Observer < ActiveRecord::Observer -end -<% end -%> diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 5f7825783b..04d0f755b6 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -109,6 +109,34 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal apple.id, citibank.firm_id end + def test_building_the_belonging_object_with_implicit_sti_base_class + account = Account.new + company = account.build_firm + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_belonging_object_with_explicit_sti_base_class + account = Account.new + company = account.build_firm(:type => "Company") + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_belonging_object_with_sti_subclass + account = Account.new + company = account.build_firm(:type => "Firm") + assert_kind_of Firm, company, "Expected #{company.class} to be a Firm" + end + + def test_building_the_belonging_object_with_an_invalid_type + account = Account.new + assert_raise(ActiveRecord::SubclassNotFound) { account.build_firm(:type => "InvalidType") } + end + + def test_building_the_belonging_object_with_an_unrelated_type + account = Account.new + assert_raise(ActiveRecord::SubclassNotFound) { account.build_firm(:type => "Account") } + end + def test_building_the_belonging_object_with_primary_key client = Client.create(:name => "Primary key client") apple = client.build_firm_with_primary_key("name" => "Apple") diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 2ded97582d..d25aca760f 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -144,6 +144,34 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 'defaulty', bulb.name end + def test_building_the_associated_object_with_implicit_sti_base_class + firm = DependentFirm.new + company = firm.companies.build + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_explicit_sti_base_class + firm = DependentFirm.new + company = firm.companies.build(:type => "Company") + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_sti_subclass + firm = DependentFirm.new + company = firm.companies.build(:type => "Client") + assert_kind_of Client, company, "Expected #{company.class} to be a Client" + end + + def test_building_the_associated_object_with_an_invalid_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.companies.build(:type => "Invalid") } + end + + def test_building_the_associated_object_with_an_unrelated_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.companies.build(:type => "Account") } + end + def test_association_keys_bypass_attribute_protection car = Car.create(:name => 'honda') @@ -1579,7 +1607,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [tagging], post.taggings end - def test_build_with_polymotphic_has_many_does_not_allow_to_override_type_and_id + def test_build_with_polymorphic_has_many_does_not_allow_to_override_type_and_id welcome = posts(:welcome) tagging = welcome.taggings.build(:taggable_id => 99, :taggable_type => 'ShouldNotChange') diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index ea1cfa0805..4ed09a3bf7 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -6,6 +6,8 @@ require 'models/ship' require 'models/pirate' require 'models/car' require 'models/bulb' +require 'models/author' +require 'models/post' class HasOneAssociationsTest < ActiveRecord::TestCase self.use_transactional_fixtures = false unless supports_savepoints? @@ -212,6 +214,34 @@ class HasOneAssociationsTest < ActiveRecord::TestCase } end + def test_building_the_associated_object_with_implicit_sti_base_class + firm = DependentFirm.new + company = firm.build_company + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_explicit_sti_base_class + firm = DependentFirm.new + company = firm.build_company(:type => "Company") + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_sti_subclass + firm = DependentFirm.new + company = firm.build_company(:type => "Client") + assert_kind_of Client, company, "Expected #{company.class} to be a Client" + end + + def test_building_the_associated_object_with_an_invalid_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.build_company(:type => "Invalid") } + end + + def test_building_the_associated_object_with_an_unrelated_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.build_company(:type => "Account") } + end + def test_build_and_create_should_not_happen_within_scope pirate = pirates(:blackbeard) scoped_count = pirate.association(:foo_bulb).scope.where_values.count diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index c0f1945cec..d7f25f760e 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -289,6 +289,14 @@ class OverridingAssociationsTest < ActiveRecord::TestCase DifferentPeopleList.reflect_on_association(:has_one) ) end + + def test_requires_symbol_argument + assert_raises ArgumentError do + Class.new(Post) do + belongs_to "author" + end + end + end end class GeneratedMethodsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb b/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb index 3e3d6e2769..1eb9bf60e1 100644 --- a/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb +++ b/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb @@ -10,19 +10,18 @@ module ActiveRecord end def test_in_use? - # FIXME: change to refute in Rails 4.0 / mt - assert !adapter.in_use?, 'adapter is not in use' + refute adapter.in_use?, 'adapter is not in use' assert adapter.lease, 'lease adapter' assert adapter.in_use?, 'adapter is in use' end def test_lease_twice assert adapter.lease, 'should lease adapter' - assert !adapter.lease, 'should not lease adapter' + refute adapter.lease, 'should not lease adapter' end def test_last_use - assert !adapter.last_use + refute adapter.last_use adapter.lease assert adapter.last_use end @@ -31,7 +30,7 @@ module ActiveRecord assert adapter.lease, 'lease adapter' assert adapter.in_use?, 'adapter is in use' adapter.expire - assert !adapter.in_use?, 'adapter is in use' + refute adapter.in_use?, 'adapter is in use' end def test_close @@ -45,7 +44,7 @@ module ActiveRecord # Close should put the adapter back in the pool adapter.close - assert !adapter.in_use? + refute adapter.in_use? assert_equal adapter, pool.connection end diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 2ddabe058f..3e33b30144 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -4,8 +4,8 @@ module ActiveRecord module ConnectionAdapters class ConnectionHandlerTest < ActiveRecord::TestCase def setup - @klass = Class.new(Base) - @subklass = Class.new(@klass) + @klass = Class.new(Base) { def self.name; 'klass'; end } + @subklass = Class.new(@klass) { def self.name; 'subklass'; end } @handler = ConnectionHandler.new @pool = @handler.establish_connection(@klass, Base.connection_pool.spec) @@ -36,13 +36,11 @@ module ActiveRecord end def test_retrieve_connection_pool_uses_superclass_pool_after_subclass_establish_and_remove - @handler.establish_connection 'north america', Base.connection_pool.spec - assert_same @handler.retrieve_connection_pool(@klass), - @handler.retrieve_connection_pool(@subklass) + sub_pool = @handler.establish_connection(@subklass, Base.connection_pool.spec) + assert_same sub_pool, @handler.retrieve_connection_pool(@subklass) @handler.remove_connection @subklass - assert_same @handler.retrieve_connection_pool(@klass), - @handler.retrieve_connection_pool(@subklass) + assert_same @pool, @handler.retrieve_connection_pool(@subklass) end def test_connection_pools diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index d4fc5f204b..55ee066cda 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -12,7 +12,7 @@ class Pirate # Just reopening it, not defining it after_update :check_changes private - # after_save/update in sweepers, observers, and the model itself + # after_save/update and the model itself # can end up checking dirty status and acting on the results def check_changes if self.changed? @@ -203,6 +203,20 @@ class DirtyTest < ActiveRecord::TestCase end end + def test_nullable_datetime_not_marked_as_changed_if_new_value_is_blank + in_time_zone 'Edinburgh' do + target = Class.new(ActiveRecord::Base) + target.table_name = 'topics' + + topic = target.create + assert_nil topic.written_on + + topic.written_on = "" + assert_nil topic.written_on + assert !topic.written_on_changed? + end + end + def test_integer_zero_to_string_zero_not_marked_as_changed pirate = Pirate.new pirate.parrot_id = 0 diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb index 6dce8ccdd1..a7e5fdf709 100644 --- a/activerecord/test/cases/explain_test.rb +++ b/activerecord/test/cases/explain_test.rb @@ -108,6 +108,16 @@ if ActiveRecord::Base.connection.supports_explain? assert_equal expected, base.exec_explain(queries) end + def test_unsupported_connection_adapter + connection.stubs(:supports_explain?).returns(false) + + base.logger.expects(:warn).never + + with_threshold(0) do + Car.where(:name => 'honda').to_a + end + end + def test_silence_auto_explain base.expects(:collecting_sqls_for_explain).never base.logger.expects(:warn).never diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index 9a2172f41e..490b599fb6 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -1,6 +1,7 @@ require 'cases/helper' require 'active_support/core_ext/hash/indifferent_access' require 'models/person' +require 'models/company' class ProtectedParams < ActiveSupport::HashWithIndifferentAccess attr_accessor :permitted @@ -40,6 +41,20 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase assert_equal 'm', person.gender end + def test_forbidden_attributes_cannot_be_used_for_sti_inheritance_column + params = ProtectedParams.new(type: 'Client') + assert_raises(ActiveModel::ForbiddenAttributesError) do + Company.new(params) + end + end + + def test_permitted_attributes_can_be_used_for_sti_inheritance_column + params = ProtectedParams.new(type: 'Client') + params.permit! + person = Company.new(params) + assert_equal person.class, Client + end + def test_regular_hash_should_still_be_used_for_mass_assignment person = Person.new(first_name: 'Guille', gender: 'm') diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 1bff005510..3a315d843b 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -22,8 +22,6 @@ ActiveSupport::Deprecation.debug = true # Connect to the database ARTest.connect -require 'support/mysql' - # Quote "type" if it's a reserved word for the current connection. QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name('type') diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index aab7aa51dd..189066eb41 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -156,6 +156,29 @@ class InheritanceTest < ActiveRecord::TestCase assert_kind_of Cabbage, savoy end + def test_inheritance_new_with_default_class + company = Company.new + assert_equal Company, company.class + end + + def test_inheritance_new_with_base_class + company = Company.new(:type => 'Company') + assert_equal Company, company.class + end + + def test_inheritance_new_with_subclass + firm = Company.new(:type => 'Firm') + assert_equal Firm, firm.class + end + + def test_new_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.new(:type => 'InvalidType') } + end + + def test_new_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.new(:type => 'Account') } + end + def test_inheritance_condition assert_equal 10, Company.count assert_equal 2, Firm.count diff --git a/activerecord/test/cases/lifecycle_test.rb b/activerecord/test/cases/lifecycle_test.rb deleted file mode 100644 index 0b78f2e46b..0000000000 --- a/activerecord/test/cases/lifecycle_test.rb +++ /dev/null @@ -1,256 +0,0 @@ -require 'cases/helper' -require 'models/topic' -require 'models/developer' -require 'models/reply' -require 'models/minimalistic' -require 'models/comment' - -class SpecialDeveloper < Developer; end - -class DeveloperObserver < ActiveRecord::Observer - def calls - @calls ||= [] - end - - def before_save(developer) - calls << developer - end -end - -class SalaryChecker < ActiveRecord::Observer - observe :special_developer - attr_accessor :last_saved - - def before_save(developer) - return developer.salary > 80000 - end - - module Implementation - def after_save(developer) - self.last_saved = developer - end - end - include Implementation - -end - -class TopicaAuditor < ActiveRecord::Observer - observe :topic - - attr_reader :topic - - def after_find(topic) - @topic = topic - end -end - -class TopicObserver < ActiveRecord::Observer - attr_reader :topic - - def after_find(topic) - @topic = topic - end - - # Create an after_save callback, so a notify_observer hook is created - # on :topic. - def after_save(nothing) - end -end - -class MinimalisticObserver < ActiveRecord::Observer - attr_reader :minimalistic - - def after_find(minimalistic) - @minimalistic = minimalistic - end -end - -class MultiObserver < ActiveRecord::Observer - attr_reader :record - - def self.observed_class() [ Topic, Developer ] end - - cattr_reader :last_inherited - @@last_inherited = nil - - def observed_class_inherited_with_testing(subclass) - observed_class_inherited_without_testing(subclass) - @@last_inherited = subclass - end - - alias_method_chain :observed_class_inherited, :testing - - def after_find(record) - @record = record - end -end - -class ValidatedComment < Comment - attr_accessor :callers - - before_validation :record_callers - - after_validation do - record_callers - end - - def record_callers - callers << self.class if callers - end -end - -class ValidatedCommentObserver < ActiveRecord::Observer - attr_accessor :callers - - def after_validation(model) - callers << self.class if callers - end -end - - -class AroundTopic < Topic -end - -class AroundTopicObserver < ActiveRecord::Observer - observe :around_topic - def topic_ids - @topic_ids ||= [] - end - - def around_save(topic) - topic_ids << topic.id - yield(topic) - topic_ids << topic.id - end -end - -class LifecycleTest < ActiveRecord::TestCase - fixtures :topics, :developers, :minimalistics - - def test_before_destroy - topic = Topic.find(1) - assert_difference 'Topic.count', -(1 + topic.replies.size) do - topic.destroy - end - end - - def test_auto_observer - topic_observer = TopicaAuditor.instance - assert_nil TopicaAuditor.observed_class - assert_equal [Topic], TopicaAuditor.observed_classes.to_a - - topic = Topic.find(1) - assert_equal topic.title, topic_observer.topic.title - end - - def test_inferred_auto_observer - topic_observer = TopicObserver.instance - assert_equal Topic, TopicObserver.observed_class - - topic = Topic.find(1) - assert_equal topic.title, topic_observer.topic.title - end - - def test_observing_two_classes - multi_observer = MultiObserver.instance - - topic = Topic.find(1) - assert_equal topic.title, multi_observer.record.title - - developer = Developer.find(1) - assert_equal developer.name, multi_observer.record.name - end - - def test_observing_subclasses - multi_observer = MultiObserver.instance - - developer = SpecialDeveloper.find(1) - assert_equal developer.name, multi_observer.record.name - - klass = Class.new(Developer) - assert_equal klass, multi_observer.last_inherited - - developer = klass.find(1) - assert_equal developer.name, multi_observer.record.name - end - - def test_after_find_can_be_observed_when_its_not_defined_on_the_model - observer = MinimalisticObserver.instance - assert_equal Minimalistic, MinimalisticObserver.observed_class - - minimalistic = Minimalistic.find(1) - assert_equal minimalistic, observer.minimalistic - end - - def test_after_find_can_be_observed_when_its_defined_on_the_model - observer = TopicObserver.instance - assert_equal Topic, TopicObserver.observed_class - - topic = Topic.find(1) - assert_equal topic, observer.topic - end - - def test_invalid_observer - assert_raise(ArgumentError) { Topic.observers = Object.new; Topic.instantiate_observers } - end - - test "model callbacks fire before observers are notified" do - callers = [] - - comment = ValidatedComment.new - comment.callers = ValidatedCommentObserver.instance.callers = callers - - comment.valid? - assert_equal [ValidatedComment, ValidatedComment, ValidatedCommentObserver], callers, - "model callbacks did not fire before observers were notified" - end - - test "able to save developer" do - SalaryChecker.instance # activate - developer = SpecialDeveloper.new :name => 'Roger', :salary => 100000 - assert developer.save, "developer with normal salary failed to save" - end - - test "unable to save developer with low salary" do - SalaryChecker.instance # activate - developer = SpecialDeveloper.new :name => 'Rookie', :salary => 50000 - assert !developer.save, "allowed to save a developer with too low salary" - end - - test "able to call methods defined with included module" do # https://rails.lighthouseapp.com/projects/8994/tickets/6065-activerecordobserver-is-not-aware-of-method-added-by-including-modules - SalaryChecker.instance # activate - developer = SpecialDeveloper.create! :name => 'Roger', :salary => 100000 - assert_equal developer, SalaryChecker.instance.last_saved - end - - test "around filter from observer should accept block" do - observer = AroundTopicObserver.instance - topic = AroundTopic.new - topic.save - assert_nil observer.topic_ids.first - assert_not_nil observer.topic_ids.last - end - - test "able to disable observers" do - observer = DeveloperObserver.instance # activate - observer.calls.clear - - ActiveRecord::Base.observers.disable DeveloperObserver do - Developer.create! :name => 'Ancestor', :salary => 100000 - SpecialDeveloper.create! :name => 'Descendent', :salary => 100000 - end - - assert_equal [], observer.calls - end - - def test_observer_is_called_once - observer = DeveloperObserver.instance # activate - observer.calls.clear - - developer = Developer.create! :name => 'Ancestor', :salary => 100000 - special_developer = SpecialDeveloper.create! :name => 'Descendent', :salary => 100000 - - assert_equal [developer, special_developer], observer.calls - end - -end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 98e278df82..92dc575d37 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -6,26 +6,26 @@ module ActiveRecord class RelationTest < ActiveRecord::TestCase fixtures :posts, :comments - class FakeKlass < Struct.new(:table_name) + class FakeKlass < Struct.new(:table_name, :name) end def test_construction relation = nil assert_nothing_raised do - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b end - assert_equal :a, relation.klass + assert_equal FakeKlass, relation.klass assert_equal :b, relation.table assert !relation.loaded, 'relation is not loaded' end def test_responds_to_model_and_returns_klass - relation = Relation.new :a, :b - assert_equal :a, relation.model + relation = Relation.new FakeKlass, :b + assert_equal FakeKlass, relation.model end def test_initialize_single_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end @@ -33,19 +33,19 @@ module ActiveRecord end def test_multi_value_initialize - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b Relation::MULTI_VALUE_METHODS.each do |method| assert_equal [], relation.send("#{method}_values"), method.to_s end end def test_extensions - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal [], relation.extensions end def test_empty_where_values_hash - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal({}, relation.where_values_hash) relation.where! :hello @@ -79,7 +79,7 @@ module ActiveRecord end def test_scope_for_create - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal({}, relation.scope_for_create) end @@ -110,31 +110,31 @@ module ActiveRecord end def test_empty_eager_loading? - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert !relation.eager_loading? end def test_eager_load_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation.eager_load! :b assert relation.eager_loading? end def test_references_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal [], relation.references_values relation = relation.references(:foo).references(:omg, :lol) assert_equal ['foo', 'omg', 'lol'], relation.references_values end def test_references_values_dont_duplicate - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation = relation.references(:foo).references(:foo) assert_equal ['foo'], relation.references_values end test 'merging a hash into a relation' do - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation = relation.merge where: :lol, readonly: true assert_equal [:lol], relation.where_values @@ -142,7 +142,7 @@ module ActiveRecord end test 'merging an empty hash into a relation' do - assert_equal [], Relation.new(:a, :b).merge({}).where_values + assert_equal [], Relation.new(FakeKlass, :b).merge({}).where_values end test 'merging a hash with unknown keys raises' do @@ -150,7 +150,7 @@ module ActiveRecord end test '#values returns a dup of the values' do - relation = Relation.new(:a, :b).where! :foo + relation = Relation.new(FakeKlass, :b).where! :foo values = relation.values values[:where] = nil @@ -158,18 +158,18 @@ module ActiveRecord end test 'relations can be created with a values hash' do - relation = Relation.new(:a, :b, where: [:foo]) + relation = Relation.new(FakeKlass, :b, where: [:foo]) assert_equal [:foo], relation.where_values end test 'merging a single where value' do - relation = Relation.new(:a, :b) + relation = Relation.new(FakeKlass, :b) relation.merge!(where: :foo) assert_equal [:foo], relation.where_values end test 'merging a hash interpolates conditions' do - klass = stub + klass = stub_everything klass.stubs(:sanitize_sql).with(['foo = ?', 'bar']).returns('foo = bar') relation = Relation.new(klass, :b) @@ -179,8 +179,11 @@ module ActiveRecord end class RelationMutationTest < ActiveSupport::TestCase + class FakeKlass < Struct.new(:table_name, :name) + end + def relation - @relation ||= Relation.new :a, :b + @relation ||= Relation.new FakeKlass, :b end (Relation::MULTI_VALUE_METHODS - [:references, :extending]).each do |method| diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index c34aeaf925..0cd838c0b0 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1452,4 +1452,33 @@ class RelationTest < ActiveRecord::TestCase assert_equal expected, result end end + + test "delegations do not leak to other classes" do + Topic.all.by_lifo + assert Topic.all.class.method_defined?(:by_lifo) + assert !Post.all.respond_to?(:by_lifo) + end + + class OMGTopic < ActiveRecord::Base + self.table_name = 'topics' + + def self.__omg__ + "omgtopic" + end + end + + test "delegations do not clash across classes" do + begin + class ::Array + def __omg__ + "array" + end + end + + assert_equal "array", Topic.all.__omg__ + assert_equal "omgtopic", OMGTopic.all.__omg__ + ensure + Array.send(:remove_method, :__omg__) + end + end end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 961ba8d9ba..2ddc449c12 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -247,87 +247,6 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end -class TransactionObserverCallbacksTest < ActiveRecord::TestCase - self.use_transactional_fixtures = false - fixtures :topics - - class TopicWithObserverAttached < ActiveRecord::Base - self.table_name = :topics - def history - @history ||= [] - end - end - - class TopicWithObserverAttachedObserver < ActiveRecord::Observer - def after_commit(record) - record.history.push "after_commit" - end - - def after_rollback(record) - record.history.push "after_rollback" - end - end - - def test_after_commit_called - assert TopicWithObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithObserverAttached.new - topic.save! - - assert_equal %w{ after_commit }, topic.history - end - - def test_after_rollback_called - assert TopicWithObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithObserverAttached.new - - Topic.transaction do - topic.save! - raise ActiveRecord::Rollback - end - - assert topic.id.nil? - assert !topic.persisted? - assert_equal %w{ after_rollback }, topic.history - end - - class TopicWithManualRollbackObserverAttached < ActiveRecord::Base - self.table_name = :topics - def history - @history ||= [] - end - end - - class TopicWithManualRollbackObserverAttachedObserver < ActiveRecord::Observer - def after_save(record) - record.history.push "after_save" - raise ActiveRecord::Rollback - end - end - - def test_after_save_called_with_manual_rollback - assert TopicWithManualRollbackObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithManualRollbackObserverAttached.new - - assert !topic.save - assert_equal nil, topic.id - assert !topic.persisted? - assert_equal %w{ after_save }, topic.history - end - def test_after_save_called_with_manual_rollback_bang - assert TopicWithManualRollbackObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithManualRollbackObserverAttached.new - - topic.save! - assert_equal nil, topic.id - assert !topic.persisted? - assert_equal %w{ after_save }, topic.history - end -end - class SaveFromAfterCommitBlockTest < ActiveRecord::TestCase self.use_transactional_fixtures = false diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 77f4a2ec87..6935cfb0ea 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -1,5 +1,6 @@ class Author < ActiveRecord::Base has_many :posts + has_one :post has_many :very_special_comments, :through => :posts has_many :posts_with_comments, -> { includes(:comments) }, :class_name => "Post" has_many :popular_grouped_posts, -> { includes(:comments).group("type").having("SUM(comments_count) > 1").select("type") }, :class_name => "Post" diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 17b17724e8..3ca8f69646 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -111,6 +111,7 @@ end class DependentFirm < Company has_one :account, :foreign_key => "firm_id", :dependent => :nullify has_many :companies, :foreign_key => 'client_of', :dependent => :nullify + has_one :company, :foreign_key => 'client_of', :dependent => :nullify end class RestrictedFirm < Company diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index 0cfde83778..ab2a63d3ea 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -152,7 +152,7 @@ _SQL ); _SQL -begin + begin execute <<_SQL CREATE TABLE postgresql_partitioned_table_parent ( id SERIAL PRIMARY KEY, @@ -174,14 +174,14 @@ begin BEFORE INSERT ON postgresql_partitioned_table_parent FOR EACH ROW EXECUTE PROCEDURE partitioned_insert_trigger(); _SQL -rescue ActiveRecord::StatementInvalid => e - if e.message =~ /language "plpgsql" does not exist/ - execute "CREATE LANGUAGE 'plpgsql';" - retry - else - raise e + rescue ActiveRecord::StatementInvalid => e + if e.message =~ /language "plpgsql" does not exist/ + execute "CREATE LANGUAGE 'plpgsql';" + retry + else + raise e + end end -end begin execute <<_SQL @@ -190,9 +190,10 @@ end data xml ); _SQL -rescue #This version of PostgreSQL either has no XML support or is was not compiled with XML support: skipping table + rescue #This version of PostgreSQL either has no XML support or is was not compiled with XML support: skipping table end + # This table is to verify if the :limit option is being ignored for text and binary columns create_table :limitless_fields, force: true do |t| t.binary :binary, limit: 100_000 t.text :text, limit: 100_000 diff --git a/activerecord/test/support/mysql.rb b/activerecord/test/support/mysql.rb deleted file mode 100644 index 7a66415e64..0000000000 --- a/activerecord/test/support/mysql.rb +++ /dev/null @@ -1,11 +0,0 @@ -if defined?(Mysql) - class Mysql - class Error - # This monkey patch fixes annoy warning with mysql-2.8.1.gem when executing testcases. - def errno_with_fix_warnings - silence_warnings { errno_without_fix_warnings } - end - alias_method_chain :errno, :fix_warnings - end - end -end |