-
Notifications
You must be signed in to change notification settings - Fork 1
LEGLINK-922: Add support for ignoring validation results based on configurable rules #1801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package com.lantanagroup.link.validation.configs; | ||
|
|
||
| import com.lantanagroup.link.validation.entities.ResultField; | ||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| @Getter | ||
| @Setter | ||
| public class ValidationResultIgnoreRuleConfig { | ||
| private String id; | ||
| private String description; | ||
| private MatcherConfig matcher; | ||
|
|
||
| @Getter | ||
| @Setter | ||
| public static class MatcherConfig { | ||
| private ResultField field; | ||
| private String regex; | ||
| private boolean inverted; | ||
| private boolean requiresAllChildren; | ||
| private List<MatcherConfig> children; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| package com.lantanagroup.link.validation.services; | ||
|
|
||
| import com.lantanagroup.link.validation.configs.LinkConfig; | ||
| import com.lantanagroup.link.validation.configs.ValidationResultIgnoreRuleConfig; | ||
| import com.lantanagroup.link.validation.entities.Result; | ||
| import com.lantanagroup.link.validation.matchers.CompositeMatcher; | ||
| import com.lantanagroup.link.validation.matchers.Matcher; | ||
| import com.lantanagroup.link.validation.matchers.RegexMatcher; | ||
| import org.apache.commons.collections4.CollectionUtils; | ||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| @Service | ||
| public class ValidationResultIgnoreService { | ||
| private static final Logger logger = LoggerFactory.getLogger(ValidationResultIgnoreService.class); | ||
|
|
||
| private final List<CompiledValidationResultIgnoreRule> rules; | ||
|
|
||
| public ValidationResultIgnoreService(LinkConfig linkConfig) { | ||
| List<ValidationResultIgnoreRuleConfig> configuredRules = linkConfig.getValidationResultIgnoreRules(); | ||
| this.rules = configuredRules == null | ||
| ? List.of() | ||
| : configuredRules.stream() | ||
| .map(this::compileRule) | ||
| .toList(); | ||
| } | ||
|
|
||
| public List<Result> filterIgnored(List<Result> results) { | ||
| if (CollectionUtils.isEmpty(results) || CollectionUtils.isEmpty(rules)) { | ||
| return results; | ||
| } | ||
|
|
||
| List<Result> filtered = new ArrayList<>(results.size()); | ||
| int ignoredCount = 0; | ||
| for (Result result : results) { | ||
| CompiledValidationResultIgnoreRule matchingRule = getFirstMatchingRule(result); | ||
| if (matchingRule != null) { | ||
| ignoredCount++; | ||
| logger.debug( | ||
| "Ignoring validation result via rule {}: expression='{}', message='{}'", | ||
| StringUtils.defaultIfBlank(matchingRule.id(), "<unnamed>"), | ||
| StringUtils.defaultString(result.getExpression()), | ||
| StringUtils.defaultString(result.getMessage())); | ||
| continue; | ||
| } | ||
|
|
||
| filtered.add(result); | ||
| } | ||
|
|
||
| if (ignoredCount > 0) { | ||
| logger.debug("Ignored {} validation result(s) using configured validation-result-ignore rules", ignoredCount); | ||
| } | ||
|
|
||
| return filtered; | ||
| } | ||
|
|
||
| String getFirstMatchingRuleId(Result result) { | ||
| CompiledValidationResultIgnoreRule rule = getFirstMatchingRule(result); | ||
| return rule == null ? null : rule.id(); | ||
| } | ||
|
|
||
| private CompiledValidationResultIgnoreRule getFirstMatchingRule(Result result) { | ||
| for (CompiledValidationResultIgnoreRule rule : rules) { | ||
| if (rule.matcher().isMatch(result)) { | ||
| return rule; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| private CompiledValidationResultIgnoreRule compileRule(ValidationResultIgnoreRuleConfig config) { | ||
| ValidationResultIgnoreRuleConfig.MatcherConfig matcherConfig = config.getMatcher(); | ||
| if (matcherConfig == null) { | ||
| throw new IllegalStateException("Validation-result-ignore rule '" + StringUtils.defaultIfBlank(config.getId(), "<unnamed>") + "' is missing matcher"); | ||
| } | ||
|
|
||
| return new CompiledValidationResultIgnoreRule(config.getId(), buildMatcher(matcherConfig)); | ||
| } | ||
|
|
||
| private Matcher buildMatcher(ValidationResultIgnoreRuleConfig.MatcherConfig config) { | ||
| if (CollectionUtils.isNotEmpty(config.getChildren())) { | ||
| CompositeMatcher matcher = new CompositeMatcher(); | ||
| matcher.setInverted(config.isInverted()); | ||
| matcher.setRequiresAllChildren(config.isRequiresAllChildren()); | ||
| matcher.setChildren(config.getChildren().stream().map(this::buildMatcher).toList()); | ||
| return matcher; | ||
| } | ||
|
|
||
| RegexMatcher matcher = new RegexMatcher(); | ||
| matcher.setInverted(config.isInverted()); | ||
| matcher.setField(config.getField()); | ||
| matcher.setRegex(config.getRegex()); | ||
| return matcher; | ||
| } | ||
|
|
||
| private record CompiledValidationResultIgnoreRule(String id, Matcher matcher) { | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,27 @@ authentication: | |
| link: | ||
| info-route: /api/validation/info | ||
| terminology-service-url: '' | ||
| 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'\\]$" | ||
|
Comment on lines
+114
to
+124
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Constrain these rules to the known false-positive warning. Each rule matches only 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 |
||
| - id: ignore_measureeval_measure_report_population_description_unknown_extension | ||
| description: Ignore known MeasureEval-generated MeasureReport population description unknown extension warnings | ||
| matcher: | ||
| field: MESSAGE | ||
| regex: "^Unknown extension http://hl7\\.org/fhir/5\\.0/StructureDefinition/extension-MeasureReport\\.population\\.description$" | ||
| - id: ignore_deprecated_criteria_reference_extension | ||
| description: Ignore deprecated criteriaReference extension warnings from measure-generated content | ||
| matcher: | ||
| field: MESSAGE | ||
| regex: "^The extension http://hl7\\.org/fhir/us/davinci-deqm/StructureDefinition/extension-criteriaReference\\|5\\.0\\.0 is deprecated$" | ||
| # Retry for all HAPI FHIR REST clients (terminology validate-code/$lookup/etc.). | ||
| # Transient failures only: connection IOExceptions, HTTP 429, and 5xx (e.g. Envoy | ||
| # "503 no healthy upstream" during a terminology-service rollout). 4xx never retries. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package com.lantanagroup.link.validation.services; | ||
|
|
||
| import com.lantanagroup.link.validation.configs.LinkConfig; | ||
| import com.lantanagroup.link.validation.configs.ValidationResultIgnoreRuleConfig; | ||
| import com.lantanagroup.link.validation.entities.Result; | ||
| import com.lantanagroup.link.validation.entities.ResultField; | ||
| import org.hl7.fhir.r4.model.OperationOutcome; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import static org.junit.jupiter.api.Assertions.assertNull; | ||
|
|
||
| class ValidationResultIgnoreServiceTest { | ||
|
|
||
| @Test | ||
| void filterIgnored_removesResultWhenAnyRuleMatches() { | ||
| LinkConfig config = new LinkConfig(); | ||
| config.setValidationResultIgnoreRules(List.of(messageRule("ignore_deprecated", "deprecated"))); | ||
| ValidationResultIgnoreService service = new ValidationResultIgnoreService(config); | ||
|
|
||
| Result ignored = new Result(); | ||
| ignored.setMessage("The extension http://example is deprecated"); | ||
|
|
||
| Result kept = new Result(); | ||
| kept.setMessage("A different validation message"); | ||
|
|
||
| List<Result> filtered = service.filterIgnored(List.of(ignored, kept)); | ||
|
|
||
| assertEquals(1, filtered.size()); | ||
| assertEquals(kept, filtered.get(0)); | ||
| } | ||
|
|
||
| @Test | ||
| void getFirstMatchingRule_requiresAllConfiguredFieldsToMatch() { | ||
| LinkConfig config = new LinkConfig(); | ||
| config.setValidationResultIgnoreRules(List.of(expressionAndMessageRule())); | ||
| ValidationResultIgnoreService service = new ValidationResultIgnoreService(config); | ||
|
|
||
| Result result = new Result(); | ||
| result.setExpression("Bundle.entry[0].resource.ofType(MeasureReport).extension[0][url='http://hl7.org/fhir/5.0/StructureDefinition/extension-MeasureReport.population.description']"); | ||
| result.setMessage("Unknown extension http://hl7.org/fhir/5.0/StructureDefinition/extension-MeasureReport.population.description"); | ||
|
|
||
| String ruleId = service.getFirstMatchingRuleId(result); | ||
|
|
||
| assertNotNull(ruleId); | ||
| assertEquals("ignore_measure_report_population_description", ruleId); | ||
| } | ||
|
|
||
| @Test | ||
| void getFirstMatchingRule_returnsNullWhenOneRequiredFieldDoesNotMatch() { | ||
| LinkConfig config = new LinkConfig(); | ||
| config.setValidationResultIgnoreRules(List.of(expressionAndMessageRule())); | ||
| ValidationResultIgnoreService service = new ValidationResultIgnoreService(config); | ||
|
|
||
| Result result = new Result(); | ||
| result.setExpression("Bundle.entry[0].resource.ofType(Patient).extension[0]"); | ||
| result.setMessage("Unknown extension http://hl7.org/fhir/5.0/StructureDefinition/extension-MeasureReport.population.description"); | ||
| result.setSeverity(OperationOutcome.IssueSeverity.INFORMATION); | ||
|
|
||
| assertNull(service.getFirstMatchingRuleId(result)); | ||
| } | ||
|
|
||
| private static ValidationResultIgnoreRuleConfig messageRule(String id, String regex) { | ||
| ValidationResultIgnoreRuleConfig rule = new ValidationResultIgnoreRuleConfig(); | ||
| rule.setId(id); | ||
| ValidationResultIgnoreRuleConfig.MatcherConfig matcher = new ValidationResultIgnoreRuleConfig.MatcherConfig(); | ||
| matcher.setField(ResultField.MESSAGE); | ||
| matcher.setRegex(regex); | ||
| rule.setMatcher(matcher); | ||
| return rule; | ||
| } | ||
|
|
||
| private static ValidationResultIgnoreRuleConfig expressionAndMessageRule() { | ||
| ValidationResultIgnoreRuleConfig rule = new ValidationResultIgnoreRuleConfig(); | ||
| rule.setId("ignore_measure_report_population_description"); | ||
|
|
||
| ValidationResultIgnoreRuleConfig.MatcherConfig expressionMatcher = new ValidationResultIgnoreRuleConfig.MatcherConfig(); | ||
| expressionMatcher.setField(ResultField.EXPRESSION); | ||
| expressionMatcher.setRegex("Bundle\\.entry\\[[0-9]+\\]\\.resource\\.ofType\\(MeasureReport\\)\\.extension\\[[0-9]+\\]\\[url='http://hl7\\.org/fhir/5\\.0/StructureDefinition/extension-MeasureReport\\.population\\.description'\\]"); | ||
|
|
||
| ValidationResultIgnoreRuleConfig.MatcherConfig messageMatcher = new ValidationResultIgnoreRuleConfig.MatcherConfig(); | ||
| messageMatcher.setField(ResultField.MESSAGE); | ||
| messageMatcher.setRegex("^Unknown extension http://hl7\\.org/fhir/5\\.0/StructureDefinition/extension-MeasureReport\\.population\\.description$"); | ||
|
|
||
| ValidationResultIgnoreRuleConfig.MatcherConfig compositeMatcher = new ValidationResultIgnoreRuleConfig.MatcherConfig(); | ||
| compositeMatcher.setChildren(List.of(expressionMatcher, messageMatcher)); | ||
| compositeMatcher.setRequiresAllChildren(true); | ||
| rule.setMatcher(compositeMatcher); | ||
| return rule; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sanitize every logging argument before the logger call.
StringUtils.defaultIfBlankandStringUtils.defaultStringdo not sanitize values. PassmatchingRule.id(),result.getExpression(), andresult.getMessage()through the repository sanitizer beforelogger.debug.As per path instructions, all logging message arguments must be sanitized before they are passed to logger methods.
🤖 Prompt for AI Agents
Source: Path instructions