Skip to content

Conversation

egg528
Copy link
Contributor

@egg528 egg528 commented May 8, 2025

Motivation / Background

This Pull Request has been created because the HTTP Cache-Control request directives were not fully supported in Rails. According to RFC 9111, HTTP clients can use various cache control directives in their requests to control how caching is handled. This implementation adds comprehensive support for these directives, with special attention to the max-stale directive which can be used with or without a value.

Detail

This Pull Request changes the following:

  1. Adds a new CacheControlDirectives class that parses and provides access to HTTP Cache-Control request directives
  2. Implements support for boolean directives (no-cache, no-store, no-transform, only-if-cached)
  3. Implements support for value directives (max-age, min-fresh, stale-if-error)
  4. Provides special handling for the max-stale directive which can appear both with a value (e.g., max-stale=300) or without a value (indicating unlimited staleness)
  5. Adds comprehensive test coverage for all supported directives
  6. Updates the CHANGELOG to document the new functionality

This enhancement allows Rails applications to better integrate with HTTP caching mechanisms and provides more control over how requests interact with cached resources.

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the actionpack label May 8, 2025
@egg528 egg528 force-pushed the add-http-request-cache-control-directives-support branch 2 times, most recently from b0eae55 to e47f341 Compare May 8, 2025 15:02

# Value directives
request.cache_control_directives.max_age # => integer or nil
request.cache_control_directives.max_stale # => integer or nil (or true for valueless max-stale)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a problem, users are likely to assume max_stale if truthy is an Integer and not handle true correct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I see max_stale_unlimited?. I suppose that is acceptable

Copy link
Member

@byroot byroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface look good to me. But I'd rather have an implementation that doesn't use a Hash.


# Value directives
request.cache_control_directives.max_age # => integer or nil
request.cache_control_directives.max_stale # => integer or nil (or true for valueless max-stale)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I see max_stale_unlimited?. I suppose that is acceptable

@byroot byroot force-pushed the add-http-request-cache-control-directives-support branch from e47f341 to 5c9904d Compare May 13, 2025 06:51
@byroot byroot enabled auto-merge May 13, 2025 06:52
Co-Authored-By: Jean Boussier <jean.boussier@gmail.com>
@byroot byroot force-pushed the add-http-request-cache-control-directives-support branch from 5c9904d to 55b54a3 Compare May 13, 2025 07:00
@byroot byroot merged commit 0da7aaf into rails:main May 13, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants