Skip to content

Feat/extra resource loading from npm #784

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 27 commits into from
Aug 27, 2025
Merged

Feat/extra resource loading from npm #784

merged 27 commits into from
Aug 27, 2025

Conversation

jkiddo
Copy link
Collaborator

@jkiddo jkiddo commented Feb 23, 2025

This feature is backwards compatible and introduces the option to load e.g. example packages from IG's based on configuration

@jamesagnew
Copy link
Contributor

This functionality would probably make sense to just roll into the base PackageInstallationSpec. I'd rather not have this project implementing novel features not actually found in the base project, that's hard for support.

@jkiddo
Changed to set
4b509a8
@jkiddo
Merge branch 'hapi-master' into feat/extra-resource-loading-from-npm
578ee8b 
# Conflicts:
#	src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java
@jkiddo
Changed PostConstruct to Component and introduced use of PartitionCon…
e6b4866 
…dition

Added try-load of HAPI optimized dialect
@jkiddo
Copy link
Collaborator Author

jkiddo commented Feb 24, 2025

Sure ... I can make a PR to the base getting the additionalResourceFolders added the PackageInstallationSpec.

@jkiddo
Copy link
Collaborator Author

jkiddo commented Feb 24, 2025

@jamesagnew whats your take on the refactored version of the PostConstruct I've done here in https://github.com/hapifhir/hapi-fhir-jpaserver-starter/pull/784/files#diff-0f42a12718125be49d417ad9c402f5f2cecddfb7cb0ecfde364ad09132fa2a99R21 as mentioned in #758

@jkiddo
Copy link
Collaborator Author

jkiddo commented Feb 24, 2025

and btw - the same thing I did on https://github.com/hapifhir/hapi-fhir-jpaserver-starter/pull/784/files#diff-cca286d85a1c14aea8b529430ecd47aa4a803b53f4c7816deb7d602bd2af3268R11 can be done for a loooooong range of interceptor logic currently handled in the already way-too-big

hapi-fhir-jpaserver-starter/src/main/java/ca/uhn/fhir/jpa/starter/common/StarterJpaConfig.java

Line 262 in 792f520

public RestfulServer restfulServer(
method (I'm not pointing fingers here as I most likely authored a lot of it myself).

@jkiddo jkiddo marked this pull request as ready for review February 25, 2025 07:56
@jkiddo jkiddo requested a review from dotasek February 25, 2025 07:56
@jkiddo
Copy link
Collaborator Author

jkiddo commented Feb 27, 2025

@jamesagnew I'll move the additional sources logic once its been merged in here - acceptable for you? Then we can merge this into #785 and remove it from the starter as soon as it can be resolved from a snapshot in core.

@jkiddo
Reverted to default config
dea6fc6
@jkiddo
Merge branch 'hapi-master' into feat/extra-resource-loading-from-npm
224b2bd 
# Conflicts:
#	src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java
@jkiddo
Apply spotless
bad73a5
@jkiddo
Merge branch 'master' into feat/extra-resource-loading-from-npm
99e76ae 
# Conflicts:
#	src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java
jkiddo added a commit to jkiddo/hapi-fhir that referenced this pull request Mar 14, 2025
@jkiddo
Added reusable feature set from hapifhir/hapi-fhir-jpaserver-starter#784
81d0811
@jkiddo
Copy link
Collaborator Author

jkiddo commented Mar 14, 2025

@jamesagnew / @dotasek - I've isolated the feature set now into hapifhir/hapi-fhir#6802. Let me know if thats the feature set you had in mind, before I start writing tests and all.

@jkiddo jkiddo mentioned this pull request Mar 19, 2025
Merged
@jkiddo jkiddo requested a review from Copilot April 29, 2025 07:38
Copilot

This comment was marked as outdated.

Sign in to view

tadgh added a commit to hapifhir/hapi-fhir that referenced this pull request May 2, 2025
@jkiddo @tadgh
Feat/additional folder options (#6802)
cbec00e 
* Added reusable feature set from hapifhir/hapi-fhir-jpaserver-starter#784

* Added a few sanity tests

* Update hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallationSpec.java

Co-authored-by: tadgh <[email protected]>

* Addressed most comments

* Reduced package size and added exception

* Added changelog

---------

Co-authored-by: tadgh <[email protected]>
@jkiddo
Merge branch 'hapi-master' into feat/extra-resource-loading-from-npm
3206140 
# Conflicts:
#	src/main/resources/application.yaml
@robogary
Copy link

robogary commented May 7, 2025

Formatting check succeeded!

@robogary
Copy link

Formatting check succeeded!

@robogary
Copy link

Formatting check succeeded!

@robogary
Copy link

robogary commented Jun 5, 2025

Formatting check succeeded!

@robogary
Copy link

robogary commented Jun 5, 2025

Formatting check succeeded!

@jkiddo
Copy link
Collaborator Author

jkiddo commented Jun 5, 2025

@dotasek once 8.4 is out the door this should be good to go.

@jkiddo
Copy link
Collaborator Author

jkiddo commented Jul 11, 2025

Which would be after #842

@robogary
Copy link

This Pull Request has failed the formatting check

Please run mvn spotless:apply or mvn clean install -DskipTests to fix the formatting issues.

You can automate this auto-formatting process to execute on the git pre-push hook, by installing pre-commit and then calling pre-commit install --hook-type pre-push. This will cause formatting to run automatically whenever you push.

@robogary
Copy link

Formatting check succeeded!

@jkiddo jkiddo requested a review from Copilot August 25, 2025 14:24
Copilot
Copilot AI reviewed Aug 25, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for loading additional resources (e.g., example packages) from Implementation Guides based on configuration, extending the existing IG installation functionality while maintaining backwards compatibility.

  • Introduces ExtendedPackageInstallationSpec class to support additional resource folder configuration
  • Updates package installation logic to process additional resources from specified folders
  • Refactors partition mode configuration to use constructor injection and conditional bean creation

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/main/java/ca/uhn/fhir/jpa/starter/ig/ExtendedPackageInstallationSpec.java New class extending PackageInstallationSpec with additionalResourceFolders property
src/main/java/ca/uhn/fhir/jpa/starter/common/StarterJpaConfig.java Updated package installer to handle additional resources and changed type references
src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java Modified to use ExtendedPackageInstallationSpec and added new property
src/main/java/ca/uhn/fhir/jpa/starter/common/PartitionModeConfigurer.java Refactored to use constructor injection and conditional bean creation
src/main/java/ca/uhn/fhir/jpa/starter/common/OnPartitionModeEnabled.java New condition class for partition mode enablement
src/main/resources/application.yaml Updated example configuration with additional resource folder example
pom.xml Updated HAPI FHIR version and revision number
Dockerfile Updated OpenTelemetry Java agent version

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@robogary
Copy link

This Pull Request has failed the formatting check

Please run mvn spotless:apply or mvn clean install -DskipTests to fix the formatting issues.

You can automate this auto-formatting process to execute on the git pre-push hook, by installing pre-commit and then calling pre-commit install --hook-type pre-push. This will cause formatting to run automatically whenever you push.

@jkiddo jkiddo marked this pull request as ready for review August 25, 2025 14:33
@robogary
Copy link

This Pull Request has failed the formatting check

Please run mvn spotless:apply or mvn clean install -DskipTests to fix the formatting issues.

You can automate this auto-formatting process to execute on the git pre-push hook, by installing pre-commit and then calling pre-commit install --hook-type pre-push. This will cause formatting to run automatically whenever you push.

@robogary
Copy link

Formatting check succeeded!

@robogary
Copy link

Formatting check succeeded!

Copilot
Copilot AI reviewed Aug 25, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@robogary
Copy link

Formatting check succeeded!

@jkiddo jkiddo requested a review from patrick-werner August 25, 2025 18:50
@jkiddo jkiddo merged commit 8621c0d into hapifhir:master Aug 27, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@patrick-werner patrick-werner patrick-werner approved these changes

@dotasek dotasek Awaiting requested review from dotasek

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkiddo @jamesagnew @robogary @patrick-werner