From 17902e0656dd11db9965f084c874675d40f3363d Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Fri, 3 Oct 2025 00:07:41 +0300 Subject: [PATCH 1/2] Fixes multiple Error Prone bug patterns --- .../fhir/analytics/BulkExportApiClient.java | 10 ++++++--- .../google/fhir/analytics/BulkExportUtil.java | 3 ++- .../fhir/analytics/ConvertResourceFn.java | 11 +++++----- .../google/fhir/analytics/FetchPatients.java | 11 ++-------- .../com/google/fhir/analytics/FhirEtl.java | 4 +--- .../google/fhir/analytics/FhirEtlOptions.java | 22 +++++++++++-------- .../fhir/analytics/ProcessGenericRecords.java | 2 +- .../google/fhir/analytics/ResourceTag.java | 6 ++--- .../analytics/metrics/PipelineMetrics.java | 8 ++----- .../fhir/analytics/AvroConversionUtil.java | 5 +++-- .../com/google/fhir/analytics/DwhFiles.java | 22 +++++++++++-------- .../google/fhir/analytics/FhirStoreUtil.java | 4 ++-- .../analytics/converter/JsonDateCodec.java | 2 +- .../model/DatabaseConfiguration.java | 2 +- .../analytics/model/EventConfiguration.java | 2 +- .../fhir/analytics/view/ViewApplicator.java | 12 +++++----- .../fhir/analytics/view/ViewDefinition.java | 6 +++++ .../fhir/analytics/view/ViewSchema.java | 2 +- .../fhir/analytics/DwhFilesManager.java | 18 ++++++++++----- .../fhir/analytics/FlinkConfiguration.java | 13 ++++++----- .../fhir/analytics/HiveTableManager.java | 3 ++- .../fhir/analytics/PipelineManager.java | 21 ++++++------------ .../metrics/PipelineMetricsEndpoint.java | 1 + .../fhir/analytics/metrics/ProgressStats.java | 2 +- .../google/fhir/analytics/metrics/Stats.java | 3 --- pipelines/pom.xml | 2 +- 26 files changed, 102 insertions(+), 95 deletions(-) diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportApiClient.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportApiClient.java index 0758e4593..ac00fc4e9 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportApiClient.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportApiClient.java @@ -26,6 +26,8 @@ import com.google.gson.Gson; import java.io.IOException; import java.io.Reader; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; import java.util.Arrays; import java.util.Date; import java.util.HashMap; @@ -94,7 +96,7 @@ public String triggerBulkExportJob( * * @param bulkExportStatusUrl the url of the bulk export job * @return BulkExportHttpResponse - the status and details of the bulk export job - * @throws IOException + * @throws IOException in case of any error while fetching the status of bulk export job */ public BulkExportHttpResponse fetchBulkExportHttpResponse(String bulkExportStatusUrl) throws IOException { @@ -103,12 +105,14 @@ public BulkExportHttpResponse fetchBulkExportHttpResponse(String bulkExportStatu httpResponseBuilder.httpStatus(httpResponse.getStatus()); if (httpResponse.getHeaders(EXPIRES) != null && !httpResponse.getHeaders(EXPIRES).isEmpty()) { String expiresString = httpResponse.getHeaders(EXPIRES).get(0); - Date expires = new Date(expiresString); + DateTimeFormatter formatter = DateTimeFormatter.RFC_1123_DATE_TIME; + ZonedDateTime zonedDateTime = ZonedDateTime.parse(expiresString, formatter); + Date expires = Date.from(zonedDateTime.toInstant()); httpResponseBuilder.expires(expires); } if (!CollectionUtils.isEmpty(httpResponse.getHeaders(RETRY_AFTER))) { String retryHeaderString = httpResponse.getHeaders(RETRY_AFTER).get(0); - httpResponseBuilder.retryAfter(Integer.valueOf(retryHeaderString)); + httpResponseBuilder.retryAfter(Integer.parseInt(retryHeaderString)); } if (!CollectionUtils.isEmpty(httpResponse.getHeaders(X_PROGRESS))) { String xProgress = httpResponse.getHeaders(X_PROGRESS).get(0); diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportUtil.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportUtil.java index d584fb821..2d9738f2b 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportUtil.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/BulkExportUtil.java @@ -47,7 +47,8 @@ public class BulkExportUtil { * @param since - the fhir resources fetched should have updated timestamp greater than this * @param fhirVersionEnum - the fhir version of resource types * @return the BulkExportResponse - * @throws IOException + * @throws IOException in case of any error while triggering or fetching the status of bulk export + * job */ public BulkExportResponse triggerBulkExport( List resourceTypes, @Nullable String since, FhirVersionEnum fhirVersionEnum) diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/ConvertResourceFn.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/ConvertResourceFn.java index 3a5aaf61e..5e504bb8b 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/ConvertResourceFn.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/ConvertResourceFn.java @@ -26,6 +26,7 @@ import java.sql.SQLException; import java.text.ParseException; import java.text.SimpleDateFormat; +import java.time.Instant; import java.util.ArrayList; import java.util.Date; import java.util.HashMap; @@ -121,7 +122,7 @@ public void writeResource(HapiRowDescriptor element) // deleted resource = createNewFhirResource(element.fhirVersion(), resourceType); ActionType removeAction = ActionType.REMOVE; - meta.setLastUpdated(new Date()); + meta.setLastUpdated(Date.from(Instant.now())); meta.addTag( new Coding(removeAction.getSystem(), removeAction.toCode(), removeAction.getDisplay())); } else { @@ -213,10 +214,10 @@ private Resource createNewFhirResource(String fhirVersion, String resourceType) try { // TODO create tests for this method and different versions of FHIR; casting to R4 resource // does not seem right! - return (Resource) - Class.forName(getFhirBasePackageName(fhirVersion) + "." + resourceType) - .getConstructor() - .newInstance(); + return Class.forName(getFhirBasePackageName(fhirVersion) + "." + resourceType) + .asSubclass(Resource.class) + .getConstructor() + .newInstance(); } catch (InstantiationException | IllegalAccessException | ClassNotFoundException diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/FetchPatients.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/FetchPatients.java index e2ef28d3b..4bd1f7725 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/FetchPatients.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/FetchPatients.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 Google LLC + * Copyright 2020-2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,8 +21,6 @@ import com.google.fhir.analytics.view.ViewApplicationException; import java.io.IOException; import java.sql.SQLException; -import java.util.List; -import org.apache.avro.Schema; import org.apache.beam.sdk.transforms.PTransform; import org.apache.beam.sdk.transforms.ParDo; import org.apache.beam.sdk.values.KV; @@ -40,15 +38,10 @@ public class FetchPatients extends PTransform>, private final FetchSearchPageFn> fetchSearchPageFn; - private final Schema schema; - - FetchPatients(FhirEtlOptions options, Schema schema) { + FetchPatients(FhirEtlOptions options) { Preconditions.checkState(!options.getActivePeriod().isEmpty()); - List dateRange = FhirSearchUtil.getDateRange(options.getActivePeriod()); int count = options.getBatchSize(); - this.schema = schema; - fetchSearchPageFn = new FetchSearchPageFn>(options, "PatientById") { diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtl.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtl.java index 0a052d445..e7fdaf310 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtl.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtl.java @@ -132,9 +132,7 @@ static void fetchPatientHistory( PCollection> flattenedPatients = patientIdList.apply(Flatten.pCollections()); PCollection> mergedPatients = flattenedPatients.apply(Sum.integersPerKey()); - final String patientType = "Patient"; - FetchPatients fetchPatients = - new FetchPatients(options, avroConversionUtil.getResourceSchema(patientType)); + FetchPatients fetchPatients = new FetchPatients(options); mergedPatients.apply(fetchPatients); for (String resourceType : patientAssociatedResources) { FetchPatientHistory fetchPatientHistory = new FetchPatientHistory(options, resourceType); diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtlOptions.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtlOptions.java index 6fa43d2a3..a17146585 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtlOptions.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/FhirEtlOptions.java @@ -214,21 +214,25 @@ public interface FhirEtlOptions extends BasePipelineOptions { void setActivePeriod(String value); @Description( - "Fetch only FHIR resources that were updated after the given timestamp." - + "The date format follows the dateTime format in the FHIR standard, without time-zone:\n" - + "https://www.hl7.org/fhir/datatypes.html#dateTime\n" - + "This feature is currently implemented only for HAPI JDBC mode.") + """ + Fetch only FHIR resources that were updated after the given timestamp. + The date format follows the dateTime format in the FHIR standard, without time-zone: + https://www.hl7.org/fhir/datatypes.html#dateTime + This feature is currently implemented only for HAPI JDBC mode. + """) @Default.String("") String getSince(); void setSince(String value); @Description( - "Path to the sink database config; if not set, no sink DB is used.\n" - + "If viewDefinitionsDir is set, the output tables will be the generated views\n" - + "(the `name` field value will be used as the table name); if not, one table\n" - + "per resource type is created with the JSON content of a resource and its\n" - + "`id` column for each row.") + """ + Path to the sink database config; if not set, no sink DB is used. + If viewDefinitionsDir is set, the output tables will be the generated views + (the `name` field value will be used as the table name); if not, one table + per resource type is created with the JSON content of a resource and its + `id` column for each row. + """) @Default.String("") String getSinkDbConfigPath(); diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/ProcessGenericRecords.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/ProcessGenericRecords.java index ada76949b..d0cb6419f 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/ProcessGenericRecords.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/ProcessGenericRecords.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 Google LLC + * Copyright 2020-2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/ResourceTag.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/ResourceTag.java index 62f552a42..b5bbdf51a 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/ResourceTag.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/ResourceTag.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 Google LLC + * Copyright 2020-2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,12 +43,10 @@ public boolean equals(Object other) { return true; } - if (!(other instanceof ResourceTag)) { + if (!(other instanceof ResourceTag resourceTag)) { return false; } - ResourceTag resourceTag = (ResourceTag) other; - return coding != null && coding.equalsDeep(resourceTag.coding) && StringUtils.compare(resourceId, resourceTag.getResourceId()) == 0 diff --git a/pipelines/batch/src/main/java/com/google/fhir/analytics/metrics/PipelineMetrics.java b/pipelines/batch/src/main/java/com/google/fhir/analytics/metrics/PipelineMetrics.java index 15059f409..db642bfe8 100644 --- a/pipelines/batch/src/main/java/com/google/fhir/analytics/metrics/PipelineMetrics.java +++ b/pipelines/batch/src/main/java/com/google/fhir/analytics/metrics/PipelineMetrics.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 Google LLC + * Copyright 2020-2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,10 +27,6 @@ public interface PipelineMetrics { /** Clears all the metrics */ void clearAllMetrics(); - /** - * Sets the total number of resources - * - * @param totalNumOfResources - */ + /** Sets the total number of resources */ void setTotalNumOfResources(long totalNumOfResources); } diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/AvroConversionUtil.java b/pipelines/common/src/main/java/com/google/fhir/analytics/AvroConversionUtil.java index a502b374d..3d4e681c4 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/AvroConversionUtil.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/AvroConversionUtil.java @@ -164,10 +164,11 @@ Resource convertToHapi(GenericRecord record, String resourceType) throws Profile AvroConverter converter = getConverter(resourceType); IBaseResource resource = converter.avroToResource(record); // TODO: fix this for other FHIR versions: https://github.com/google/fhir-data-pipes/issues/400 - if (!(resource instanceof Resource)) { + if (resource instanceof Resource res) { + return res; + } else { throw new IllegalArgumentException("Cannot convert input record to resource!"); } - return (Resource) resource; } /** diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/DwhFiles.java b/pipelines/common/src/main/java/com/google/fhir/analytics/DwhFiles.java index 6a199db1f..cf19c650a 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/DwhFiles.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/DwhFiles.java @@ -131,7 +131,7 @@ public class DwhFiles { * @param dwhRoot the root of the DWH * @param viewRoot the root of the view dir under DWH; note this should be an absolute path, but * it is usually "under" `dwhRoot`. - * @param fhirContext + * @param fhirContext the FHIR Context of the FHIR version we are working with. */ private DwhFiles(String dwhRoot, String viewRoot, FhirContext fhirContext) { Preconditions.checkArgument(!Strings.isNullOrEmpty(dwhRoot)); @@ -199,8 +199,10 @@ public ResourceId getViewPath(String viewName) { } /** - * @param resourceType the type of the FHIR resources - * @return The file pattern for Parquet files of `resourceType` in this DWH. + * Returns the file-pattern for all Parquet files of a specific resource type. + * + * @param resourceType the type of the FHIR resources {@return The file pattern for Parquet files + * of `resourceType` in this DWH.} */ public String getResourceFilePattern(String resourceType) { return String.format( @@ -221,14 +223,14 @@ public String getViewFilePattern(String viewName) { /** * Returns all the child directories under the given base directory which are 1-level deep. Note * in many cloud/distributed file-systems, we do not have "directories"; there are only buckets - * and files in those buckets. We use file-seprators (e.g., `/`) to simulate the concept of + * and files in those buckets. We use file-separators (e.g., `/`) to simulate the concept of * directories. So for example, this method returns an empty set if `baseDir` is `bucket/test` and * the only file in that bucket is `bucket/test/dir1/dir2/file.txt`. If `baseDir` is * `bucket/test/dir1`, in the above example, `dir2` is returned. * * @param baseDir the path under which "directories" are looked for. * @return The list of all child directories under the base directory - * @throws IOException + * @throws IOException if there is an error accessing the base directory */ static Set getAllChildDirectories(String baseDir) throws IOException { return getAllChildDirectories(baseDir, ""); @@ -241,8 +243,8 @@ static Set getAllChildDirectories(String baseDir) throws IOException * * @param baseDir the baseDir of the DWH * @param commonPrefix the prefix of directories under `baseDir` that we are looking for - * @return the list of found directory names. - * @throws IOException + * @return the set of found directory names. + * @throws IOException if there is an error accessing the base directory */ static Set getAllChildDirectories(String baseDir, String commonPrefix) throws IOException { @@ -280,7 +282,7 @@ static Set getAllChildDirectories(String baseDir, String commonPrefi * * @param commonPrefix the common prefix for the timestamped directories to search. * @return the latest directory with the `commonPrefix`. - * @throws IOException + * @throws IOException if there is an error accessing the dwhRoot directory */ @Nullable private static ResourceId getLatestPath(String dwhRoot, String commonPrefix) throws IOException { @@ -300,6 +302,8 @@ private static ResourceId newTimestampedPath(String dwhRoot, String commonPrefix } /** + * Gets the latest incremental-run path under the given DWH root. + * * @return the current incremental run path if one found; null otherwise. */ @Nullable @@ -578,7 +582,7 @@ public static String getFileSeparatorForDwhFiles(String dwhRootPrefix) { }; } - /** This method returns the {@code path} by appending the {@code fileseparator} if required. */ + /** This method returns the {@code path} by appending the {@code fileSeparator} if required. */ public static String getPathEndingWithFileSeparator(String path, String fileSeparator) { Preconditions.checkNotNull(path); Preconditions.checkNotNull(fileSeparator); diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/FhirStoreUtil.java b/pipelines/common/src/main/java/com/google/fhir/analytics/FhirStoreUtil.java index e57b19db2..4e6d1c8f6 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/FhirStoreUtil.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/FhirStoreUtil.java @@ -184,8 +184,8 @@ protected Collection uploadBundle( } Resource outcome = responseComponent.getOutcome(); - if (outcome instanceof IBaseOperationOutcome) { - methodOutcome.setOperationOutcome((IBaseOperationOutcome) outcome); + if (outcome instanceof IBaseOperationOutcome operationOutcome) { + methodOutcome.setOperationOutcome(operationOutcome); } responses.add(methodOutcome); diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/converter/JsonDateCodec.java b/pipelines/common/src/main/java/com/google/fhir/analytics/converter/JsonDateCodec.java index 80a257264..8da11c454 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/converter/JsonDateCodec.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/converter/JsonDateCodec.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 Google LLC + * Copyright 2020-2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/model/DatabaseConfiguration.java b/pipelines/common/src/main/java/com/google/fhir/analytics/model/DatabaseConfiguration.java index c231a5507..561305ab1 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/model/DatabaseConfiguration.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/model/DatabaseConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 Google LLC + * Copyright 2020-2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/model/EventConfiguration.java b/pipelines/common/src/main/java/com/google/fhir/analytics/model/EventConfiguration.java index adf5a79e0..006c18cb5 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/model/EventConfiguration.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/model/EventConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 Google LLC + * Copyright 2020-2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewApplicator.java b/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewApplicator.java index 7b3764c66..219803e07 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewApplicator.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewApplicator.java @@ -290,11 +290,10 @@ FlatRow applyColumns(@Nullable IBase element, List columns) List rowElements = new ArrayList<>(); for (Column col : columns) { if (GET_RESOURCE_KEY.equals(col.getPath())) { - if (element == null || !(element instanceof IBaseResource)) { + if (element == null || !(element instanceof IBaseResource baseResource)) { throw new ViewApplicationException( GET_RESOURCE_KEY + " can only be applied at the root!"); } - IBaseResource baseResource = (IBaseResource) element; rowElements.add( new RowElement( // TODO move all type inference to a single place outside View application. @@ -321,11 +320,11 @@ FlatRow applyColumns(@Nullable IBase element, List columns) eval = evaluateFhirPath(element, fhirPathForRef); } for (IBase refElem : eval) { - if (!(refElem instanceof IBaseReference)) { + if (!(refElem instanceof IBaseReference refElemBaseReference)) { throw new ViewApplicationException( "getReferenceKey can only be applied to Reference elements; got " + fhirPathForRef); } - IIdType ref = ((IBaseReference) refElem).getReferenceElement(); + IIdType ref = refElemBaseReference.getReferenceElement(); if (resType.isEmpty()) { refs.add(ref); } else { @@ -364,7 +363,9 @@ public static class RowList { private final ImmutableList rows; - private RowList(List rows, LinkedHashMap columnInfos) { + private RowList( + List rows, + @SuppressWarnings("NonApiType") LinkedHashMap columnInfos) { this.rows = ImmutableList.copyOf(rows); this.columnInfos = ImmutableMap.copyOf(columnInfos); } @@ -465,6 +466,7 @@ private static Builder builder() { } // This can eventually be public, but currently it is not used outside this file. + @SuppressWarnings("EffectivelyPrivate") private static class Builder { private final LinkedHashMap columnInfos = new LinkedHashMap<>(); private final List rows = new ArrayList<>(); diff --git a/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewDefinition.java b/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewDefinition.java index c2c12e53c..b2390f315 100644 --- a/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewDefinition.java +++ b/pipelines/common/src/main/java/com/google/fhir/analytics/view/ViewDefinition.java @@ -143,6 +143,7 @@ void validateAndSetUp(boolean checkName, @Nullable String fhirVersion) * @return the [ordered] map of new column names and their types as string. * @throws ViewDefinitionException for repeated columns or other requirements not satisfied. */ + @SuppressWarnings("NonApiType") private LinkedHashMap validateAndReplaceConstantsInSelects( @Nullable List