adoroszlai commented on code in PR #3263:
URL: https://github.com/apache/ozone/pull/3263#discussion_r847670579
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -110,4 +123,160 @@ public void testHeadObject() throws Exception {
long curMetric = metrics.getHeadKeySuccess();
assertEquals(1L, curMetric - oriMetric);
}
-}
+
+ @Test
+ public void testGetBucketSuccess() throws Exception {
+ long oriMetric = metrics.getGetBucketSuccess();
+
+ clientStub = createClientWithKeys("file1");
+ bucketEndpoint.setClient(clientStub);
+ bucketEndpoint.get(bucketName, null,
+ null, null, 1000, null,
+ null, "random", null,
+ null, null).getEntity();
+
+ long curMetric = metrics.getGetBucketSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetBucketFailure() throws Exception {
+ long oriMetric = metrics.getGetBucketFailure();
+
+ try {
+ // Searching for a bucket that does not exist
+ bucketEndpoint.get("newBucket", null,
+ null, null, 1000, null,
+ null, "random", null,
+ null, null);
+ fail();
+ } catch (OS3Exception e) {
+ }
+
+ long curMetric = metrics.getGetBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testCreateBucketSuccess() throws Exception {
+
+ long oriMetric = metrics.getCreateBucketSuccess();
+
+ bucketEndpoint.put(bucketName, null,
+ null, null);
+ long curMetric = metrics.getCreateBucketSuccess();
+
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testCreateBucketFailure() throws Exception {
+ // Creating an error by trying to create a bucket that already exists
+ long oriMetric = metrics.getCreateBucketFailure();
+
+ bucketEndpoint.put(bucketName, null, null, null);
+
+ long curMetric = metrics.getCreateBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testDeleteBucketSuccess() throws Exception {
+ long oriMetric = metrics.getDeleteBucketSuccess();
+
+ bucketEndpoint.delete(bucketName);
+
+ long curMetric = metrics.getDeleteBucketSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testDeleteBucketFailure() throws Exception {
+ long oriMetric = metrics.getDeleteBucketFailure();
+ bucketEndpoint.delete(bucketName);
+ try {
+ // Deleting a bucket that does not exist will result in delete failure
+ bucketEndpoint.delete(bucketName);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), ex.getCode());
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getErrorMessage(),
+ ex.getErrorMessage());
+ }
+
+ long curMetric = metrics.getDeleteBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetAclSuccess() throws Exception {
+ long oriMetric = metrics.getGetAclSuccess();
+
+ Response response =
+ bucketEndpoint.get(bucketName, null, null,
+ null, 0, null, null,
+ null, null, "acl", null);
+ long curMetric = metrics.getGetAclSuccess();
+ assertEquals(HTTP_OK, response.getStatus());
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetAclFailure() throws Exception {
+ long oriMetric = metrics.getGetAclFailure();
+ try {
+ // Failing the getACL endpoint by applying ACL on a non-Existent Bucket
+ bucketEndpoint.get("random_bucket", null,
+ null, null, 0, null,
+ null, null, null, "acl", null);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), ex.getCode());
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getErrorMessage(),
+ ex.getErrorMessage());
+ }
+ long curMetric = metrics.getGetAclFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testPutAclSuccess() throws Exception {
+ long oriMetric = metrics.getPutAclSuccess();
+
+ clientStub.getObjectStore().createS3Bucket("b1");
+ InputStream inputBody = TestBucketAcl.class.getClassLoader()
+ .getResourceAsStream("userAccessControlList.xml");
+
+ bucketEndpoint.put("b1", ACL_MARKER, headers, inputBody);
+ inputBody.close();
Review Comment:
Nit: using try-with-resources we can ensure that the stream is closed even
in case of failure.
```suggestion
try (InputStream inputBody = TestBucketAcl.class.getClassLoader()
.getResourceAsStream("userAccessControlList.xml")) {
bucketEndpoint.put("b1", ACL_MARKER, headers, inputBody);
}
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/metrics/S3GatewayMetrics.java:
##########
@@ -317,4 +317,52 @@ public long getHeadBucketSuccess() {
public long getHeadKeySuccess() {
return headKeySuccess.value();
}
+
+ public long getGetBucketSuccess() {
+ return getBucketSuccess.value();
+ }
+
+ public long getGetBucketFailure() {
+ return getBucketFailure.value();
+ }
+
+ public long getCreateBucketSuccess() {
+ return createBucketSuccess.value();
+ }
+
+ public long getCreateBucketFailure() {
+ return createBucketFailure.value();
+ }
+
+ public long getDeleteBucketSuccess() {
+ return deleteBucketSuccess.value();
+ }
+
+ public long getDeleteBucketFailure() {
+ return deleteBucketFailure.value();
+ }
+
+ public long getGetAclSuccess() {
+ return getAclSuccess.value();
+ }
+
+ public long getGetAclFailure() {
+ return getAclFailure.value();
+ }
+
+ public long getPutAclSuccess() {
+ return putAclSuccess.value();
+ }
+
+ public long getPutAclFailure() {
+ return putAclFailure.value();
+ }
+
+ public long getListMultipartUploadsSuccess() {
+ return listMultipartUploadsSuccess.value();
+ }
+
+ public long getListMultipartUploadsFailure() {
+ return listMultipartUploadsSuccess.value();
Review Comment:
Sorry, I just noticed this typo:
```suggestion
public long getListMultipartUploadsFailure() {
return listMultipartUploadsFailure.value();
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -110,4 +123,160 @@ public void testHeadObject() throws Exception {
long curMetric = metrics.getHeadKeySuccess();
assertEquals(1L, curMetric - oriMetric);
}
-}
+
+ @Test
+ public void testGetBucketSuccess() throws Exception {
+ long oriMetric = metrics.getGetBucketSuccess();
+
+ clientStub = createClientWithKeys("file1");
+ bucketEndpoint.setClient(clientStub);
+ bucketEndpoint.get(bucketName, null,
+ null, null, 1000, null,
+ null, "random", null,
+ null, null).getEntity();
+
+ long curMetric = metrics.getGetBucketSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetBucketFailure() throws Exception {
+ long oriMetric = metrics.getGetBucketFailure();
+
+ try {
+ // Searching for a bucket that does not exist
+ bucketEndpoint.get("newBucket", null,
+ null, null, 1000, null,
+ null, "random", null,
+ null, null);
+ fail();
+ } catch (OS3Exception e) {
+ }
+
+ long curMetric = metrics.getGetBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testCreateBucketSuccess() throws Exception {
+
+ long oriMetric = metrics.getCreateBucketSuccess();
+
+ bucketEndpoint.put(bucketName, null,
+ null, null);
+ long curMetric = metrics.getCreateBucketSuccess();
+
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testCreateBucketFailure() throws Exception {
+ // Creating an error by trying to create a bucket that already exists
+ long oriMetric = metrics.getCreateBucketFailure();
+
+ bucketEndpoint.put(bucketName, null, null, null);
+
+ long curMetric = metrics.getCreateBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testDeleteBucketSuccess() throws Exception {
+ long oriMetric = metrics.getDeleteBucketSuccess();
+
+ bucketEndpoint.delete(bucketName);
+
+ long curMetric = metrics.getDeleteBucketSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testDeleteBucketFailure() throws Exception {
+ long oriMetric = metrics.getDeleteBucketFailure();
+ bucketEndpoint.delete(bucketName);
+ try {
+ // Deleting a bucket that does not exist will result in delete failure
+ bucketEndpoint.delete(bucketName);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), ex.getCode());
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getErrorMessage(),
+ ex.getErrorMessage());
+ }
+
+ long curMetric = metrics.getDeleteBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetAclSuccess() throws Exception {
+ long oriMetric = metrics.getGetAclSuccess();
+
+ Response response =
+ bucketEndpoint.get(bucketName, null, null,
+ null, 0, null, null,
+ null, null, "acl", null);
+ long curMetric = metrics.getGetAclSuccess();
+ assertEquals(HTTP_OK, response.getStatus());
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetAclFailure() throws Exception {
+ long oriMetric = metrics.getGetAclFailure();
+ try {
+ // Failing the getACL endpoint by applying ACL on a non-Existent Bucket
+ bucketEndpoint.get("random_bucket", null,
+ null, null, 0, null,
+ null, null, null, "acl", null);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), ex.getCode());
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getErrorMessage(),
+ ex.getErrorMessage());
+ }
+ long curMetric = metrics.getGetAclFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testPutAclSuccess() throws Exception {
+ long oriMetric = metrics.getPutAclSuccess();
+
+ clientStub.getObjectStore().createS3Bucket("b1");
+ InputStream inputBody = TestBucketAcl.class.getClassLoader()
+ .getResourceAsStream("userAccessControlList.xml");
+
+ bucketEndpoint.put("b1", ACL_MARKER, headers, inputBody);
+ inputBody.close();
+ long curMetric = metrics.getPutAclSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testPutAclFailure() throws Exception {
+ // Failing the putACL endpoint by applying ACL on a non-Existent Bucket
+ long oriMetric = metrics.getPutAclFailure();
+
+ clientStub.getObjectStore().createS3Bucket("b1");
Review Comment:
Nit: `b1` bucket is not needed for this test case.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -110,4 +123,160 @@ public void testHeadObject() throws Exception {
long curMetric = metrics.getHeadKeySuccess();
assertEquals(1L, curMetric - oriMetric);
}
-}
+
+ @Test
+ public void testGetBucketSuccess() throws Exception {
+ long oriMetric = metrics.getGetBucketSuccess();
+
+ clientStub = createClientWithKeys("file1");
+ bucketEndpoint.setClient(clientStub);
+ bucketEndpoint.get(bucketName, null,
+ null, null, 1000, null,
+ null, "random", null,
+ null, null).getEntity();
+
+ long curMetric = metrics.getGetBucketSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetBucketFailure() throws Exception {
+ long oriMetric = metrics.getGetBucketFailure();
+
+ try {
+ // Searching for a bucket that does not exist
+ bucketEndpoint.get("newBucket", null,
+ null, null, 1000, null,
+ null, "random", null,
+ null, null);
+ fail();
+ } catch (OS3Exception e) {
+ }
+
+ long curMetric = metrics.getGetBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testCreateBucketSuccess() throws Exception {
+
+ long oriMetric = metrics.getCreateBucketSuccess();
+
+ bucketEndpoint.put(bucketName, null,
+ null, null);
+ long curMetric = metrics.getCreateBucketSuccess();
+
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testCreateBucketFailure() throws Exception {
+ // Creating an error by trying to create a bucket that already exists
+ long oriMetric = metrics.getCreateBucketFailure();
+
+ bucketEndpoint.put(bucketName, null, null, null);
+
+ long curMetric = metrics.getCreateBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testDeleteBucketSuccess() throws Exception {
+ long oriMetric = metrics.getDeleteBucketSuccess();
+
+ bucketEndpoint.delete(bucketName);
+
+ long curMetric = metrics.getDeleteBucketSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testDeleteBucketFailure() throws Exception {
+ long oriMetric = metrics.getDeleteBucketFailure();
+ bucketEndpoint.delete(bucketName);
+ try {
+ // Deleting a bucket that does not exist will result in delete failure
+ bucketEndpoint.delete(bucketName);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), ex.getCode());
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getErrorMessage(),
+ ex.getErrorMessage());
+ }
+
+ long curMetric = metrics.getDeleteBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetAclSuccess() throws Exception {
+ long oriMetric = metrics.getGetAclSuccess();
+
+ Response response =
+ bucketEndpoint.get(bucketName, null, null,
+ null, 0, null, null,
+ null, null, "acl", null);
+ long curMetric = metrics.getGetAclSuccess();
+ assertEquals(HTTP_OK, response.getStatus());
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testGetAclFailure() throws Exception {
+ long oriMetric = metrics.getGetAclFailure();
+ try {
+ // Failing the getACL endpoint by applying ACL on a non-Existent Bucket
+ bucketEndpoint.get("random_bucket", null,
+ null, null, 0, null,
+ null, null, null, "acl", null);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), ex.getCode());
+ assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getErrorMessage(),
+ ex.getErrorMessage());
+ }
+ long curMetric = metrics.getGetAclFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testPutAclSuccess() throws Exception {
+ long oriMetric = metrics.getPutAclSuccess();
+
+ clientStub.getObjectStore().createS3Bucket("b1");
+ InputStream inputBody = TestBucketAcl.class.getClassLoader()
+ .getResourceAsStream("userAccessControlList.xml");
+
+ bucketEndpoint.put("b1", ACL_MARKER, headers, inputBody);
+ inputBody.close();
+ long curMetric = metrics.getPutAclSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+ }
+
+ @Test
+ public void testPutAclFailure() throws Exception {
+ // Failing the putACL endpoint by applying ACL on a non-Existent Bucket
+ long oriMetric = metrics.getPutAclFailure();
+
+ clientStub.getObjectStore().createS3Bucket("b1");
+ InputStream inputBody = TestBucketAcl.class.getClassLoader()
+ .getResourceAsStream("userAccessControlList.xml");
+
+ try {
+ bucketEndpoint.put("unknown_bucket", ACL_MARKER, headers, inputBody);
+ fail();
+ } catch (OS3Exception ex) {
+ }
+ inputBody.close();
Review Comment:
Nit: using try-with-resources we can ensure that the stream is closed even
in case of failure.
```suggestion
try (InputStream inputBody = TestBucketAcl.class.getClassLoader()
.getResourceAsStream("userAccessControlList.xml")) {
bucketEndpoint.put("unknown_bucket", ACL_MARKER, headers, inputBody);
fail();
} catch (OS3Exception ex) {
}
```
--
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]
