Add silent meta update #5144

LTA-Thinking · 2025-09-15T16:38:31Z

Description

Adds a query parameter for PUT updates that allows changes that only update metadata fields to happen without adding a historical version.
The parameter is _meta-history
ex: PUT <fhir base>/Patient/12456?_meta-history=false

Related issues

Addresses User Story 167335

Testing

Manual testing, new and existing unit and e2e tests

FHIR Team Checklist

Update the title of the PR to be succinct and less than 65 characters
Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
CI is green before merge
Review squash-merge requirements

Semver Change (docs)

Patch

…adata

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs

SergeyGaluzo · 2025-09-16T16:09:26Z

Are any of these meta fields searchable?

LTA-Thinking · 2025-09-17T14:27:49Z

Yes, tags are a default search parameter in meta and other custom search parameters can be created.

SergeyGaluzo · 2025-09-20T06:01:58Z

@LTA-Thinking, @brendankowitz
Planned "meta update" performs updates of resources in place without version and last updated change. This is a violation of current FHIR storage principal - append only. I think this functionality should not be supported.

Copilot

Pull Request Overview

This PR introduces the _silent-meta query parameter for PUT update operations, allowing metadata-only changes to occur without incrementing version numbers or updating the lastUpdated timestamp. This addresses scenarios where frequent metadata updates would otherwise cause unnecessary version history bloat.

Key changes:

Added _silent-meta query parameter that prevents version changes when only metadata fields are modified
Implemented deep comparison logic to detect metadata-only changes
Added end-to-end tests covering various scenarios of the silent metadata update feature

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/Microsoft.Health.Fhir.Core/Features/KnownQueryParameterNames.cs	Adds the `SilentMeta` constant for the new query parameter
src/Microsoft.Health.Fhir.Core/Messages/Upsert/UpsertResourceRequest.cs	Adds `MetaSilent` property to track the silent metadata flag
src/Microsoft.Health.Fhir.Core/Features/Persistence/ResourceWrapperOperation.cs	Adds `MetaSilent` property to resource operation wrapper
src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Upsert/UpsertResourceHandler.cs	Passes the `MetaSilent` flag through to the data store operation
src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs	Adds `metaSilent` parameter to the Update endpoint
src/Microsoft.Health.Fhir.Shared.Client/IFhirClient.cs	Adds `silentMeta` parameter to update method signatures
src/Microsoft.Health.Fhir.Shared.Client/FhirClient.cs	Implements silentMeta parameter handling and URI construction
src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs	Implements the core logic to detect metadata-only changes and control history creation
src/Microsoft.Health.Fhir.Core/Extensions/ElementValueExtensions.cs	Adds recursive comparison logic for ElementValue instances
src/Microsoft.Health.Fhir.Core.UnitTests/Extensions/ElementValueExtensionsTests.cs	Unit tests for the ElementValue comparison logic
test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/UpdateTests.cs	E2E tests validating the silent metadata update behavior
test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs	Test fixture updates to support the new functionality
src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeResources.sql	Documentation comment clarification
src/Microsoft.Health.Fhir.Shared.Web/appsettings.Development.json	Changes log level from Debug to Information
docs/arch/adr-2510-Silent-meta.md	Architecture decision record documenting the design

Copilot · 2025-10-16T21:27:13Z

test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/UpdateTests.cs

+                provenanceHeader: null,
+                silentMeta: true);
+
+            // Assert: Historical record should not be created (version and lastUpdated remain change)


Corrected spelling of 'change' to 'unchanged'.

Suggested change

// Assert: Historical record should not be created (version and lastUpdated remain change)

// Assert: Historical record should not be created (version and lastUpdated remain unchanged)

Copilot · 2025-10-16T21:27:13Z

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs

+
+            if (inputMeta.Equals(existingMeta))
+            {
+                return false; // Difference is in a non-meta field


The logic is inverted. When inputMeta.Equals(existingMeta) returns true, it means the metadata is the same, so changes must be in non-meta fields. However, the method should return true (indicating metadata-only changes) only when the metadata differs AND everything else is the same. This condition should return true to continue checking other fields, not false.

Suggested change

return false; // Difference is in a non-meta field

return false; // No metadata changes; any difference must be in non-meta fields

Copilot · 2025-10-16T21:27:13Z

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs

+                        return false;
+                    }
+
+                    // The end of the file has been reached


The comment says 'end of the file' but should say 'end of the resource' or 'end of the children', as this is iterating through resource children, not file content.

Suggested change

// The end of the file has been reached

// The end of the resource's children has been reached

Copilot · 2025-10-16T21:27:14Z

src/Microsoft.Health.Fhir.Core/Extensions/ElementValueExtensions.cs

+using System.Text;
+using System.Threading.Tasks;


Remove unused imports: System.Collections.Generic, System.Linq, System.Text, and System.Threading.Tasks are not used in this file.

Suggested change

using System.Text;

using System.Threading.Tasks;

Copilot · 2025-10-16T21:27:14Z

src/Microsoft.Health.Fhir.Core.UnitTests/Extensions/ElementValueExtensionsTests.cs

+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;


Remove unused imports: System, System.Text, and System.Threading.Tasks are not used in this file.

Suggested change

using System;

using System.Collections.Generic;

using System.Linq;

using System.Text;

using System.Threading.Tasks;

using System.Collections.Generic;

using System.Linq;

Copilot · 2025-10-16T21:27:14Z

...crosoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs

+            if (serviceType == typeof(ResourceDeserializer))
+            {
+                return ResourceDeserializer as ResourceDeserializer;


The cast is redundant and will always be null. ResourceDeserializer is of type IResourceDeserializer (from line 241), but this is casting it to the concrete type ResourceDeserializer. This should either check for typeof(IResourceDeserializer) or cast to IResourceDeserializer.

Suggested change

if (serviceType == typeof(ResourceDeserializer))

{

return ResourceDeserializer as ResourceDeserializer;

if (serviceType == typeof(IResourceDeserializer))

{

return ResourceDeserializer;

Copilot · 2025-10-16T21:27:15Z

src/Microsoft.Health.Fhir.Shared.Web/appsettings.Development.json

-        }
+      "LogLevel": {
+        "Default": "Information",
+        "Microsoft.Health": "Information",


[nitpick] This appears to be an unrelated configuration change in a development settings file. Changes to logging levels should typically be in a separate PR or should be documented in the PR description if intentional.

Suggested change

"Microsoft.Health": "Information",

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs

SergeyGaluzo

Please use SilentMetaUpdate instead of SilentMeta

LTA-Thinking · 2025-10-27T16:19:39Z

/azp run

azure-pipelines · 2025-10-27T16:19:55Z

Azure Pipelines successfully started running 1 pipeline(s).

LTA-Thinking · 2025-10-30T21:14:59Z

I changed the name to Meta History as the only thing it now does is decide to store or not store a historical version.

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs

brendankowitz

Code Review: PR #5144 - Add Silent Meta Update

Overview

This PR adds a _meta-history query parameter for PUT requests that allows metadata-only changes to be made without creating a historical version of the resource. This is an optimization to reduce database bloat when only metadata (like tags) is updated.

Critical Issue - Potential PATCH Regression and Data Loss

HIGH SEVERITY: Default Value Mismatch Causes History Loss for All PATCH Operations

Location: UpsertResourceRequest.cs:18

public UpsertResourceRequest(ResourceElement resource, BundleResourceContext bundleResourceContext = null, 
    WeakETag weakETag = null, bool metaHistory = false)  // ⚠️ Default is FALSE

Location: PatchResourceHandler.cs:82

return await _mediator.Send<UpsertResourceResponse>(
    new UpsertResourceRequest(patchedResource, request.BundleResourceContext, request.WeakETag), 
    cancellationToken);  // ⚠️ metaHistory not passed, uses default FALSE

The Problem:

UpsertResourceRequest constructor defaults metaHistory = false
PatchResourceHandler doesn't pass the metaHistory parameter
When metaHistory = false AND ChangesAreOnlyInMetadata() returns true, NO HISTORY IS KEPT

Location: SqlServerFhirDataStore.cs:304-307

else if (!resourceExt.MetaHistory && ChangesAreOnlyInMetadata(resource, existingResource))
{
    metaHistory = false;  // Prevents history creation!
}

Location: SqlServerFhirDataStore.cs:350

mergeWrappersWithVersions.Add((new MergeResourceWrapper(resource, resourceExt.KeepHistory && metaHistory, ...

Impact Scenario:

Any PATCH operation that only modifies metadata (tags, security labels, etc.) will silently skip creating a historical version, even though:

The user never requested this behavior
PATCH should always create history (standard FHIR behavior)
The ADR explicitly states: "This is planned to be added to PATCH...in the future" - meaning it was NOT intended for PATCH yet

Inconsistency: ResourceWrapperOperation vs UpsertResourceRequest Defaults

Class	`metaHistory` Default
`ResourceWrapperOperation`	`true` (safe)
`UpsertResourceRequest`	`false` (dangerous)

This inconsistency is confusing and error-prone.

Additional Issues

2. Fragile JSON String Parsing

Location: StringExtensions.cs:154-207 (GetJsonSection)

public static string GetJsonSection(this string input, int startingIndex)

This method parses JSON by tracking brackets and quotes manually. Risks:

Unicode escapes not handled (\u0022 for quote)
Edge cases with nested objects containing "meta": in string values could cause incorrect parsing
Returns empty string for unmatched brackets instead of throwing (silent failures)

Location: SqlServerFhirDataStore.cs:974

var inputMeta = inputData.GetJsonSection(inputMetaStartIndex);
inputDataWithoutMeta = inputData.Replace(inputMeta, string.Empty, StringComparison.Ordinal);

Replacing the meta section content but leaving "meta":{} structure could cause comparison mismatches.

3. Inconsistent Default in FhirController vs UpsertResourceRequest

Location: FhirController.cs:228

[FromQuery(Name = KnownQueryParameterNames.MetaHistory)] bool metaHistory = true

The controller defaults to true, but:

UpsertResourceRequest defaults to false
This means explicitly calling the constructor without the parameter behaves opposite to the HTTP API

4. Unused Import

Location: StringExtensions.cs:9

using Hl7.FhirPath.Sprache;  // Not used in file

Recommendations

Critical Fix Required:

Change UpsertResourceRequest.metaHistory default to true to match safe behavior and ResourceWrapperOperation
Update PatchResourceHandler to explicitly pass metaHistory: true to ensure PATCH always creates history

Suggested Fix:

// UpsertResourceRequest.cs
public UpsertResourceRequest(ResourceElement resource, BundleResourceContext bundleResourceContext = null, 
    WeakETag weakETag = null, bool metaHistory = true)  // Change to TRUE

// PatchResourceHandler.cs - Be explicit
return await _mediator.Send<UpsertResourceResponse>(
    new UpsertResourceRequest(patchedResource, request.BundleResourceContext, request.WeakETag, metaHistory: true), 
    cancellationToken);

Summary

Category	Rating
Code Correctness	❌ Critical bug - PATCH silently loses history
Project Conventions	⚠️ Inconsistent defaults between classes
Performance	✅ Good - Reduces unnecessary DB writes
Test Coverage	⚠️ Missing tests for PATCH with meta-only changes
Security	⚠️ Data integrity risk (audit trail loss)

Verdict: This PR contains a critical regression where PATCH operations modifying only metadata will silently lose historical versions. This should be patched immediately.

🤖 Generated with Claude Code

Robert Johnson added 8 commits August 29, 2025 07:32

Start adding metadata comparison

dd13d55

Small addition

ad98003

Add flag

181a52a

Add silent meta

fcae89c

Merge remote-tracking branch 'origin/main' into personal/rojo/put-met…

d9e439e

…adata

It's working!

bf4f4b1

Fix tests

dbd85c1

Add parameter name

bc6fac3

github-advanced-security bot found potential problems Sep 15, 2025

View reviewed changes

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs Fixed Show fixed Hide fixed

Robert Johnson added 5 commits October 9, 2025 08:01

Remove 96

a1563c5

Merge from main

72caff8

Schema 98

1971bd5

Use KeepHistory

5efca08

Fix v97

649cf3b

LTA-Thinking added Enhancement Enhancement on existing functionality. Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs labels Oct 13, 2025

LTA-Thinking added this to the CY25Q4/2Wk01 milestone Oct 13, 2025

Add tests

17dcd4d

LTA-Thinking marked this pull request as ready for review October 14, 2025 21:33

LTA-Thinking requested a review from a team as a code owner October 14, 2025 21:33

Add test traits

7606003

feordin requested a review from Copilot October 16, 2025 21:25

Copilot AI reviewed Oct 16, 2025

View reviewed changes

feordin reviewed Oct 16, 2025

View reviewed changes

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs Outdated Show resolved Hide resolved

feordin reviewed Oct 16, 2025

View reviewed changes

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs Outdated Show resolved Hide resolved

feordin reviewed Oct 16, 2025

View reviewed changes

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs Outdated Show resolved Hide resolved

SergeyGaluzo reviewed Oct 16, 2025

View reviewed changes

Robert Johnson added 2 commits October 22, 2025 14:37

Change comparison method

ae00677

Address build issues

125d2a2

Robert Johnson added 2 commits October 27, 2025 14:40

Change name to meta history

9a3de9d

Fix test failures

db59ae3

feordin reviewed Oct 31, 2025

View reviewed changes

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs Outdated Show resolved Hide resolved

Robert Johnson added 2 commits November 3, 2025 12:31

address comment

2a2aed3

update adr

a41b654

feordin approved these changes Nov 4, 2025

View reviewed changes

LTA-Thinking merged commit 8c505de into main Nov 4, 2025
50 checks passed

LTA-Thinking deleted the personal/rojo/put-metadata branch November 4, 2025 17:26

brendankowitz reviewed Nov 26, 2025

View reviewed changes

	// Assert: Historical record should not be created (version and lastUpdated remain change)
	// Assert: Historical record should not be created (version and lastUpdated remain unchanged)

	return false; // Difference is in a non-meta field
	return false; // No metadata changes; any difference must be in non-meta fields

	// The end of the file has been reached
	// The end of the resource's children has been reached

Add silent meta update #5144

Add silent meta update #5144

Uh oh!

Conversation

LTA-Thinking commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Testing

FHIR Team Checklist

Semver Change (docs)

Uh oh!

Uh oh!

SergeyGaluzo commented Sep 16, 2025

Uh oh!

LTA-Thinking commented Sep 17, 2025

Uh oh!

SergeyGaluzo commented Sep 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SergeyGaluzo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LTA-Thinking commented Oct 27, 2025

Uh oh!

azure-pipelines bot commented Oct 27, 2025

Uh oh!

LTA-Thinking commented Oct 30, 2025

Uh oh!

Uh oh!

Uh oh!

brendankowitz left a comment

Choose a reason for hiding this comment

Code Review: PR #5144 - Add Silent Meta Update

Overview

Critical Issue - Potential PATCH Regression and Data Loss

HIGH SEVERITY: Default Value Mismatch Causes History Loss for All PATCH Operations

Impact Scenario:

Inconsistency: ResourceWrapperOperation vs UpsertResourceRequest Defaults

Additional Issues

2. Fragile JSON String Parsing

3. Inconsistent Default in FhirController vs UpsertResourceRequest

4. Unused Import

Recommendations

Critical Fix Required:

Suggested Fix:

Summary

Uh oh!

Reviewers

Assignees

Labels

LTA-Thinking commented Sep 15, 2025 •

edited

Loading

SergeyGaluzo left a comment •

edited

Loading