fixes #138 (#140)

* fixes #138
/gcbrun

dicom_util/src/main/java/com/google/cloud/healthcare/DicomWebClient.java

@@ -15,27 +15,28 @@
 package com.google.cloud.healthcare;
 
 import com.google.api.client.http.GenericUrl;
+import com.google.api.client.http.HttpMediaType;
 import com.google.api.client.http.HttpRequest;
 import com.google.api.client.http.HttpRequestFactory;
-import com.google.api.client.http.HttpMediaType;
 import com.google.api.client.http.HttpResponse;
 import com.google.api.client.http.HttpResponseException;
 import com.google.api.client.http.HttpStatusCodes;
 import com.google.api.client.http.InputStreamContent;
 import com.google.api.client.http.MultipartContent;
-import java.util.UUID;
 import com.google.common.io.CharStreams;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
+import java.util.UUID;
 import javax.inject.Inject;
+import org.dcm4che3.data.Attributes;
+import org.dcm4che3.data.Tag;
+import org.dcm4che3.io.DicomInputStream;
 import org.dcm4che3.net.Status;
 import org.json.JSONArray;
 
-/**
- * A client for communicating with the Cloud Healthcare API.
- */
+/** A client for communicating with the Cloud Healthcare API. */
 public class DicomWebClient implements IDicomWebClient {
 
   // Factory to create HTTP requests with proper credentials.
@@ -44,9 +45,12 @@ public class DicomWebClient implements IDicomWebClient {
   // Service prefix all dicomWeb paths will be appended to.
   private final String serviceUrlPrefix;
 
-  // The path for a StowRS request to be appened to serviceUrlPrefix.
+  // The path for a StowRS request to be appended to serviceUrlPrefix.
   private final String stowPath;
 
+  // If we will delete and retry when we receive HTTP 409.
+  private final Boolean useStowOverwrite;
+
   @Inject
   public DicomWebClient(
       HttpRequestFactory requestFactory,
@@ -55,51 +59,65 @@ public DicomWebClient(
     this.requestFactory = requestFactory;
     this.serviceUrlPrefix = serviceUrlPrefix;
     this.stowPath = stowPath;
+    this.useStowOverwrite = false;
+  }
 
+  @Inject
+  public DicomWebClient(
+      HttpRequestFactory requestFactory,
+      @Annotations.DicomwebAddr String serviceUrlPrefix,
+      String stowPath,
+      Boolean useStowOverwrite) {
+    this.requestFactory = requestFactory;
+    this.serviceUrlPrefix = serviceUrlPrefix;
+    this.stowPath = stowPath;
+    this.useStowOverwrite = useStowOverwrite;
   }
 
-  /**
-   * Makes a WADO-RS call and returns the response InputStream.
-   */
+  /** Makes a WADO-RS call and returns the response InputStream. */
+  @Override
   public InputStream wadoRs(String path) throws IDicomWebClient.DicomWebException {
     try {
       HttpRequest httpRequest =
-          requestFactory.buildGetRequest(new GenericUrl(serviceUrlPrefix + "/"
-              + StringUtil.trim(path)));
+          requestFactory.buildGetRequest(
+              new GenericUrl(serviceUrlPrefix + "/" + StringUtil.trim(path)));
       httpRequest.getHeaders().put("Accept", "application/dicom; transfer-syntax=*");
       HttpResponse httpResponse = httpRequest.execute();
 
       return httpResponse.getContent();
     } catch (HttpResponseException e) {
       throw new DicomWebException(
           String.format("WadoRs: %d, %s", e.getStatusCode(), e.getStatusMessage()),
-          e, e.getStatusCode(), Status.ProcessingFailure);
+          e,
+          e.getStatusCode(),
+          Status.ProcessingFailure);
     } catch (IOException | IllegalArgumentException e) {
       throw new IDicomWebClient.DicomWebException(e);
     }
   }
 
-  /**
-   * Makes a QIDO-RS call and returns a JSON array.
-   */
+  /** Makes a QIDO-RS call and returns a JSON array. */
+  @Override
   public JSONArray qidoRs(String path) throws IDicomWebClient.DicomWebException {
     try {
       HttpRequest httpRequest =
-          requestFactory.buildGetRequest(new GenericUrl(serviceUrlPrefix + "/"
-              + StringUtil.trim(path)));
+          requestFactory.buildGetRequest(
+              new GenericUrl(serviceUrlPrefix + "/" + StringUtil.trim(path)));
       HttpResponse httpResponse = httpRequest.execute();
 
       // dcm4che server can return 204 responses.
       if (httpResponse.getStatusCode() == HttpStatusCodes.STATUS_CODE_NO_CONTENT) {
         return new JSONArray();
       }
       return new JSONArray(
-          CharStreams
-              .toString(new InputStreamReader(httpResponse.getContent(), StandardCharsets.UTF_8)));
+          CharStreams.toString(
+              new InputStreamReader(httpResponse.getContent(), StandardCharsets.UTF_8)));
     } catch (HttpResponseException e) {
       throw new DicomWebException(
           String.format("QidoRs: %d, %s", e.getStatusCode(), e.getStatusMessage()),
-          e, e.getStatusCode(), Status.UnableToCalculateNumberOfMatches);
+          e,
+          e.getStatusCode(),
+          Status.UnableToCalculateNumberOfMatches);
     } catch (IOException | IllegalArgumentException e) {
       throw new IDicomWebClient.DicomWebException(e);
     }
@@ -110,6 +128,7 @@ public JSONArray qidoRs(String path) throws IDicomWebClient.DicomWebException {
    *
    * @param in The DICOM input stream.
    */
+  @Override
   public void stowRs(InputStream in) throws IDicomWebClient.DicomWebException {
     GenericUrl url = new GenericUrl(StringUtil.joinPath(serviceUrlPrefix, this.stowPath));
 
@@ -128,18 +147,73 @@ public void stowRs(InputStream in) throws IDicomWebClient.DicomWebException {
     } catch (HttpResponseException e) {
       throw new DicomWebException(
           String.format("StowRs: %d, %s", e.getStatusCode(), e.getStatusMessage()),
-          e, e.getStatusCode(), Status.ProcessingFailure);
+          e,
+          e.getStatusCode(),
+          Status.ProcessingFailure);
     } catch (IOException e) {
       throw new IDicomWebClient.DicomWebException(e);
-    }
-    finally {
+    } finally {
       try {
         if ((resp) != null) {
           resp.disconnect();
         }
-      } catch(IOException e) {
+      } catch (IOException e) {
         throw new IDicomWebClient.DicomWebException(e);
       }
     }
   }
+
+  /** Retry the STOW-RS on an HTTP409, after DELETE. */
+  @Override
+  public Boolean getStowOverwrite() {
+    return this.useStowOverwrite;
+  }
+
+  /**
+   * Deletes the resource and returns void.
+   *
+   * @param path The resource URL path to delete.
+   */
+  @Override
+  public void delete(String path) throws IDicomWebClient.DicomWebException {
+    try {
+      HttpRequest httpRequest =
+          requestFactory.buildDeleteRequest(
+              new GenericUrl(serviceUrlPrefix + "/" + StringUtil.trim(path)));
+      httpRequest.execute();
+    } catch (HttpResponseException e) {
+      throw new DicomWebException(
+          String.format("delete: %d, %s", e.getStatusCode(), e.getStatusMessage()),
+          e,
+          e.getStatusCode(),
+          Status.ProcessingFailure);
+    } catch (IOException | IllegalArgumentException e) {
+      throw new IDicomWebClient.DicomWebException(e);
+    }
+  }
+
+  /**
+   * Reads the resource to figure out the study/series/instance UID path, deletes it and returns
+   * void.
+   *
+   * @param stream The unread stream containing the DICOM instance.
+   */
+  @Override
+  public void delete(InputStream stream) throws IDicomWebClient.DicomWebException {
+    try {
+      // TODO: HTTP 409 content has RetrieveURL as StudyUID, so we need to construct the instance
+      // path. Can we receive the RetrieveURL as the full instance path, as we're constructing here?
+      DicomInputStream dis = new DicomInputStream(stream);
+      Attributes attrs = dis.readDataset(-1, Tag.PixelData);
+      String instanceUrl =
+          String.format(
+              "studies/%s/series/%s/instances/%s",
+              attrs.getString(Tag.StudyInstanceUID, 0),
+              attrs.getString(Tag.SeriesInstanceUID, 0),
+              attrs.getString(Tag.SOPInstanceUID, 0));
+      this.delete(instanceUrl);
+    } catch (IOException e) {
+      throw new IDicomWebClient.DicomWebException(e);
+    }
+  }
 }

dicom_util/src/main/java/com/google/cloud/healthcare/DicomWebClientJetty.java

@@ -59,6 +59,16 @@ public JSONArray qidoRs(String path) throws DicomWebException {
     throw new UnsupportedOperationException("Not Implemented, use DicomWebClient");
   }
 
+  @Override
+  public void delete(String path) throws DicomWebException {
+    throw new UnsupportedOperationException("Not Implemented, use DicomWebClient");
+  }
+
+  @Override
+  public void delete(InputStream stream) throws DicomWebException {
+    throw new UnsupportedOperationException("Not Implemented, use DicomWebClient");
+  }
+
   @Override
   public void stowRs(InputStream in) throws DicomWebException {
     try {
@@ -155,6 +165,11 @@ public void onData(Stream stream, DataFrame frame, Callback callback) {
     }
   }
 
+  @Override
+  public Boolean getStowOverwrite() {
+    return false;
+  }
+
   private static class DataStream {
 
     private static final int BUFFER_SIZE = 8192;

dicom_util/src/main/java/com/google/cloud/healthcare/IDicomWebClient.java

@@ -27,6 +27,12 @@ public interface IDicomWebClient {
 
   void stowRs(InputStream in) throws DicomWebException;
 
+  Boolean getStowOverwrite();
+
+  void delete(String path) throws DicomWebException;
+
+  void delete(InputStream stream) throws DicomWebException;
+
   /**
    * An exception for errors returned by the DicomWeb server.
    */

import/src/main/java/com/google/cloud/healthcare/imaging/dicomadapter/Flags.java

@@ -70,6 +70,13 @@ public class Flags {
   )
   Boolean useHttp2ForStow = false;
 
+  @Parameter(
+      names = {"--stow_overwrite"},
+      description = "If STOW-RS request receives HTTP 409 (instance conflict), then perform HTTP DELETE on"
+        + " instance and retry. False by default."
+  )
+  Boolean useStowOverwrite = false;
+
   @Parameter(
       names = {"--oauth_scopes"},
       description = "Comma seperated OAuth scopes used by adapter."

import/src/main/java/com/google/cloud/healthcare/imaging/dicomadapter/ImportAdapter.java

@@ -19,22 +19,22 @@
 import com.google.api.client.http.javanet.NetHttpTransport;
 import com.google.auth.http.HttpCredentialsAdapter;
 import com.google.auth.oauth2.GoogleCredentials;
+import com.google.cloud.healthcare.DicomWebClient;
 import com.google.cloud.healthcare.DicomWebClientJetty;
-import com.google.cloud.healthcare.IDicomWebClient;
-import com.google.cloud.healthcare.StringUtil;
 import com.google.cloud.healthcare.DicomWebValidation;
+import com.google.cloud.healthcare.IDicomWebClient;
 import com.google.cloud.healthcare.LogUtil;
-import com.google.cloud.healthcare.DicomWebClient;
+import com.google.cloud.healthcare.StringUtil;
 import com.google.cloud.healthcare.deid.redactor.DicomRedactor;
 import com.google.cloud.healthcare.deid.redactor.protos.DicomConfigProtos;
 import com.google.cloud.healthcare.deid.redactor.protos.DicomConfigProtos.DicomConfig;
 import com.google.cloud.healthcare.deid.redactor.protos.DicomConfigProtos.DicomConfig.TagFilterProfile;
+import com.google.cloud.healthcare.imaging.dicomadapter.cmove.CMoveSenderFactory;
+import com.google.cloud.healthcare.imaging.dicomadapter.cstore.backup.BackupUploadService;
 import com.google.cloud.healthcare.imaging.dicomadapter.cstore.backup.DelayCalculator;
 import com.google.cloud.healthcare.imaging.dicomadapter.cstore.backup.GcpBackupUploader;
-import com.google.cloud.healthcare.imaging.dicomadapter.cstore.backup.BackupUploadService;
 import com.google.cloud.healthcare.imaging.dicomadapter.cstore.backup.IBackupUploader;
 import com.google.cloud.healthcare.imaging.dicomadapter.cstore.backup.LocalBackupUploader;
-import com.google.cloud.healthcare.imaging.dicomadapter.cmove.CMoveSenderFactory;
 import com.google.cloud.healthcare.imaging.dicomadapter.cstore.destination.IDestinationClientFactory;
 import com.google.cloud.healthcare.imaging.dicomadapter.cstore.destination.MultipleDestinationClientFactory;
 import com.google.cloud.healthcare.imaging.dicomadapter.cstore.destination.SingleDestinationClientFactory;
@@ -43,6 +43,10 @@
 import com.google.cloud.healthcare.imaging.dicomadapter.monitoring.Event;
 import com.google.cloud.healthcare.imaging.dicomadapter.monitoring.MonitoringService;
 import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.util.Arrays;
+import java.util.List;
 import org.dcm4che3.net.Device;
 import org.dcm4che3.net.service.BasicCEchoSCP;
 import org.dcm4che3.net.service.DicomServiceRegistry;
@@ -51,15 +55,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.security.GeneralSecurityException;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-
-
 public class ImportAdapter {
 
   private static final Logger log = LoggerFactory.getLogger(ImportAdapter.class);
@@ -163,13 +158,17 @@ private static IDicomWebClient configureDefaultDicomWebClient(
       Flags flags) {
     IDicomWebClient defaultCstoreDicomWebClient;
     if (flags.useHttp2ForStow) {
+      if (flags.useStowOverwrite) {
+        throw new IllegalArgumentException("--stow_overwrite is not supported with --stow_http2.");
+      }
       defaultCstoreDicomWebClient =
-        new DicomWebClientJetty(
-            credentials,
-            StringUtil.joinPath(cstoreDicomwebAddr, cstoreDicomwebStowPath));
+          new DicomWebClientJetty(
+              credentials, StringUtil.joinPath(cstoreDicomwebAddr, cstoreDicomwebStowPath));
     } else {
+      log.debug("--stow_overwrite set to " + (flags.useStowOverwrite ? "true" : "false"));
       defaultCstoreDicomWebClient =
-        new DicomWebClient(requestFactory, cstoreDicomwebAddr, cstoreDicomwebStowPath);
+          new DicomWebClient(
+              requestFactory, cstoreDicomwebAddr, cstoreDicomwebStowPath, flags.useStowOverwrite);
     }
     return defaultCstoreDicomWebClient;
   }
@@ -181,8 +180,10 @@ private static IDestinationClientFactory configureDestinationClientFactory(
     IDestinationClientFactory destinationClientFactory;
     if (flags.sendToAllMatchingDestinations) {
       if (backupServicePresent == false) {
-        throw new IllegalArgumentException("backup is not configured properly. '--send_to_all_matching_destinations' " +
-            "flag must be used only in pair with backup, local or GCP. Please see readme to configure backup.");
+        throw new IllegalArgumentException(
+            "backup is not configured properly. '--send_to_all_matching_destinations' flag must be"
+                + " used only in pair with backup, local or GCP. Please see readme to configure"
+                + " backup.");
       }
       Pair<ImmutableList<Pair<DestinationFilter, IDicomWebClient>>,
           ImmutableList<Pair<DestinationFilter, AetDictionary.Aet>>> multipleDestinations = configureMultipleDestinationTypesMap(
@@ -217,9 +218,15 @@ private static MultipleDestinationUploadService configureMultipleDestinationUplo
     return null;
   }
 
-  private static BackupUploadService configureBackupUploadService(Flags flags, GoogleCredentials credentials) throws IOException {
+  private static BackupUploadService configureBackupUploadService(
+      Flags flags, GoogleCredentials credentials) throws IOException {
     String uploadPath = flags.persistentFileStorageLocation;
-
+    if (flags.useStowOverwrite
+        && (uploadPath.isBlank() || flags.persistentFileUploadRetryAmount < 1)) {
+      throw new IllegalArgumentException(
+          "--stow_overwrite requires the use of --persistent_file_storage_location and"
+              + " --persistent_file_upload_retry_amount >= 1");
+    }
     if (!uploadPath.isBlank()) {
       final IBackupUploader backupUploader;
       if (uploadPath.startsWith(GCP_PATH_PREFIX)) {
@@ -231,10 +238,8 @@ private static BackupUploadService configureBackupUploadService(Flags flags, Goo
           backupUploader,
           flags.persistentFileUploadRetryAmount,
           ImmutableList.copyOf(flags.httpErrorCodesToRetry),
-          new DelayCalculator(
-              flags.minUploadDelay,
-              flags.maxWaitingTimeBetweenUploads));
-      }
+          new DelayCalculator(flags.minUploadDelay, flags.maxWaitingTimeBetweenUploads));
+    }
     return null;
   }