Implement FHIR Bulk Export client. #1781

piotrszul · 2024-02-19T00:30:28Z

Resolves #1776

…AMESPACE file)

Adding a basic progress tracking callback mechanism.

…setup.

…r status.

Improved exception reporting.

…u.csiro.pathling logger)

Adding support for handling of Too Many Requests (429) status.

…ration).

…also support a timestamp (which is what we are getting from the test server).

codecov · 2024-02-19T00:30:43Z

Codecov Report

Attention: Patch coverage is 79.04564% with 202 lines in your changes are missing coverage. Please review.

Project coverage is 83.94%. Comparing base (76e21ab) to head (f7fd125).
Report is 13 commits behind head on main.

❗ Current head f7fd125 differs from pull request most recent head e626f38. Consider uploading reports for the commit e626f38 to get more accurate results

Files	Patch %	Lines
...siro/pathling/library/fs/HdfsFileStoreFactory.java	0.00%	28 Missing ⚠️
...ava/au/csiro/pathling/export/BulkExportClient.java	77.00%	20 Missing and 3 partials ⚠️
...u/csiro/pathling/export/ws/BulkExportTemplate.java	75.58%	14 Missing and 7 partials ⚠️
.../pathling/export/download/UrlDownloadTemplate.java	80.70%	7 Missing and 4 partials ⚠️
...iro/pathling/auth/SMARTTokenCredentialFactory.java	74.35%	8 Missing and 2 partials ⚠️
...ava/au/csiro/pathling/library/PathlingContext.java	0.00%	10 Missing ⚠️
...va/au/csiro/pathling/config/AuthConfiguration.java	50.00%	7 Missing and 1 partial ⚠️
...au/csiro/pathling/export/ws/BulkExportRequest.java	87.50%	6 Missing and 2 partials ⚠️
...siro/pathling/auth/AsymmetricClientAuthMethod.java	76.00%	5 Missing and 1 partial ⚠️
.../au/csiro/pathling/export/BulkExportException.java	81.81%	6 Missing ⚠️
... and 32 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1781      +/-   ##
============================================
- Coverage     84.67%   83.94%   -0.74%     
  Complexity      139      139              
============================================
  Files           341      384      +43     
  Lines          7868     8818     +950     
  Branches        528      592      +64     
============================================
+ Hits           6662     7402     +740     
- Misses          912     1067     +155     
- Partials        294      349      +55

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gitguardian · 2024-03-28T06:35:31Z

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

^{_{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!}}

…n bulkexport client.

…enCredentials.

Adding unit tests for AuthTokenProvider

…ntication.

- adding support for SMART configuration discovery - removing 'identifier' field from FHRI Reference()

Adding an example of python bulk export from Smart Health IT server with asymmetric keys.

johngrimes

Thanks for all of your work on this @piotrszul!

I've gone through it and come up with some feedback for you to take a look at.

johngrimes · 2024-04-11T01:00:07Z

bulkclient/pom.xml

+          <artifactId>*</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>


I think we should remove this dependency as the first step to separating this module from the main Pathling code base.

johngrimes · 2024-04-11T01:03:59Z

bulkclient/src/main/java/au/csiro/pathling/auth/AuthConst.java

+ * Constants for authentication.
+ */
+@UtilityClass
+public class AuthConst {


Perhaps these could be moved into SmartClientAuthMethodBase?

Or into the specific subclass where they are more specific, e.g. CLIENT_ASSERTION_TYPE_JWT_BEARER.

Reafactored.

johngrimes · 2024-04-11T01:06:32Z

bulkclient/src/main/java/au/csiro/pathling/auth/ClientAuthMethod.java

+ * Authentication method for one of the FHIR SMART client authentication profiles.
+ */
+
+public interface ClientAuthMethod {


Do we need this level of interface? Aren't all the authentication methods we support SMART client authentication methods?

Refactored.

johngrimes · 2024-04-11T01:12:49Z

bulkclient/src/main/java/au/csiro/pathling/auth/JWTUtils.java

+    return jwkAlgorithm
+        .replace("RS", "RSA")
+        .replace("ES", "ECDSA")
+        .replace("HC", "HMAC");


Is this a undocumented supported algorithm?

It should actually be "HS" not "HC" as if HS256.
it's one of the symmetric signing algorithms supported by auth0.
Added for completness as it's supported by 'Algorithm' class but we do not use that since we only expose asymmetric algorithms.

johngrimes · 2024-04-11T01:19:01Z

bulkclient/examples/bulk_export_smilecdr.py

+        auth_scope="system/*.read",
+    ),
+).export()
+print(f"TransactionTime: {as_fhir_instant(result.transaction_time)}")


I get the following error when running this script. This is probably because I didn't set the client secret environment variable, but perhaps it should handle this more gracefully.

:: loading settings :: url = jar:file:/opt/homebrew/Caskroom/miniconda/base/envs/pathling-dev/lib/python3.12/site-packages/pyspark/jars/ivy-2.5.1.jar!/org/apache/ivy/core/settings/ivysettings.xml Ivy Default Cache set to: /Users/gri306/.ivy2/cache The jars for the packages stored in: /Users/gri306/.ivy2/jars au.csiro.pathling#library-runtime added as a dependency io.delta#delta-core_2.12 added as a dependency org.apache.hadoop#hadoop-aws added as a dependency :: resolving dependencies :: org.apache.spark#spark-submit-parent-56feae64-188a-4257-9fe1-6edb5a66c4cd;1.0 confs: [default] found au.csiro.pathling#library-runtime;6.6.0-SNAPSHOT in local-m2-cache found io.delta#delta-core_2.12;2.4.0 in local-m2-cache found io.delta#delta-storage;2.4.0 in local-m2-cache found org.antlr#antlr4-runtime;4.9.3 in local-m2-cache found org.apache.hadoop#hadoop-aws;3.3.4 in local-m2-cache found com.amazonaws#aws-java-sdk-bundle;1.12.262 in local-m2-cache found org.wildfly.openssl#wildfly-openssl;1.0.7.Final in local-m2-cache :: resolution report :: resolve 172ms :: artifacts dl 7ms :: modules in use: au.csiro.pathling#library-runtime;6.6.0-SNAPSHOT from local-m2-cache in [default] com.amazonaws#aws-java-sdk-bundle;1.12.262 from local-m2-cache in [default] io.delta#delta-core_2.12;2.4.0 from local-m2-cache in [default] io.delta#delta-storage;2.4.0 from local-m2-cache in [default] org.antlr#antlr4-runtime;4.9.3 from local-m2-cache in [default] org.apache.hadoop#hadoop-aws;3.3.4 from local-m2-cache in [default] org.wildfly.openssl#wildfly-openssl;1.0.7.Final from local-m2-cache in [default] --------------------------------------------------------------------- | | modules || artifacts | | conf | number| search|dwnlded|evicted|| number|dwnlded| --------------------------------------------------------------------- | default | 7 | 0 | 0 | 0 || 7 | 0 | --------------------------------------------------------------------- :: retrieving :: org.apache.spark#spark-submit-parent-56feae64-188a-4257-9fe1-6edb5a66c4cd confs: [default] 0 artifacts copied, 7 already retrieved (0kB/8ms) 24/04/11 11:15:12 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable Setting default log level to "WARN". To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel). 24/04/11 11:15:15 DEBUG PathlingContext: Setting log level for 'au.csiro.pathling' to DEBUG 24/04/11 11:15:16 WARN SimpleFunctionRegistry: The function date_diff replaced a previously registered function. Exporting from: https://aehrc-cdr.cc/fhir_r4 to /Users/gri306/Code/pathling/bulkclient/target/export-1712798116 24/04/11 11:15:16 DEBUG BulkExportClient: Creating FileStore of: au.csiro.pathling.library.fs.HdfsFileStoreFactory@36abae5e for outputDir: /Users/gri306/Code/pathling/bulkclient/target/export-1712798116 24/04/11 11:15:16 DEBUG BulkExportClient: Creating HttpClient with configuration: HttpClientConfiguration(socketTimeout=60000, maxConnectionsTotal=32, maxConnectionsPerRoute=16, retryEnabled=true, retryCount=2) 24/04/11 11:15:16 DEBUG SMARTTokenCredentialFactory: Retrieving SMART configuration for fhirEndpoint: https://aehrc-cdr.cc/fhir_r4 24/04/11 11:15:16 DEBUG SMARTTokenCredentialFactory: SMART configuration retrieved: SMARTDiscoveryResponse(tokenEndpoint=https://aehrc-cdr.cc/smartsec_r4/oauth/token, grantTypesSupported=null, tokenEndpointAuthMethodsSupported=null, tokenEndpointAuthSigningAlgValuesSupported=null, capabilities=[launch-ehr, launch-standalone, client-public, client-confidential-symmetric, client-confidential-asymmetric, sso-openid-connect, context-ehr-patient, context-standalone-patient, permission-offline, permission-patient]) Traceback (most recent call last): File "/Users/gri306/Code/pathling/bulkclient/examples/bulk_export_smilecdr.py", line 66, in <module> ).export() ^^^^^^^^ File "/Users/gri306/Code/pathling/lib/python/pathling/bulkexport.py", line 157, in export return BulkExportResult.from_java(self._jclient.export()) ^^^^^^^^^^^^^^^^^^^^^^ File "/opt/homebrew/Caskroom/miniconda/base/envs/pathling-dev/lib/python3.12/site-packages/py4j/java_gateway.py", line 1322, in __call__ return_value = get_return_value( ^^^^^^^^^^^^^^^^^ File "/opt/homebrew/Caskroom/miniconda/base/envs/pathling-dev/lib/python3.12/site-packages/pyspark/errors/exceptions/captured.py", line 169, in deco return f(*a, **kw) ^^^^^^^^^^^ File "/opt/homebrew/Caskroom/miniconda/base/envs/pathling-dev/lib/python3.12/site-packages/py4j/protocol.py", line 326, in get_return_value raise Py4JJavaError( py4j.protocol.Py4JJavaError: An error occurred while calling o103.export. : java.lang.NullPointerException at java.base/java.util.Objects.requireNonNull(Objects.java:221) at au.csiro.pathling.auth.ClientAuthMethod.create(ClientAuthMethod.java:123) at au.csiro.pathling.auth.SMARTTokenCredentialFactory.createSMARTAuthMethod(SMARTTokenCredentialFactory.java:123) at au.csiro.pathling.auth.SMARTTokenCredentialFactory.createAuthMethod(SMARTTokenCredentialFactory.java:105) at au.csiro.pathling.auth.SMARTTokenCredentialFactory.createCredentials(SMARTTokenCredentialFactory.java:91) at au.csiro.pathling.export.BulkExportClient.createHttpClient(BulkExportClient.java:365) at au.csiro.pathling.export.BulkExportClient.export(BulkExportClient.java:250) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at py4j.reflection.MethodInvoker.invoke(MethodInvoker.java:244) at py4j.reflection.ReflectionEngine.invoke(ReflectionEngine.java:374) at py4j.Gateway.invoke(Gateway.java:282) at py4j.commands.AbstractCommand.invokeMethod(AbstractCommand.java:132) at py4j.commands.CallCommand.execute(CallCommand.java:79) at py4j.ClientServerConnection.waitForCommands(ClientServerConnection.java:182) at py4j.ClientServerConnection.run(ClientServerConnection.java:106) at java.base/java.lang.Thread.run(Thread.java:829)

Yes, this should be resolved by JSR-380 validation of config.
I was having some issues with the making it work (in this project - some dependency issues).

Added JRS-380 validation for the client configuration and it's subconfig beans.

bulkclient/src/main/java/au/csiro/pathling/export/ws/BulkExportTemplate.java

bulkclient/src/main/java/au/csiro/pathling/export/ws/BulkExportRequest.java

johngrimes · 2024-04-11T02:09:19Z

bulkclient/src/main/java/au/csiro/pathling/export/ws/BulkExportRequest.java

+   */
+  @Nonnull
+  @Builder.Default
+  List<Reference> patient = Collections.emptyList();


Is this the full list of parameters that I can use? Is it possible to pass parameters that are not in this class, e.g. includeAssociatedData?

Yes, this is the full list.
Actually whatever is defined in BulkExportClient but this is then mapped to the BulkExportRequest.
I think includeAssociatedData is the only one missing from the spec.

I think we should add this to make the list complete.

Implemented.

johngrimes · 2024-04-11T02:11:29Z

bulkclient/src/main/java/au/csiro/pathling/export/BulkExportClient.java

+          executorServiceResource.getExecutorService());
+
+      final BulkExportResult result = doExport(fileStore, bulkExportTemplate, downloadTemplate);
+      log.info("Export successful: {}", result);


Should we issue a Bulk Data Delete Request once the download has successfully completed (if the server supports it)?

The spec say that we MAY issue it to either cancel the request in progress or to cleanup.
So I guess it depends what we want to consider to be required for the first release of the bulk client.
I this this might be something to be added subsequent releases.

I think we should add this to mitigate the problem of exported data accumulating in the source server, which has been a problem that we have encountered with HAPI JPA.

Implemented.

bulkclient/src/test/java/au/csiro/pathling/example/RunBulkExport.java

Minor spelling fixes.

…on methods.

…thentication method classes.

…rt Java API.

…rt Python API.

… of an error or successful download.

johngrimes · 2025-02-03T03:41:03Z

We ended up shipping this client within a separate repo: https://github.com/aehrc/fhir-bulk-java

piotrszul added 29 commits January 29, 2024 16:05

Fixing the R build (inclue Roxygen always as it is need to generate N…

e7dc3f6

…AMESPACE file)

Initial version of the Bulk Export Client module.

cadceba

Initial version of the Bulk Export Client

0434ae4

Adding support for '_type' parameter.

103fb31

Adding basic python interface for bulk export.

c9ad57f

Adding support for '_since' parameter.

ecba42b

Adding support for '_since' parameter to Python API.

fa431ae

Fixing support for '_since' parameter to Python API.

0812fbf

Adding a basic progress tracking callback mechanism.

Moving the example of running the client to tests and fixing logging …

23bade4

…setup.

Added identification and handling of transient error while pooling fo…

3b09287

…r status.

Added support for http-date in retry-after header.

8e23469

Added WireMock test cases for some error scenarios.

88c4176

Improved exception reporting.

Added wiremock test scenario for persistent transient errors.

db5ae97

Migrating to Apache Http Client

805cf53

Adding java and python code for setting the log level for Pathling (a…

0b67ba1

…u.csiro.pathling logger)

Refactoring the FileSystem access abstraction.

5c44b01

Refactoring of the file download process.

688f5b0

Defining required FHIR resources and types

bf304fd

Refactoring: moving web service related classes to 'ws' package.

8bf1ac8

Refactoring: reimplementing web service using ResponseHandler.

7339203

Improving reporting of HTTP Errors.

11cd03a

Adding support for handling of Too Many Requests (429) status.

Refactoring: exposing bulk export configuration (AsyncRequest configu…

28b4f29

…ration).

Adding support got System/Group and Patien level export in Java.

1bdcb9b

Adding wirmock test for Group level export.

bf68008

Adding wiremock tests for Patient level export.

08f2314

Refactoring: Improved creation and disposal of Closeable resources.

12215f3

Refactoring: Extracting Async Config from BulkExport;

16fe449

Refactoring: Adding native Gson deserialization for instant that can …

42d4f3a

…also support a timestamp (which is what we are getting from the test server).

Refactoring: Finalising the interface for the client.

7daf106

piotrszul force-pushed the issue/1776 branch from 6d0005d to e9c5e0b Compare March 28, 2024 01:55

Refactoring and adding tests for authentication in bulkexport client.

f575261

piotrszul added 10 commits April 2, 2024 12:07

Refactoring and adding tests for authentication request interceptor i…

5b17741

…n bulkexport client.

Refactoring the authentication integration with HttpClient to use Tok…

be59b98

…enCredentials.

Adding SMART authentication discovery capabilites.

7a9292c

Refactoring of the Token authentication for better testability.

ca6ed69

Adding unit tests for AuthTokenProvider

Further refactoring and unit testing of SMART authentication methods.

4a5506f

Further refactoring and unit testing of SMART configuration discovery.

17def27

Further refactoring and unit testing of SMART configuration discovery.

41f0fe2

Further refactoring and unit testing of SMART configuration and authe…

00f2df7

…ntication.

Aligning the python API for bulkexport with Java implementation.

24c896c

- adding support for SMART configuration discovery - removing 'identifier' field from FHRI Reference()

Refactoring and improving logging for AuthTokenProvider.

f7fd125

Adding an example of python bulk export from Smart Health IT server with asymmetric keys.

piotrszul force-pushed the issue/1776 branch from d122ede to f7fd125 Compare April 5, 2024 06:23

johngrimes added 2 commits April 8, 2024 11:47

Add missing Javadoc comments, needs review

fb8ce1c

Update version to 6.6.0-SNAPSHOT

d80062d

piotrszul requested a review from johngrimes April 8, 2024 02:25

johngrimes added the new feature New feature or request label Apr 8, 2024

johngrimes requested changes Apr 11, 2024

View reviewed changes

piotrszul added 9 commits April 15, 2024 08:25

Refactoring out common code to read resources as strings.

e8cff65

Minor spelling fixes.

Refactoring: Simplifying class hierarchy for bulk export authenticati…

ce4359f

…on methods.

Refactoring: Redistributing client authentication constants to the au…

72e5973

…thentication method classes.

Fixing an issues with accessing of test resources.

9fe2bbf

Adding support for 'includeAssociatedData' parameter in the bulk expo…

3041802

…rt Java API.

Adding support for 'includeAssociatedData' parameter in the bulk expo…

bf723df

…rt Python API.

Adding support for cleaning up the export state on the server in case…

1d42149

… of an error or successful download.

Removing dependency of bulkclient on 'utilities'.

b73e39c

Removing dependency of bulkclient on amazon-sdk-bundle

e626f38

johngrimes closed this Feb 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement FHIR Bulk Export client. #1781

Implement FHIR Bulk Export client. #1781

piotrszul commented Feb 19, 2024

codecov bot commented Feb 19, 2024 •

edited

Loading

gitguardian bot commented Mar 28, 2024 •

edited

Loading

johngrimes left a comment

johngrimes Apr 11, 2024

johngrimes Apr 11, 2024

piotrszul Apr 14, 2024

johngrimes Apr 11, 2024

piotrszul Apr 14, 2024

johngrimes Apr 11, 2024

piotrszul Apr 12, 2024

johngrimes Apr 11, 2024

piotrszul Apr 12, 2024

piotrszul Apr 21, 2024

johngrimes Apr 11, 2024

piotrszul Apr 12, 2024

johngrimes Apr 12, 2024

piotrszul Apr 16, 2024

johngrimes Apr 11, 2024

piotrszul Apr 12, 2024

johngrimes Apr 12, 2024

piotrszul Apr 16, 2024

johngrimes commented Feb 3, 2025

Implement FHIR Bulk Export client. #1781

Implement FHIR Bulk Export client. #1781

Conversation

piotrszul commented Feb 19, 2024

codecov bot commented Feb 19, 2024 • edited Loading

Codecov Report

gitguardian bot commented Mar 28, 2024 • edited Loading

️✅ There are no secrets present in this pull request anymore.

johngrimes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johngrimes commented Feb 3, 2025

codecov bot commented Feb 19, 2024 •

edited

Loading

gitguardian bot commented Mar 28, 2024 •

edited

Loading