Skip to content

DMS-1264: HTTP If-None-Match support (conditional GET 304 + write create-guard)#1095

Draft
stephenfuqua wants to merge 20 commits into
etag-content-versionfrom
DMS-1264
Draft

DMS-1264: HTTP If-None-Match support (conditional GET 304 + write create-guard)#1095
stephenfuqua wants to merge 20 commits into
etag-content-versionfrom
DMS-1264

Conversation

@stephenfuqua

@stephenfuqua stephenfuqua commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements HTTP If-None-Match support (DMS-1264), building on the etag→ContentVersion work (DMS-1252). Two parts:

  • Conditional GET (read path). GET-by-id now emits an ETag response header and returns 304 Not Modified when the client's If-None-Match matches the current representation. Comparison is against the full served tag (representation-sensitive: a different profile / format / links mode does not match — deliberately distinct from the write-side projection). Weak (W/) and unquoted values are accepted; a bare * returns 304 when the resource exists.
  • Write create-guard (write path). If-None-Match: * is insert-only — a POST resolving to an insert proceeds; a POST resolving to an existing document, or a PUT against an existing resource, returns 412; a PUT to a missing target returns 404 (not 412). A specific tag returns 412 when it matches the current representation and proceeds otherwise (state-significant projection). If-Match takes precedence when both headers are present. Honored on both the relational and descriptor paths, on both the before-authorization and deferred-post-authorization routes; DELETE ignores If-None-Match.

Behavior is specified in the ADR amendment "Amendment (2026-07-06): If-None-Match support" (reference/adr-etag-from-content-version.md) and reflects the ODS-6853 legacy finding: legacy ODS/API returns 304 only for an unquoted If-None-Match, so unquoted acceptance is a compatibility requirement (the server still emits quoted strong ETags).

Base branch

Targets etag-content-version, not main, so reviewers can merge the base etag work with or without this If-None-Match feature.

Test plan

  • Backend unit: 1920 pass
  • Core unit: 2582 pass · Frontend unit: 218 pass
  • Cross-engine integration (PostgreSQL and SQL Server): conditional-GET 304 (10) + write create-guard (14), including the deferred-authorization write path on both engines
  • E2E scenarios added (build-verified locally; executed in CI)

Out of scope

If-Modified-Since, If-Unmodified-Since, and If-Range remain unimplemented (date-based validators are deferred in favor of the strong ContentVersion-derived entity tag; If-Range needs HTTP range support the DMS does not provide).

AI-use disclosure

This feature was implemented and reviewed with substantial AI assistance (Claude Code). Each task passed a two-stage (spec-compliance + code-quality) review, plus a whole-feature final review for each of the read and write parts. Human review is required before merge; accountability for the final result rests with the development team.

🤖 Generated with Claude Code

@stephenfuqua stephenfuqua marked this pull request as draft July 6, 2026 22:04
@bradbanister

bradbanister commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review

Findings

  1. Medium: If-None-Match does not support entity-tag lists.
    The ADR says “full If-None-Match support,” but EtagValue.cs explicitly parses only one value. GetByIdHandler.cs and WritePreconditionFactory.cs treat the whole header as a single tag. A compliant request like If-None-Match: "stale", "current" would miss on GET and return 200, or allow a write, instead of matching any listed tag and returning 304/412. AspNetCoreFrontend.cs also drops repeated physical header values by taking only the first nonblank value.

  2. Low: If-None-Match 412 responses still say If-Match.
    If an If-None-Match write fails, the outward problem text still says the mismatch was against the If-Match header. See UpsertHandler.cs and UpdateByIdHandler.cs. The status code is correct, but the response body is inaccurate for create-guard failures like If-None-Match: * on an existing resource.

  3. Low: EtagPreconditionEvaluator fails open for future precondition arms.
    EtagPreconditionEvaluator.cs returns true for anything other than IfMatch/IfNoneMatch via _ => true. Since this is central security/concurrency-adjacent write precondition logic, prefer explicit WritePrecondition.None => true and throw for unknown arms.

Simplicity Notes

  • TryParseConditionalTag returns false only for null/empty input, so fallback branches in callers are effectively dead for real ASP.NET requests. This would be cleaner as a normalizer that returns a parsed value, or as part of a real list parser.

  • IfMatchPreconditionEvaluation / DeferredIfMatch now apply to both If-Match and If-None-Match; renaming them to ETag/precondition-neutral names would reduce mental overhead.

  • The new Get-by-id unit tests repeat the same nested repository/setup/assertion structure across many If-None-Match cases. Parameterized cases over header value, expected status, and body/etag behavior would be simpler and make list-case coverage cheaper.

  • The new E2E If-None-Match header helpers duplicate existing generic “with header name/value” steps. Low risk, but it adds another path to keep in sync with auth/content-type behavior.

Open Question

Jira says If-Match takes precedence when both headers are present. The branch covers that for writes, but GET only checks If-None-Match. The ADR says there is no read-side If-Match today, so this may be intentional; worth confirming.

@bradbanister

bradbanister commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

If it's intentional to have no entity-tag list support, then the ticket/design needs updating.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds full If-None-Match support to the DMS HTTP pipeline: conditional GET now returns 304 Not Modified with ETag when appropriate, and writes honor If-None-Match as a create-guard (including wildcard * semantics and precedence vs If-Match). This builds on the existing ContentVersion-derived ETag work and extends behavior consistently across relational and descriptor write paths, including the deferred-authorization branch.

Changes:

  • Implement conditional GET for GET-by-id (If-None-Match304 + ETag, no body) using full served-tag comparison.
  • Implement write-side create-guard semantics for If-None-Match (wildcard and specific tag) across relational + descriptor paths, with centralized evaluation logic.
  • Add unit/integration/E2E coverage for read + write behavior across PostgreSQL and SQL Server, including deferred-authorization scenarios.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/dms/core/EdFi.DataManagementService.Core/Utilities/EtagValue.cs Adds parsing helper for If-None-Match (weak W/ + quoted/unquoted tolerance).
src/dms/core/EdFi.DataManagementService.Core/Handler/GetByIdHandler.cs Emits ETag on 200 and short-circuits to 304 on satisfied If-None-Match.
src/dms/core/EdFi.DataManagementService.Core/Backend/WritePreconditionFactory.cs Adds If-None-Match precondition parsing with If-Match precedence.
src/dms/core/EdFi.DataManagementService.Core.External/Backend/WritePrecondition.cs Introduces WritePrecondition.IfNoneMatch record.
src/dms/backend/EdFi.DataManagementService.Backend/Etag/EtagPreconditionEvaluator.cs Centralizes proceed-vs-412 evaluation for If-Match and If-None-Match.
src/dms/backend/EdFi.DataManagementService.Backend/RelationalWriteExecutionStateResolver.cs Broadens precondition gating to include If-None-Match (incl. deferred path safety).
src/dms/backend/EdFi.DataManagementService.Backend/RelationalCurrentEtagPreconditionChecker.cs Generalizes checker to accept WritePrecondition and returns IsSatisfied.
src/dms/backend/EdFi.DataManagementService.Backend/DefaultRelationalWriteExecutor.cs Ensures If-None-Match participates in before/deferred precondition flow and CreateNew behavior.
src/dms/backend/EdFi.DataManagementService.Backend/DescriptorWriteHandler.cs Adds If-None-Match parity for descriptor POST/PUT (and explicitly ignores on DELETE).
src/dms/core/EdFi.DataManagementService.Core.Tests.Unit/Utilities/EtagValueTests.cs Unit tests for conditional-tag parsing (quoted/unquoted/weak/wildcard).
src/dms/core/EdFi.DataManagementService.Core.Tests.Unit/Handler/GetByIdHandlerTests.cs Unit tests for 200 + ETag and conditional-GET 304 behavior variants.
src/dms/core/EdFi.DataManagementService.Core.Tests.Unit/Backend/WritePreconditionFactoryTests.cs Unit tests for If-None-Match parsing and precedence vs If-Match.
src/dms/frontend/EdFi.DataManagementService.Frontend.AspNetCore.Tests.Unit/AspNetCoreFrontendResponseHeaderTests.cs Verifies frontend mapping for 304 + quoted ETag and no body writes.
src/dms/backend/EdFi.DataManagementService.Backend.Tests.Unit/Etag/EtagPreconditionEvaluatorTests.cs Unit tests for If-Match/If-None-Match satisfaction polarity and null-etag paths.
src/dms/backend/EdFi.DataManagementService.Backend.Tests.Unit/RelationalCurrentEtagPreconditionCheckerTests.cs Extends checker tests to cover If-None-Match satisfied/unsatisfied conditions.
src/dms/backend/EdFi.DataManagementService.Backend.Tests.Unit/RelationalWriteExecutorResultsTests.cs Locks behavior for stale guarded no-op outcomes under If-None-Match.
src/dms/backend/EdFi.DataManagementService.Backend.Tests.Unit/DefaultRelationalWriteExecutorTests.cs Adds execution-path tests for If-None-Match across create/update/missing/deferred cases.
src/dms/backend/EdFi.DataManagementService.Backend.Tests.Unit/DescriptorWriteHandlerPreconditionTests.cs Adds descriptor parity tests for If-None-Match create-guard and missing-target behavior.
src/dms/backend/EdFi.DataManagementService.Backend.Tests.Unit/DescriptorWriteHandlerNamespaceAuthorizationTests.cs Ensures proposed-namespace authorization still precedes insert on If-None-Match create path.
src/dms/backend/EdFi.DataManagementService.Backend.Tests.Unit/RelationalDocumentStoreRepositoryTests.cs Confirms DELETE ignores If-None-Match.
src/dms/backend/EdFi.DataManagementService.Backend.Postgresql.Tests.Integration/PostgresqlRelationalWriteAuthoritativeSampleStudentSchoolAssociationSmokeTests.cs Updates assertions for renamed precondition result flag (IsSatisfied).
src/dms/backend/EdFi.DataManagementService.Backend.Mssql.Tests.Integration/MssqlRelationalWriteAuthoritativeSampleStudentSchoolAssociationSmokeTests.cs Updates assertions for renamed precondition result flag (IsSatisfied).
src/dms/tests/EdFi.DataManagementService.Tests.Integration/Scenarios/ConditionalGetIfNoneMatchScenario.cs Integration scenario coverage for conditional GET behaviors.
src/dms/tests/EdFi.DataManagementService.Tests.Integration/Scenarios/WriteCreateGuardIfNoneMatchScenario.cs Integration scenario coverage for write create-guard behaviors (incl. deferred auth path).
src/dms/tests/EdFi.DataManagementService.Tests.Integration/Tests/Postgresql/Given_Postgresql_ConditionalGetIfNoneMatch.cs Wires PostgreSQL integration tests to conditional GET scenarios.
src/dms/tests/EdFi.DataManagementService.Tests.Integration/Tests/Postgresql/Given_Postgresql_WriteCreateGuardIfNoneMatch.cs Wires PostgreSQL integration tests to write create-guard scenarios (incl. deferred auth).
src/dms/tests/EdFi.DataManagementService.Tests.Integration/Tests/Mssql/Given_Mssql_ConditionalGetIfNoneMatch.cs Wires SQL Server integration tests to conditional GET scenarios.
src/dms/tests/EdFi.DataManagementService.Tests.Integration/Tests/Mssql/Given_Mssql_WriteCreateGuardIfNoneMatch.cs Wires SQL Server integration tests to write create-guard scenarios (incl. deferred auth).
src/dms/tests/EdFi.DataManagementService.Tests.E2E/StepDefinitions/StepDefinitions.cs Adds E2E steps to send GET/PUT with If-None-Match.
src/dms/tests/EdFi.DataManagementService.Tests.E2E/Features/Resources/etag.feature Adds E2E scenarios for conditional GET and write create-guard.
reference/adr-etag-from-content-version.md Documents the If-None-Match amendment and related RFC updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread reference/adr-etag-from-content-version.md Outdated
@stephenfuqua

Copy link
Copy Markdown
Contributor Author

If it's intentional to have no entity-tag list support, then the ticket/design needs updating.

Although the ODS/API does not support the entity-tag list, this is a good opportunity for DMS to get it right. Good call out.

@stephenfuqua stephenfuqua force-pushed the etag-content-version branch 3 times, most recently from 65f2c45 to 0bc8bd1 Compare July 8, 2026 22:33
stephenfuqua and others added 17 commits July 8, 2026 21:01
…ility finding

Add "Amendment (2026-07-06): If-None-Match support" specifying full RFC 9110
If-None-Match handling: conditional GET returning 304 (full served-tag, weak
comparison) and a write create-guard returning 412 (state-significant
projection), with the inverted wildcard, If-Match precedence, and DELETE /
If-Modified-Since / If-Unmodified-Since / If-Range out of scope.

Record the ODS-6853 finding: legacy ODS/API returns 304 only for an unquoted
If-None-Match, making DMS unquoted-acceptance a compatibility requirement.
Marked Proposed pending team sign-off.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The If-None-Match amendment's "Scope of the code change" described the read
path recomposing the served tag via EtagComposer; the shipped handler instead
echoes the already-composed body _etag (functionally identical, cannot drift
from read materialization). Note the intentional divergence.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…valuator

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… gates

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…on-matching retries)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds API-level scenarios for the write-side If-None-Match create-guard:
wildcard/specific-tag PUT and POST-upsert against existing targets (412),
wildcard POST-insert (201), wildcard PUT to a missing target (404), and
If-Match-governs-when-both-present. Also adds a deferred-authorization
integration scenario (RelationshipsWithEdOrgsOnly Update boundary) proving
the create-guard still 412s on the post-proposed-authorization branch,
matching the existing backend unit regression.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
stephenfuqua and others added 2 commits July 8, 2026 21:31
…g ADR

Document that If-None-Match accepts a comma-separated list of entity-tags
(RFC 9110 13.1.2), matching if any tag matches, and clarify the weak
comparison function (a W/-prefixed tag is compared by opaque value only).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement RFC 9110 13.1.2 entity-tag list support for the If-None-Match
header. The header may now carry a comma-separated list of entity-tags
(e.g. "t1", "t2", W/"t3"); the precondition triggers when ANY tag matches:
304 on conditional GET, 412 on the write create-guard. Per-tag weak
comparison (RFC 9110 2.1) treats W/"1" the same as stored "1". A bare *
remains the wildcard only as the sole value.

Production changes:
- WritePrecondition.IfNoneMatch now holds an ordered list of values with
  ordinal SequenceEqual equality plus a single-value convenience ctor.
- EtagValue.ParseConditionalTagList splits and normalizes each list element
  (W/ strip, quote strip, unquoted tolerance).
- WritePreconditionFactory builds the list precondition; If-Match precedence
  and bare-* wildcard detection preserved.
- EtagPreconditionEvaluator: write is unsatisfied when any tag's projection
  matches.
- GetByIdHandler: conditional GET returns 304 when any listed tag equals the
  full served etag.

Tests:
- Unit coverage for list parsing, weak-in-list, quoted/unquoted mix,
  whitespace/empty handling, and evaluator any-match cases.
- PostgreSQL + SQL Server integration scenarios for multi-tag conditional GET
  and write create-guard, wired into all fixtures.
- Fixed two pre-existing test compile errors (missing ComposedWriteResultEtag
  argument; IsMatch -> IsSatisfied) so the backend unit test project builds.

Documents the behavior recorded in
reference/adr-etag-from-content-version.md (2026-07-06 amendment).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

3 participants