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_names<http://api.rubyonrails.org/classes/ActiveRecord/AttributeMethods/ClassMethods.html#method-i-attribute_names>on 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_column<https://github.com/rails/rails/blob/c9346322b15c15f51234c33a3db1b3895ffe84ab/activerecord/lib/active_record/relation/calculations.rb#L220-L226>that 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? 4. 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 -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/ef276779-1ef7-47aa-9e98-2abc4e7d7bad%40googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
