Best Way To Manage Mutable Public Rails Return Types

Our team ran into a problem in a Rails application today where someone
unknowingly modified Rails internals leading to bad query generation.
The
developer modified the list of attribute names for a model by reading
and
then mutating the contents of
attribute_nameshttp://api.rubyonrails.org/classes/ActiveRecord/AttributeMethods/ClassMethods.html#method-i-attribute_nameson
an
ActiveRecord model class. For a variety of reasons it was not
immediately
obvious that they were modifying the results of this call so it took a
while to track down.

Certain query generation code paths, such as distinct counts, can rely
on
private methods like
aggregate_columnhttps://github.com/rails/rails/blob/c9346322b15c15f51234c33a3db1b3895ffe84ab/activerecord/lib/active_record/relation/calculations.rb#L220-L226that
depend on an accurate list of column names to properly prefix column
names with table names. Because attribute_names happens to expose the
internal array used to track column names any modifications to the
return
value of attribute_names can break query generation.

For example (using the ActiveRecord master bug report demo models):
https://gist.github.com/matt-glover/9202013

In our case developers did not intend to modify a core array utilized by
Rails for query generation when they modified the results of the
public-facing method. On the other hand I imagine there are cases where
this level of mutability in Ruby and Rails is helpful to people writing
plugins. My core question is how others recommend my team avoid this
particular class of issue going forward.

A few possibilities immediately come to mind:

  1. Someone much smarter than me comes up with a better solution than
    anything that follows
  2. Via thorough testing, static analysis, code review, and careful
    attention try to avoid this class of issue going forward
    • Interested in any tools or other advice to aid this effort
  3. Find documentation that identifies the publicly visible Rails
    methods
    intended to be part of the public API
    • Perhaps some documentation considers methods like
      attribute_names that
      can trigger this type of issue to be excluded from the Rails API
    • Is there a clear line between publicly visible Rails methods and
      intended parts of the API?
    1. Claim this behavior is a bug and submit a patch for it because
      mutation of the resulting array has problematic implications and it
      arguably breaks encapsulation
    • I do not plan to pursue this approach unless there is broad
      agreement that the current behavior is incorrect as mutability is
      commonplace in Ruby
    • For the sake of this post I am interested in this particular
      method
      call but not interested in a theoretical discussion about
      mutability and
      encapsulation in general
    • I am happy to have a separate theoretical discussion on those
      broader topics via a channel that does not spam the entire
      mailing list

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs