Skip to content

Replace deprecated DateTimeInterface::RFC7231 with literal format string#173

Merged
koriym merged 3 commits into1.xfrom
deprecate-datetime-rfc7231
Feb 7, 2026
Merged

Replace deprecated DateTimeInterface::RFC7231 with literal format string#173
koriym merged 3 commits into1.xfrom
deprecate-datetime-rfc7231

Conversation

@koriym
Copy link
Member

@koriym koriym commented Feb 7, 2026

Summary

  • DateTimeInterface::RFC7231 is deprecated in PHP 8.5 (php-src#12989)
  • Replaced with literal format string 'D, d M Y H:i:s \G\M\T' in EtagSetter and DevEtagSetter
  • No behavioral change since gmdate() always outputs GMT

Test plan

  • composer test all tests pass
  • composer cs-fix no violations

DateTimeInterface::RFC7231 is deprecated in PHP 8.5 because the format
hardcodes GMT timezone, ignoring the actual timezone of the instance.

Since gmdate() is used here (always GMT), the behavior is unchanged.

See: php/php-src#12989
@koriym
Copy link
Member Author

koriym commented Feb 7, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 7, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review
📝 Walkthrough

Walkthrough

Added a new RFC7231 date format constant to Header and updated ETag-related classes to use that constant (removing the DateTimeInterface import) when generating the Last-Modified header.

Changes

Cohort / File(s) Summary
Header constant
src/Header.php
Added public const RFC7231 = 'D, d M Y H:i:s \G\M\T' to expose the RFC 7231 date format string.
ETag header generation
src/DevEtagSetter.php, src/EtagSetter.php
Removed DateTimeInterface import and replaced gmdate(DateTimeInterface::RFC7231, 0) with gmdate(Header::RFC7231, 0) for Last-Modified header generation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title states the PR replaces deprecated DateTimeInterface::RFC7231 with a literal format string, but the actual changes move this to Header::RFC7231, which is a semantic distinction that the title misses. Consider revising the title to better reflect that the constant is now defined in Header class, e.g., 'Replace deprecated DateTimeInterface::RFC7231 with Header::RFC7231 constant'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description accurately explains the deprecation context, the replacement strategy, and includes test results, making it clearly related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch deprecate-datetime-rfc7231

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps
Copy link

greptile-apps bot commented Feb 7, 2026

Greptile Overview

Greptile Summary

  • Replaces deprecated DateTimeInterface::RFC7231 usage in EtagSetter and DevEtagSetter with a centralized Header::RFC7231 date format constant.
  • Adds Header::RFC7231 = 'D, d M Y H:i:s \\G\\M\\T' for RFC 7231 HTTP date formatting.
  • Keeps Last-Modified header formatting behavior consistent for updated call sites via gmdate(...).
  • One remaining inconsistency: MobileEtagSetter still uses a hardcoded/concatenated date format instead of the new constant.

Confidence Score: 4/5

  • This PR is low-risk and likely safe to merge after addressing one small consistency gap in HTTP date formatting.
  • The changes are straightforward: swapping a deprecated constant for an equivalent literal format centralized in Header::RFC7231. No behavioral change is expected for the updated setters. The only concrete gap found is an existing setter (MobileEtagSetter) that still uses a different hardcoded RFC7231-equivalent format, which undermines the PR’s centralization goal but is unlikely to break runtime behavior.
  • src/MobileEtagSetter.php (not in changeset) for consistent use of Header::RFC7231

Important Files Changed

Filename Overview
src/DevEtagSetter.php Replaces deprecated DateTimeInterface::RFC7231 usage with Header::RFC7231 when formatting Last-Modified.
src/EtagSetter.php Switches gmdate format for Last-Modified from DateTimeInterface::RFC7231 to Header::RFC7231; removes DateTimeInterface import.
src/Header.php Adds Header::RFC7231 format constant for HTTP date formatting; otherwise unchanged.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant EtagSetter
    participant DevEtagSetter
    participant Header
    participant RO as ResourceObject

    Note over Caller,RO: Setting ETag + Last-Modified

    Caller->>EtagSetter: __invoke(ro, time, httpCache)
    EtagSetter->>RO: headers['ETag']=getEtag(...)
    EtagSetter->>Header: RFC7231
    EtagSetter->>RO: headers['Last-Modified']=gmdate(Header::RFC7231, time)

    Caller->>DevEtagSetter: __invoke(ro, time?, httpCache?)
    DevEtagSetter->>RO: headers['ETag']=uriEtag
    DevEtagSetter->>Header: RFC7231
    DevEtagSetter->>RO: headers['Last-Modified']=gmdate(Header::RFC7231, 0)

    Note over Header: RFC7231 = "D, d M Y H:i:s \\G\\M\\T"
Loading

@codecov
Copy link

codecov bot commented Feb 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (8e4ab37) to head (0d58311).
⚠️ Report is 4 commits behind head on 1.x.

Additional details and impacted files
@@             Coverage Diff             @@
##                 1.x      #173   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       246       246           
===========================================
  Files             53        53           
  Lines            746       746           
===========================================
  Hits             746       746           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@koriym
Copy link
Member Author

koriym commented Feb 7, 2026

@greptileai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 7, 2026

@koriym: I'll review the changes in this PR. This appears to be a straightforward deprecation fix to maintain PHP 8.5 compatibility by replacing the deprecated DateTimeInterface::RFC7231 constant with the literal format string.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link

greptile-apps bot commented Feb 7, 2026

Additional Comments (1)

src/Header.php
Centralize RFC7231 usage

This PR introduces Header::RFC7231, but src/MobileEtagSetter.php:27 still formats Last-Modified with a hardcoded/concatenated string (gmdate('D, d M Y H:i:s', $time) . ' GMT'). That leaves date formatting inconsistent across ETag setters. Consider switching MobileEtagSetter to gmdate(Header::RFC7231, $time) so all paths use the same definition.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@koriym
Copy link
Member Author

koriym commented Feb 7, 2026

@greptile-apps Fixed in 0d58311. All three EtagSetter implementations now use Header::RFC7231.

@greptile-apps
Copy link

greptile-apps bot commented Feb 7, 2026

Perfect! The fix looks good. You've:

  1. ✅ Added Header::RFC7231 constant with the correct format string 'D, d M Y H:i:s \G\M\T'
  2. ✅ Updated all three EtagSetter implementations to use it consistently:
    • EtagSetter.php
    • DevEtagSetter.php
    • MobileEtagSetter.php
  3. ✅ Removed the deprecated DateTimeInterface imports
  4. ✅ Added proper documentation with RFC reference

This centralizes the date format in one place and avoids the PHP 8.5 deprecation. The implementation is cleaner and more maintainable than having the literal string or the deprecated constant scattered across multiple files.

@koriym koriym merged commit 41468fa into 1.x Feb 7, 2026
36 of 37 checks passed
@koriym koriym deleted the deprecate-datetime-rfc7231 branch February 7, 2026 01:47
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.

1 participant