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

Support for external terminology service #2832

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

feordin
Copy link
Contributor

@feordin feordin commented Sep 27, 2022

Description

Adds necessary support for configuring an external terminology service and creates endpoints to pass-thru terminology requests.

Related issues

Addresses [issue #].

Testing

Manual testing performed as well as automated tests added.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 50 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, Dependencies, Enhancement, or New-Feature
  • Tag the PR with Azure API for FHIR if this will release to the Azure API for FHIR managed service (CosmosDB or common code related to service)
  • Tag the PR with Azure Healthcare APIs if this will release to the Azure Healthcare APIs managed service (Sql server or common code related to service)
  • CI is green before merge
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)
Semver=Feature

@feordin feordin added New Feature Label for a new feature in FHIR OSS Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs labels Sep 27, 2022
@feordin feordin added this to the S98 milestone Sep 27, 2022
@feordin feordin requested a review from a team as a code owner September 27, 2022 23:43
@lgtm-com
Copy link

lgtm-com bot commented Sep 28, 2022

This pull request introduces 5 alerts and fixes 1 when merging f431f72 into dd9129f - view on LGTM.com

new alerts:

  • 4 for Missing Dispose call on local IDisposable
  • 1 for Dereferenced variable may be null

fixed alerts:

  • 1 for Dereferenced variable may be null

"ExternalTerminologyServer": "http://tx.fhir.org/r4",
"Validate": {
"CacheDurationInSeconds": 14400,
"ProfileValidationTerminologyServer": "http://hapi.fhir.org/baseR4"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the issues we had with this branch was the test severs not being reliable for running E2E tests. We wanted to replace them with our in house server, but that didn't get done in time.
We could skip all the E2E tests for terminology, but I don't feel great about including a feature that doesn't have tests run against it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be done with an in-memory terminology server making it less dependent on external systems.

SamuilDIntern and others added 6 commits February 3, 2023 09:44
* Support for external terminology server

* Fixed Issue with $export

* Bug #1459: FIxed issue with $export error code when request type is Batch or Transaction.

* Resources can now be validated against US Core

* Added tests for $validate using external terminology service
TODO: Need to make tests better.

* Updated US-Core profiles in normative from STU3 -> R4.
Wrote Tests for $Validate on a patient resource against us-core.

* Improved added tests
Added GenderIdentity USCORE-profile to normative
Set "Definitions.zip" as "content" and "Copy Always" so that its path can be found
Modified some pre-existing tests so that they account for profile validation.

* Fixed appsettings

* Removed unrelated bug-fix code for ExportNotSupportedInBundle

* Validate tests pass on STU3 and R4, most pass on R5.
Fixed issue with ZipSource which prevented defintion file summaries from being loaded.
Removed last bit of code irrelevant work.

* Added ValueSet-BirthSex to R4 and R5
Validate Tests passing for all versions

* Cleaned up csproj for Stu3, R4, and R5 .web

* Updated TestConfiguration.json to use new external terminology service endpoint

* Changed endpoint back to hapi

Co-authored-by: Jared Erwin <[email protected]>
* Support for external terminology server

* Fixed Issue with $export

* Bug #1459: FIxed issue with $export error code when request type is Batch or Transaction.

* Resources can now be validated against US Core

* Added tests for $validate using external terminology service
TODO: Need to make tests better.

* Updated US-Core profiles in normative from STU3 -> R4.
Wrote Tests for $Validate on a patient resource against us-core.

* Improved added tests
Added GenderIdentity USCORE-profile to normative
Set "Definitions.zip" as "content" and "Copy Always" so that its path can be found
Modified some pre-existing tests so that they account for profile validation.

* Fixed appsettings

* Removed unrelated bug-fix code for ExportNotSupportedInBundle

* Validate tests pass on STU3 and R4, most pass on R5.
Fixed issue with ZipSource which prevented defintion file summaries from being loaded.
Removed last bit of code irrelevant work.

* Added ValueSet-BirthSex to R4 and R5
Validate Tests passing for all versions

* Cleaned up csproj for Stu3, R4, and R5 .web

* Updated TestConfiguration.json to use new external terminology service endpoint

* Changed endpoint back to hapi

* Validate-Code operationHandler. OperationRequest, and OpeartionResponse.

* ValueSet $Validate-Code is mostly working and testing is partially implemented using OntoServer as external TS

* Added Testing for Get version of valueset $validate-code

* Created Terminology Controller and profile validator now uses external terminology service by default

* Implemented POST and GET $Validate-Code for both ValueSets and CodeSystems.
Some E2E tests are implemented but more are to come.

* Added more testing for $validate-code
Implemented GET version of $lookup

* $validate-code does not work for STU3 as the TS is on R4
Fixed error where some operations would return an error saying server could not read conformance statement.

* Implemented $lookup and wrote E2E tests for said operation

* Finished Implementing  and testing $expand
Cleaned up terminology related code.

* Added logging when definitions folder is not found or external/fallback terminology service cannot be created

Added ProfileValidationTerminologServer endpoint in the test configuration file

* Fixed Test names in TerminologyOperationTests

* Final code clean up before submitting PR
Terminology related tests will be skipped if there is no external endpoint set.

* Removed unused variable

* Fixed bug where tests were failing because CreateAsync was using PUT request instead of POST

* Added:
Terminology Operation Filters for TerminologyControlle,
Terminology Operations to Capability Statement

Fixed:
Batch tests failing because $lookup was assuming to not work,
Tests failing due to InProcTestServer only available in local environment

Improved code standard to fit with the rest of FHIR server codebase

* Fixed Capability Statement

* Added temporary operatioDefinition json files.

* Added Operation definition for terminology operations.

* Fixed problem with testConfiguration settings not being updated.
Fixed small bug in TerminologyOperator

* Removed code in CheckResults method from Terminology Operator that retried terminology operation if the error was related to conformance statement read issues.

Fixed issue in bundle-batch that assumed $lookup was not implemented.

* Added checks in terminology controller to make sure operation that is to be executed is enabled in appsettings.

Added separate testconfiguration.json for each version of FHIR.

Placed profiler resolver in a try catch if the profile validation terminology endpoint is empty

* Removed old testconfiguration.json file that was causing tests to not be skipped when necessary.

* Updated test configuration path in provision-deploy.yml

Added terminology module logging messages to API.Resources.resx

* Added break out of for each look once bool has been set to true in Lookup and Expand parameters filter.

More code clean up

* Fixed package.yml to account for separate, version specific, testconfiguration files.

* Trying to fix build & deploy for test configuration

* Corrected HAPI endpoint for STU3 in testconfiguration.json

Corrected validate and terminology tests where they would break out of for each loop early.

* Fixed tests that were not working correctly.

* Fixed comment typo

* Fixed Typos

* Fixed test name typo.

* $validate was already being added to the capability statement somewhere else within the FHIR service, so I removed it from where I was adding the terminology operations.

Also fixed typo.

* Added Terminology Operations Documentation.

* Update TerminologyOperations.md

* Update TerminologyOperations.md

Co-authored-by: Jared Erwin <[email protected]>
@feordin feordin force-pushed the feature/terminologyservice branch from f431f72 to f832f3a Compare February 9, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs New Feature Label for a new feature in FHIR OSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants