Add If-Match support with 412 response for precondition failures, imp…

[OskarLundqvist33]

Add If-Match support with 412 response for precondition failures, improve ETag parsing. Add tests for If-Match

Summary by CodeRabbit

• Bug Fixes

  • Improved ETag normalization and handling for static file responses
  • Added support for HTTP If-Match preconditions with proper error responses
  • Enhanced cache validation logic for conditional requests

• Tests

  • Added comprehensive test coverage for ETag handling and HTTP preconditions

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces If-Match HTTP precondition handling to static file responses, returning 412 Precondition Failed when the ETag does not match. Additionally, ETag normalization logic is refined to properly strip weak indicators and quotes before comparison, and the precondition check order is adjusted to evaluate If-Match before If-None-Match.

Changes

Cohort / File(s)	Summary
ETag and Precondition Handling src/main/java/org/juv25d/handler/StaticFileHandler.java	Introduces If-Match precondition validation with normalized ETag comparison; refactors opaqueTag method to strip weak prefixes and quotes; adds 412 Precondition Failed response with logging when If-Match fails; reorders precondition checks to prioritize If-Match.
Precondition Test Coverage src/test/java/org/juv25d/handler/StaticFileHandlerTest.java	Adds comprehensive test suite for If-Match handling, including 412 failure cases, successful matches, precedence over If-None-Match, unquoted ETag handling, and header verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Implement If-Match for static files (return 412 Precondition Failed) #154: Directly addresses the If-Match precondition behavior and 412 response handling that this PR implements for static file responses.

Possibly related PRs

• Add ETag support to StaticFileHandler with caching for static files #108: Established the foundational ETag/If-None-Match implementation that this PR builds upon by adding If-Match precondition logic and refining ETag normalization.

Suggested reviewers

• addee1
• DennSel

Poem

🐰 Hopping through ETags with care,
If-Match checks everywhere!
Preconditions now aligned,
No more mismatches to find,
A rabbit's HTTP delight! 🏷️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding If-Match support with 412 Precondition Failed response handling.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 154-implement-if-match-for-static-files-return-412-precondition-failed

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

src/test/java/org/juv25d/handler/StaticFileHandlerTest.java (1)

131-131: Consider adding test for If-Match: * wildcard.

The current tests don't cover the If-Match: * case, which should return 200 when the resource exists. This is an important edge case per RFC 7232.

🧪 Suggested additional test

`@Test`
void shouldReturn200WhenIfMatchIsWildcard() {
    Map<String, String> headers = new HashMap<>();
    headers.put("If-Match", "*");

    HttpRequest request = createRequest("GET", "/index.html", headers);
    HttpResponse response = StaticFileHandler.handle(request);

    assertThat(response.statusCode()).isEqualTo(200);
    assertThat(response.statusText()).isEqualTo("OK");
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/handler/StaticFileHandlerTest.java` at line 131, Add
a unit test in StaticFileHandlerTest that verifies the If-Match: * wildcard
returns 200 when the resource exists: create a headers map with "If-Match" ->
"*" (use the existing createRequest helper), call
StaticFileHandler.handle(request), and assert response.statusCode() == 200 and
response.statusText().equals("OK"); name the test e.g.
shouldReturn200WhenIfMatchIsWildcard and place it alongside the other handler
tests to cover this RFC 7232 edge case.

src/main/java/org/juv25d/handler/StaticFileHandler.java (1)

251-273: Consider renaming method for clarity.

The method etagMatches is now used for both If-Match (line 81) and If-None-Match (line 115) header processing, but the parameter is still named ifNoneMatchHeader. This could cause confusion for future maintainers.

♻️ Suggested rename for clarity

-    private static boolean etagMatches(`@Nullable` String ifNoneMatchHeader, String currentEtag) {
-        if (ifNoneMatchHeader == null || ifNoneMatchHeader.isBlank()) {
+    private static boolean etagMatches(`@Nullable` String headerValue, String currentEtag) {
+        if (headerValue == null || headerValue.isBlank()) {
             return false;
         }
-        String value = ifNoneMatchHeader.trim();
+        String value = headerValue.trim();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/handler/StaticFileHandler.java` around lines 251 -
273, Rename the etagMatches method parameter from ifNoneMatchHeader to a neutral
name (e.g., headerValue or matchHeader) and optionally rename the method to
something unambiguous like etagMatchesHeader or matchesEtagHeader; update all
call sites that use etagMatches (including the If-Match and If-None-Match
usages) to pass the new parameter name and new method name if changed, and
ensure opaqueTag(...) references remain correct inside the method.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/main/java/org/juv25d/handler/StaticFileHandler.java`:
- Around line 251-273: Rename the etagMatches method parameter from
ifNoneMatchHeader to a neutral name (e.g., headerValue or matchHeader) and
optionally rename the method to something unambiguous like etagMatchesHeader or
matchesEtagHeader; update all call sites that use etagMatches (including the
If-Match and If-None-Match usages) to pass the new parameter name and new method
name if changed, and ensure opaqueTag(...) references remain correct inside the
method.

In `@src/test/java/org/juv25d/handler/StaticFileHandlerTest.java`:
- Line 131: Add a unit test in StaticFileHandlerTest that verifies the If-Match:
* wildcard returns 200 when the resource exists: create a headers map with
"If-Match" -> "*" (use the existing createRequest helper), call
StaticFileHandler.handle(request), and assert response.statusCode() == 200 and
response.statusText().equals("OK"); name the test e.g.
shouldReturn200WhenIfMatchIsWildcard and place it alongside the other handler
tests to cover this RFC 7232 edge case.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e609337 and 4fbd8c7.

📒 Files selected for processing (2)

• src/main/java/org/juv25d/handler/StaticFileHandler.java
• src/test/java/org/juv25d/handler/StaticFileHandlerTest.java