Populate the env route information when calling the automatically added endpoints #2076
Conversation
anakinj
force-pushed
the
support-env-route-information-for-allowed-methods-endpoints
branch
2 times, most recently
from
fb70f81 to
7910a98
Compare
lib/grape/api/instance.rb
Outdated
| route_settings[:path] = route.origin | ||
| route_settings = routes_map[route.pattern.to_regexp] ||= {} | ||
|
|
||
| route_settings.merge!(options: {}, |
There was a problem hiding this comment.
This is a bit hard to grok IMO. Feels like we should decide whether we want to use merge! or assign fields. Are you trying to optimize because merge can do things in bulk? I would merge everything or assign fields one by one. I also really dislike the side effect of ||= and then taking a route_settings reference on that.
There was a problem hiding this comment.
Totally agree. I've initially re-wrote it to try to understand what it does. Also RuboCop had some issues with the length of the method when I added that one :options key into the hash.
So now as a second attempt to understand what it does i re-wrote it again :).
anakinj
force-pushed
the
support-env-route-information-for-allowed-methods-endpoints
branch
from
7910a98 to
f1d06b1
Compare
|
The more I look into this the more I feel that the automatically generated endpoints do not belong into the core (was not even aware of this functionality before the before hook started to give us a hard time). Was thinking that maybe it could be possible to extract the features into a separate GEM or something. |
anakinj
force-pushed
the
support-env-route-information-for-allowed-methods-endpoints
branch
from
f1d06b1 to
c532d64
Compare
|
I merged the change, thank you.
Generally unless we feel that nobody wants a feature and we feel that it was a mistake, we try not to introduce backwards incompatible changes. Either way open an issue (or a PR) and describe what you think we should do? We can discuss it there? |
Noticed that calling the
routemethod in abeforehook causes anNoMethodErrorwhen the hook is invoked for the automatically generated Method Not Allowed endpoints. The reason seems to be that the route information dictionary is not populated in these cases.These changes will populate the route information before calling the method not allowed endpoint. Also included some minor refactoring for the methods involved in all this.