Skip to content

Add header for recursive validation#5410

Draft
LTA-Thinking wants to merge 1 commit intomainfrom
personal/rojo/recursive-validate
Draft

Add header for recursive validation#5410
LTA-Thinking wants to merge 1 commit intomainfrom
personal/rojo/recursive-validate

Conversation

@LTA-Thinking
Copy link
Contributor

Description

Adds a header x-ms-recursive-validation to turn on recursive validation for resources. Without this header only the top level resources are validated.
x-ms-recursive-validation: true

Related issues

Addresses [issue #].

Testing

Manual and new unit 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 Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

[Fact]
public void GivenAResource_WhenValidatingWithNullRequestContext_ThenRecursiveValidationDefaultsToDisabled()
{
_requestContextAccessor.RequestContext.Returns((IFhirRequestContext)null);

Check warning

Code scanning / CodeQL

Useless upcast Warning

There is no need to upcast from
null
to
IFhirRequestContext
- the conversion can be done implicitly.
Comment on lines +100 to +107
if (fhirContext.RequestHeaders.ContainsKey(KnownHeaders.RecursiveValidation)
&& fhirContext.RequestHeaders.TryGetValue(KnownHeaders.RecursiveValidation, out var headerValue))
{
if (bool.TryParse(headerValue, out bool recursiveValidation))
{
return recursiveValidation;
}
}

Check notice

Code scanning / CodeQL

Nested 'if' statements can be combined Note

These 'if' statements can be combined.

Copilot Autofix

AI about 23 hours ago

In general, to fix this issue you should merge nested if statements that lack else branches into a single if whose condition is the logical AND (&&) of the original conditions. This maintains the same logical requirement (all conditions must be true) while simplifying control flow and reducing indentation.

In this file, within GetRecursiveValidationSetting, there is an outer if on lines 100–102 checking for the presence of the RecursiveValidation header and trying to get its value, then an inner if on lines 103–106 attempting to parse that value to a boolean. The behavior is: only when the header exists, is retrievable, and parses successfully do we return recursiveValidation; otherwise we fall through and return false. To fix this without changing functionality, we should combine the header presence check and the TryParse call into a single if statement. That means replacing the outer if block (including the inner if and its braces) with one if whose condition first checks ContainsKey and TryGetValue, and then bool.TryParse. All of the logic stays in GetRecursiveValidationSetting and no new imports or methods are required.

Concretely:

  • In src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceContentValidator.cs, in GetRecursiveValidationSetting(), replace the block starting with if (fhirContext.RequestHeaders.ContainsKey(... through the closing brace before return false; with a single combined if that calls bool.TryParse directly in the condition and returns recursiveValidation if successful.
  • No additional dependencies, using directives, or helper methods are needed.
Suggested changeset 1
src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceContentValidator.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceContentValidator.cs b/src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceContentValidator.cs
--- a/src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceContentValidator.cs
+++ b/src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceContentValidator.cs
@@ -98,12 +98,10 @@
 
             var fhirContext = _contextAccessor.RequestContext;
             if (fhirContext.RequestHeaders.ContainsKey(KnownHeaders.RecursiveValidation)
-                && fhirContext.RequestHeaders.TryGetValue(KnownHeaders.RecursiveValidation, out var headerValue))
+                && fhirContext.RequestHeaders.TryGetValue(KnownHeaders.RecursiveValidation, out var headerValue)
+                && bool.TryParse(headerValue, out bool recursiveValidation))
             {
-                if (bool.TryParse(headerValue, out bool recursiveValidation))
-                {
-                    return recursiveValidation;
-                }
+                return recursiveValidation;
             }
 
             return false;
EOF
@@ -98,12 +98,10 @@

var fhirContext = _contextAccessor.RequestContext;
if (fhirContext.RequestHeaders.ContainsKey(KnownHeaders.RecursiveValidation)
&& fhirContext.RequestHeaders.TryGetValue(KnownHeaders.RecursiveValidation, out var headerValue))
&& fhirContext.RequestHeaders.TryGetValue(KnownHeaders.RecursiveValidation, out var headerValue)
&& bool.TryParse(headerValue, out bool recursiveValidation))
{
if (bool.TryParse(headerValue, out bool recursiveValidation))
{
return recursiveValidation;
}
return recursiveValidation;
}

return false;
Copilot is powered by AI and may make mistakes. Always verify output.
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