Fix single rel caching issue. #1712
Conversation
| @@ -111,13 +111,17 @@ def init_cache | |||
|
|
|||
| def add_to_cache(object, rel = nil) | |||
| (@cached_rels ||= []) << rel if rel | |||
| (@cached_result ||= []).tap { |results| results << object if object && !results.include?(object) } | |||
| (@cached_result ||= []).tap { |results| results << object if !results.include?(object) } if object | |||
There was a problem hiding this comment.
do you realize that this change is not equivalent? If object is nil then @cached_result will not be initialized if nil, while it was before. However this will make a difference only in case of a call with only nil parametesr add_to_cache(nil, nil).
There was a problem hiding this comment.
There is a reason why I changed this code. After adding the rel function to AssociationProxy, I noticed that the following case is working incorrectly.
role = Role.find(xxx)
role.manager.source_rel
role.manager.source
Even though the manager has a source, if we call source after calling source_rel, it returns nil. Because we used the add_to_cache method in line has_n.rb:118, @cached_result was initialized as an empty array then in line has_n.rb:81 we check @cached_result, since it is initialized we use it, i.e. the empty array. To avoid this, I made @cached_result initialize only when the object exists. All specs and manual tests passed successfully.
- Object parameter is nil only when we use the
add_to_cachemethod for rels.
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1712 +/- ##
===========================================
- Coverage 93.43% 43.26% -50.17%
===========================================
Files 128 106 -22
Lines 5909 4946 -963
===========================================
- Hits 5521 2140 -3381
- Misses 388 2806 +2418 ☔ View full report in Codecov by Sentry. |
Fixes #
This pull introduces/changes: