Skip to content

Commit 4fd8360

Browse files
adoroszlaiadoroszlai
authored
HDDS-14209. Reduce parameter count in ObjectEndpoint (#9532)
1 parent fd181c6 commit 4fd8360

File tree

3 files changed

+82
-66
lines changed

3 files changed

+82
-66
lines changed

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

Lines changed: 42 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@
8383
import javax.annotation.PostConstruct;
8484
import javax.ws.rs.Consumes;
8585
import javax.ws.rs.DELETE;
86-
import javax.ws.rs.DefaultValue;
8786
import javax.ws.rs.GET;
8887
import javax.ws.rs.HEAD;
8988
import javax.ws.rs.HeaderParam;
@@ -92,7 +91,6 @@
9291
import javax.ws.rs.Path;
9392
import javax.ws.rs.PathParam;
9493
import javax.ws.rs.Produces;
95-
import javax.ws.rs.QueryParam;
9694
import javax.ws.rs.core.HttpHeaders;
9795
import javax.ws.rs.core.MediaType;
9896
import javax.ws.rs.core.MultivaluedMap;
@@ -222,23 +220,23 @@ public void init() {
222220
* See: https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectPUT.html for
223221
* more details.
224222
*/
225-
@SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
223+
@SuppressWarnings("checkstyle:MethodLength")
226224
@PUT
227225
public Response put(
228226
@PathParam(BUCKET) String bucketName,
229227
@PathParam(PATH) String keyPath,
230228
@HeaderParam(HttpHeaders.CONTENT_LENGTH) long length,
231-
@QueryParam(QueryParams.PART_NUMBER) int partNumber,
232-
@QueryParam(QueryParams.UPLOAD_ID) @DefaultValue("") String uploadID,
233-
@QueryParam(QueryParams.TAGGING) String taggingMarker,
234-
@QueryParam(QueryParams.ACL) String aclMarker,
235-
final InputStream body) throws IOException, OS3Exception {
229+
final InputStream body
230+
) throws IOException, OS3Exception {
231+
final String aclMarker = queryParams().get(QueryParams.ACL);
232+
final String taggingMarker = queryParams().get(QueryParams.TAGGING);
233+
final String uploadID = queryParams().get(QueryParams.UPLOAD_ID);
236234
long startNanos = Time.monotonicNowNanos();
237235
S3GAction s3GAction = S3GAction.CREATE_KEY;
238236
boolean auditSuccess = true;
239237
PerformanceStringBuilder perf = new PerformanceStringBuilder();
240238

241-
String copyHeader = null, storageType = null, storageConfig = null;
239+
String copyHeader = null;
242240
MultiDigestInputStream multiDigestInputStream = null;
243241
try {
244242
if (aclMarker != null) {
@@ -261,17 +259,13 @@ public Response put(
261259
}
262260
// If uploadID is specified, it is a request for upload part
263261
return createMultipartKey(volume, bucket, keyPath, length,
264-
partNumber, uploadID, body, perf);
262+
body, perf);
265263
}
266264

267265
copyHeader = getHeaders().getHeaderString(COPY_SOURCE_HEADER);
268-
storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
269-
storageConfig = getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX + STORAGE_CONFIG_HEADER);
270-
boolean storageTypeDefault = StringUtils.isEmpty(storageType);
271266

272267
// Normal put object
273-
ReplicationConfig replicationConfig =
274-
getReplicationConfig(bucket, storageType, storageConfig);
268+
ReplicationConfig replicationConfig = getReplicationConfig(bucket);
275269

276270
boolean enableEC = false;
277271
if ((replicationConfig != null &&
@@ -284,8 +278,7 @@ public Response put(
284278
//Copy object, as copy source available.
285279
s3GAction = S3GAction.COPY_OBJECT;
286280
CopyObjectResponse copyObjectResponse = copyObject(volume,
287-
copyHeader, bucketName, keyPath, replicationConfig,
288-
storageTypeDefault, perf);
281+
bucketName, keyPath, replicationConfig, perf);
289282
return Response.status(Status.OK).entity(copyObjectResponse).header(
290283
"Connection", "close").build();
291284
}
@@ -431,17 +424,18 @@ public Response put(
431424
* https://docs.aws.amazon.com/AmazonS3/latest/API/mpUploadListParts.html
432425
* for more details.
433426
*/
434-
@SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
427+
@SuppressWarnings("checkstyle:MethodLength")
435428
@GET
436429
public Response get(
437430
@PathParam(BUCKET) String bucketName,
438-
@PathParam(PATH) String keyPath,
439-
@QueryParam(QueryParams.PART_NUMBER) int partNumber,
440-
@QueryParam(QueryParams.UPLOAD_ID) String uploadId,
441-
@QueryParam(QueryParams.MAX_PARTS) @DefaultValue("1000") int maxParts,
442-
@QueryParam(QueryParams.PART_NUMBER_MARKER) String partNumberMarker,
443-
@QueryParam(QueryParams.TAGGING) String taggingMarker)
444-
throws IOException, OS3Exception {
431+
@PathParam(PATH) String keyPath
432+
) throws IOException, OS3Exception {
433+
final int maxParts = queryParams().getInt(QueryParams.MAX_PARTS, 1000);
434+
final int partNumber = queryParams().getInt(QueryParams.PART_NUMBER, 0);
435+
final String partNumberMarker = queryParams().get(QueryParams.PART_NUMBER_MARKER);
436+
final String taggingMarker = queryParams().get(QueryParams.TAGGING);
437+
final String uploadId = queryParams().get(QueryParams.UPLOAD_ID);
438+
445439
long startNanos = Time.monotonicNowNanos();
446440
S3GAction s3GAction = S3GAction.GET_KEY;
447441
PerformanceStringBuilder perf = new PerformanceStringBuilder();
@@ -748,10 +742,11 @@ private Response abortMultipartUpload(OzoneVolume volume, String bucket,
748742
@SuppressWarnings("emptyblock")
749743
public Response delete(
750744
@PathParam(BUCKET) String bucketName,
751-
@PathParam(PATH) String keyPath,
752-
@QueryParam(QueryParams.UPLOAD_ID) @DefaultValue("") String uploadId,
753-
@QueryParam(QueryParams.TAGGING) String taggingMarker) throws
754-
IOException, OS3Exception {
745+
@PathParam(PATH) String keyPath
746+
) throws IOException, OS3Exception {
747+
final String taggingMarker = queryParams().get(QueryParams.TAGGING);
748+
final String uploadId = queryParams().get(QueryParams.UPLOAD_ID);
749+
755750
long startNanos = Time.monotonicNowNanos();
756751
S3GAction s3GAction = S3GAction.DELETE_KEY;
757752

@@ -826,24 +821,20 @@ public Response delete(
826821
public Response initializeMultipartUpload(
827822
@PathParam(BUCKET) String bucket,
828823
@PathParam(PATH) String key
829-
)
830-
throws IOException, OS3Exception {
824+
) throws IOException, OS3Exception {
831825
long startNanos = Time.monotonicNowNanos();
832826
S3GAction s3GAction = S3GAction.INIT_MULTIPART_UPLOAD;
833827

834828
try {
835829
OzoneBucket ozoneBucket = getBucket(bucket);
836830
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucket, ozoneBucket.getOwner());
837-
String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
838-
String storageConfig = getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX + STORAGE_CONFIG_HEADER);
839831

840832
Map<String, String> customMetadata =
841833
getCustomMetadataFromHeaders(getHeaders().getRequestHeaders());
842834

843835
Map<String, String> tags = getTaggingFromHeaders(getHeaders());
844836

845-
ReplicationConfig replicationConfig =
846-
getReplicationConfig(ozoneBucket, storageType, storageConfig);
837+
ReplicationConfig replicationConfig = getReplicationConfig(ozoneBucket);
847838

848839
OmMultipartInfo multipartInfo =
849840
ozoneBucket.initiateMultipartUpload(key, replicationConfig, customMetadata, tags);
@@ -873,8 +864,9 @@ public Response initializeMultipartUpload(
873864
}
874865
}
875866

876-
private ReplicationConfig getReplicationConfig(OzoneBucket ozoneBucket,
877-
String storageType, String storageConfig) throws OS3Exception {
867+
private ReplicationConfig getReplicationConfig(OzoneBucket ozoneBucket) throws OS3Exception {
868+
String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
869+
String storageConfig = getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX + STORAGE_CONFIG_HEADER);
878870

879871
ReplicationConfig clientConfiguredReplicationConfig =
880872
OzoneClientUtils.getClientConfiguredReplicationConfig(getOzoneConfiguration());
@@ -891,9 +883,9 @@ private ReplicationConfig getReplicationConfig(OzoneBucket ozoneBucket,
891883
public Response completeMultipartUpload(
892884
@PathParam(BUCKET) String bucket,
893885
@PathParam(PATH) String key,
894-
@QueryParam(QueryParams.UPLOAD_ID) @DefaultValue("") String uploadID,
895-
CompleteMultipartUploadRequest multipartUploadRequest)
896-
throws IOException, OS3Exception {
886+
CompleteMultipartUploadRequest multipartUploadRequest
887+
) throws IOException, OS3Exception {
888+
final String uploadID = queryParams().get(QueryParams.UPLOAD_ID, "");
897889
long startNanos = Time.monotonicNowNanos();
898890
S3GAction s3GAction = S3GAction.COMPLETE_MULTIPART_UPLOAD;
899891
OzoneVolume volume = getVolume();
@@ -962,12 +954,14 @@ public Response completeMultipartUpload(
962954
}
963955
}
964956

965-
@SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
957+
@SuppressWarnings("checkstyle:MethodLength")
966958
private Response createMultipartKey(OzoneVolume volume, OzoneBucket ozoneBucket,
967-
String key, long length, int partNumber, String uploadID,
959+
String key, long length,
968960
final InputStream body, PerformanceStringBuilder perf)
969961
throws IOException, OS3Exception {
970962
long startNanos = Time.monotonicNowNanos();
963+
final String uploadID = queryParams().get(QueryParams.UPLOAD_ID);
964+
final int partNumber = queryParams().getInt(QueryParams.PART_NUMBER, 0);
971965
String copyHeader = null;
972966
MultiDigestInputStream multiDigestInputStream = null;
973967
final String bucketName = ozoneBucket.getName();
@@ -979,10 +973,7 @@ private Response createMultipartKey(OzoneVolume volume, OzoneBucket ozoneBucket,
979973
length = chunkInputStreamInfo.getEffectiveLength();
980974

981975
copyHeader = getHeaders().getHeaderString(COPY_SOURCE_HEADER);
982-
String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
983-
String storageConfig = getHeaders().getHeaderString(CUSTOM_METADATA_HEADER_PREFIX + STORAGE_CONFIG_HEADER);
984-
ReplicationConfig replicationConfig =
985-
getReplicationConfig(ozoneBucket, storageType, storageConfig);
976+
ReplicationConfig replicationConfig = getReplicationConfig(ozoneBucket);
986977

987978
boolean enableEC = false;
988979
if ((replicationConfig != null &&
@@ -1227,12 +1218,14 @@ void copy(OzoneVolume volume, DigestInputStream src, long srcKeyLen,
12271218
perf.appendSizeBytes(copyLength);
12281219
}
12291220

1230-
@SuppressWarnings("checkstyle:ParameterNumber")
12311221
private CopyObjectResponse copyObject(OzoneVolume volume,
1232-
String copyHeader, String destBucket, String destkey,
1233-
ReplicationConfig replicationConfig, boolean storageTypeDefault,
1222+
String destBucket, String destkey, ReplicationConfig replicationConfig,
12341223
PerformanceStringBuilder perf)
12351224
throws OS3Exception, IOException {
1225+
String copyHeader = getHeaders().getHeaderString(COPY_SOURCE_HEADER);
1226+
String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
1227+
boolean storageTypeDefault = StringUtils.isEmpty(storageType);
1228+
12361229
long startNanos = Time.monotonicNowNanos();
12371230
Pair<String, String> result = parseSourceHeader(copyHeader);
12381231

‎hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/EndpointTestUtils.java‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
import javax.ws.rs.core.Response;
2929
import org.apache.hadoop.ozone.OzoneConsts;
3030
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
31+
import org.apache.hadoop.ozone.s3.util.S3Consts;
3132
import org.apache.http.HttpStatus;
33+
import org.apache.ratis.util.function.CheckedRunnable;
3234
import org.apache.ratis.util.function.CheckedSupplier;
3335

3436
/** Utilities for unit-testing S3 endpoints. */
@@ -40,7 +42,7 @@ public static Response get(
4042
String bucket,
4143
String key
4244
) throws IOException, OS3Exception {
43-
return subject.get(bucket, key, 0, null, 0, null, null);
45+
return subject.get(bucket, key);
4446
}
4547

4648
/** Get key tags. */
@@ -49,7 +51,8 @@ public static Response getTagging(
4951
String bucket,
5052
String key
5153
) throws IOException, OS3Exception {
52-
return subject.get(bucket, key, 0, null, 0, null, "");
54+
subject.queryParamsForTest().set(S3Consts.QueryParams.TAGGING, "");
55+
return subject.get(bucket, key);
5356
}
5457

5558
/** List parts of MPU. */
@@ -61,7 +64,10 @@ public static Response listParts(
6164
int maxParts,
6265
int nextPart
6366
) throws IOException, OS3Exception {
64-
return subject.get(bucket, key, 0, uploadID, maxParts, String.valueOf(nextPart), null);
67+
subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID, uploadID);
68+
subject.queryParamsForTest().setInt(S3Consts.QueryParams.MAX_PARTS, maxParts);
69+
subject.queryParamsForTest().setInt(S3Consts.QueryParams.PART_NUMBER_MARKER, nextPart);
70+
return subject.get(bucket, key);
6571
}
6672

6773
/** Put without content. */
@@ -90,12 +96,13 @@ public static Response putTagging(
9096
String key,
9197
String content
9298
) throws IOException, OS3Exception {
99+
subject.queryParamsForTest().set(S3Consts.QueryParams.TAGGING, "");
93100
if (content == null) {
94-
return subject.put(bucket, key, 0, 0, null, "", null, null);
101+
return subject.put(bucket, key, 0, null);
95102
} else {
96103
final long length = content.length();
97104
try (ByteArrayInputStream body = new ByteArrayInputStream(content.getBytes(UTF_8))) {
98-
return subject.put(bucket, key, length, 0, null, "", null, body);
105+
return subject.put(bucket, key, length, body);
99106
}
100107
}
101108
}
@@ -109,12 +116,17 @@ public static Response put(
109116
String uploadID,
110117
String content
111118
) throws IOException, OS3Exception {
119+
if (uploadID != null) {
120+
subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID, uploadID);
121+
}
122+
subject.queryParamsForTest().setInt(S3Consts.QueryParams.PART_NUMBER, partNumber);
123+
112124
if (content == null) {
113-
return subject.put(bucket, key, 0, partNumber, uploadID, null, null, null);
125+
return subject.put(bucket, key, 0, null);
114126
} else {
115127
final long length = content.length();
116128
try (ByteArrayInputStream body = new ByteArrayInputStream(content.getBytes(UTF_8))) {
117-
return subject.put(bucket, key, length, partNumber, uploadID, null, null, body);
129+
return subject.put(bucket, key, length, body);
118130
}
119131
}
120132
}
@@ -125,7 +137,7 @@ public static Response delete(
125137
String bucket,
126138
String key
127139
) throws IOException, OS3Exception {
128-
return subject.delete(bucket, key, null, null);
140+
return subject.delete(bucket, key);
129141
}
130142

131143
/** Delete key tags. */
@@ -134,7 +146,8 @@ public static Response deleteTagging(
134146
String bucket,
135147
String key
136148
) throws IOException, OS3Exception {
137-
return subject.delete(bucket, key, null, "");
149+
subject.queryParamsForTest().set(S3Consts.QueryParams.TAGGING, "");
150+
return subject.delete(bucket, key);
138151
}
139152

140153
/** Initiate multipart upload.
@@ -185,7 +198,9 @@ public static void completeMultipartUpload(
185198
CompleteMultipartUploadRequest completeMultipartUploadRequest = new CompleteMultipartUploadRequest();
186199
completeMultipartUploadRequest.setPartList(parts);
187200

188-
try (Response response = subject.completeMultipartUpload(bucket, key, uploadID, completeMultipartUploadRequest)) {
201+
subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID, uploadID);
202+
203+
try (Response response = subject.completeMultipartUpload(bucket, key, completeMultipartUploadRequest)) {
189204
assertEquals(HttpStatus.SC_OK, response.getStatus());
190205

191206
CompleteMultipartUploadResponse completeMultipartUploadResponse =
@@ -205,7 +220,8 @@ public static Response abortMultipartUpload(
205220
String key,
206221
String uploadID
207222
) throws IOException, OS3Exception {
208-
return subject.delete(bucket, key, uploadID, null);
223+
subject.queryParamsForTest().set(S3Consts.QueryParams.UPLOAD_ID, uploadID);
224+
return subject.delete(bucket, key);
209225
}
210226

211227
/** Verify response is success for {@code request}. */
@@ -220,7 +236,15 @@ public static <E extends Exception> void assertStatus(int status, CheckedSupplie
220236
}
221237
}
222238

223-
/** Verify error response for {@code request} matching {@code expected} {@link OS3Exception}. */
239+
/** Verify error response for {@code request} matches {@code expected} {@link OS3Exception}. */
240+
public static OS3Exception assertErrorResponse(OS3Exception expected, CheckedRunnable<?> request) {
241+
OS3Exception actual = assertThrows(OS3Exception.class, request::run);
242+
assertEquals(expected.getCode(), actual.getCode());
243+
assertEquals(expected.getHttpCode(), actual.getHttpCode());
244+
return actual;
245+
}
246+
247+
/** Verify error response for {@code request} matches {@code expected} {@link OS3Exception}. */
224248
public static OS3Exception assertErrorResponse(OS3Exception expected, CheckedSupplier<Response, ?> request) {
225249
OS3Exception actual = assertThrows(OS3Exception.class, () -> request.get().close());
226250
assertEquals(expected.getCode(), actual.getCode());

‎hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919

2020
import static java.net.HttpURLConnection.HTTP_CONFLICT;
2121
import static java.net.HttpURLConnection.HTTP_OK;
22+
import static java.util.Collections.emptyList;
2223
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.abortMultipartUpload;
2324
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertErrorResponse;
2425
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertStatus;
2526
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertSucceeds;
27+
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.completeMultipartUpload;
2628
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.delete;
2729
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.deleteTagging;
2830
import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.get;
@@ -54,7 +56,6 @@
5456
import org.apache.hadoop.ozone.client.OzoneClient;
5557
import org.apache.hadoop.ozone.client.OzoneClientStub;
5658
import org.apache.hadoop.ozone.s3.endpoint.BucketEndpoint;
57-
import org.apache.hadoop.ozone.s3.endpoint.CompleteMultipartUploadRequest;
5859
import org.apache.hadoop.ozone.s3.endpoint.EndpointBuilder;
5960
import org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils;
6061
import org.apache.hadoop.ozone.s3.endpoint.ObjectEndpoint;
@@ -406,9 +407,8 @@ public void testAbortMultiPartUploadFailure() {
406407
public void testCompleteMultiPartUploadSuccess() throws Exception {
407408
long oriMetric = metrics.getCompleteMultiPartUploadSuccess();
408409
String uploadID = initiateMultipartUpload(bucketName, keyName);
409-
CompleteMultipartUploadRequest request = new CompleteMultipartUploadRequest();
410410

411-
assertSucceeds(() -> keyEndpoint.completeMultipartUpload(bucketName, keyName, uploadID, request));
411+
completeMultipartUpload(keyEndpoint, bucketName, keyName, uploadID, emptyList());
412412

413413
long curMetric = metrics.getCompleteMultiPartUploadSuccess();
414414
assertEquals(1L, curMetric - oriMetric);
@@ -417,10 +417,9 @@ public void testCompleteMultiPartUploadSuccess() throws Exception {
417417
@Test
418418
public void testCompleteMultiPartUploadFailure() {
419419
long oriMetric = metrics.getCompleteMultiPartUploadFailure();
420-
CompleteMultipartUploadRequest request = new CompleteMultipartUploadRequest();
421420

422421
assertErrorResponse(S3ErrorTable.NO_SUCH_UPLOAD,
423-
() -> keyEndpoint.completeMultipartUpload(bucketName, "key2", "random", request));
422+
() -> completeMultipartUpload(keyEndpoint, bucketName, "key2", "random", emptyList()));
424423

425424
long curMetric = metrics.getCompleteMultiPartUploadFailure();
426425
assertEquals(1L, curMetric - oriMetric);

0 commit comments

Comments
 (0)