Skip to content

Commit ef6be85

Browse files
adoroszlaiadoroszlai
committed
HDDS-14209. Reduce parameter count in ObjectEndpoint
1 parent 3bdec91 commit ef6be85

3 files changed

Lines changed: 83 additions & 66 deletions

File tree

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

Lines changed: 43 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,24 @@ 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 int partNumber = queryParams().getInt(QueryParams.PART_NUMBER, 0);
233+
final String taggingMarker = queryParams().get(QueryParams.TAGGING);
234+
final String uploadID = queryParams().get(QueryParams.UPLOAD_ID);
236235
long startNanos = Time.monotonicNowNanos();
237236
S3GAction s3GAction = S3GAction.CREATE_KEY;
238237
boolean auditSuccess = true;
239238
PerformanceStringBuilder perf = new PerformanceStringBuilder();
240239

241-
String copyHeader = null, storageType = null, storageConfig = null;
240+
String copyHeader = null;
242241
MultiDigestInputStream multiDigestInputStream = null;
243242
try {
244243
if (aclMarker != null) {
@@ -261,17 +260,13 @@ public Response put(
261260
}
262261
// If uploadID is specified, it is a request for upload part
263262
return createMultipartKey(volume, bucket, keyPath, length,
264-
partNumber, uploadID, body, perf);
263+
body, perf);
265264
}
266265

267266
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);
271267

272268
// Normal put object
273-
ReplicationConfig replicationConfig =
274-
getReplicationConfig(bucket, storageType, storageConfig);
269+
ReplicationConfig replicationConfig = getReplicationConfig(bucket);
275270

276271
boolean enableEC = false;
277272
if ((replicationConfig != null &&
@@ -284,8 +279,7 @@ public Response put(
284279
//Copy object, as copy source available.
285280
s3GAction = S3GAction.COPY_OBJECT;
286281
CopyObjectResponse copyObjectResponse = copyObject(volume,
287-
copyHeader, bucketName, keyPath, replicationConfig,
288-
storageTypeDefault, perf);
282+
bucketName, keyPath, replicationConfig, perf);
289283
return Response.status(Status.OK).entity(copyObjectResponse).header(
290284
"Connection", "close").build();
291285
}
@@ -431,17 +425,18 @@ public Response put(
431425
* https://docs.aws.amazon.com/AmazonS3/latest/API/mpUploadListParts.html
432426
* for more details.
433427
*/
434-
@SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
428+
@SuppressWarnings("checkstyle:MethodLength")
435429
@GET
436430
public Response get(
437431
@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 {
432+
@PathParam(PATH) String keyPath
433+
) throws IOException, OS3Exception {
434+
final int maxParts = queryParams().getInt(QueryParams.MAX_PARTS, 1000);
435+
final int partNumber = queryParams().getInt(QueryParams.PART_NUMBER, 0);
436+
final String partNumberMarker = queryParams().get(QueryParams.PART_NUMBER_MARKER);
437+
final String taggingMarker = queryParams().get(QueryParams.TAGGING);
438+
final String uploadId = queryParams().get(QueryParams.UPLOAD_ID);
439+
445440
long startNanos = Time.monotonicNowNanos();
446441
S3GAction s3GAction = S3GAction.GET_KEY;
447442
PerformanceStringBuilder perf = new PerformanceStringBuilder();
@@ -748,10 +743,11 @@ private Response abortMultipartUpload(OzoneVolume volume, String bucket,
748743
@SuppressWarnings("emptyblock")
749744
public Response delete(
750745
@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 {
746+
@PathParam(PATH) String keyPath
747+
) throws IOException, OS3Exception {
748+
final String taggingMarker = queryParams().get(QueryParams.TAGGING);
749+
final String uploadId = queryParams().get(QueryParams.UPLOAD_ID);
750+
755751
long startNanos = Time.monotonicNowNanos();
756752
S3GAction s3GAction = S3GAction.DELETE_KEY;
757753

@@ -826,24 +822,20 @@ public Response delete(
826822
public Response initializeMultipartUpload(
827823
@PathParam(BUCKET) String bucket,
828824
@PathParam(PATH) String key
829-
)
830-
throws IOException, OS3Exception {
825+
) throws IOException, OS3Exception {
831826
long startNanos = Time.monotonicNowNanos();
832827
S3GAction s3GAction = S3GAction.INIT_MULTIPART_UPLOAD;
833828

834829
try {
835830
OzoneBucket ozoneBucket = getBucket(bucket);
836831
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);
839832

840833
Map<String, String> customMetadata =
841834
getCustomMetadataFromHeaders(getHeaders().getRequestHeaders());
842835

843836
Map<String, String> tags = getTaggingFromHeaders(getHeaders());
844837

845-
ReplicationConfig replicationConfig =
846-
getReplicationConfig(ozoneBucket, storageType, storageConfig);
838+
ReplicationConfig replicationConfig = getReplicationConfig(ozoneBucket);
847839

848840
OmMultipartInfo multipartInfo =
849841
ozoneBucket.initiateMultipartUpload(key, replicationConfig, customMetadata, tags);
@@ -873,8 +865,9 @@ public Response initializeMultipartUpload(
873865
}
874866
}
875867

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

879872
ReplicationConfig clientConfiguredReplicationConfig =
880873
OzoneClientUtils.getClientConfiguredReplicationConfig(getOzoneConfiguration());
@@ -891,9 +884,9 @@ private ReplicationConfig getReplicationConfig(OzoneBucket ozoneBucket,
891884
public Response completeMultipartUpload(
892885
@PathParam(BUCKET) String bucket,
893886
@PathParam(PATH) String key,
894-
@QueryParam(QueryParams.UPLOAD_ID) @DefaultValue("") String uploadID,
895-
CompleteMultipartUploadRequest multipartUploadRequest)
896-
throws IOException, OS3Exception {
887+
CompleteMultipartUploadRequest multipartUploadRequest
888+
) throws IOException, OS3Exception {
889+
final String uploadID = queryParams().get(QueryParams.UPLOAD_ID, "");
897890
long startNanos = Time.monotonicNowNanos();
898891
S3GAction s3GAction = S3GAction.COMPLETE_MULTIPART_UPLOAD;
899892
OzoneVolume volume = getVolume();
@@ -962,12 +955,14 @@ public Response completeMultipartUpload(
962955
}
963956
}
964957

965-
@SuppressWarnings({"checkstyle:MethodLength", "checkstyle:ParameterNumber"})
958+
@SuppressWarnings("checkstyle:MethodLength")
966959
private Response createMultipartKey(OzoneVolume volume, OzoneBucket ozoneBucket,
967-
String key, long length, int partNumber, String uploadID,
960+
String key, long length,
968961
final InputStream body, PerformanceStringBuilder perf)
969962
throws IOException, OS3Exception {
970963
long startNanos = Time.monotonicNowNanos();
964+
final String uploadID = queryParams().get(QueryParams.UPLOAD_ID);
965+
final int partNumber = queryParams().getInt(QueryParams.PART_NUMBER, 0);
971966
String copyHeader = null;
972967
MultiDigestInputStream multiDigestInputStream = null;
973968
final String bucketName = ozoneBucket.getName();
@@ -979,10 +974,7 @@ private Response createMultipartKey(OzoneVolume volume, OzoneBucket ozoneBucket,
979974
length = chunkInputStreamInfo.getEffectiveLength();
980975

981976
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);
977+
ReplicationConfig replicationConfig = getReplicationConfig(ozoneBucket);
986978

987979
boolean enableEC = false;
988980
if ((replicationConfig != null &&
@@ -1227,12 +1219,14 @@ void copy(OzoneVolume volume, DigestInputStream src, long srcKeyLen,
12271219
perf.appendSizeBytes(copyLength);
12281220
}
12291221

1230-
@SuppressWarnings("checkstyle:ParameterNumber")
12311222
private CopyObjectResponse copyObject(OzoneVolume volume,
1232-
String copyHeader, String destBucket, String destkey,
1233-
ReplicationConfig replicationConfig, boolean storageTypeDefault,
1223+
String destBucket, String destkey, ReplicationConfig replicationConfig,
12341224
PerformanceStringBuilder perf)
12351225
throws OS3Exception, IOException {
1226+
String copyHeader = getHeaders().getHeaderString(COPY_SOURCE_HEADER);
1227+
String storageType = getHeaders().getHeaderString(STORAGE_CLASS_HEADER);
1228+
boolean storageTypeDefault = StringUtils.isEmpty(storageType);
1229+
12361230
long startNanos = Time.monotonicNowNanos();
12371231
Pair<String, String> result = parseSourceHeader(copyHeader);
12381232

‎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)