HDDS-10844. Clarify snapshot create error message. (apache#6955)

xichen01 · Jul 17, 2024 · abf3a0a · abf3a0a
1 parent 6fa74bb
commit abf3a0a
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 39 deletions.
diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java
@@ -67,27 +67,26 @@ private HddsClientUtils() {
           .add(NotReplicatedException.class)
           .build();
 
-  private static void doNameChecks(String resName) {
+  private static void doNameChecks(String resName, String resType) {
     if (resName == null) {
-      throw new IllegalArgumentException("Bucket or Volume name is null");
+      throw new IllegalArgumentException(resType + " name is null");
     }
 
     if (resName.length() < OzoneConsts.OZONE_MIN_BUCKET_NAME_LENGTH ||
         resName.length() > OzoneConsts.OZONE_MAX_BUCKET_NAME_LENGTH) {
-      throw new IllegalArgumentException(
-          "Bucket or Volume length is illegal, "
-              + "valid length is 3-63 characters");
+      throw new IllegalArgumentException(resType +
+          " length is illegal, " + "valid length is 3-63 characters");
     }
 
     if (resName.charAt(0) == '.' || resName.charAt(0) == '-') {
-      throw new IllegalArgumentException(
-          "Bucket or Volume name cannot start with a period or dash");
+      throw new IllegalArgumentException(resType +
+          " name cannot start with a period or dash");
     }
 
     if (resName.charAt(resName.length() - 1) == '.' ||
         resName.charAt(resName.length() - 1) == '-') {
-      throw new IllegalArgumentException("Bucket or Volume name "
-          + "cannot end with a period or dash");
+      throw new IllegalArgumentException(resType +
+          " name cannot end with a period or dash");
     }
   }
 
@@ -108,27 +107,27 @@ private static boolean isSupportedCharacter(char c, boolean isStrictS3) {
     return false;
   }
 
-  private static void doCharacterChecks(char currChar, char prev,
+  private static void doCharacterChecks(char currChar, char prev, String resType,
       boolean isStrictS3) {
     if (Character.isUpperCase(currChar)) {
-      throw new IllegalArgumentException(
-          "Bucket or Volume name does not support uppercase characters");
+      throw new IllegalArgumentException(resType +
+          " name does not support uppercase characters");
     }
     if (!isSupportedCharacter(currChar, isStrictS3)) {
-      throw new IllegalArgumentException("Bucket or Volume name has an " +
-          "unsupported character : " + currChar);
+      throw new IllegalArgumentException(resType +
+          " name has an unsupported character : " + currChar);
     }
     if (prev == '.' && currChar == '.') {
-      throw new IllegalArgumentException("Bucket or Volume name should not " +
-          "have two contiguous periods");
+      throw new IllegalArgumentException(resType +
+          " name should not have two contiguous periods");
     }
     if (prev == '-' && currChar == '.') {
-      throw new IllegalArgumentException(
-          "Bucket or Volume name should not have period after dash");
+      throw new IllegalArgumentException(resType +
+          " name should not have period after dash");
     }
     if (prev == '.' && currChar == '-') {
-      throw new IllegalArgumentException(
-          "Bucket or Volume name should not have dash after period");
+      throw new IllegalArgumentException(resType +
+          " name should not have dash after period");
     }
   }
 
@@ -140,7 +139,11 @@ private static void doCharacterChecks(char currChar, char prev,
    * @throws IllegalArgumentException
    */
   public static void verifyResourceName(String resName) {
-    verifyResourceName(resName, true);
+    verifyResourceName(resName, "resource", true);
+  }
+
+  public static void verifyResourceName(String resName, String resType) {
+    verifyResourceName(resName, resType, true);
   }
 
   /**
@@ -150,9 +153,9 @@ public static void verifyResourceName(String resName) {
    *
    * @throws IllegalArgumentException
    */
-  public static void verifyResourceName(String resName, boolean isStrictS3) {
+  public static void verifyResourceName(String resName, String resType, boolean isStrictS3) {
 
-    doNameChecks(resName);
+    doNameChecks(resName, resType);
 
     boolean isIPv4 = true;
     char prev = (char) 0;
@@ -162,13 +165,13 @@ public static void verifyResourceName(String resName, boolean isStrictS3) {
       if (currChar != '.') {
         isIPv4 = ((currChar >= '0') && (currChar <= '9')) && isIPv4;
       }
-      doCharacterChecks(currChar, prev, isStrictS3);
+      doCharacterChecks(currChar, prev, resType, isStrictS3);
       prev = currChar;
     }
 
     if (isIPv4) {
-      throw new IllegalArgumentException(
-          "Bucket or Volume name cannot be an IPv4 address or all numeric");
+      throw new IllegalArgumentException(resType +
+          " name cannot be an IPv4 address or all numeric");
     }
   }
 

diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@@ -709,7 +709,7 @@ public void createBucket(
 
   private static void verifyVolumeName(String volumeName) throws OMException {
     try {
-      HddsClientUtils.verifyResourceName(volumeName, false);
+      HddsClientUtils.verifyResourceName(volumeName, "volume", false);
     } catch (IllegalArgumentException e) {
       throw new OMException(e.getMessage(),
           OMException.ResultCodes.INVALID_VOLUME_NAME);
@@ -718,7 +718,7 @@ private static void verifyVolumeName(String volumeName) throws OMException {
 
   private static void verifyBucketName(String bucketName) throws OMException {
     try {
-      HddsClientUtils.verifyResourceName(bucketName, false);
+      HddsClientUtils.verifyResourceName(bucketName, "bucket", false);
     } catch (IllegalArgumentException e) {
       throw new OMException(e.getMessage(),
           OMException.ResultCodes.INVALID_BUCKET_NAME);

diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
@@ -522,7 +522,7 @@ public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo,
   public static void validateVolumeName(String volumeName, boolean isStrictS3)
       throws OMException {
     try {
-      HddsClientUtils.verifyResourceName(volumeName, isStrictS3);
+      HddsClientUtils.verifyResourceName(volumeName, "volume", isStrictS3);
     } catch (IllegalArgumentException e) {
       throw new OMException("Invalid volume name: " + volumeName,
           OMException.ResultCodes.INVALID_VOLUME_NAME);
@@ -535,7 +535,7 @@ public static void validateVolumeName(String volumeName, boolean isStrictS3)
   public static void validateBucketName(String bucketName, boolean isStrictS3)
       throws OMException {
     try {
-      HddsClientUtils.verifyResourceName(bucketName, isStrictS3);
+      HddsClientUtils.verifyResourceName(bucketName, "bucket", isStrictS3);
     } catch (IllegalArgumentException e) {
       throw new OMException("Invalid bucket name: " + bucketName,
           OMException.ResultCodes.INVALID_BUCKET_NAME);
@@ -571,9 +571,9 @@ public static void validateSnapshotName(String snapshotName)
       return;
     }
     try {
-      HddsClientUtils.verifyResourceName(snapshotName);
+      HddsClientUtils.verifyResourceName(snapshotName, "snapshot");
     } catch (IllegalArgumentException e) {
-      throw new OMException("Invalid snapshot name: " + snapshotName,
+      throw new OMException("Invalid snapshot name: " + snapshotName + "\n" + e.getMessage(),
           OMException.ResultCodes.INVALID_SNAPSHOT_ERROR);
     }
   }

diff --git a/...ntegration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java b/...ntegration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java
@@ -799,7 +799,7 @@ public void testInvalidBucketCreation() throws Exception {
     OzoneVolume volume = store.getVolume(volumeName);
     OMException omException = assertThrows(OMException.class,
         () -> volume.createBucket(bucketName));
-    assertEquals("Bucket or Volume name has an unsupported character : #",
+    assertEquals("bucket name has an unsupported character : #",
         omException.getMessage());
   }
 

diff --git a/...rc/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java b/...rc/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java
@@ -162,8 +162,8 @@ public void testPreExecuteFailure(String snapshotName) {
         bucketName, snapshotName);
     OMException omException =
         assertThrows(OMException.class, () -> doPreExecute(omRequest));
-    assertEquals("Invalid snapshot name: " + snapshotName,
-        omException.getMessage());
+    assertTrue(omException.getMessage()
+        .contains("Invalid snapshot name: " + snapshotName));
   }
 
   @Test

diff --git a/...rc/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java b/...rc/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
@@ -159,8 +159,8 @@ public void testPreExecuteFailure(String deleteSnapshotName) {
         bucketName, deleteSnapshotName);
     OMException omException =
         assertThrows(OMException.class, () -> doPreExecute(omRequest));
-    assertEquals("Invalid snapshot name: " + deleteSnapshotName,
-        omException.getMessage());
+    assertTrue(omException.getMessage()
+        .contains("Invalid snapshot name: " + deleteSnapshotName));
   }
 
   @Test

diff --git a/...rc/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotRenameRequest.java b/...rc/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotRenameRequest.java
@@ -60,6 +60,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.doNothing;
@@ -172,8 +173,7 @@ public void testPreExecuteFailure(String toSnapshotName) {
         bucketName, currentSnapshotName, toSnapshotName);
     OMException omException =
         assertThrows(OMException.class, () -> doPreExecute(omRequest));
-    assertEquals("Invalid snapshot name: " + toSnapshotName,
-        omException.getMessage());
+    assertTrue(omException.getMessage().contains("Invalid snapshot name: " + toSnapshotName));
   }
 
   @Test