-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Corrected a hash modification while iterating issue. #2162
Corrected a hash modification while iterating issue. #2162
Conversation
98316ed
to
4453c15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this start adding duplicates into @setup
? It's a Set
.
Signed-off-by: Hermann Mayer <hermann.mayer92@gmail.com>
4453c15
to
c859eee
Compare
No, it does not add duplicates, because it's a set. Example: block1 = proc { false }
block2 = proc { true }
setup = Set.new
setup << { method: :default_format, args: [:json], block: block1 }
setup << { method: :default_format, args: [:text], block: block2 }
setup << { method: :default_format, args: [:text], block: block2 }
# =>
# #<Set: {{:method=>:default_format,
# :args=>[:json],
# :block=>#<Proc:0x000055bbcbe3a8f0 (irb):5>},
# {:method=>:default_format,
# :args=>[:text],
# :block=>#<Proc:0x000055bbcb83dce0 (irb):6>}}>
setup = Set.new
setup += [{ method: :default_format, args: [:json], block: block1 }]
setup += [{ method: :default_format, args: [:text], block: block2 }]
setup += [{ method: :default_format, args: [:text], block: block2 }]
# #<Set: {{:method=>:default_format,
# :args=>[:json],
# :block=>#<Proc:0x000055bbcbe3a8f0 (irb):5>},
# {:method=>:default_format,
# :args=>[:text],
# :block=>#<Proc:0x000055bbcb83dce0 (irb):6>}}> |
Looks good, thank you. |
@dblock can you estimate when a new version can be released including this patch? |
I can do it this week unless someone gets it before me, add your +1 to #2163 |
- What is it good for
There is a nasty bug on edge case usage of nested mounted APIs which use a shared class-helper method to abstract the creation of a route. With this fix in place, no errors are raised. Otherwise a
RuntimeError: can't add a new key into hash during iteration
is raised. This is caused by an inline modification of the @setup set inside the Grape::API class. This error is caused by the definition of the helper method in a specific way. The following form causes the issue:while this form works as expected:
- What I did
I just converted the inline set modification into a reassignment of a freshly built set and added a test that proves the bugfix.
- A picture of a cute animal (not mandatory but encouraged)