Move validation logic of checksum/algorithm parameters from 'putObjec…

…t' to 'verifyChecksumParameters' to improve code clarity
DataONEorg · Feb 1, 2024 · 45dbb31 · 45dbb31
1 parent 188d482
commit 45dbb31
Showing 1 changed file with 13 additions and 16 deletions.
diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java
@@ -1362,32 +1362,19 @@ protected ObjectMetadata putObject(
         PidRefsFileExistsException, IllegalArgumentException, NullPointerException,
         AtomicMoveNotSupportedException {
         logFileHashStore.debug("FileHashStore.putObject - Called to put object for pid: " + pid);
-
-        // Validate algorithms if not null or empty, throws exception if not supported
+        // If validation is desired, checksumAlgorithm and checksum must both be present
+        boolean requestValidation = verifyChecksumParameters(checksum, checksumAlgorithm);
+        // Validate additional algorithm if not null or empty, throws exception if not supported
         if (additionalAlgorithm != null) {
             FileHashStoreUtility.checkForEmptyString(
                 additionalAlgorithm, "additionalAlgorithm", "putObject"
             );
             validateAlgorithm(additionalAlgorithm);
         }
-        // checksumAlgorithm should be evaluated as a pair, catch it earlier
-        // The way this checks must be very clear or else it's difficult to understand
-        if (checksumAlgorithm != null) {
-            FileHashStoreUtility.checkForEmptyString(
-                checksumAlgorithm, "checksumAlgorithm", "putObject"
-            );
-            validateAlgorithm(checksumAlgorithm);
-        }
-        if (checksum != null) {
-            FileHashStoreUtility.checkForEmptyString(checksum, "checksum", "putObject");
-        }
         if (objSize != -1) {
             FileHashStoreUtility.checkNotNegativeOrZero(objSize, "putObject");
         }
 
-        // If validation is desired, checksumAlgorithm and checksum must both be present
-        boolean requestValidation = verifyChecksumParameters(checksum, checksumAlgorithm);
-
         // Generate tmp file and write to it
         logFileHashStore.debug("FileHashStore.putObject - Generating tmpFile");
         File tmpFile = FileHashStoreUtility.generateTmpFile("tmp", OBJECT_TMP_FILE_DIRECTORY);
@@ -1588,6 +1575,16 @@ private boolean isDefaultAlgorithm(String algorithm) {
      */
     protected boolean verifyChecksumParameters(String checksum, String checksumAlgorithm)
         throws NoSuchAlgorithmException {
+        // First ensure algorithm is compatible and values are valid if they aren't null
+        if (checksumAlgorithm != null) {
+            FileHashStoreUtility.checkForEmptyString(
+                checksumAlgorithm, "checksumAlgorithm", "putObject"
+            );
+            validateAlgorithm(checksumAlgorithm);
+        }
+        if (checksum != null) {
+            FileHashStoreUtility.checkForEmptyString(checksum, "checksum", "putObject");
+        }
         // If checksum is supplied, checksumAlgorithm cannot be empty
         if (checksum != null && !checksum.trim().isEmpty()) {
             FileHashStoreUtility.ensureNotNull(