From 97925134ccb30c122f9849296073575dd3ff49d9 Mon Sep 17 00:00:00 2001 From: Xinran Dong <81548653+007DXR@users.noreply.github.com> Date: Mon, 17 Jul 2023 17:50:00 +0800 Subject: [PATCH] Refactor RestApiTest and TestCase ### What changes are proposed in this pull request? I refactor `RestApiTest` and `TestCase`. I wrap the method to create `TestCase` instance. ### Why are the changes needed? In the former, every unit test creates a `TestCase` instance like this. ``` HttpURLConnection connection = new TestCase(mHostname, mPort, mBaseUri, bucketUri, NO_PARAMS, HttpMethod.PUT, options).execute(); Assert.assertEquals(Response.Status.CONFLICT.getStatusCode(), connection.getResponseCode()); S3Error response = new XmlMapper().readerFor(S3Error.class).readValue(connection.getErrorStream()); Assert.assertEquals(S3ErrorCode.Name.BUCKET_ALREADY_EXISTS, response.getCode()); ``` It's too ugly and unreadable. now you can use the following instead of above. ``` createBucketTestCase(bucketName).checkResponseCode(Status.CONFLICT.getStatusCode()) .checkErrorCode(S3ErrorCode.Name.BUCKET_ALREADY_EXISTS); ``` ### Does this PR introduce any user facing changes? none. pr-link: Alluxio/alluxio#17775 change-id: cid-bf3fb6b1675ef6c2d755e04a5ee746e191530885 --- .../alluxio/client/rest/CreateBucketTest.java | 90 ++++---------- .../alluxio/client/rest/ListStatusTest.java | 3 +- .../java/alluxio/client/rest/RestApiTest.java | 49 +++++--- .../client/rest/S3ClientRestApiTest.java | 27 +++- .../java/alluxio/client/rest/TestCase.java | 115 ++++++++++++------ 5 files changed, 154 insertions(+), 130 deletions(-) diff --git a/dora/tests/src/test/java/alluxio/client/rest/CreateBucketTest.java b/dora/tests/src/test/java/alluxio/client/rest/CreateBucketTest.java index 3cc03132c383..19e22a9390ec 100644 --- a/dora/tests/src/test/java/alluxio/client/rest/CreateBucketTest.java +++ b/dora/tests/src/test/java/alluxio/client/rest/CreateBucketTest.java @@ -14,8 +14,6 @@ import alluxio.Constants; import alluxio.client.WriteType; import alluxio.conf.PropertyKey; -import alluxio.master.journal.JournalType; -import alluxio.proxy.s3.S3Error; import alluxio.proxy.s3.S3ErrorCode; import alluxio.testutils.LocalAlluxioClusterResource; @@ -25,25 +23,23 @@ import com.amazonaws.regions.Regions; import com.amazonaws.services.s3.AmazonS3; import com.amazonaws.services.s3.AmazonS3ClientBuilder; -import com.fasterxml.jackson.dataformat.xml.XmlMapper; import org.gaul.s3proxy.junit.S3ProxyRule; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import java.net.HttpURLConnection; -import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; public class CreateBucketTest extends RestApiTest { private static final String TEST_BUCKET = "test-bucket"; + private static final int UFS_PORT = 8002; private AmazonS3 mS3Client = null; @Rule public S3ProxyRule mS3Proxy = S3ProxyRule.builder() .withBlobStoreProvider("transient") - .withPort(8001) + .withPort(UFS_PORT) .withCredentials("_", "_") .build(); @@ -51,19 +47,10 @@ public class CreateBucketTest extends RestApiTest { public LocalAlluxioClusterResource mLocalAlluxioClusterResource = new LocalAlluxioClusterResource.Builder() .setIncludeProxy(true) - .setProperty(PropertyKey.MASTER_PERSISTENCE_CHECKER_INTERVAL_MS, "10ms") - .setProperty(PropertyKey.MASTER_PERSISTENCE_SCHEDULER_INTERVAL_MS, "10ms") - .setProperty(PropertyKey.JOB_MASTER_WORKER_HEARTBEAT_INTERVAL, "200ms") - .setProperty(PropertyKey.USER_BLOCK_SIZE_BYTES_DEFAULT, Constants.MB * 16) - .setProperty(PropertyKey.MASTER_TTL_CHECKER_INTERVAL_MS, Long.MAX_VALUE) .setProperty(PropertyKey.USER_FILE_WRITE_TYPE_DEFAULT, WriteType.CACHE_THROUGH) - .setProperty(PropertyKey.USER_FILE_RESERVED_BYTES, Constants.MB * 16 / 2) - .setProperty(PropertyKey.CONF_DYNAMIC_UPDATE_ENABLED, true) .setProperty(PropertyKey.WORKER_BLOCK_STORE_TYPE, "PAGE") .setProperty(PropertyKey.WORKER_PAGE_STORE_PAGE_SIZE, Constants.KB) - .setProperty(PropertyKey.WORKER_PAGE_STORE_SIZES, "1GB") - .setProperty(PropertyKey.MASTER_JOURNAL_TYPE, JournalType.NOOP) - .setProperty(PropertyKey.UNDERFS_S3_ENDPOINT, "localhost:8001") + .setProperty(PropertyKey.UNDERFS_S3_ENDPOINT, "localhost:" + UFS_PORT) .setProperty(PropertyKey.UNDERFS_S3_ENDPOINT_REGION, "us-west-2") .setProperty(PropertyKey.UNDERFS_S3_DISABLE_DNS_BUCKETS, true) .setProperty(PropertyKey.MASTER_MOUNT_TABLE_ROOT_UFS, "s3://" + TEST_BUCKET) @@ -72,13 +59,10 @@ public class CreateBucketTest extends RestApiTest { .setProperty(PropertyKey.S3A_ACCESS_KEY, mS3Proxy.getAccessKey()) .setProperty(PropertyKey.S3A_SECRET_KEY, mS3Proxy.getSecretKey()) .setNumWorkers(2) - .setStartCluster(false) .build(); @Before public void before() throws Exception { - mLocalAlluxioClusterResource.start(); - mS3Client = AmazonS3ClientBuilder .standard() .withPathStyleAccessEnabled(true) @@ -101,59 +85,35 @@ public void after() { } /** - * Creates a bucket. - */ - @Test - public void createAndHeadBucket() throws Exception { - String bucketName = "bucket"; - Assert.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), - headBucketRestCall(bucketName).getResponseCode()); - Assert.assertEquals(Response.Status.OK.getStatusCode(), - createBucketRestCall(bucketName).getResponseCode()); - Assert.assertEquals(Response.Status.OK.getStatusCode(), - headBucketRestCall(bucketName).getResponseCode()); - } - - /** - * Creates an existent bucket. + * Creates a bucket. Creates an existent bucket. */ @Test - public void createExistentBucket() throws Exception { + public void createBucket() throws Exception { String bucketName = "bucket"; -// Ensures this bucket exists. - if (headBucketRestCall(bucketName).getResponseCode() - == Response.Status.NOT_FOUND.getStatusCode()) { - Assert.assertEquals(Response.Status.OK.getStatusCode(), - createBucketRestCall(bucketName).getResponseCode()); - } - - HttpURLConnection connection = createBucketRestCall(bucketName); - Assert.assertEquals(Response.Status.CONFLICT.getStatusCode(), connection.getResponseCode()); - S3Error response = - new XmlMapper().readerFor(S3Error.class).readValue(connection.getErrorStream()); - Assert.assertEquals(bucketName, response.getResource()); - Assert.assertEquals(S3ErrorCode.Name.BUCKET_ALREADY_EXISTS, response.getCode()); + // Heads a non-existent bucket. + headTestCase(bucketName).checkResponseCode(Status.NOT_FOUND.getStatusCode()); + // Creates a bucket. + createBucketTestCase(bucketName).checkResponseCode(Status.OK.getStatusCode()); + // Heads a bucket. + headTestCase(bucketName).checkResponseCode(Status.OK.getStatusCode()); + // Creates an existent bucket. + createBucketTestCase(bucketName).checkResponseCode(Status.CONFLICT.getStatusCode()) + .checkErrorCode(S3ErrorCode.Name.BUCKET_ALREADY_EXISTS); } /** - * 2 users creates the same bucket. + * Deletes a non-existent bucket. Deletes an existent bucket. */ @Test - public void createSameBucket() throws Exception { + public void deleteBucket() throws Exception { String bucketName = "bucket"; -// Ensures this bucket exists. - if (headBucketRestCall(bucketName, "user0").getResponseCode() - == Response.Status.NOT_FOUND.getStatusCode()) { - Assert.assertEquals(Response.Status.OK.getStatusCode(), - createBucketRestCall(bucketName, "user0").getResponseCode()); - } - - HttpURLConnection connection = createBucketRestCall(bucketName, "user1"); - - Assert.assertEquals(Response.Status.CONFLICT.getStatusCode(), connection.getResponseCode()); - S3Error response = - new XmlMapper().readerFor(S3Error.class).readValue(connection.getErrorStream()); - Assert.assertEquals(bucketName, response.getResource()); - Assert.assertEquals(S3ErrorCode.Name.BUCKET_ALREADY_EXISTS, response.getCode()); + headTestCase(bucketName).checkResponseCode(Status.NOT_FOUND.getStatusCode()); + // Deletes a non-existent bucket. + deleteTestCase(bucketName).checkResponseCode(Status.NOT_FOUND.getStatusCode()) + .checkErrorCode(S3ErrorCode.Name.NO_SUCH_BUCKET); + createBucketTestCase(bucketName).checkResponseCode(Status.OK.getStatusCode()); + // Deletes an existent bucket. + deleteTestCase(bucketName).checkResponseCode(Status.NO_CONTENT.getStatusCode()); + headTestCase(bucketName).checkResponseCode(Status.NOT_FOUND.getStatusCode()); } } diff --git a/dora/tests/src/test/java/alluxio/client/rest/ListStatusTest.java b/dora/tests/src/test/java/alluxio/client/rest/ListStatusTest.java index 1cd55328dbb8..9814386a99d3 100644 --- a/dora/tests/src/test/java/alluxio/client/rest/ListStatusTest.java +++ b/dora/tests/src/test/java/alluxio/client/rest/ListStatusTest.java @@ -923,8 +923,7 @@ public void headAndListNonExistentBucket() throws Exception { // Heads a non-existent bucket. String bucketName = "non_existent_bucket"; // Verifies 404 status will be returned by head non-existent bucket. - Assert.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), - headBucketRestCall(bucketName).getResponseCode()); + headTestCase(bucketName).checkResponseCode(Response.Status.NOT_FOUND.getStatusCode()); // Lists objects in a non-existent bucket. HttpURLConnection connection2 = new TestCase(mHostname, mPort, mBaseUri, diff --git a/dora/tests/src/test/java/alluxio/client/rest/RestApiTest.java b/dora/tests/src/test/java/alluxio/client/rest/RestApiTest.java index 5141a7811423..4c1091a8d73b 100644 --- a/dora/tests/src/test/java/alluxio/client/rest/RestApiTest.java +++ b/dora/tests/src/test/java/alluxio/client/rest/RestApiTest.java @@ -16,8 +16,9 @@ import alluxio.testutils.BaseIntegrationTest; import com.google.common.collect.ImmutableMap; +import com.google.common.io.BaseEncoding; -import java.net.HttpURLConnection; +import java.security.MessageDigest; import java.util.Map; import javax.validation.constraints.NotNull; import javax.ws.rs.HttpMethod; @@ -30,43 +31,55 @@ public abstract class RestApiTest extends BaseIntegrationTest { protected int mPort; protected String mBaseUri = Constants.REST_API_PREFIX; - protected HttpURLConnection createBucketRestCall(String bucket, @NotNull String user) - throws Exception { - return new TestCase(mHostname, mPort, mBaseUri, - bucket, NO_PARAMS, HttpMethod.PUT, - getDefaultOptionsWithAuth(user)).execute(); + protected TestCase newTestCase(String bucket, Map params, + String httpMethod, TestCaseOptions options) throws Exception { + return new TestCase(mHostname, mPort, mBaseUri, bucket, params, httpMethod, + options); } - protected HttpURLConnection createBucketRestCall(String bucket) throws Exception { - return createBucketRestCall(bucket, TEST_USER_NAME); + protected TestCase createBucketTestCase(String bucket) throws Exception { + return newTestCase(bucket, NO_PARAMS, HttpMethod.PUT, getDefaultOptionsWithAuth()); } - protected HttpURLConnection headBucketRestCall(String bucket, @NotNull String user) - throws Exception { - return new TestCase(mHostname, mPort, mBaseUri, - bucket, NO_PARAMS, HttpMethod.HEAD, - getDefaultOptionsWithAuth(user)).execute(); + protected TestCase createObjectTestCase(String bucket, byte[] object) throws Exception { + return newTestCase(bucket, NO_PARAMS, HttpMethod.PUT, getDefaultOptionsWithAuth() + .setBody(object) + .setMD5(computeObjectChecksum(object))); + } + + protected TestCase deleteTestCase(String uri) throws Exception { + return newTestCase(uri, NO_PARAMS, HttpMethod.DELETE, getDefaultOptionsWithAuth()); } - protected HttpURLConnection headBucketRestCall(String bucket) throws Exception { - return headBucketRestCall(bucket, TEST_USER_NAME); + protected TestCase headTestCase(String uri) throws Exception { + return newTestCase(uri, NO_PARAMS, HttpMethod.HEAD, getDefaultOptionsWithAuth()); + } + + protected TestCase listTestCase(String uri, Map params) throws Exception { + return newTestCase(uri, params, HttpMethod.GET, + getDefaultOptionsWithAuth().setContentType(TestCaseOptions.XML_CONTENT_TYPE)); } protected void listStatusRestCall(Map parameters, ListBucketResult expected) throws Exception { new TestCase(mHostname, mPort, mBaseUri, TEST_BUCKET_NAME, parameters, HttpMethod.GET, - getDefaultOptionsWithAuth()) + getDefaultOptionsWithAuth().setContentType(TestCaseOptions.XML_CONTENT_TYPE)) .runAndCheckResult(expected); } protected TestCaseOptions getDefaultOptionsWithAuth(@NotNull String user) { return TestCaseOptions.defaults() - .setAuthorization("AWS4-HMAC-SHA256 Credential=" + user + "/...") - .setContentType(TestCaseOptions.XML_CONTENT_TYPE); + .setAuthorization("AWS4-HMAC-SHA256 Credential=" + user + "/..."); } protected TestCaseOptions getDefaultOptionsWithAuth() { return getDefaultOptionsWithAuth(TEST_USER_NAME); } + + private String computeObjectChecksum(byte[] objectContent) throws Exception { + MessageDigest md5Hash = MessageDigest.getInstance("MD5"); + byte[] md5Digest = md5Hash.digest(objectContent); + return BaseEncoding.base64().encode(md5Digest); + } } diff --git a/dora/tests/src/test/java/alluxio/client/rest/S3ClientRestApiTest.java b/dora/tests/src/test/java/alluxio/client/rest/S3ClientRestApiTest.java index 3b587d45355c..a97192551b9f 100644 --- a/dora/tests/src/test/java/alluxio/client/rest/S3ClientRestApiTest.java +++ b/dora/tests/src/test/java/alluxio/client/rest/S3ClientRestApiTest.java @@ -2205,12 +2205,29 @@ private void testTagging(String resource, ImmutableMap expectedT Assert.assertEquals(0, deletedTags.getTagMap().size()); } + private void createBucketRestCall(String bucketUri) throws Exception { + createBucketRestCall(bucketUri, TEST_USER_NAME); + } + + private void createBucketRestCall(String bucketUri, String user) throws Exception { + TestCaseOptions options = getDefaultOptionsWithAuth(user); + new TestCase(mHostname, mPort, mBaseUri, + bucketUri, NO_PARAMS, HttpMethod.PUT, + options).runAndCheckResult(); + } + private HttpURLConnection deleteBucketRestCall(String bucketUri) throws Exception { return new TestCase(mHostname, mPort, mBaseUri, bucketUri, NO_PARAMS, HttpMethod.DELETE, getDefaultOptionsWithAuth()).executeAndAssertSuccess(); } + private HttpURLConnection headBucketRestCall(String bucketUri) throws Exception { + return new TestCase(mHostname, mPort, mBaseUri, + bucketUri, NO_PARAMS, HttpMethod.HEAD, + getDefaultOptionsWithAuth()).execute(); + } + private String computeObjectChecksum(byte[] objectContent) throws Exception { MessageDigest md5Hash = MessageDigest.getInstance("MD5"); byte[] md5Digest = md5Hash.digest(objectContent); @@ -2236,13 +2253,13 @@ private String initiateMultipartUploadRestCall(String objectUri, String user) th } private TestCase getCompleteMultipartUploadReadCallTestCase( - String objectUri, String uploadId, CompleteMultipartUploadRequest request) { + String objectUri, String uploadId, CompleteMultipartUploadRequest request) throws Exception { Map params = ImmutableMap.of("uploadId", uploadId); return new TestCase(mHostname, mPort, mBaseUri, - objectUri, params, HttpMethod.POST, - getDefaultOptionsWithAuth() - .setBody(request) - .setContentType(TestCaseOptions.XML_CONTENT_TYPE)); + objectUri, params, HttpMethod.POST, + getDefaultOptionsWithAuth() + .setBody(request) + .setContentType(TestCaseOptions.XML_CONTENT_TYPE)); } private String completeMultipartUploadRestCall( diff --git a/dora/tests/src/test/java/alluxio/client/rest/TestCase.java b/dora/tests/src/test/java/alluxio/client/rest/TestCase.java index d0dd5d148df6..eff9769323e3 100644 --- a/dora/tests/src/test/java/alluxio/client/rest/TestCase.java +++ b/dora/tests/src/test/java/alluxio/client/rest/TestCase.java @@ -12,6 +12,7 @@ package alluxio.client.rest; import alluxio.exception.status.InvalidArgumentException; +import alluxio.proxy.s3.S3Error; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; @@ -30,6 +31,7 @@ import java.util.Map; import java.util.stream.Collectors; import javax.annotation.concurrent.NotThreadSafe; +import javax.validation.constraints.NotNull; import javax.ws.rs.core.Response; /** @@ -51,18 +53,22 @@ public final class TestCase { private final String mMethod; private final TestCaseOptions mOptions; + private final HttpURLConnection mConnection; + /** * Creates a new instance of {@link TestCase}. - * @param hostname the hostname to use - * @param port the port to use - * @param baseUri the base URI of the REST server - * @param endpoint the endpoint to use + * + * @param hostname the hostname to use + * @param port the port to use + * @param baseUri the base URI of the REST server + * @param endpoint the endpoint to use * @param parameters the parameters to use - * @param method the method to use - * @param options the test case options to use + * @param method the method to use + * @param options the test case options to use */ public TestCase(String hostname, int port, String baseUri, String endpoint, - Map parameters, String method, TestCaseOptions options) { + Map parameters, String method, TestCaseOptions options) + throws Exception { // TODO(czhu): URL-encode the URI & parameters mHostname = hostname; mPort = port; @@ -71,6 +77,7 @@ public TestCase(String hostname, int port, String baseUri, String endpoint, mParameters = parameters; mMethod = method; mOptions = options; + mConnection = execute(); } /** @@ -107,12 +114,11 @@ public URL createURL() throws Exception { } /** - * @param connection the HttpURLConnection * @return the String from the InputStream of HttpURLConnection */ - public String getResponse(HttpURLConnection connection) throws Exception { + public String getResponse() throws Exception { StringBuilder sb = new StringBuilder(); - BufferedReader br = new BufferedReader(new InputStreamReader(connection.getInputStream())); + BufferedReader br = new BufferedReader(new InputStreamReader(mConnection.getInputStream())); char[] buffer = new char[1024]; int len; @@ -171,7 +177,9 @@ public HttpURLConnection execute() throws Exception { * if response is not successful, it will assert fail. * @return connection * @throws Exception + * @deprecated use checkResponseCode() instead. */ + @Deprecated public HttpURLConnection executeAndAssertSuccess() throws Exception { HttpURLConnection connection = execute(); if (Response.Status.Family.familyOf(connection.getResponseCode()) @@ -187,53 +195,80 @@ public HttpURLConnection executeAndAssertSuccess() throws Exception { /** * Runs the test case and returns the output. + * @deprecated use checkResponse() instead. */ + @Deprecated public String runAndGetResponse() throws Exception { - return getResponse(executeAndAssertSuccess()); + executeAndAssertSuccess(); + return getResponse(); } + /** + * @deprecated use checkResponseCode() instead. + */ + @Deprecated public void runAndCheckResult() throws Exception { runAndCheckResult(null); } /** * Runs the test case. + * @deprecated use checkResponse() instead. */ + @Deprecated public void runAndCheckResult(Object expectedResult) throws Exception { String expected = ""; if (expectedResult != null) { - switch (mOptions.getContentType()) { - case TestCaseOptions.JSON_CONTENT_TYPE: { - ObjectMapper mapper = new ObjectMapper(); - if (mOptions.isPrettyPrint()) { - expected = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(expectedResult); - } else { - expected = mapper.writeValueAsString(expectedResult); - } - break; - } - case TestCaseOptions.XML_CONTENT_TYPE: { - XmlMapper mapper = new XmlMapper(); - if (mOptions.isPrettyPrint()) { - expected = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(expectedResult); - } else { - expected = mapper.writeValueAsString(expectedResult); - } - break; - } - case TestCaseOptions.OCTET_STREAM_CONTENT_TYPE: { - expected = new String((byte[]) expectedResult, mOptions.getCharset()); - break; + expected = convertToString(expectedResult); + } + + String result = runAndGetResponse(); + Assert.assertEquals(mEndpoint, expected, result); + } + + public void checkResponse(@NotNull Object expectedResult) throws Exception { + String expected = convertToString(expectedResult); + Assert.assertEquals(mEndpoint, expected, getResponse()); + } + + public TestCase checkResponseCode(@NotNull int expected) throws Exception { + Assert.assertEquals(mEndpoint, expected, mConnection.getResponseCode()); + return this; + } + + public TestCase checkErrorCode(@NotNull String expectedErrorCode) throws Exception { + InputStream errorStream = mConnection.getErrorStream(); + S3Error response = XML_MAPPER.readerFor(S3Error.class).readValue(errorStream); + Assert.assertEquals(mEndpoint, expectedErrorCode, response.getCode()); + return this; + } + + private String convertToString(@NotNull Object expectedResult) throws Exception { + switch (mOptions.getContentType()) { + case TestCaseOptions.JSON_CONTENT_TYPE: { + ObjectMapper mapper = new ObjectMapper(); + if (mOptions.isPrettyPrint()) { + return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(expectedResult); + } else { + return mapper.writeValueAsString(expectedResult); } - case TestCaseOptions.TEXT_PLAIN_CONTENT_TYPE: { - expected = (String) expectedResult; - break; + } + case TestCaseOptions.XML_CONTENT_TYPE: { + XmlMapper mapper = new XmlMapper(); + if (mOptions.isPrettyPrint()) { + return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(expectedResult); + } else { + return mapper.writeValueAsString(expectedResult); } - default: - throw new InvalidArgumentException("Invalid content type in TestCaseOptions!"); } + case TestCaseOptions.OCTET_STREAM_CONTENT_TYPE: { + return new String((byte[]) expectedResult, mOptions.getCharset()); + } + case TestCaseOptions.TEXT_PLAIN_CONTENT_TYPE: { + return (String) expectedResult; + } + default: + throw new InvalidArgumentException("Invalid content type in TestCaseOptions!"); } - String result = runAndGetResponse(); - Assert.assertEquals(mEndpoint, expected, result); } }