Best Way To Manage Mutable Public Rails Return Types

[Matt Glover]

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 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 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