LEGLINK-922: Add support for ignoring validation results based on configurable rules - #1801
LEGLINK-922: Add support for ignoring validation results based on configurable rules#1801seanmcilvenna wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughValidation now supports configurable rules that match validation results by field and regular expression. Matching results are filtered after deduplication. Four MeasureReport-related rules and test coverage were added. ChangesValidation result ignore rules
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ValidationService
participant ValidationResultIgnoreService
participant ValidationResults
ValidationService->>ValidationResultIgnoreService: filterIgnored(results)
ValidationResultIgnoreService->>ValidationResults: evaluate configured matchers
ValidationResults-->>ValidationResultIgnoreService: matching fields
ValidationResultIgnoreService-->>ValidationService: filtered results
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Java/validation/src/main/java/com/lantanagroup/link/validation/services/ValidationResultIgnoreService.java (1)
86-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused matcher branch tests.
Add xUnit tests for an inverted matcher, a composite matcher with
requiresAllChildren=false, andSEVERITYandCODEfield matching. Keep these tests local and do not use network activity.As per path instructions, “If/Else or Switch/Case blocks are introduced or modified — ensure each branch has a corresponding unit test.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Java/validation/src/main/java/com/lantanagroup/link/validation/services/ValidationResultIgnoreService.java` around lines 86 - 99, Add focused local xUnit tests covering the branches in ValidationResultIgnoreService.buildMatcher: inverted matching, composite matching with requiresAllChildren=false, and RegexMatcher field matching for SEVERITY and CODE. Exercise each behavior through the service’s public API, avoid network activity, and ensure the modified conditional branches have corresponding tests.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Java/validation/src/main/java/com/lantanagroup/link/validation/services/ValidationResultIgnoreService.java`:
- Around line 44-48: Update the logger.debug call in
ValidationResultIgnoreService to sanitize matchingRule.id(),
result.getExpression(), and result.getMessage() with the repository’s
established sanitizer before passing them as logging arguments. Preserve the
existing blank/default fallback behavior while ensuring every argument is
sanitized before the logger call.
In `@Java/validation/src/main/resources/application.yml`:
- Around line 114-124: Update the validation-result-ignore-rules entries
ignore_measureeval_measure_report_population_description_invalid_version and
ignore_sde_reference_extension to use composite matchers requiring both the
existing EXPRESSION path and the specific known false-positive message or
severity. Update the associated rule tests with nearby errors that match the
path but differ in message or severity, and verify those results are not
ignored.
---
Nitpick comments:
In
`@Java/validation/src/main/java/com/lantanagroup/link/validation/services/ValidationResultIgnoreService.java`:
- Around line 86-99: Add focused local xUnit tests covering the branches in
ValidationResultIgnoreService.buildMatcher: inverted matching, composite
matching with requiresAllChildren=false, and RegexMatcher field matching for
SEVERITY and CODE. Exercise each behavior through the service’s public API,
avoid network activity, and ensure the modified conditional branches have
corresponding tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 841d7876-89f5-4431-a1a9-80bb57a0c335
📒 Files selected for processing (8)
Java/validation/src/main/java/com/lantanagroup/link/validation/configs/LinkConfig.javaJava/validation/src/main/java/com/lantanagroup/link/validation/configs/ValidationResultIgnoreRuleConfig.javaJava/validation/src/main/java/com/lantanagroup/link/validation/services/ValidationResultIgnoreService.javaJava/validation/src/main/java/com/lantanagroup/link/validation/services/ValidationService.javaJava/validation/src/main/resources/application.ymlJava/validation/src/test/java/com/lantanagroup/link/validation/services/ReadyForValidationConsumerTest.javaJava/validation/src/test/java/com/lantanagroup/link/validation/services/ValidationResultIgnoreServiceTest.javaJava/validation/src/test/java/com/lantanagroup/link/validation/services/ValidationServiceTest.java
| logger.debug( | ||
| "Ignoring validation result via rule {}: expression='{}', message='{}'", | ||
| StringUtils.defaultIfBlank(matchingRule.id(), "<unnamed>"), | ||
| StringUtils.defaultString(result.getExpression()), | ||
| StringUtils.defaultString(result.getMessage())); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sanitize every logging argument before the logger call.
StringUtils.defaultIfBlank and StringUtils.defaultString do not sanitize values. Pass matchingRule.id(), result.getExpression(), and result.getMessage() through the repository sanitizer before logger.debug.
As per path instructions, all logging message arguments must be sanitized before they are passed to logger methods.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Java/validation/src/main/java/com/lantanagroup/link/validation/services/ValidationResultIgnoreService.java`
around lines 44 - 48, Update the logger.debug call in
ValidationResultIgnoreService to sanitize matchingRule.id(),
result.getExpression(), and result.getMessage() with the repository’s
established sanitizer before passing them as logging arguments. Preserve the
existing blank/default fallback behavior while ensuring every argument is
sanitized before the logger call.
Source: Path instructions
| validation-result-ignore-rules: | ||
| - id: ignore_measureeval_measure_report_population_description_invalid_version | ||
| description: Ignore known MeasureEval-generated MeasureReport population description extension version warnings | ||
| matcher: | ||
| field: EXPRESSION | ||
| regex: "\\.extension\\[[0-9]+\\]\\[url='http://hl7\\.org/fhir/5\\.0/StructureDefinition/extension-MeasureReport\\.population\\.description'\\]$" | ||
| - id: ignore_sde_reference_extension | ||
| description: Ignore known MeasureReport.supplementalDataElement.reference extension | ||
| matcher: | ||
| field: EXPRESSION | ||
| regex: "\\.extension\\[[0-9]+\\]\\[url='http://hl7\\.org/fhir/5\\.0/StructureDefinition/extension-MeasureReport\\.supplementalDataElement\\.reference'\\]$" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Constrain these rules to the known false-positive warning.
Each rule matches only EXPRESSION. It can therefore discard any validation error at the same extension path. ValidationResultIgnoreService removes matched results before categorization, persistence, and validity calculation.
Use a composite matcher that requires the target expression and the specific expected message or severity. Add a nearby non-matching error case to the rule tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Java/validation/src/main/resources/application.yml` around lines 114 - 124,
Update the validation-result-ignore-rules entries
ignore_measureeval_measure_report_population_description_invalid_version and
ignore_sde_reference_extension to use composite matchers requiring both the
existing EXPRESSION path and the specific known false-positive message or
severity. Update the associated rule tests with nearby errors that match the
path but differ in message or severity, and verify those results are not
ignored.
🛠️ Description of Changes
Added a configuration-driven validation result ignore capability to the Java Validation service so known non-actionable false positives can be suppressed before they enter the normal validation workflow.
This change:
Also added default ignore rules for known Measure Evaluation-generated false positives.
🧪 Testing Performed
Ran targeted Validation service unit tests from the Java reactor:
Ran validation service within docker and performed a validation using Postman with a very large bundle that was generated on the TEST environment, and observed that a bunch of the false positives are no longer included in the response.
🧑🔬 Unit Testing
📓 Documentation Updated
Plan to update the Keystone documentation on the Validation service's configurability from this change, after the PR has been merged.