chore: file resource save cleanup (#15748)

dhis2 · Nov 22, 2023 · 7e3db7a · 7e3db7a
1 parent f5161c5
commit 7e3db7a
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 98 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceContentStore.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceContentStore.java
@@ -34,6 +34,8 @@
 import java.net.URI;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 
 /**
  * @author Halvdan Hoem Grelland
@@ -64,7 +66,8 @@ public interface FileResourceContentStore {
    * @param bytes the byte array.
    * @return the key on success or null if saving failed.
    */
-  String saveFileResourceContent(FileResource fileResource, byte[] bytes);
+  @CheckForNull
+  String saveFileResourceContent(@Nonnull FileResource fileResource, @Nonnull byte[] bytes);
 
   /**
    * Save the contents of the File to the file store.
@@ -73,7 +76,8 @@ public interface FileResourceContentStore {
    * @param file the File. Will be consumed upon deletion.
    * @return the key on success or null if saving failed.
    */
-  String saveFileResourceContent(FileResource fileResource, File file);
+  @CheckForNull
+  String saveFileResourceContent(@Nonnull FileResource fileResource, @Nonnull File file);
 
   /**
    * Save the content of image files.
@@ -82,8 +86,9 @@ public interface FileResourceContentStore {
    * @param imageFile will map image dimension to its associated file.
    * @return the key on success or null if saving failed.
    */
+  @CheckForNull
   String saveFileResourceContent(
-      FileResource fileResource, Map<ImageFileDimension, File> imageFile);
+      @Nonnull FileResource fileResource, @Nonnull Map<ImageFileDimension, File> imageFile);
 
   /**
    * Delete the content bytes of a file resource.

diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/BinaryFileSavedEvent.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/BinaryFileSavedEvent.java
@@ -27,24 +27,16 @@
  */
 package org.hisp.dhis.fileresource.events;
 
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
 /**
  * @Author Zubair Asghar.
  */
+@Getter
+@RequiredArgsConstructor
 public class BinaryFileSavedEvent {
-  private String fileResource;
-
-  private byte[] bytes;
-
-  public BinaryFileSavedEvent(String fileResource, byte[] bytes) {
-    this.fileResource = fileResource;
-    this.bytes = bytes;
-  }
-
-  public String getFileResource() {
-    return fileResource;
-  }
 
-  public byte[] getBytes() {
-    return bytes;
-  }
+  private final String fileResource;
+  private final byte[] bytes;
 }
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/FileDeletedEvent.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/FileDeletedEvent.java
@@ -27,33 +27,18 @@
  */
 package org.hisp.dhis.fileresource.events;
 
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
 import org.hisp.dhis.fileresource.FileResourceDomain;
 
 /**
  * @Author Zubair Asghar.
  */
+@Getter
+@RequiredArgsConstructor
 public class FileDeletedEvent {
-  private String storageKey;
 
-  private String contentType;
-
-  private FileResourceDomain domain;
-
-  public FileDeletedEvent(String storageKey, String contentType, FileResourceDomain domain) {
-    this.storageKey = storageKey;
-    this.contentType = contentType;
-    this.domain = domain;
-  }
-
-  public String getStorageKey() {
-    return storageKey;
-  }
-
-  public String getContentType() {
-    return contentType;
-  }
-
-  public FileResourceDomain getDomain() {
-    return domain;
-  }
+  private final String storageKey;
+  private final String contentType;
+  private final FileResourceDomain domain;
 }
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/FileSavedEvent.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/FileSavedEvent.java
@@ -28,25 +28,16 @@
 package org.hisp.dhis.fileresource.events;
 
 import java.io.File;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
 
 /**
  * @Author Zubair Asghar.
  */
+@Getter
+@RequiredArgsConstructor
 public class FileSavedEvent {
-  private String fileResource;
 
-  private File file;
-
-  public FileSavedEvent(String fileResource, File file) {
-    this.fileResource = fileResource;
-    this.file = file;
-  }
-
-  public String getFileResource() {
-    return fileResource;
-  }
-
-  public File getFile() {
-    return file;
-  }
+  private final String fileResource;
+  private final File file;
 }
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/ImageFileSavedEvent.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/events/ImageFileSavedEvent.java
@@ -29,26 +29,17 @@
 
 import java.io.File;
 import java.util.Map;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
 import org.hisp.dhis.fileresource.ImageFileDimension;
 
 /**
  * @Author Zubair Asghar.
  */
+@Getter
+@RequiredArgsConstructor
 public class ImageFileSavedEvent {
-  private String fileResource;
 
-  private Map<ImageFileDimension, File> imageFiles;
-
-  public ImageFileSavedEvent(String fileResource, Map<ImageFileDimension, File> imageFiles) {
-    this.fileResource = fileResource;
-    this.imageFiles = imageFiles;
-  }
-
-  public String getFileResource() {
-    return fileResource;
-  }
-
-  public Map<ImageFileDimension, File> getImageFiles() {
-    return imageFiles;
-  }
+  private final String fileResource;
+  private final Map<ImageFileDimension, File> imageFiles;
 }
diff --git a/...ervice-core/src/main/java/org/hisp/dhis/fileresource/JCloudsFileResourceContentStore.java b/...ervice-core/src/main/java/org/hisp/dhis/fileresource/JCloudsFileResourceContentStore.java
@@ -43,6 +43,8 @@
 import java.util.NoSuchElementException;
 import java.util.Properties;
 import java.util.regex.Pattern;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import lombok.extern.slf4j.Slf4j;
@@ -228,9 +230,28 @@ public long getFileResourceContentLength(String key) {
   }
 
   @Override
-  public String saveFileResourceContent(FileResource fileResource, byte[] bytes) {
-    Blob blob = createBlob(fileResource, bytes);
+  public String saveFileResourceContent(@Nonnull FileResource fr, @Nonnull byte[] bytes) {
+    return saveFileResourceContent(fr, createBlob(fr, bytes), null);
+  }
+
+  @Override
+  public String saveFileResourceContent(@Nonnull FileResource fr, @Nonnull File file) {
+    return saveFileResourceContent(
+        fr,
+        createBlob(fr, StringUtils.EMPTY, file, fr.getContentMd5()),
+        () -> {
+          try {
+            Files.deleteIfExists(file.toPath());
+          } catch (IOException ioe) {
+            log.warn(
+                String.format("Temporary file '%s' could not be deleted.", file.toPath()), ioe);
+          }
+        });
+  }
 
+  @CheckForNull
+  private String saveFileResourceContent(
+      @Nonnull FileResource fr, @CheckForNull Blob blob, @CheckForNull Runnable postPutCallback) {
     if (blob == null) {
       return null;
     }
@@ -242,35 +263,18 @@ public String saveFileResourceContent(FileResource fileResource, byte[] bytes) {
       return null;
     }
 
-    log.debug(String.format("File resource saved with key: %s", fileResource.getStorageKey()));
-
-    return fileResource.getStorageKey();
-  }
-
-  @Override
-  public String saveFileResourceContent(FileResource fileResource, File file) {
-    Blob blob = createBlob(fileResource, StringUtils.EMPTY, file, fileResource.getContentMd5());
-
-    if (blob == null) {
-      return null;
-    }
-
-    blobStore.putBlob(config.container, blob);
-
-    try {
-      Files.deleteIfExists(file.toPath());
-    } catch (IOException ioe) {
-      log.warn(String.format("Temporary file '%s' could not be deleted.", file.toPath()), ioe);
+    if (postPutCallback != null) {
+      postPutCallback.run();
     }
 
-    log.debug(String.format("File resource saved with key: %s", fileResource.getStorageKey()));
+    log.debug(String.format("File resource saved with key: %s", fr.getStorageKey()));
 
-    return fileResource.getStorageKey();
+    return fr.getStorageKey();
   }
 
   @Override
   public String saveFileResourceContent(
-      FileResource fileResource, Map<ImageFileDimension, File> imageFiles) {
+      @Nonnull FileResource fr, @Nonnull Map<ImageFileDimension, File> imageFiles) {
     if (imageFiles.isEmpty()) {
       return null;
     }
@@ -290,7 +294,7 @@ public String saveFileResourceContent(
         return null;
       }
 
-      blob = createBlob(fileResource, entry.getKey().getDimension(), file, contentMd5);
+      blob = createBlob(fr, entry.getKey().getDimension(), file, contentMd5);
 
       if (blob != null) {
         try {
@@ -308,7 +312,7 @@ public String saveFileResourceContent(
       }
     }
 
-    return fileResource.getStorageKey();
+    return fr.getStorageKey();
   }
 
   @Override
@@ -379,7 +383,7 @@ private void deleteBlob(String key) {
     blobStore.removeBlob(config.container, key);
   }
 
-  private Blob createBlob(FileResource fileResource, byte[] bytes) {
+  private Blob createBlob(@Nonnull FileResource fileResource, @Nonnull byte[] bytes) {
     return blobStore
         .blobBuilder(fileResource.getStorageKey())
         .payload(bytes)
@@ -391,7 +395,10 @@ private Blob createBlob(FileResource fileResource, byte[] bytes) {
   }
 
   private Blob createBlob(
-      FileResource fileResource, String fileDimension, File file, String contentMd5) {
+      @Nonnull FileResource fileResource,
+      String fileDimension,
+      @Nonnull File file,
+      @Nonnull String contentMd5) {
     return blobStore
         .blobBuilder(StringUtils.join(fileResource.getStorageKey(), fileDimension))
         .payload(file)