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

WIP: Implement the ITI-48 transaction (Retrieve Value Set, SOAP) #354

Merged
merged 1 commit into from
Jan 19, 2025

Conversation

qligier
Copy link
Contributor

@qligier qligier commented May 30, 2021

This is an implementation of the IHE ITI-48 transaction, Retrieve Value Set, for the SOAP binding only. It introduces new modules: ipf-commons-ihe-svs and ipf-platform-camel-ihe-svs (SVS for Sharing Value Sets profile). Models for request and response are introduced, along with Camel converters. A simple validator (based on XSD) is implemented. The audit record messages are generated.

List of issues with the specification, and how I dealt with them:

  1. In case of an error, the NAV and VERUNK codes can't be specified as a fault code, as per SOAP 1.2 specification. Instead, in the implementation, they're specified as fault subcode (without namespace) and the fault code is set to SENDER.
  2. In the audit record messages (consumer and repository), there's a typo in the transaction name: Retrieve Value Sets instead of Retrieve Value Set (as it's used elsewhere in the specification). The implementation uses the latter.
  3. In the audit record messages (consumer and repository), the participantObjectDetail contains the value set version and is mandatory on the repository side. In the DICOM specification, it's not a free text field but a type-value pair element. The specification does not define what value to use for @type; this implementation uses type="ValueSetVersion".

There are other small issues that are not preventing an implementation. Not sure if they are worth writing CPs.

@unixoid
Copy link
Member

unixoid commented May 30, 2021

The long service creation time is most probably caused by the huge XML Schemas which contain ca. 27000 lines. We have had the same problem many years ago when implementing PIXv3/PDQv3 components.

@qligier
Copy link
Contributor Author

qligier commented May 31, 2021

I found the culprit. It wasn't the schema complexity but the following import in the schema:

<xs:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="http://www.w3.org/2001/xml.xsd"/>

It was most probably hitting the HTTP request timeout, thus the constant 30 seconds delay.

I've nonetheless simplified the schema.

@unixoid
Copy link
Member

unixoid commented May 31, 2021

FYI: we have a local copy of it in /commons/ihe/hl7v3/src/main/resources/schema.

@qligier
Copy link
Contributor Author

qligier commented May 31, 2021

It does not seem to be useful, the definitions are self-contained or come from the http://www.w3.org/2001/XMLSchema namespace, so I don't think we've a good reason to reintroduce it as a local import.

@unixoid
Copy link
Member

unixoid commented Jul 15, 2021

@qligier: eHealth Suisse told us you need a new IPF release. No problem. But do you want this pull request to be part of it? If yes, please finalize it. :-)

@qligier
Copy link
Contributor Author

qligier commented Jul 16, 2021

Yes, it would be nice to start the eHealthConnector refactoring based on IPF 4.1

I've updated the only part of the code that I wanted to change but there's no urgency for this, at least on my side: we're manually including the code in one of our HUG project (so not waiting on a release), and I think SVS is out of scope of the eHC.
I've implemented as close as possible to the spec while fixing some of the issues, so it's not perfect but cannot be improved yet. I'm waiting for Lynn's answer about the specification issues and will fix the implementation once a change request is written or voted (no idea about the timeline).
So whether you want to merge it and open an issue to track IHE fixes on this or let it open and merge only when the implementation is compliant is up to you.

@unixoid
Copy link
Member

unixoid commented Jul 23, 2021

@qligier: FYI: IPF 4.1.0 is released.

@qligier qligier force-pushed the feature-tmp-svs branch 2 times, most recently from 8ac5059 to 68523cc Compare January 14, 2025 10:05
@qligier qligier marked this pull request as ready for review January 14, 2025 15:58
@qligier
Copy link
Contributor Author

qligier commented Jan 14, 2025

I've rebased the PR on top of master and fixed the parts that were not done yet. The PR should ultimately be ready for review.
Tests are passing, the CI fails submitting the dependency graph.

@unixoid unixoid self-assigned this Jan 15, 2025
@unixoid unixoid merged commit e17d1ef into oehf:master Jan 19, 2025
4 of 5 checks passed
@unixoid unixoid added this to the IPF 5.0 milestone Jan 19, 2025
@unixoid
Copy link
Member

unixoid commented Jan 19, 2025

Hello @qligier
Thank you for the contribution! I made some changes, added a Spring Boot starter and documentation.
Do I understand that you created JAXB stub classes and object factories manually instead of using xjc or wscompile?
And: can I close #458?

unixoid added a commit that referenced this pull request Jan 19, 2025
@qligier
Copy link
Contributor Author

qligier commented Jan 20, 2025

Hi @unixoid! Thank you for your work!

Yes, I wasn't pleased with the XSD provided by IHE. I took the decision to write the few classes needed by hand. And at the time of writing the JAXB factories, it was simpler to continue by hand than to find the rights commands to generate the schema from the classes, and then regenerate the classes with the factories from the schema.
Luckily, the SVS class hierarchy is really simple.

I also tweaked the XSD file to improve the validation coverage, and added the XML XSD because xml:lang was causing issues with the XSD validator.

@qligier qligier deleted the feature-tmp-svs branch February 20, 2025 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants