Merge pull request rails#55285 from rails/fxn/deprecated-associations

Deprecated associations

Author: fxn

activerecord/CHANGELOG.md

@@ -1,3 +1,19 @@
+*   Implement support for deprecating associations:
+
+    ```ruby
+    has_many :posts, deprecated: true
+    ```
+
+    With that, Active Record will report any usage of the `posts` association.
+
+    Three reporting modes are supported (`:warn`, `:raise`, and `:notify`), and
+    backtraces can be enabled or disabled. Defaults are `:warn` mode and
+    disabled backtraces.
+
+    Please, check the docs for further details.
+
+    *Xavier Noria*
+
 *   PostgreSQL adapter create DB now supports `locale_provider` and `locale`.
 
     *Bengt-Ove Hollaender*

activerecord/lib/active_record.rb

@@ -26,6 +26,7 @@
 require "active_support"
 require "active_support/rails"
 require "active_support/ordered_options"
+require "active_support/core_ext/array/conversions"
 require "active_model"
 require "arel"
 require "yaml"
@@ -470,6 +471,29 @@ def self.permanent_connection_checkout=(value)
   singleton_class.attr_accessor :generate_secure_token_on
   self.generate_secure_token_on = :create
 
+  def self.deprecated_associations_options=(options)
+    raise ArgumentError, "deprecated_associations_options must be a hash" unless options.is_a?(Hash)
+
+    valid_keys = [:mode, :backtrace]
+
+    invalid_keys = options.keys - valid_keys
+    unless invalid_keys.empty?
+      inflected_key = invalid_keys.size == 1 ? "key" : "keys"
+      raise ArgumentError, "invalid deprecated_associations_options #{inflected_key} #{invalid_keys.map(&:inspect).to_sentence} (valid keys are #{valid_keys.map(&:inspect).to_sentence})"
+    end
+
+    options.each do |key, value|
+      ActiveRecord::Associations::Deprecation.send("#{key}=", value)
+    end
+  end
+
+  def self.deprecated_associations_options
+    {
+      mode: ActiveRecord::Associations::Deprecation.mode,
+      backtrace: ActiveRecord::Associations::Deprecation.backtrace
+    }
+  end
+
   def self.marshalling_format_version
     Marshalling.format_version
   end

activerecord/lib/active_record/associations.rb

@@ -39,6 +39,8 @@ module Builder # :nodoc:
       autoload :AssociationScope
       autoload :DisableJoinsAssociationScope
       autoload :AliasTracker
+
+      autoload :Deprecation
     end
 
     def self.eager_load!
@@ -87,6 +89,14 @@ def association_instance_set(name, association)
         @association_cache[name] = association
       end
 
+      def deprecated_associations_api_guard(association, method_name)
+        Deprecation.guard(association.reflection) { "the method #{method_name} was invoked" }
+      end
+
+      def report_deprecated_association(reflection, context:)
+        Deprecation.report(reflection, context: context)
+      end
+
       # = Active Record \Associations
       #
       # \Associations are a set of macro-like class methods for tying objects together through
@@ -1020,6 +1030,116 @@ def association_instance_set(name, association)
       # associated records themselves, you can always do something along the lines of
       # <tt>person.tasks.each(&:destroy)</tt>.
       #
+      # == Deprecated Associations
+      #
+      # Associations can be marked as deprecated by passing <tt>deprecated: true</tt>:
+      #
+      #     has_many :posts, deprecated: true
+      #
+      # When a deprecated association is used, a warning is issued using the
+      # Active Record logger, though more options are available via
+      # configuration.
+      #
+      # The message includes some context that helps understand the deprecated
+      # usage:
+      #
+      #     The association Author#posts is deprecated, the method post_ids was invoked (...)
+      #     The association Author#posts is deprecated, referenced in query to preload records (...)
+      #
+      # The dots in the examples above would have the application-level spot
+      # where usage occurred, to help locate what triggered the warning. That
+      # location is computed using the Active Record backtrace cleaner.
+      #
+      # === What is considered to be usage?
+      #
+      # * Invocation of any association methods like +posts+, <tt>posts=</tt>,
+      #   etc.
+      #
+      # * If the association accepts nested attributes, assignment to those
+      #   attributes.
+      #
+      # * If the association is a through association and some of its nested
+      #   associations are deprecated, you'll get warnings for them whenever the
+      #   top-level through is used. This is so regardless of whether the
+      #   through itself is deprecated.
+      #
+      # * Execution of queries that refer to the association. Think execution of
+      #   <tt>eager_load(:posts)</tt>, <tt>joins(author: :posts)</tt>, etc.
+      #
+      # * If the association has a +:dependent+ option, destroying the
+      #   associated record issues warnings (because that has a side-effect that
+      #   would not happen if the association was removed).
+      #
+      # * If the association has a +:touch+ option, saving or destroying the
+      #   record issues a warning (because that has a side-effect that would not
+      #   happen if the association was removed).
+      #
+      # === Things that do NOT issue warnings
+      #
+      # The rationale behind most of the following edge cases is that Active
+      # Record accesses associations lazily, when used. Before that, the
+      # reference to the association is basically just a Ruby symbol.
+      #
+      # * If +posts+ is deprecated, <tt>has_many :comments, through: :posts</tt>
+      #   does not warn. Usage of the +comments+ association reports usage of
+      #   +posts+, as we explained above, but the definition of the +has_many+
+      #   itself does not.
+      #
+      # * Similarly, <tt>accepts_nested_attributes_for :posts</tt> does not
+      #   warn. Assignment to the posts attributes warns, as explained above,
+      #   but the +accepts_nested_attributes_for+ call itself does not.
+      #
+      # * Same if an association declares to be inverse of a deprecated one, the
+      #   macro itself does not warn.
+      #
+      # * In the same line, the declaration <tt>validates_associated :posts</tt>
+      #   does not warn by itself, though access is reported when the validation
+      #   runs.
+      #
+      # * Relation query methods like <tt>Author.includes(:posts)</tt> do not
+      #   warn by themselves. At that point, that is a relation that internally
+      #   stores a symbol for later use. As explained in the previous section,
+      #   you get a warning when/if the query is executed.
+      #
+      # * Access to the reflection object of the association as in
+      #   <tt>Author.reflect_on_association(:posts)</tt> or
+      #   <tt>Author.reflect_on_all_associations</tt> does not warn.
+      #
+      # === Configuration
+      #
+      # Reporting deprecated usage can be configured:
+      #
+      #     config.active_record.deprecated_associations_options = { ... }
+      #
+      # If present, this has to be a hash with keys +:mode+ and/or +:backtrace+.
+      #
+      # ==== Mode
+      #
+      # * In +:warn+ mode, usage issues a warning that includes the
+      #   application-level place where the access happened, if any. This is the
+      #   default mode.
+      #
+      # * In +:raise+ mode, usage raises an
+      #   ActiveRecord::DeprecatedAssociationError with a similar message and a
+      #   clean backtrace in the exception object.
+      #
+      # * In +:notify+ mode, a <tt>deprecated_association.active_record</tt>
+      #   Active Support notification is published. The event payload has the
+      #   association reflection (+:reflection+), the application-level location
+      #   (+:location+) where the access happened (a Thread::Backtrace::Location
+      #   object, or +nil+), and a deprecation message (+:message+).
+      #
+      # ==== Backtrace
+      #
+      # If :backtrace is true, warnings include a clean backtrace in the message
+      # and notifications have a +:backtrace+ key in the payload with an array
+      # of clean Thread::Backtrace::Location objects. Exceptions always get a
+      # clean stack trace set.
+      #
+      # Clean backtraces are computed using the Active Record backtrace cleaner.
+      # In Rails applications, that is by the default the same as
+      # <tt>Rails.backtrace_cleaner</tt>.
+      #
       # == Type safety with ActiveRecord::AssociationTypeMismatch
       #
       # If you attempt to assign an object to an association that doesn't match the inferred
@@ -1287,6 +1407,9 @@ module ClassMethods
         #   Defines an {association callback}[rdoc-ref:Associations::ClassMethods@Association+callbacks] that gets triggered <b>before an object is removed</b> from the association collection.
         # [:after_remove]
         #   Defines an {association callback}[rdoc-ref:Associations::ClassMethods@Association+callbacks] that gets triggered <b>after an object is removed</b> from the association collection.
+        # [+:deprecated+]
+        #   If true, marks the association as deprecated. Usage of deprecated associations is reported.
+        #   Please, check the class documentation above for details.
         #
         # Option examples:
         #   has_many :comments, -> { order("posted_on") }
@@ -1484,6 +1607,9 @@ def has_many(name, scope = nil, **options, &extension)
         #   Serves as a composite foreign key. Defines the list of columns to be used to query the associated object.
         #   This is an optional option. By default Rails will attempt to derive the value automatically.
         #   When the value is set the Array size must match associated model's primary key or +query_constraints+ size.
+        # [+:deprecated+]
+        #   If true, marks the association as deprecated. Usage of deprecated associations is reported.
+        #   Please, check the class documentation above for details.
         #
         # Option examples:
         #   has_one :credit_card, dependent: :destroy  # destroys the associated credit card
@@ -1676,6 +1802,9 @@ def has_one(name, scope = nil, **options)
         #   Serves as a composite foreign key. Defines the list of columns to be used to query the associated object.
         #   This is an optional option. By default Rails will attempt to derive the value automatically.
         #   When the value is set the Array size must match associated model's primary key or +query_constraints+ size.
+        # [+:deprecated+]
+        #   If true, marks the association as deprecated. Usage of deprecated associations is reported.
+        #   Please, check the class documentation above for details.
         #
         # Option examples:
         #   belongs_to :firm, foreign_key: "client_of"
@@ -1865,6 +1994,9 @@ def belongs_to(name, scope = nil, **options)
         #   <tt>:autosave</tt> to <tt>true</tt>.
         # [+:strict_loading+]
         #   Enforces strict loading every time an associated record is loaded through this association.
+        # [+:deprecated+]
+        #   If true, marks the association as deprecated. Usage of deprecated associations is reported.
+        #   Please, check the class documentation above for details.
         #
         # Option examples:
         #   has_and_belongs_to_many :projects

activerecord/lib/active_record/associations/builder/association.rb

@@ -19,7 +19,7 @@ class << self
     self.extensions = []
 
     VALID_OPTIONS = [
-      :anonymous_class, :primary_key, :foreign_key, :dependent, :validate, :inverse_of, :strict_loading, :query_constraints
+      :anonymous_class, :primary_key, :foreign_key, :dependent, :validate, :inverse_of, :strict_loading, :query_constraints, :deprecated
     ].freeze # :nodoc:
 
     def self.build(model, name, scope, options, &block)
@@ -102,15 +102,19 @@ def self.define_accessors(model, reflection)
     def self.define_readers(mixin, name)
       mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name}
-          association(:#{name}).reader
+          association = association(:#{name})
+          deprecated_associations_api_guard(association, __method__)
+          association.reader
         end
       CODE
     end
 
     def self.define_writers(mixin, name)
       mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name}=(value)
-          association(:#{name}).writer(value)
+          association = association(:#{name})
+          deprecated_associations_api_guard(association, __method__)
+          association.writer(value)
         end
       CODE
     end
@@ -138,8 +142,15 @@ def self.check_dependent_options(dependent, model)
     end
 
     def self.add_destroy_callbacks(model, reflection)
-      name = reflection.name
-      model.before_destroy(->(o) { o.association(name).handle_dependency })
+      if reflection.deprecated?
+        # If :dependent is set, destroying the record has a side effect that
+        # would no longer happen if the association is removed.
+        model.before_destroy do
+          report_deprecated_association(reflection, context: ":dependent has a side effect here")
+        end
+      end
+
+      model.before_destroy(->(o) { o.association(reflection.name).handle_dependency })
     end
 
     def self.add_after_commit_jobs_callback(model, dependent)

activerecord/lib/active_record/associations/builder/belongs_to.rb

@@ -108,6 +108,14 @@ def self.add_default_callbacks(model, reflection)
     end
 
     def self.add_destroy_callbacks(model, reflection)
+      if reflection.deprecated?
+        # If :dependent is set, destroying the record has some side effect that
+        # would no longer happen if the association is removed.
+        model.before_destroy do
+          report_deprecated_association(reflection, context: ":dependent has a side effect here")
+        end
+      end
+
       model.after_destroy lambda { |o| o.association(reflection.name).handle_dependency }
     end
 
@@ -145,11 +153,15 @@ def self.define_validations(model, reflection)
     def self.define_change_tracking_methods(model, reflection)
       model.generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{reflection.name}_changed?
-          association(:#{reflection.name}).target_changed?
+          association = association(:#{reflection.name})
+          deprecated_associations_api_guard(association, __method__)
+          association.target_changed?
         end
 
         def #{reflection.name}_previously_changed?
-          association(:#{reflection.name}).target_previously_changed?
+          association = association(:#{reflection.name})
+          deprecated_associations_api_guard(association, __method__)
+          association.target_previously_changed?
         end
       CODE
     end

activerecord/lib/active_record/associations/builder/collection_association.rb

@@ -60,7 +60,9 @@ def self.define_readers(mixin, name)
 
       mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name.to_s.singularize}_ids
-          association(:#{name}).ids_reader
+          association = association(:#{name})
+          deprecated_associations_api_guard(association, __method__)
+          association.ids_reader
         end
       CODE
     end
@@ -70,7 +72,9 @@ def self.define_writers(mixin, name)
 
       mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name.to_s.singularize}_ids=(ids)
-          association(:#{name}).ids_writer(ids)
+          association = association(:#{name})
+          deprecated_associations_api_guard(association, __method__)
+          association.ids_writer(ids)
         end
       CODE
     end