This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 4fd83602c30 HDDS-14209. Reduce parameter count in ObjectEndpoint
(#9532)
4fd83602c30 is described below
commit 4fd83602c3080a8174876bc9c19c356af857fd56
Author: Doroszlai, Attila <[email protected]>
AuthorDate: Wed Jan 7 17:38:45 2026 +0100
HDDS-14209. Reduce parameter count in ObjectEndpoint (#9532)
---
.../hadoop/ozone/s3/endpoint/ObjectEndpoint.java | 91 ++++++++++------------
.../ozone/s3/endpoint/EndpointTestUtils.java | 48 +++++++++---
.../ozone/s3/metrics/TestS3GatewayMetrics.java | 9 +--
3 files changed, 82 insertions(+), 66 deletions(-)
diff --git
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
index 45e20230337..d33b79761f7 100644
---
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
+++
b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
@@ -83,7 +83,6 @@
import javax.annotation.PostConstruct;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
-import javax.ws.rs.DefaultValue;
import javax.ws.rs.GET;
import javax.ws.rs.HEAD;
import javax.ws.rs.HeaderParam;
@@ -92,7 +91,6 @@
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
-import javax.ws.rs.QueryParam;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
@@ -222,23 +220,23 @@ public void init() {
* See: https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectPUT.html
for
* more details.
*/
- @SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
+ @SuppressWarnings("checkstyle:MethodLength")
@PUT
public Response put(
@PathParam(BUCKET) String bucketName,
@PathParam(PATH) String keyPath,
@HeaderParam(HttpHeaders.CONTENT_LENGTH) long length,
- @QueryParam(QueryParams.PART_NUMBER) int partNumber,
- @QueryParam(QueryParams.UPLOAD_ID) @DefaultValue("") String uploadID,
- @QueryParam(QueryParams.TAGGING) String taggingMarker,
- @QueryParam(QueryParams.ACL) String aclMarker,
- final InputStream body) throws IOException, OS3Exception {
+ final InputStream body
+ ) throws IOException, OS3Exception {
+ final String aclMarker = queryParams().get(QueryParams.ACL);
+ final String taggingMarker = queryParams().get(QueryParams.TAGGING);
+ final String uploadID = queryParams().get(QueryParams.UPLOAD_ID);
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.CREATE_KEY;
boolean auditSuccess = true;
PerformanceStringBuilder perf = new PerformanceStringBuilder();
- String copyHeader = null, storageType = null, storageConfig = null;
+ String copyHeader = null;
MultiDigestInputStream multiDigestInputStream = null;
try {
if (aclMarker != null) {
@@ -261,17 +259,13 @@ public Response put(
}
// If uploadID is specified, it is a request for upload part
return createMultipartKey(volume, bucket, keyPath, length,
- partNumber, uploadID, body, perf);
+ body, perf);
}
copyHeader = getHeaders().getHeaderString(COPY_SOURCE_HEADER);
- storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
- storageConfig =
getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX +
STORAGE_CONFIG_HEADER);
- boolean storageTypeDefault = StringUtils.isEmpty(storageType);
// Normal put object
- ReplicationConfig replicationConfig =
- getReplicationConfig(bucket, storageType, storageConfig);
+ ReplicationConfig replicationConfig = getReplicationConfig(bucket);
boolean enableEC = false;
if ((replicationConfig != null &&
@@ -284,8 +278,7 @@ public Response put(
//Copy object, as copy source available.
s3GAction = S3GAction.COPY_OBJECT;
CopyObjectResponse copyObjectResponse = copyObject(volume,
- copyHeader, bucketName, keyPath, replicationConfig,
- storageTypeDefault, perf);
+ bucketName, keyPath, replicationConfig, perf);
return Response.status(Status.OK).entity(copyObjectResponse).header(
"Connection", "close").build();
}
@@ -431,17 +424,18 @@ public Response put(
* https://docs.aws.amazon.com/AmazonS3/latest/API/mpUploadListParts.html
* for more details.
*/
- @SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
+ @SuppressWarnings("checkstyle:MethodLength")
@GET
public Response get(
@PathParam(BUCKET) String bucketName,
- @PathParam(PATH) String keyPath,
- @QueryParam(QueryParams.PART_NUMBER) int partNumber,
- @QueryParam(QueryParams.UPLOAD_ID) String uploadId,
- @QueryParam(QueryParams.MAX_PARTS) @DefaultValue("1000") int maxParts,
- @QueryParam(QueryParams.PART_NUMBER_MARKER) String partNumberMarker,
- @QueryParam(QueryParams.TAGGING) String taggingMarker)
- throws IOException, OS3Exception {
+ @PathParam(PATH) String keyPath
+ ) throws IOException, OS3Exception {
+ final int maxParts = queryParams().getInt(QueryParams.MAX_PARTS, 1000);
+ final int partNumber = queryParams().getInt(QueryParams.PART_NUMBER, 0);
+ final String partNumberMarker =
queryParams().get(QueryParams.PART_NUMBER_MARKER);
+ final String taggingMarker = queryParams().get(QueryParams.TAGGING);
+ final String uploadId = queryParams().get(QueryParams.UPLOAD_ID);
+
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.GET_KEY;
PerformanceStringBuilder perf = new PerformanceStringBuilder();
@@ -748,10 +742,11 @@ private Response abortMultipartUpload(OzoneVolume volume,
String bucket,
@SuppressWarnings("emptyblock")
public Response delete(
@PathParam(BUCKET) String bucketName,
- @PathParam(PATH) String keyPath,
- @QueryParam(QueryParams.UPLOAD_ID) @DefaultValue("") String uploadId,
- @QueryParam(QueryParams.TAGGING) String taggingMarker) throws
- IOException, OS3Exception {
+ @PathParam(PATH) String keyPath
+ ) throws IOException, OS3Exception {
+ final String taggingMarker = queryParams().get(QueryParams.TAGGING);
+ final String uploadId = queryParams().get(QueryParams.UPLOAD_ID);
+
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.DELETE_KEY;
@@ -826,24 +821,20 @@ public Response delete(
public Response initializeMultipartUpload(
@PathParam(BUCKET) String bucket,
@PathParam(PATH) String key
- )
- throws IOException, OS3Exception {
+ ) throws IOException, OS3Exception {
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.INIT_MULTIPART_UPLOAD;
try {
OzoneBucket ozoneBucket = getBucket(bucket);
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucket,
ozoneBucket.getOwner());
- String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
- String storageConfig =
getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX +
STORAGE_CONFIG_HEADER);
Map<String, String> customMetadata =
getCustomMetadataFromHeaders(getHeaders().getRequestHeaders());
Map<String, String> tags = getTaggingFromHeaders(getHeaders());
- ReplicationConfig replicationConfig =
- getReplicationConfig(ozoneBucket, storageType, storageConfig);
+ ReplicationConfig replicationConfig = getReplicationConfig(ozoneBucket);
OmMultipartInfo multipartInfo =
ozoneBucket.initiateMultipartUpload(key, replicationConfig,
customMetadata, tags);
@@ -873,8 +864,9 @@ public Response initializeMultipartUpload(
}
}
- private ReplicationConfig getReplicationConfig(OzoneBucket ozoneBucket,
- String storageType, String storageConfig) throws OS3Exception {
+ private ReplicationConfig getReplicationConfig(OzoneBucket ozoneBucket)
throws OS3Exception {
+ String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
+ String storageConfig =
getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX +
STORAGE_CONFIG_HEADER);
ReplicationConfig clientConfiguredReplicationConfig =
OzoneClientUtils.getClientConfiguredReplicationConfig(getOzoneConfiguration());
@@ -891,9 +883,9 @@ private ReplicationConfig getReplicationConfig(OzoneBucket
ozoneBucket,
public Response completeMultipartUpload(
@PathParam(BUCKET) String bucket,
@PathParam(PATH) String key,
- @QueryParam(QueryParams.UPLOAD_ID) @DefaultValue("") String uploadID,
- CompleteMultipartUploadRequest multipartUploadRequest)
- throws IOException, OS3Exception {
+ CompleteMultipartUploadRequest multipartUploadRequest
+ ) throws IOException, OS3Exception {
+ final String uploadID = queryParams().get(QueryParams.UPLOAD_ID, "");
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.COMPLETE_MULTIPART_UPLOAD;
OzoneVolume volume = getVolume();
@@ -962,12 +954,14 @@ public Response completeMultipartUpload(
}
}
- @SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
+ @SuppressWarnings("checkstyle:MethodLength")
private Response createMultipartKey(OzoneVolume volume, OzoneBucket
ozoneBucket,
- String key, long length, int partNumber, String uploadID,
+ String key, long length,
final InputStream body, PerformanceStringBuilder perf)
throws IOException, OS3Exception {
long startNanos = Time.monotonicNowNanos();
+ final String uploadID = queryParams().get(QueryParams.UPLOAD_ID);
+ final int partNumber = queryParams().getInt(QueryParams.PART_NUMBER, 0);
String copyHeader = null;
MultiDigestInputStream multiDigestInputStream = null;
final String bucketName = ozoneBucket.getName();
@@ -979,10 +973,7 @@ private Response createMultipartKey(OzoneVolume volume,
OzoneBucket ozoneBucket,
length = chunkInputStreamInfo.getEffectiveLength();
copyHeader = getHeaders().getHeaderString(COPY_SOURCE_HEADER);
- String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
- String storageConfig =
getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX +
STORAGE_CONFIG_HEADER);
- ReplicationConfig replicationConfig =
- getReplicationConfig(ozoneBucket, storageType, storageConfig);
+ ReplicationConfig replicationConfig = getReplicationConfig(ozoneBucket);
boolean enableEC = false;
if ((replicationConfig != null &&
@@ -1227,12 +1218,14 @@ void copy(OzoneVolume volume, DigestInputStream src,
long srcKeyLen,
perf.appendSizeBytes(copyLength);
}
- @SuppressWarnings("checkstyle:ParameterNumber")
private CopyObjectResponse copyObject(OzoneVolume volume,
- String copyHeader, String destBucket, String destkey,
- ReplicationConfig replicationConfig, boolean storageTypeDefault,
+ String destBucket, String destkey, ReplicationConfig replicationConfig,
PerformanceStringBuilder perf)
throws OS3Exception, IOException {
+ String copyHeader = getHeaders().getHeaderString(COPY_SOURCE_HEADER);
+ String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
+ boolean storageTypeDefault = StringUtils.isEmpty(storageType);
+
long startNanos = Time.monotonicNowNanos();
Pair<String, String> result = parseSourceHeader(copyHeader);
diff --git
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/EndpointTestUtils.java
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/EndpointTestUtils.java
index ae776993555..c82a0772c93 100644
---
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/EndpointTestUtils.java
+++
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/EndpointTestUtils.java
@@ -28,7 +28,9 @@
import javax.ws.rs.core.Response;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.util.S3Consts;
import org.apache.http.HttpStatus;
+import org.apache.ratis.util.function.CheckedRunnable;
import org.apache.ratis.util.function.CheckedSupplier;
/** Utilities for unit-testing S3 endpoints. */
@@ -40,7 +42,7 @@ public static Response get(
String bucket,
String key
) throws IOException, OS3Exception {
- return subject.get(bucket, key, 0, null, 0, null, null);
+ return subject.get(bucket, key);
}
/** Get key tags. */
@@ -49,7 +51,8 @@ public static Response getTagging(
String bucket,
String key
) throws IOException, OS3Exception {
- return subject.get(bucket, key, 0, null, 0, null, "");
+ subject.queryParamsForTest().set(S3Consts.QueryParams.TAGGING, "");
+ return subject.get(bucket, key);
}
/** List parts of MPU. */
@@ -61,7 +64,10 @@ public static Response listParts(
int maxParts,
int nextPart
) throws IOException, OS3Exception {
- return subject.get(bucket, key, 0, uploadID, maxParts,
String.valueOf(nextPart), null);
+ subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID, uploadID);
+ subject.queryParamsForTest().setInt(S3Consts.QueryParams.MAX_PARTS,
maxParts);
+
subject.queryParamsForTest().setInt(S3Consts.QueryParams.PART_NUMBER_MARKER,
nextPart);
+ return subject.get(bucket, key);
}
/** Put without content. */
@@ -90,12 +96,13 @@ public static Response putTagging(
String key,
String content
) throws IOException, OS3Exception {
+ subject.queryParamsForTest().set(S3Consts.QueryParams.TAGGING, "");
if (content == null) {
- return subject.put(bucket, key, 0, 0, null, "", null, null);
+ return subject.put(bucket, key, 0, null);
} else {
final long length = content.length();
try (ByteArrayInputStream body = new
ByteArrayInputStream(content.getBytes(UTF_8))) {
- return subject.put(bucket, key, length, 0, null, "", null, body);
+ return subject.put(bucket, key, length, body);
}
}
}
@@ -109,12 +116,17 @@ public static Response put(
String uploadID,
String content
) throws IOException, OS3Exception {
+ if (uploadID != null) {
+ subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID,
uploadID);
+ }
+ subject.queryParamsForTest().setInt(S3Consts.QueryParams.PART_NUMBER,
partNumber);
+
if (content == null) {
- return subject.put(bucket, key, 0, partNumber, uploadID, null, null,
null);
+ return subject.put(bucket, key, 0, null);
} else {
final long length = content.length();
try (ByteArrayInputStream body = new
ByteArrayInputStream(content.getBytes(UTF_8))) {
- return subject.put(bucket, key, length, partNumber, uploadID, null,
null, body);
+ return subject.put(bucket, key, length, body);
}
}
}
@@ -125,7 +137,7 @@ public static Response delete(
String bucket,
String key
) throws IOException, OS3Exception {
- return subject.delete(bucket, key, null, null);
+ return subject.delete(bucket, key);
}
/** Delete key tags. */
@@ -134,7 +146,8 @@ public static Response deleteTagging(
String bucket,
String key
) throws IOException, OS3Exception {
- return subject.delete(bucket, key, null, "");
+ subject.queryParamsForTest().set(S3Consts.QueryParams.TAGGING, "");
+ return subject.delete(bucket, key);
}
/** Initiate multipart upload.
@@ -185,7 +198,9 @@ public static void completeMultipartUpload(
CompleteMultipartUploadRequest completeMultipartUploadRequest = new
CompleteMultipartUploadRequest();
completeMultipartUploadRequest.setPartList(parts);
- try (Response response = subject.completeMultipartUpload(bucket, key,
uploadID, completeMultipartUploadRequest)) {
+ subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID, uploadID);
+
+ try (Response response = subject.completeMultipartUpload(bucket, key,
completeMultipartUploadRequest)) {
assertEquals(HttpStatus.SC_OK, response.getStatus());
CompleteMultipartUploadResponse completeMultipartUploadResponse =
@@ -205,7 +220,8 @@ public static Response abortMultipartUpload(
String key,
String uploadID
) throws IOException, OS3Exception {
- return subject.delete(bucket, key, uploadID, null);
+ subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID, uploadID);
+ return subject.delete(bucket, key);
}
/** Verify response is success for {@code request}. */
@@ -220,7 +236,15 @@ public static <E extends Exception> void assertStatus(int
status, CheckedSupplie
}
}
- /** Verify error response for {@code request} matching {@code expected}
{@link OS3Exception}. */
+ /** Verify error response for {@code request} matches {@code expected}
{@link OS3Exception}. */
+ public static OS3Exception assertErrorResponse(OS3Exception expected,
CheckedRunnable<?> request) {
+ OS3Exception actual = assertThrows(OS3Exception.class, request::run);
+ assertEquals(expected.getCode(), actual.getCode());
+ assertEquals(expected.getHttpCode(), actual.getHttpCode());
+ return actual;
+ }
+
+ /** Verify error response for {@code request} matches {@code expected}
{@link OS3Exception}. */
public static OS3Exception assertErrorResponse(OS3Exception expected,
CheckedSupplier<Response, ?> request) {
OS3Exception actual = assertThrows(OS3Exception.class, () ->
request.get().close());
assertEquals(expected.getCode(), actual.getCode());
diff --git
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
index 8df792be49b..ccc8521a44f 100644
---
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
+++
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
@@ -19,10 +19,12 @@
import static java.net.HttpURLConnection.HTTP_CONFLICT;
import static java.net.HttpURLConnection.HTTP_OK;
+import static java.util.Collections.emptyList;
import static
org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.abortMultipartUpload;
import static
org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertErrorResponse;
import static
org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertStatus;
import static
org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertSucceeds;
+import static
org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.completeMultipartUpload;
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.delete;
import static
org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.deleteTagging;
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.get;
@@ -54,7 +56,6 @@
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneClientStub;
import org.apache.hadoop.ozone.s3.endpoint.BucketEndpoint;
-import org.apache.hadoop.ozone.s3.endpoint.CompleteMultipartUploadRequest;
import org.apache.hadoop.ozone.s3.endpoint.EndpointBuilder;
import org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils;
import org.apache.hadoop.ozone.s3.endpoint.ObjectEndpoint;
@@ -406,9 +407,8 @@ public void testAbortMultiPartUploadFailure() {
public void testCompleteMultiPartUploadSuccess() throws Exception {
long oriMetric = metrics.getCompleteMultiPartUploadSuccess();
String uploadID = initiateMultipartUpload(bucketName, keyName);
- CompleteMultipartUploadRequest request = new
CompleteMultipartUploadRequest();
- assertSucceeds(() -> keyEndpoint.completeMultipartUpload(bucketName,
keyName, uploadID, request));
+ completeMultipartUpload(keyEndpoint, bucketName, keyName, uploadID,
emptyList());
long curMetric = metrics.getCompleteMultiPartUploadSuccess();
assertEquals(1L, curMetric - oriMetric);
@@ -417,10 +417,9 @@ public void testCompleteMultiPartUploadSuccess() throws
Exception {
@Test
public void testCompleteMultiPartUploadFailure() {
long oriMetric = metrics.getCompleteMultiPartUploadFailure();
- CompleteMultipartUploadRequest request = new
CompleteMultipartUploadRequest();
assertErrorResponse(S3ErrorTable.NO_SUCH_UPLOAD,
- () -> keyEndpoint.completeMultipartUpload(bucketName, "key2",
"random", request));
+ () -> completeMultipartUpload(keyEndpoint, bucketName, "key2",
"random", emptyList()));
long curMetric = metrics.getCompleteMultiPartUploadFailure();
assertEquals(1L, curMetric - oriMetric);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
