Add OpenMRS FHIR IGenericClient spring bean #65
Conversation
corneliouzbett
commented
Jul 30, 2024
corneliouzbett
commented
- This PR adds a convenient IGenericClient Spring bean to be used by downstream projects.
camel-openmrs-fhir/src/main/java/org/openmrs/eip/fhir/spring/OpenmrsFhirConfiguration.java
Outdated
Show resolved
Hide resolved
| @@ -51,16 +69,7 @@ public void beforeApplicationStart(CamelContext camelContext) { | |||
| FhirConfiguration fhirConfiguration = new FhirConfiguration(); | |||
| FhirContext ctx = FhirContext.forR4(); | |||
There was a problem hiding this comment.
Aren't you supposed to get rid of these lines 69 and 70 since you're are now using the new spring bean?
There was a problem hiding this comment.
No, still need both. The new spring bean IGenericClient should be set to FhirConfiguration which is then set camel FHIR component.
| @@ -42,28 +44,29 @@ public boolean isOauthEnabled() { | |||
| return isOauthEnabled; | |||
| } | |||
|
|
|||
| @Bean | |||
| @Qualifier("openmrsFhirClient") | |||
| IGenericClient openmrsFhirClient() { | |||
There was a problem hiding this comment.
Is there a reason why these are not public?
There was a problem hiding this comment.
openmrsFhirClient does not need to be public because it is only used within the OpenmrsFhirConfiguration class, not intended to publicly exposed and I don't want it to be misused. I could ask the reverse, why public?
| IGenericClient client = FhirContext.forR4().newRestfulGenericClient(fhirServerUrl); | ||
| if (isOauthEnabled) { | ||
| client.registerInterceptor(oauth2Interceptor); | ||
| } else if (StringUtils.isNotBlank(fhirUsername) && StringUtils.isNotBlank(fhirPassword)) { |
There was a problem hiding this comment.
Does this mean they are optional?
| // verify BasicAuthInterceptor is registered | ||
| assertNotNull(interceptors); | ||
| assertTrue(interceptors.stream().anyMatch(BasicAuthInterceptor.class::isInstance)); | ||
| } |
There was a problem hiding this comment.
From a pure unit test perspective, I don't think I like the way these tests are setup, because instead of calling the logic you're testing from the test method i.e. constructor, you're testing a pre-constructed bean by spring. I guess it's okay but I'd have preferred you call the constructor from the test method itself.
