Skip to content

Conversation

ericproulx
Copy link
Contributor

Fix routes memoization

Base: master
Branch: fix_routes_memoization

Summary

Ensure routes memoization works reliably by making routes a non-overridable method and tightening specs to verify the memoization behavior directly.

Motivation / Context

  • The routes method was eligible to be overridden, which could inadvertently break memoization and force repeated route computation.
  • This PR safeguards memoization and updates tests to validate the behavior without relying on private implementation details.

Changes

  • Make routes non-overridable to preserve memoization guarantees.
  • Improve specs so they assert actual memoization effects rather than depending on private methods.

Implementation Notes

  • Commit(s):
    • 017e762 — Add routes as a non overridable methods so that memoization works properly. Added a better specs so that a private method is not expected and memoization is really tested. (Eric Proulx)

Files Changed (4)

  • lib/grape/api.rb
  • lib/grape/api/instance.rb
  • lib/grape/dsl/routing.rb
  • spec/grape/dsl/routing_spec.rb

Diff Stats

4 files changed, 15 insertions(+), 18 deletions(-)

Testing

  • Added/updated specs in spec/grape/dsl/routing_spec.rb to validate memoization behavior.
  • Run:
    • bundle install
    • bundle exec rspec spec/grape/dsl/routing_spec.rb
    • bundle exec rspec

Backwards Compatibility / Risk

  • Low risk for typical usage; behavior is preserved with improved guarantees.
  • Potentially breaking only if external code previously overrode routes. Marking it non-overridable is intentional to protect memoization semantics.

Checklist

  • Tests updated/added
  • No public API changes beyond making routes non-overridable
  • Changelog entry considered (if maintainers require)

Notes for Reviewers

  • Focus on how routes is protected from overriding and how tests assert memoization without relying on private methods.

@ericproulx ericproulx marked this pull request as ready for review August 21, 2025 13:00
@ericproulx ericproulx requested a review from dblock August 21, 2025 13:00
…roperly. Added a better specs so that a private method is not expected and memoization is really tested.
@ericproulx ericproulx force-pushed the fix_routes_memoization branch from 017e762 to decebae Compare August 21, 2025 13:01
@dblock dblock merged commit ce766c8 into master Aug 21, 2025
93 checks passed
@ericproulx ericproulx deleted the fix_routes_memoization branch August 21, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants