Skip to content
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

Use of valid code from profile's extended binding results in misleading warning #3052

Closed
prb112 opened this issue Nov 29, 2021 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@prb112
Copy link
Contributor

prb112 commented Nov 29, 2021

Describe the bug
A clear and concise description of what the bug is.

{
    "severity": "warning",
    "code": "code-invalid",
    "details": {
        "text": "Code 'hl7-fhir-opn' is invalid"
    },
    "expression": [
        "Endpoint.connectionType"
    ]
}

Environment
Which version of IBM FHIR Server? latest release / main

To Reproduce
Steps to reproduce the behavior:

  1. Create an test project linked to fhir-ig-davinci-plannet
  2. Create an Endpoint without a display name
Endpoint.Builder builder = Endpoint.builder();
        builder.setValidating(false);
        builder.meta(Meta.builder()
                            .profile(
                                Canonical.of("http://hl7.org/fhir/us/davinci-pdex-plan-net/StructureDefinition/plannet-Endpoint"))
                            .lastUpdated(Instant.now())
                            .build());
        builder.connectionType(Coding.builder()
            .code(Code.of("hl7-fhir-opn"))
            .system(Uri.of("http://hl7.org/fhir/us/davinci-pdex-plan-net/CodeSystem/EndpointConnectionTypeCS"))
            .build());
        System.out.println(builder.build());
        Endpoint endpoint = builder.build()
  1. Run the following:
        List<Issue> issues = FHIRValidator.validator().validate(endpoint);
        issues.forEach(System.out::println);
  1. Generates the following warning...
{
    "severity": "warning",
    "code": "code-invalid",
    "details": {
        "text": "Code 'hl7-fhir-opn' is invalid"
    },
    "expression": [
        "Endpoint.connectionType"
    ]
}

To work around the issue...

        //.display("HL7 FHIR Operation") // Commented out we emit a warning
{
    "system": "http://hl7.org/fhir/us/davinci-pdex-plan-net/CodeSystem/EndpointConnectionTypeCS",
    "code": "hl7-fhir-opn",
    "display": "HL7 FHIR Operation"
}
    private ValidationOutcome validateCode(CodeSystem codeSystem, Coding coding, boolean result, LookupOutcome outcome) {
        java.lang.String message = null;
        if (!result && coding != null && coding.getCode() != null) {
            message = java.lang.String.format("Code '%s' is invalid", coding.getCode().getValue());
        }
        if (result && outcome != null && coding != null &&
                outcome.getDisplay() != null && coding.getDisplay() != null &&
                outcome.getDisplay().getValue() != null && coding.getDisplay().getValue() != null) {
            java.lang.String system = null;
            if (coding.getSystem() != null) {
                system = coding.getSystem().getValue();
            } else if (codeSystem != null && codeSystem.getUrl() != null) {
                system = codeSystem.getUrl().getValue();
            }
            boolean caseSensitive = (codeSystem != null) ? CodeSystemSupport.isCaseSensitive(codeSystem) : false;
            if (codeSystem == null && system != null) {
                java.lang.String version = (coding.getVersion() != null) ? coding.getVersion().getValue() : null;
                java.lang.String url = (version != null) ? system + "|" + version : system;
                caseSensitive = CodeSystemSupport.isCaseSensitive(url);
            }
            result = caseSensitive ? outcome.getDisplay().equals(coding.getDisplay()) : normalize(outcome.getDisplay().getValue()).equals(normalize(coding.getDisplay().getValue()));
            message = !result ? java.lang.String.format("The display '%s' is incorrect for code '%s' from code system '%s'", coding.getDisplay().getValue(), coding.getCode().getValue(), system) : null;
        }
        return ValidationOutcome.builder()
                .result(result ? Boolean.TRUE : Boolean.FALSE)
                .message((message != null) ? string(message) : null)
                .display((outcome != null) ? outcome.getDisplay() : null)
                .build();
    }

Expected behavior
A clear and concise description of what you expected to happen.

Expected this to work without a display

Additional context
Add any other context about the problem here.

@prb112 prb112 added the bug Something isn't working label Nov 29, 2021
@prb112 prb112 added this to the Sprint 2021-16 milestone Nov 29, 2021
@prb112 prb112 self-assigned this Nov 29, 2021
@prb112
Copy link
Contributor Author

prb112 commented Nov 29, 2021

Note:

  1. The memberOf function is run multiple times regardless of first success.
  2. The memberOf function also aggregate across the providers rather than single successful result.

@lmsurpre
Copy link
Member

I circled back with Paul and we were unable to reproduce the original issue.
However, after some discussion, we felt that we should:

  1. use this issue to produce a better error message. currently we say "code [x] is invalid" but I think it would be better to indicate what we mean by "invalid" and why (e.g. which valueset / codesystem we validated against)
  2. open a separate issue to improve our handling in cases where
    • the base spec has an extensible binding
    • an IG has extended that binding with some additional options
    • an instance is using one of the extended codes from that IG

@lmsurpre lmsurpre changed the title Missing Display in Coding during validation results in Error Use of valid code from profile's extended binding results in misleading warning Nov 30, 2021
@lmsurpre
Copy link
Member

I opened #3056 to address the misleading warning.
After this update, the described scenario's Code 'hl7-fhir-opn' is invalid issue will be replaced with issue detail like the following:

Code 'hl7-fhir-opn' in system 'http://hl7.org/fhir/us/davinci-pdex-plan-net/CodeSystem/EndpointConnectionTypeCS' is not a valid member of ValueSet with URL=http://hl7.org/fhir/ValueSet/endpoint-connection-type and version=4.0.1"

@lmsurpre
Copy link
Member

lmsurpre commented Dec 2, 2021

After further discussion we also opted to reduce the severity level of these messages from "warning" to "information" (#3063).

Additionally we reduced the severity of the corresponding issues produced by the FHIRPath memberOf function.

Old:

binding strength severity detail text
extensible warning The concept in this element must be from the specified value set '[url]' if possible
preferred warning The concept in this element should be from the specified value set '[url]' if possible
warning Code 'hl7-fhir-opn' is invalid

New:

binding strength severity detail text
extensible information A code in this element must be from the specified value set '[url]' if possible
preferred information A code in this element should be from the specified value set '[url]' if possible
information Code '[code]' in system '[url]' is not a valid member of ValueSet with URL=[vs_url] and version=[vs_version]

@prb112
Copy link
Contributor Author

prb112 commented Dec 2, 2021

Ran my test case and altered it and the valueset to hit the new messages.
Works as expected

package com.ibm.fhir.ig.davinci.plannet.test;

import java.util.List;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;

import org.testng.annotations.Test;

import com.ibm.fhir.model.resource.Endpoint;
import com.ibm.fhir.model.resource.OperationOutcome.Issue;
import com.ibm.fhir.model.type.Canonical;
import com.ibm.fhir.model.type.Code;
import com.ibm.fhir.model.type.Coding;
import com.ibm.fhir.model.type.Instant;
import com.ibm.fhir.model.type.Meta;
import com.ibm.fhir.model.type.Uri;
import com.ibm.fhir.path.exception.FHIRPathException;
import com.ibm.fhir.profile.ConstraintGenerator;
import com.ibm.fhir.validation.FHIRValidator;
import com.ibm.fhir.validation.exception.FHIRValidationException;

public class PPNetTest {
    @Test
    public void testEndpoint() throws FHIRValidationException, FHIRPathException {
        Logger logger = Logger.getLogger(ConstraintGenerator.class.getName());
        logger.setLevel(Level.FINEST);
        Handler handler = new Handler() {
            @Override
            public void publish(LogRecord record) {
                System.out.println(record.getMessage());
            }

            @Override
            public void flush() {
                System.out.flush();
            }

            @Override
            public void close() throws SecurityException { }
        };
        handler.setLevel(Level.FINEST);
        logger.addHandler(handler);

        Endpoint.Builder builder = Endpoint.builder();
        builder.setValidating(false);
        builder.meta(Meta.builder()
                            .profile(
                                Canonical.of("http://hl7.org/fhir/us/davinci-pdex-plan-net/StructureDefinition/plannet-Endpoint"))
                            .lastUpdated(Instant.now())
                            .build());
        builder.connectionType(Coding.builder()
            .code(Code.of("hl7-fhir-opn"))
            .display("HL7 FHIR Operation") // Commented out we emit a warning
            .system(Uri.of("http://hl7.org/fhir/us/davinci-pdex-plan-net/CodeSystem/EndpointConnectionTypeCS"))
            .build());
        System.out.println(builder.build());
        Endpoint endpoint = builder.build();

        List<Issue> issues = FHIRValidator.validator().validate(endpoint);
        issues.forEach(System.out::println);
    }
}

@prb112 prb112 closed this as completed Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants