peterxcli commented on code in PR #10072:
URL: https://github.com/apache/ozone/pull/10072#discussion_r3071340073


##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -114,7 +115,7 @@ public static String createSignatureBase(
     strToSign.append(signatureInfo.getAlgorithm()).append(NEWLINE);
     if (signatureInfo.getDateTime() == null) {
       LOG.error("DateTime Header not found.");
-      throw S3_AUTHINFO_CREATION_ERROR;
+      throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);

Review Comment:
   ```suggestion
         throw newError(S3_AUTHINFO_CREATION_ERROR);
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -187,13 +188,13 @@ public static String buildCanonicalRequest(
           validateSignedHeader(schema, header, headerValue);
         } catch (DateTimeParseException ex) {
           LOG.error("DateTime format invalid.", ex);
-          throw S3_AUTHINFO_CREATION_ERROR;
+          throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);

Review Comment:
   ```suggestion
             throw newError(S3_AUTHINFO_CREATION_ERROR);
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -187,13 +188,13 @@ public static String buildCanonicalRequest(
           validateSignedHeader(schema, header, headerValue);
         } catch (DateTimeParseException ex) {
           LOG.error("DateTime format invalid.", ex);
-          throw S3_AUTHINFO_CREATION_ERROR;
+          throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);
         }
 
       } else {
         LOG.error("Header " + header + " not present in "
             + "request but requested to be signed.");
-        throw S3_AUTHINFO_CREATION_ERROR;
+        throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);

Review Comment:
   ```suggestion
           throw newError(S3_AUTHINFO_CREATION_ERROR);
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java:
##########
@@ -27,171 +27,181 @@
 import static 
org.apache.hadoop.ozone.OzoneConsts.S3_REQUEST_HEADER_METADATA_SIZE_LIMIT_KB;
 import static org.apache.hadoop.ozone.s3.util.S3Consts.RANGE_NOT_SATISFIABLE;
 
-import java.util.function.Function;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * This class represents errors from Ozone S3 service.
  * This class needs to be updated to add new errors when required.
  */
-public final class S3ErrorTable {
+public enum S3ErrorTable {
 
-  private static final Logger LOG = LoggerFactory.getLogger(
-      S3ErrorTable.class);
+  INVALID_URI("InvalidURI",
+      "Couldn't parse the specified URI.", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception INVALID_URI = new OS3Exception("InvalidURI",
-      "Couldn't parse the specified URI.", HTTP_BAD_REQUEST);
+  NO_SUCH_VOLUME("NoSuchVolume",
+      "The specified volume does not exist", HTTP_NOT_FOUND),
 
-  public static final OS3Exception NO_SUCH_VOLUME = new OS3Exception(
-      "NoSuchVolume", "The specified volume does not exist", HTTP_NOT_FOUND);
+  NO_SUCH_BUCKET("NoSuchBucket",
+      "The specified bucket does not exist", HTTP_NOT_FOUND),
 
-  public static final OS3Exception NO_SUCH_BUCKET = new OS3Exception(
-      "NoSuchBucket", "The specified bucket does not exist", HTTP_NOT_FOUND);
+  AUTH_PROTOCOL_NOT_SUPPORTED("AuthProtocolNotSupported",
+      "Auth protocol used for this request is not supported.", 
HTTP_BAD_REQUEST),
 
-  public static final OS3Exception AUTH_PROTOCOL_NOT_SUPPORTED =
-      new OS3Exception("AuthProtocolNotSupported", "Auth protocol used for" +
-          " this request is not supported.", HTTP_BAD_REQUEST);
-
-  public static final OS3Exception S3_AUTHINFO_CREATION_ERROR =
-      new OS3Exception("InvalidRequest", "Error creating s3 auth info. " +
+  S3_AUTHINFO_CREATION_ERROR("InvalidRequest", "Error creating s3 auth info. " 
+
           "The request may not be signed using AWS V4 signing algorithm," +
-          " or might be invalid", HTTP_FORBIDDEN);
+          " or might be invalid", HTTP_FORBIDDEN),
 
-  public static final OS3Exception BUCKET_NOT_EMPTY = new OS3Exception(
+  BUCKET_NOT_EMPTY(
       "BucketNotEmpty", "The bucket you tried to delete is not empty. " +
       "If you are using --force option to delete all objects in the bucket, " +
       "please ensure that the bucket layout is OBJECT_STORE or " +
       "that the bucket is completely empty before delete.",
-      HTTP_CONFLICT);
+      HTTP_CONFLICT),
 
-  public static final OS3Exception MALFORMED_HEADER = new OS3Exception(
+  MALFORMED_HEADER(
       "AuthorizationHeaderMalformed", "The authorization header you provided " 
+
-      "is invalid.", HTTP_BAD_REQUEST);
+      "is invalid.", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception NO_SUCH_KEY = new OS3Exception(
-      "NoSuchKey", "The specified key does not exist", HTTP_NOT_FOUND);
+  NO_SUCH_KEY(
+      "NoSuchKey", "The specified key does not exist", HTTP_NOT_FOUND),
 
-  public static final OS3Exception INVALID_ARGUMENT = new OS3Exception(
-      "InvalidArgument", "Invalid Argument", HTTP_BAD_REQUEST);
+  INVALID_ARGUMENT(
+      "InvalidArgument", "Invalid Argument", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception INVALID_REQUEST = new OS3Exception(
-      "InvalidRequest", "Invalid Request", HTTP_BAD_REQUEST);
+  INVALID_REQUEST(
+      "InvalidRequest", "Invalid Request", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception INVALID_RANGE = new OS3Exception(
+  INVALID_RANGE(
       "InvalidRange", "The requested range is not satisfiable",
-      RANGE_NOT_SATISFIABLE);
+      RANGE_NOT_SATISFIABLE),
 
-  public static final OS3Exception NO_SUCH_UPLOAD = new OS3Exception(
+  NO_SUCH_UPLOAD(
       "NoSuchUpload", "The specified multipart upload does not exist. The " +
       "upload ID might be invalid, or the multipart upload might have " +
-      "been aborted or completed.", HTTP_NOT_FOUND);
+      "been aborted or completed.", HTTP_NOT_FOUND),
 
-  public static final OS3Exception INVALID_BUCKET_NAME = new OS3Exception(
+  INVALID_BUCKET_NAME(
       "InvalidBucketName", "The specified bucket is not valid.",
-      HTTP_BAD_REQUEST);
+      HTTP_BAD_REQUEST),
 
-  public static final OS3Exception INVALID_PART = new OS3Exception(
+  INVALID_PART(
       "InvalidPart", "One or more of the specified parts could not be found." +
       " The part might not have been uploaded, or the specified entity " +
-      "tag might not have matched the part's entity tag.", HTTP_BAD_REQUEST);
+      "tag might not have matched the part's entity tag.", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception INVALID_PART_ORDER = new OS3Exception(
+  INVALID_PART_ORDER(
       "InvalidPartOrder", "The list of parts was not in ascending order. The " 
+
       "parts list must be specified in order by part number.",
-      HTTP_BAD_REQUEST);
+      HTTP_BAD_REQUEST),
 
-  public static final OS3Exception ENTITY_TOO_SMALL = new OS3Exception(
+  ENTITY_TOO_SMALL(
       "EntityTooSmall", "Your proposed upload is smaller than the minimum " +
       "allowed object size. Each part must be at least 5 MB in size, except " +
-      "the last part.", HTTP_BAD_REQUEST);
+      "the last part.", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception INTERNAL_ERROR = new OS3Exception(
+  INTERNAL_ERROR(
       "InternalError", "We encountered an internal error. Please try again.",
-      HTTP_INTERNAL_ERROR);
+      HTTP_INTERNAL_ERROR),
 
-  public static final OS3Exception ACCESS_DENIED = new OS3Exception(
+  ACCESS_DENIED(
       "AccessDenied", "User doesn't have the right to access this " +
-      "resource.", HTTP_FORBIDDEN);
+      "resource.", HTTP_FORBIDDEN),
 
-  public static final OS3Exception PRECOND_FAILED = new OS3Exception(
+  PRECOND_FAILED(
       "PreconditionFailed", "At least one of the pre-conditions you " +
-      "specified did not hold", HTTP_PRECON_FAILED);
+      "specified did not hold", HTTP_PRECON_FAILED),
 
-  public static final OS3Exception CONDITIONAL_REQUEST_CONFLICT =
-      new OS3Exception("ConditionalRequestConflict",
+  CONDITIONAL_REQUEST_CONFLICT("ConditionalRequestConflict",
           "A conflicting conditional operation occurred. Retry the request.",
-          HTTP_CONFLICT);
+          HTTP_CONFLICT),
   
-  public static final OS3Exception NOT_IMPLEMENTED = new OS3Exception(
+  NOT_IMPLEMENTED(
       "NotImplemented", "This part of feature is not implemented yet.",
-      HTTP_NOT_IMPLEMENTED);
+      HTTP_NOT_IMPLEMENTED),
 
-  public static final OS3Exception NO_OVERWRITE = new OS3Exception(
-      "Conflict", "Cannot overwrite file with directory", HTTP_CONFLICT);
+  NO_OVERWRITE(
+      "Conflict", "Cannot overwrite file with directory", HTTP_CONFLICT),
 
-  public static final OS3Exception METADATA_TOO_LARGE = new OS3Exception(
+  METADATA_TOO_LARGE(
       "MetadataTooLarge", "Illegal user defined metadata. Combined size " +
       "exceeds the maximum allowed metadata size of " +
-      S3_REQUEST_HEADER_METADATA_SIZE_LIMIT_KB + "KB", HTTP_BAD_REQUEST);
+      S3_REQUEST_HEADER_METADATA_SIZE_LIMIT_KB + "KB", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception BUCKET_ALREADY_EXISTS = new OS3Exception(
+  BUCKET_ALREADY_EXISTS(
       "BucketAlreadyExists", "The requested bucket name is not available" +
-      " as it already exists.", HTTP_CONFLICT);
+      " as it already exists.", HTTP_CONFLICT),
 
-  public static final OS3Exception INVALID_TAG = new OS3Exception(
-      "InvalidTag", "Your request contains tag input that is not valid.", 
HTTP_BAD_REQUEST);
+  INVALID_TAG(
+      "InvalidTag", "Your request contains tag input that is not valid.", 
HTTP_BAD_REQUEST),
 
-  public static final OS3Exception NO_SUCH_TAG_SET = new OS3Exception(
-      "NoSuchTagSet", "The specified tag does not exist.", HTTP_NOT_FOUND);
+  NO_SUCH_TAG_SET(
+      "NoSuchTagSet", "The specified tag does not exist.", HTTP_NOT_FOUND),
 
-  public static final OS3Exception MALFORMED_XML = new OS3Exception(
+  MALFORMED_XML(
       "MalformedXML", "The XML you provided was not well-formed or did not " +
-      "validate against our published schema", HTTP_BAD_REQUEST);
+      "validate against our published schema", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception QUOTA_EXCEEDED = new OS3Exception(
+  QUOTA_EXCEEDED(
       "QuotaExceeded", "The quota has been exceeded. " +
       "Please review your disk space or namespace usage and adjust 
accordingly.",
       HTTP_FORBIDDEN
-  );
+  ),
 
-  public static final OS3Exception INVALID_STORAGE_CLASS = new OS3Exception(
+  INVALID_STORAGE_CLASS(
       "InvalidStorageClass", "The storage class that you specified is not 
valid. " +
       "Provide a supported storage 
class[STANDARD|REDUCED_REDUNDANCY|STANDARD_IA] or " +
       "a valid custom EC storage config for if using STANDARD_IA.",
-      HTTP_BAD_REQUEST);
+      HTTP_BAD_REQUEST),
 
-  public static final OS3Exception BUCKET_OWNER_MISMATCH = new OS3Exception(
+  BUCKET_OWNER_MISMATCH(
       "Access Denied", "User doesn't have permission to access this resource 
due to a " +
-      "bucket ownership mismatch.", HTTP_FORBIDDEN);
+      "bucket ownership mismatch.", HTTP_FORBIDDEN),
 
-  public static final OS3Exception X_AMZ_CONTENT_SHA256_MISMATCH = new 
OS3Exception(
+  X_AMZ_CONTENT_SHA256_MISMATCH(
       "XAmzContentSHA256Mismatch", "The provided 'x-amz-content-sha256' header 
does " +
-      "not match the computed hash.", HTTP_BAD_REQUEST);
+      "not match the computed hash.", HTTP_BAD_REQUEST),
 
-  public static final OS3Exception BAD_DIGEST = new OS3Exception(
+  BAD_DIGEST(
       "BadDigest", "The Content-MD5 or checksum value that you specified did 
not match what the server received.",
       HTTP_BAD_REQUEST);
 
-  private static Function<Exception, OS3Exception> generateInternalError =
-      e -> new OS3Exception("InternalError", e.getMessage(), 
HTTP_INTERNAL_ERROR);
+  private static final Logger LOG = 
LoggerFactory.getLogger(S3ErrorTable.class);
+
+  private final String code;
+  private final String message;
+  private final int httpCode;
+
+  S3ErrorTable(String code, String message, int httpCode) {
+    this.code = code;
+    this.message = message;
+    this.httpCode = httpCode;
+  }
 
-  private S3ErrorTable() {
-    //No one should construct this object.
+  public String getCode() {
+    return code;
   }
 
-  public static OS3Exception newError(OS3Exception e, String resource) {
+  public String getErrorMessage() {
+    return message;
+  }
+
+  public int getHttpCode() {
+    return httpCode;
+  }
+
+  public static OS3Exception newError(S3ErrorTable e, String resource) {
     return newError(e, resource, null);
   }

Review Comment:
   ```suggestion
     public static OS3Exception newError(S3ErrorTable e) {
       return newError(e, null, null);
     }
   
     public static OS3Exception newError(S3ErrorTable e, String resource) {
       return newError(e, resource, null);
     }
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -225,7 +226,7 @@ private static String getPayloadHash(Map<String, String> 
headers, boolean isUsin
     if (contentSignatureHeaderValue == null) {
       LOG.error("The request must include " + X_AMZ_CONTENT_SHA256
           + " header for signed payload");
-      throw S3_AUTHINFO_CREATION_ERROR;
+      throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);

Review Comment:
   ```suggestion
         throw newError(S3_AUTHINFO_CREATION_ERROR);
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/AuthorizationFilter.java:
##########
@@ -71,14 +71,14 @@ public void filter(ContainerRequestContext context) throws
       } else {
         LOG.debug("Unsupported AWS signature version: {}",
             signatureInfo.getVersion());
-        throw S3_AUTHINFO_CREATION_ERROR;
+        throw S3ErrorTable.newError(S3_AUTHINFO_CREATION_ERROR, 
String.valueOf(signatureInfo.getVersion()), null);
       }
 
       String awsAccessId = signatureInfo.getAwsAccessId();
       // ONLY validate aws access id when needed.
       if (awsAccessId == null || awsAccessId.equals("")) {
         LOG.debug("Malformed s3 header. awsAccessID: {}", awsAccessId);
-        throw ACCESS_DENIED;
+        throw S3ErrorTable.newError(ACCESS_DENIED, null, null);

Review Comment:
   ```suggestion
           throw S3ErrorTable.newError(ACCESS_DENIED);
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java:
##########
@@ -50,11 +51,13 @@ public CompleteMultipartUploadRequest readFrom(
       InputStream inputStream) throws WebApplicationException {
     try {
       if (inputStream.available() == 0) {
-        throw wrapOS3Exception(INVALID_REQUEST.withMessage("You must specify 
at least one part"));
+        throw wrapOS3Exception(newError(INVALID_REQUEST, null, null)

Review Comment:
   ```suggestion
           throw wrapOS3Exception(newError(INVALID_REQUEST)
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -375,7 +376,7 @@ private static void validateCanonicalHeaders(
         }
         LOG.error("The SignedHeaders list must include all "
             + "x-amz-* headers in the request");
-        throw S3_AUTHINFO_CREATION_ERROR;
+        throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);

Review Comment:
   ```suggestion
           throw newError(S3_AUTHINFO_CREATION_ERROR);
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -361,7 +362,7 @@ private static void validateCanonicalHeaders(
   ) throws OS3Exception {
     if (!canonicalHeaders.contains(HOST + ":")) {
       LOG.error("The SignedHeaders list must include HTTP Host header");
-      throw S3_AUTHINFO_CREATION_ERROR;
+      throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);

Review Comment:
   ```suggestion
         throw newError(S3_AUTHINFO_CREATION_ERROR);
   ```



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -332,7 +333,7 @@ static void validateSignedHeader(
         LOG.error("AWS date not in valid range. Request timestamp:{} should "
             + "not be older than {} seconds.",
             headerValue, PRESIGN_URL_MAX_EXPIRATION_SECONDS);
-        throw S3_AUTHINFO_CREATION_ERROR;
+        throw newError(S3_AUTHINFO_CREATION_ERROR, null, null);

Review Comment:
   ```suggestion
           throw newError(S3_AUTHINFO_CREATION_ERROR);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to