HDDS-15003. Inline EndpointBase#createS3Bucket/deleteS3Bucket

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketAclHandler.java

@@ -88,7 +88,7 @@ Response handleGetRequest(S3RequestContext context, String bucketName)
     context.setAction(S3GAction.GET_ACL);
 
     try {
-      OzoneBucket bucket = getBucket(bucketName);
+      OzoneBucket bucket = context.getVolume().getBucket(bucketName);
       S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
       S3Owner owner = S3Owner.of(bucket.getOwner());
 
@@ -139,9 +139,9 @@ Response handlePutRequest(S3RequestContext context, String bucketName, InputStre
     String grantFull = getHeaders().getHeaderString(S3Acl.GRANT_FULL_CONTROL);
 
     try {
-      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = context.getVolume();
+      OzoneBucket bucket = volume.getBucket(bucketName);
       S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
-      OzoneVolume volume = getVolume();
 
       List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
       List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketCrudHandler.java

@@ -19,6 +19,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
 import org.apache.hadoop.ozone.audit.S3GAction;
 import org.apache.hadoop.ozone.client.OzoneBucket;
@@ -62,10 +63,16 @@ Response handlePutRequest(S3RequestContext context, String bucketName, InputStre
 
     context.setAction(S3GAction.CREATE_BUCKET);
 
-    String location = createS3Bucket(bucketName);
-    getMetrics().updateCreateBucketSuccessStats(context.getStartNanos());
-    return Response.status(HttpStatus.SC_OK).header("Location", location)
-        .build();
+    try {
+      getClient().getObjectStore().createS3Bucket(bucketName);
+      getMetrics().updateCreateBucketSuccessStats(context.getStartNanos());
+      return Response.status(HttpStatus.SC_OK)
+          .header(HttpHeaders.LOCATION, "/" + bucketName)
+          .build();
+    } catch (Exception e) {
+      getMetrics().updateCreateBucketFailureStats(context.getStartNanos());
+      throw e;
+    }
   }
 
   /**
@@ -83,10 +90,10 @@ Response handleDeleteRequest(S3RequestContext context, String bucketName)
 
     try {
       if (S3Owner.hasBucketOwnershipVerificationConditions(getHeaders())) {
-        OzoneBucket bucket = getBucket(bucketName);
+        OzoneBucket bucket = context.getVolume().getBucket(bucketName);
         S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
       }
-      deleteS3Bucket(bucketName);
+      context.getVolume().deleteBucket(bucketName);
     } catch (Exception ex) {
       getMetrics().updateDeleteBucketFailureStats(context.getStartNanos());
       throw ex;

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java

@@ -136,7 +136,7 @@ Response handleGetRequest(S3RequestContext context, String bucketName) throws IO
       boolean shallow = listKeysShallowEnabled
           && OZONE_URI_DELIMITER.equals(delimiter);
 
-      bucket = getBucket(bucketName);
+      bucket = context.getVolume().getBucket(bucketName);
       S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
 
       ozoneKeyIterator = bucket.listKeys(prefix, prevKey, shallow);
@@ -293,11 +293,14 @@ public Response head(@PathParam(BUCKET) String bucketName)
     long startNanos = Time.monotonicNowNanos();
     S3GAction s3GAction = S3GAction.HEAD_BUCKET;
     try {
-      OzoneBucket bucket = getBucket(bucketName);
+      OzoneBucket bucket = getVolume().getBucket(bucketName);
       S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
       auditReadSuccess(s3GAction);
       getMetrics().updateHeadBucketSuccessStats(startNanos);
       return Response.ok().build();
+    } catch (OMException e) {
+      auditReadFailure(s3GAction, e);
+      throw newError(translateException(e), bucketName, e);
     } catch (Exception e) {
       auditReadFailure(s3GAction, e);
       throw e;
@@ -342,7 +345,7 @@ public MultiDeleteResponse multiDelete(
   ) throws OS3Exception, IOException {
     S3GAction s3GAction = S3GAction.MULTI_DELETE;
 
-    OzoneBucket bucket = getBucket(bucketName);
+    OzoneBucket bucket = getVolume().getBucket(bucketName);
     MultiDeleteResponse result = new MultiDeleteResponse();
     List<String> deleteKeys = new ArrayList<>();
 
@@ -429,15 +432,21 @@ protected void init() {
   }
 
   private static OS3Exception translateException(OMException ex) {
-    if (isAccessDenied(ex)) {
+    switch (ex.getResult()) {
+    case ACCESS_DENIED:
+    case INVALID_TOKEN:
+    case PERMISSION_DENIED:
       return S3ErrorTable.ACCESS_DENIED;
-    } else if (ex.getResult() == ResultCodes.BUCKET_NOT_EMPTY) {
+    case BUCKET_ALREADY_EXISTS:
+      return S3ErrorTable.BUCKET_ALREADY_EXISTS;
+    case BUCKET_NOT_EMPTY:
       return S3ErrorTable.BUCKET_NOT_EMPTY;
-    } else if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+    case BUCKET_NOT_FOUND:
+    case VOLUME_NOT_FOUND:
       return S3ErrorTable.NO_SUCH_BUCKET;
-    } else if (ex.getResult() == ResultCodes.INVALID_BUCKET_NAME) {
+    case INVALID_BUCKET_NAME:
       return S3ErrorTable.INVALID_BUCKET_NAME;
-    } else {
+    default:
       return INTERNAL_ERROR;
     }
   }

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java

@@ -108,7 +108,6 @@
 import org.apache.hadoop.ozone.s3.util.AuditUtils;
 import org.apache.hadoop.ozone.s3.util.S3Utils;
 import org.apache.hadoop.ozone.web.utils.OzoneUtils;
-import org.apache.hadoop.util.Time;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.utils.URLEncodedUtils;
 import org.slf4j.Logger;
@@ -186,27 +185,6 @@ public RequestParameters.Mutable queryParamsForTest() {
     return queryParams;
   }
 
-  protected OzoneBucket getBucket(OzoneVolume volume, String bucketName)
-      throws OS3Exception, IOException {
-    OzoneBucket bucket;
-    try {
-      bucket = volume.getBucket(bucketName);
-    } catch (OMException ex) {
-      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
-        throw newError(S3ErrorTable.NO_SUCH_BUCKET, bucketName, ex);
-      } else if (ex.getResult() == ResultCodes.INVALID_TOKEN) {
-        throw newError(S3ErrorTable.ACCESS_DENIED,
-            s3Auth.getAccessID(), ex);
-      } else if (ex.getResult() == ResultCodes.TIMEOUT ||
-          ex.getResult() == ResultCodes.INTERNAL_ERROR) {
-        throw newError(S3ErrorTable.INTERNAL_ERROR, bucketName, ex);
-      } else {
-        throw ex;
-      }
-    }
-    return bucket;
-  }
-
   /**
    * Initializes the object post construction. Calls init() from any
    * child classes to work around the issue of only one method can be annotated.
@@ -274,62 +252,6 @@ protected OzoneVolume getVolume() throws IOException {
     return client.getObjectStore().getS3Volume();
   }
 
-  /**
-   * Create an S3Bucket, and also it creates mapping needed to access via
-   * ozone and S3.
-   * @param bucketName
-   * @return location of the S3Bucket.
-   * @throws IOException
-   */
-  protected String createS3Bucket(String bucketName) throws
-      IOException, OS3Exception {
-    long startNanos = Time.monotonicNowNanos();
-    try {
-      client.getObjectStore().createS3Bucket(bucketName);
-    } catch (OMException ex) {
-      getMetrics().updateCreateBucketFailureStats(startNanos);
-      if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
-        throw newError(S3ErrorTable.ACCESS_DENIED, bucketName, ex);
-      } else if (ex.getResult() == ResultCodes.INVALID_TOKEN) {
-        throw newError(S3ErrorTable.ACCESS_DENIED,
-            s3Auth.getAccessID(), ex);
-      } else if (ex.getResult() == ResultCodes.TIMEOUT ||
-          ex.getResult() == ResultCodes.INTERNAL_ERROR) {
-        throw newError(S3ErrorTable.INTERNAL_ERROR, bucketName, ex);
-      } else if (ex.getResult() == ResultCodes.BUCKET_ALREADY_EXISTS) {
-        throw newError(S3ErrorTable.BUCKET_ALREADY_EXISTS, bucketName, ex);
-      } else {
-        throw ex;
-      }
-    }
-    return "/" + bucketName;
-  }
-
-  /**
-   * Deletes an s3 bucket and removes mapping of Ozone volume/bucket.
-   * @param s3BucketName - S3 Bucket Name.
-   * @throws  IOException in case the bucket cannot be deleted.
-   */
-  protected void deleteS3Bucket(String s3BucketName)
-      throws IOException, OS3Exception {
-    try {
-      client.getObjectStore().deleteS3Bucket(s3BucketName);
-    } catch (OMException ex) {
-      if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
-        throw newError(S3ErrorTable.ACCESS_DENIED,
-            s3BucketName, ex);
-      } else if (ex.getResult() == ResultCodes.INVALID_TOKEN) {
-        throw newError(S3ErrorTable.ACCESS_DENIED,
-            s3Auth.getAccessID(), ex);
-      } else if (ex.getResult() == ResultCodes.TIMEOUT ||
-          ex.getResult() == ResultCodes.INTERNAL_ERROR) {
-        throw newError(S3ErrorTable.INTERNAL_ERROR, s3BucketName, ex);
-      } else {
-        throw ex;
-      }
-    }
-  }
-
   /**
    * Returns Iterator to iterate over all buckets for a specific user.
    * The result can be restricted using bucket prefix, will return all

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ListMultipartUploadsHandler.java

@@ -56,7 +56,7 @@ Response handleGetRequest(S3RequestContext context, String bucketName)
 
     long startNanos = context.getStartNanos();
 
-    OzoneBucket bucket = getBucket(bucketName);
+    OzoneBucket bucket = context.getVolume().getBucket(bucketName);
 
     try {
       S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());

hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ObjectStoreStub.java

@@ -17,9 +17,6 @@
 
 package org.apache.hadoop.ozone.client;
 
-import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_ALREADY_EXISTS;
-import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_EMPTY;
-import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND;
 import static org.mockito.Mockito.mock;
 
@@ -40,20 +37,20 @@
  */
 public class ObjectStoreStub extends ObjectStore {
 
-  private static OzoneConfiguration conf = new OzoneConfiguration();
-  private Map<String, OzoneVolumeStub> volumes = new HashMap<>();
-  private Map<String, Boolean> bucketEmptyStatus = new HashMap<>();
+  private final Map<String, OzoneVolumeStub> volumes = new HashMap<>();
+  private final String s3VolumeName;
 
   public ObjectStoreStub() {
-    super(conf, mock(ClientProtocol.class));
+    this(new OzoneConfiguration(), mock(ClientProtocol.class));
   }
 
   public ObjectStoreStub(ConfigurationSource conf, ClientProtocol proxy) {
     super(conf, proxy);
+    s3VolumeName = HddsClientUtils.getDefaultS3VolumeName(conf);
   }
 
   @Override
-  public void createVolume(String volumeName) throws IOException {
+  public void createVolume(String volumeName) {
     createVolume(volumeName,
         VolumeArgs.newBuilder()
             .setAdmin("root")
@@ -86,8 +83,7 @@ public OzoneVolume getVolume(String volumeName) throws IOException {
   }
 
   @Override
-  public Iterator<? extends OzoneVolume> listVolumes(String volumePrefix)
-      throws IOException {
+  public Iterator<? extends OzoneVolume> listVolumes(String volumePrefix) {
     return volumes.values()
         .stream()
         .filter(volume -> volume.getName().startsWith(volumePrefix))
@@ -98,7 +94,7 @@ public Iterator<? extends OzoneVolume> listVolumes(String volumePrefix)
 
   @Override
   public Iterator<? extends OzoneVolume> listVolumes(String volumePrefix,
-      String prevVolume) throws IOException {
+      String prevVolume) {
     return volumes.values()
         .stream()
         .filter(volume -> volume.getName().compareTo(prevVolume) > 0)
@@ -109,7 +105,7 @@ public Iterator<? extends OzoneVolume> listVolumes(String volumePrefix,
 
   @Override
   public Iterator<? extends OzoneVolume> listVolumesByUser(String user,
-      String volumePrefix, String prevVolume) throws IOException {
+      String volumePrefix, String prevVolume) {
     return volumes.values()
         .stream()
         .filter(volume -> volume.getOwner().equals(user))
@@ -120,49 +116,33 @@ public Iterator<? extends OzoneVolume> listVolumesByUser(String user,
   }
 
   @Override
-  public void deleteVolume(String volumeName) throws IOException {
+  public void deleteVolume(String volumeName) {
     volumes.remove(volumeName);
   }
 
   @Override
   public OzoneVolume getS3Volume() throws IOException {
     // Always return default S3 volume. This class will not be used for
     // multitenant testing.
-    String volumeName = HddsClientUtils.getDefaultS3VolumeName(conf);
-    return getVolume(volumeName);
+    return getVolume(s3VolumeName);
   }
 
   @Override
   public void createS3Bucket(String s3BucketName) throws
       IOException {
-    if (!bucketEmptyStatus.containsKey(s3BucketName)) {
-      String volumeName = HddsClientUtils.getDefaultS3VolumeName(conf);
-      bucketEmptyStatus.put(s3BucketName, true);
-      if (!volumes.containsKey(volumeName)) {
-        createVolume(volumeName);
-      }
-      volumes.get(volumeName).createBucket(s3BucketName);
-    } else {
-      throw new OMException("", BUCKET_ALREADY_EXISTS);
+    if (!volumes.containsKey(s3VolumeName)) {
+      createVolume(s3VolumeName);
     }
+    volumes.get(s3VolumeName).createBucket(s3BucketName);
   }
 
   @Override
   public void deleteS3Bucket(String s3BucketName) throws
       IOException {
-    if (bucketEmptyStatus.containsKey(s3BucketName)) {
-      if (bucketEmptyStatus.get(s3BucketName)) {
-        bucketEmptyStatus.remove(s3BucketName);
-      } else {
-        throw new OMException("", BUCKET_NOT_EMPTY);
-      }
-    } else {
-      throw new OMException("", BUCKET_NOT_FOUND);
+    OzoneVolume volume = getS3Volume();
+    if (volume == null) {
+      throw new OMException("", VOLUME_NOT_FOUND);
     }
+    volume.deleteBucket(s3BucketName);
   }
-
-  public void setBucketEmptyStatus(String bucketName, boolean status) {
-    bucketEmptyStatus.computeIfPresent(bucketName, (k, v) -> status);
-  }
-
 }

hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java

@@ -105,6 +105,10 @@ public OzoneBucketStub build() {
     }
   }
 
+  boolean isEmpty() {
+    return keyDetails.isEmpty();
+  }
+
   @Override
   public OzoneOutputStream createKey(String key, long size) throws IOException {
     return createKey(key, size,