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

Update to HAPI FHIR 7.0.0 #618

Merged
merged 22 commits into from
Feb 27, 2024
Merged

Update to HAPI FHIR 7.0.0 #618

merged 22 commits into from
Feb 27, 2024

Conversation

jamesagnew
Copy link
Contributor

This PR is tracking the HAPI FHIR 7.0.x changes, using Spring 6 / Boot 3, and the new Jakarta APIs

@jamesagnew
Copy link
Contributor Author

Note: WebSockets don't currently work in this PR. Spring-Test 6.x requires Jetty 12, but Spring Boot 3.x seems to be incompatible with Jetty 12, so this is causing some weirdness as we've upgraded hapi-fhir to 12.

It looks like this is solved in Spring 6.1 so I'm going to try and get us up to that version soon.

@jamesagnew
Copy link
Contributor Author

Websockets are now enabled, and this branch has been upgraded to use Jetty 12

stmsat and others added 14 commits December 15, 2023 15:05
…t_hapi_7_0

# Conflicts:
#	src/main/java/ca/uhn/fhir/jpa/starter/Application.java
…rting with tomcat, and running as web application doesn't work if the bean is not at least lazy)

I'm removing it, it shouldn't be necessary (to run spring-spring boot with jetty, it's enough to enable the jetty profile?)
…nability with jetty and tomcat, tests, and deployment as web application
added required jetty dependencies through spring-boot-starter-jetty so this is the only dependency with scope=testing and transitive dependencies (core jetty libraries) aren't marked as such

mvn -P jetty spring-boot:run seems to work
jetty-maven-plugin is obsolete, and jetty-ee10-maven-plugin doesn't work at the moment. Using spring-boot to run local server.
…ing-boot:run by defaults starts tomcat, while the jetty profile enables jetty

added a fix to run the generated war with spring-boot on tomcat, due to issues with spring-boot 3.2.0 spring-projects/spring-boot#38585
the jar generated with jetty doesn't work at the moment
…ess "jetty" profile is active (in which case they're done with jetty)

Test class JpaStarterWebsocketDispatcherConfig emptied, not sure if necessary (it's now missing jetty dependencies)
@stmsat
Copy link
Contributor

stmsat commented Jan 3, 2024

Hi James, I opened a merge request with your branch as the destination (#624) because I encountered issues when trying it (running spring-boot, tests, packaging a runnable war with at least tomcat, because jetty still has some issues), and had a somewhat hard time solving (or trying to solve) them; I hope you'll find it useful, in whole or part.

stmsat and others added 3 commits January 25, 2024 08:53
… JpaStarterWebsocketDispatcherConfig and references

unrequested changes: commons-logging dependency marked provided (to avoid redundant jar inclusion), removed unused imports in classes that were modified
Revision of jetty dependencies and maven profiles to preserve runnability as webapp, spring-boot, and testing
@XcrigX
Copy link
Contributor

XcrigX commented Feb 7, 2024

Any idea on when this will be merged?

@XcrigX
Copy link
Contributor

XcrigX commented Feb 14, 2024

I'm hitting an issue adding spring security to this branch.
I haven't fully debugged it, but I think the problem is that hapi-fhir-storage-cr has an explicit/older spring-security-core version:

https://github.com/hapifhir/hapi-fhir/blob/master/hapi-fhir-storage-cr/pom.xml#L22
<spring-security-core.version>5.7.8</spring-security-core.version>

I'm seeing app context creation errors when adding my own spring-boot-starter-security dependency to the starter project. I would guess SMILE would have similar issues?

My two cents on the side, it might be much easier to maintain things if the HAPI root POM imported the SpringBoot BOM so all dependencies are vetted and compatible with a given SpringBoot version. HAPI is a super-complex project, so possible there are roadblocks to doing that, but.. maybe?

EDIT to confirm: I moved my spring-boot-starter-security above hapi-fhir-storage-cr in my POM so it takes precedence and the problems went away. Still - I think hapi-fhir-storage-cr needs to be updated to inherit the spring-security-core version.

@jkiddo
Copy link
Collaborator

jkiddo commented Feb 27, 2024

@dotasek will this be merged anytime soon?

@dotasek dotasek changed the title Update to HAPI FHIR 7.0.0 Draft - Do not merge yet! Update to HAPI FHIR 7.0.0 Feb 27, 2024
@dotasek dotasek self-requested a review February 27, 2024 19:46
@dotasek dotasek merged commit 4ad7c3c into master Feb 27, 2024
5 checks passed
@patrick-werner patrick-werner deleted the ja_20231203_hapi_7_0 branch April 23, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants