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

Refactor fhir-term module #1530

Closed
JohnTimm opened this issue Sep 29, 2020 · 5 comments
Closed

Refactor fhir-term module #1530

JohnTimm opened this issue Sep 29, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request terminology

Comments

@JohnTimm
Copy link
Collaborator

JohnTimm commented Sep 29, 2020

The fhir-term module should be updated as follows:

  1. Add support to FHIRTermService for multiple FHIRTermServiceProvider instances:
	private FHIRTermServiceProvider findProvider(CodeSystem codeSystem) {
            return providers.stream()
			.filter(provider -> provider.isSupported(codeSystem))
			.findFirst()
			.orElse(null);
	}
  1. Add methods to FHIRTermServiceProvider interface:
    boolean isSupported(CodeSystem)
    Concept findConcept(CodeSystem, Code)
    Concept findConcept(CodeSystem, Code, Code)

  2. Remove methods from FHIRTermServiceProvider interface:
    boolean isExpandable(ValueSet)
    Consider removing validateCode(ValueSet, ...) methods
    Consider removing expand(ValueSet) method

  3. Update methods in FHIRTermService class:
    boolean isExpandable(ValueSet) should be updated to call ValueSetSupport.isExpandable(ValueSet)
    ValidationResult validateCode(ValueSet, ...) methods should be updated to call ValueSetSupport.validateCode(ValueSet, ...)
    Consider removing dependency on FHIRTermServiceProvider interface

  4. Add implementation for new methods in DefaultTermServiceProvider:
    boolean isSupported(CodeSystem) -> CodeSystem.content = 'complete'
    Concept findConcept(CodeSystem, Code) -> calls CodeSystemSupport.findConcept(...)
    Concept findConcept(CodeSystem, Code, Code) -> calls CodeSystemSupport.findConcept(...)

  5. Updated methods and inner classes in ValueSetSupport
    Update isExpandable to use FHIRTermService.getInstance().isSupported(CodeSystem)
    Update createDescendOfFilter, createGeneralizesFilter, createIsAFilter, createIsNotFilter to use FHIRTermService.getInstance().findConcept(...) method
    Update DescendantOfFilter, GeneralizesFilter, IsAFilter, EqualsFilter to use FHIRTermService.getInstance().findConcept(...) method

  6. Centeralize code set caching: move code set caching from DefaultTermServiceProvider back into the ValueSetSupport class

@JohnTimm JohnTimm added the enhancement New feature or request label Sep 29, 2020
@JohnTimm JohnTimm self-assigned this Sep 29, 2020
@JohnTimm
Copy link
Collaborator Author

JohnTimm commented Nov 30, 2020

Proposed simplified FHIRTermServiceProvider interface:

public interface FHIRTermServiceProvider {
    boolean isSupported(CodeSystem codeSystem);
    Concept findConcept(CodeSystem codeSystem, Code code);
    Concept findConcept(CodeSysten codeSystem, Concept concept, Code code);
    LookupOutcome lookup(CodeSystem codeSystem, Code code, LookupParameters parameters);
    default LookupOutcome lookup(CodeSystem codeSystem, Code code) {
        return lookup(codeSystem, code, LookupParameter.EMPTY);
    }
    ConceptSubsumptionOutcome subsumes(CodeSystem codeSystem, Code codeA, Code codeB);
    ConceptMap closure(CodeSystem codeSystem, Code code);
    ValidationOutcome validateCode(CodeSystem codeSystem, Code code, String display, ValidationParameters parameters);
    default ValidationOutcome validateCode(CodeSystem codeSystem, Code code, String display) {
        return validateCode(codeSystem, code, display, ValidationParameters.EMPTY);
    }
}

@lmsurpre lmsurpre added this to the Sprint 2021-03 milestone Feb 17, 2021
JohnTimm added a commit that referenced this issue Feb 22, 2021
Signed-off-by: John T.E. Timm <johntimm@us.ibm.com>
JohnTimm added a commit that referenced this issue Feb 22, 2021
Signed-off-by: John T.E. Timm <johntimm@us.ibm.com>
JohnTimm added a commit that referenced this issue Feb 22, 2021
* Upgrade Pom.xml to latest versions of Dependencies #1959

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Update to include Bulkdata

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Push version dependencies into parent

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Update Dependencies and Resolve Duplicates in the pom.xmls, shifted the dependency versions to dependencyManagement in the parent pom

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* fix: update per code review and fix compilation

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Remove changes to Apache Derby and add a comment not to upgrade

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Update Test Certificates to Expire in Year 2051 (#1900)

* Update Test Certificates to Expire in Year 2051

- Update CI/CD Audit resources (jks)
- Update Minio Resources (crt,key)
- Update fhir-server trust and key store (p12)
- Update fhir-client (p12)
- Create build/certificates with documentation on certificates and utility scripts

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Unify the CA that signs the Client/Server

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Unify the Certificates including Minio

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Unify the Certificates including Minio

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* fhir-server-test relies on certificates in p12 files expire in April 2021 #1276

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* fhir-server-test relies on certificates in p12 files expire in April 2021 #1276

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Update to include subject alternate names

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Update to include subject alternate names

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* update for notification ci and per review comments

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Fix: pom.xml

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

* Issue #1530 - fhir-term refactoring (#1975)

Signed-off-by: John T.E. Timm <johntimm@us.ibm.com>

Co-authored-by: Paul Bastide <pbastide@us.ibm.com>
@lmsurpre
Copy link
Member

I think we need to update the javadoc for subsumes:

    /**
     * Find the concept in tree rooted by the provided concept that matches the specified code.
     *
     * @param codeSystem
     *     the code system
     * @param codeA
     *     the root of the hierarchy to search
     * @param codeB
     *     the code to match
     * @return
     *     the code system concept that matches the specified code, or null if not such concept exists
     */
    boolean subsumes(CodeSystem codeSystem, Code codeA, Code codeB);

It returns a boolean but the javadoc implies it will return a Concept

@lmsurpre
Copy link
Member

was just reading at https://www.hl7.org/fhir/operation-codesystem-subsumes.html

The “A” Coding that is to be tested. The code system does not have to match the specified subsumption code system, but the relationships between the code systems must be well established

we don’t support subsumption testing across codesystems, right? is that a limitation of the current interface?

lmsurpre added a commit that referenced this issue Mar 31, 2021
1. clarify the contract for FHIRTermServiceProvider.getConcept
2. update stale javadoc for FHIRTermServiceProvider.subsumes

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Copy link
Member

I opened https://github.com/IBM/FHIR/pull/2181/files with my recommended javadoc updates.

lmsurpre added a commit that referenced this issue Apr 5, 2021
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Copy link
Member

lmsurpre commented Apr 5, 2021

was just reading at https://www.hl7.org/fhir/operation-codesystem-subsumes.html

The “A” Coding that is to be tested. The code system does not have to match the specified subsumption code system, but the relationships between the code systems must be well established

we don’t support subsumption testing across codesystems, right? is that a limitation of the current interface?

I discussed this with @JohnTimm and he wanted to keep just the single-system variant for now (since this is all we support at the moment). If/when we get a FHIRTermServiceProvider implementation that can test for subsumption across systems, we'll need to update the interface to support that.

@lmsurpre lmsurpre closed this as completed Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request terminology
Projects
None yet
Development

No branches or pull requests

2 participants