Make reflection and aggregate_reflection caches work with string keys. #14675
Conversation
with strings with string keys. Related #14668.
|
It definitely shouldn't be touching the has directly like that. Do we have accessor functions for the hash? Aaron Patterson
|
…rks_with_string_keys Make reflection and aggregate_reflection caches work with string keys.
Yes, they are using the |
| end | ||
|
|
||
| def source_reflection_name # :nodoc: | ||
| return @source_reflection_name.to_sym if @source_reflection_name | ||
| return @source_reflection_name if @source_reflection_name | ||
|
|
||
| names = [name.to_s.singularize, name].collect { |n| n.to_sym }.uniq |
There was a problem hiding this comment.
Changing that doesn't actually affect this piece where it looks for the real name, right? The "names" method is only used in an exception as far as I could see.
There was a problem hiding this comment.
@carlosantoniodasilva yes, you are right. Thanks bro!
In PR rails#14675 reflection cache / associations were changed to accept string as keys. The problem is this messed up the ability to accepts symbols as keys. There were no tests to make sure both strings and symbols could be accepts. This was brought to light when I attempted to fix rails#15671.
Update the code that uses the internal rails api to be compatible with rails 4.2. > ActiveRecord::Base#reflections now returns a hash with string keys instead of symbol keys. [PR] rails/rails#14675
Following @tenderlove's suggestion on #14668 (comment), now the
reflectionand theaggregate_reflectioncaches use String as keys. This is needed to avoidto_symin some cases, such as did in #14668.I think we won't have problems changing these caches, since they are only used in
reflection.rbfile.Another point is the test. I think it is wrong, it should not use the
reflectionHash, right?cc/ @tenderlove