Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.ozone.s3.commontypes;

import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MultivaluedMap;
import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;

/** Allow looking up query parameters as primitive types. */
public interface RequestParameters {

String get(String key);

static MultivaluedMapImpl of(MultivaluedMap<String, String> params) {
return new MultivaluedMapImpl(params);
}

default String get(String key, String defaultValue) {
final String value = get(key);
return value != null ? value : defaultValue;
}

default int getInt(String key, int defaultValue) {
final String value = get(key);
if (value == null) {
return defaultValue;
}

try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
throw translateException(e);
}
}

default WebApplicationException translateException(RuntimeException e) {
return new WebApplicationException(e.getMessage(), S3ErrorTable.INVALID_ARGUMENT.getHttpCode());
}

/** Additional methods for tests. */
interface Mutable extends RequestParameters {

void set(String key, String value);

void unset(String key);

default void setInt(String key, int value) {
set(key, String.valueOf(value));
}
}

/** Mutable implementation based on {@link MultivaluedMap}. */
final class MultivaluedMapImpl implements Mutable {
private final MultivaluedMap<String, String> params;

private MultivaluedMapImpl(MultivaluedMap<String, String> params) {
this.params = params;
}

@Override
public String get(String key) {
return params.getFirst(key);
}

@Override
public void set(String key, String value) {
params.putSingle(key, value);
}

@Override
public void unset(String key) {
params.remove(key);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import java.util.Set;
import javax.annotation.PostConstruct;
import javax.ws.rs.DELETE;
import javax.ws.rs.DefaultValue;
import javax.ws.rs.GET;
import javax.ws.rs.HEAD;
import javax.ws.rs.POST;
Expand Down Expand Up @@ -106,28 +105,28 @@ public class BucketEndpoint extends EndpointBase {
@GET
@SuppressWarnings("methodlength")
public Response get(
@PathParam(BUCKET) String bucketName,
@DefaultValue("1000") @QueryParam(QueryParams.MAX_KEYS) int maxKeys,
@DefaultValue("1000") @QueryParam(QueryParams.MAX_UPLOADS) int maxUploads
@PathParam(BUCKET) String bucketName
) throws OS3Exception, IOException {
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.GET_BUCKET;
PerformanceStringBuilder perf = new PerformanceStringBuilder();

final String continueToken = getQueryParam(QueryParams.CONTINUATION_TOKEN);
final String delimiter = getQueryParam(QueryParams.DELIMITER);
final String encodingType = getQueryParam(QueryParams.ENCODING_TYPE);
final String marker = getQueryParam(QueryParams.MARKER);
String prefix = getQueryParam(QueryParams.PREFIX);
String startAfter = getQueryParam(QueryParams.START_AFTER);
final String continueToken = queryParams().get(QueryParams.CONTINUATION_TOKEN);
final String delimiter = queryParams().get(QueryParams.DELIMITER);
final String encodingType = queryParams().get(QueryParams.ENCODING_TYPE);
final String marker = queryParams().get(QueryParams.MARKER);
int maxKeys = queryParams().getInt(QueryParams.MAX_KEYS, 1000);
final int maxUploads = queryParams().getInt(QueryParams.MAX_UPLOADS, 1000);
Comment on lines +118 to +119
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems here change to use getInt() not JAX-RS. should we add error hadling in getInt().

now:

default int getInt(String key, int defaultValue) {
String value = get(key);
return value != null ? Integer.parseInt(value) : defaultValue;
}

just a suggestion:

@Override
public int getInt(String key, int defaultValue) {
  String value = get(key);
  if (value == null) {
    return defaultValue;
  }
  try {
    return Integer.parseInt(value.trim());
  } catch (NumberFormatException e) {
    throw new OS3Exception(S3ErrorTable.INVALID_ARGUMENT, 
        key + " must be a valid integer");
  }
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OS3Exception is a checked exception, needs to be declared, but the interface does not allow that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can translate to WebApplicationException(400), but doing so for every type seems cumbersome. Then it may be better to create a new converter just for params, initially supporting only getInt/setInt.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure! I think it makes sense to me.

String prefix = queryParams().get(QueryParams.PREFIX);
String startAfter = queryParams().get(QueryParams.START_AFTER);

Iterator<? extends OzoneKey> ozoneKeyIterator = null;
ContinueToken decodedToken =
ContinueToken.decodeFromString(continueToken);
OzoneBucket bucket = null;

try {
final String aclMarker = getQueryParam(QueryParams.ACL);
final String aclMarker = queryParams().get(QueryParams.ACL);
if (aclMarker != null) {
s3GAction = S3GAction.GET_ACL;
S3BucketAcl result = getAcl(bucketName);
Expand All @@ -136,11 +135,11 @@ public Response get(
return Response.ok(result, MediaType.APPLICATION_XML_TYPE).build();
}

final String uploads = getQueryParam(QueryParams.UPLOADS);
final String uploads = queryParams().get(QueryParams.UPLOADS);
if (uploads != null) {
s3GAction = S3GAction.LIST_MULTIPART_UPLOAD;
final String uploadIdMarker = getQueryParam(QueryParams.UPLOAD_ID_MARKER);
final String keyMarker = getQueryParam(QueryParams.KEY_MARKER);
final String uploadIdMarker = queryParams().get(QueryParams.UPLOAD_ID_MARKER);
final String keyMarker = queryParams().get(QueryParams.KEY_MARKER);
return listMultipartUploads(bucketName, prefix, keyMarker, uploadIdMarker, maxUploads);
}

Expand Down Expand Up @@ -314,7 +313,7 @@ public Response put(
S3GAction s3GAction = S3GAction.CREATE_BUCKET;

try {
final String aclMarker = getQueryParam(QueryParams.ACL);
final String aclMarker = queryParams().get(QueryParams.ACL);
if (aclMarker != null) {
s3GAction = S3GAction.PUT_ACL;
Response response = putAcl(bucketName, body);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
import org.apache.hadoop.ozone.om.protocol.S3Auth;
import org.apache.hadoop.ozone.s3.RequestIdentifier;
import org.apache.hadoop.ozone.s3.commontypes.RequestParameters;
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
import org.apache.hadoop.ozone.s3.metrics.S3GatewayMetrics;
Expand Down Expand Up @@ -106,6 +107,9 @@ public abstract class EndpointBase {
@Context
private HttpHeaders headers;

// initialized in @PostConstruct
private RequestParameters.MultivaluedMapImpl queryParams;

private final Set<String> excludeMetadataFields =
new HashSet<>(Arrays.asList(OzoneConsts.GDPR_FLAG, STORAGE_CONFIG_HEADER));
private static final Logger LOG =
Expand All @@ -114,12 +118,14 @@ public abstract class EndpointBase {
protected static final AuditLogger AUDIT =
new AuditLogger(AuditLoggerType.S3GLOGGER);

protected String getQueryParam(String key) {
return getQueryParameters().getFirst(key);
/** Read-only access to query parameters. */
protected RequestParameters queryParams() {
return queryParams;
}

public MultivaluedMap<String, String> getQueryParameters() {
return context.getUriInfo().getQueryParameters();
/** For setting multiple values use {@link #getContext()}. */
public RequestParameters.Mutable queryParamsForTest() {
return queryParams;
}

protected OzoneBucket getBucket(OzoneVolume volume, String bucketName)
Expand Down Expand Up @@ -149,6 +155,7 @@ protected OzoneBucket getBucket(OzoneVolume volume, String bucketName)
*/
@PostConstruct
public void initialization() {
queryParams = RequestParameters.of(context.getUriInfo().getQueryParameters());
// Note: userPrincipal is initialized to be the same value as accessId,
// could be updated later in RpcClient#getS3Volume
s3Auth = new S3Auth(signatureInfo.getStringToSign(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void setup() throws IOException {
.setClient(client)
.setHeaders(headers)
.build();
bucketEndpoint.getQueryParameters().add(QueryParams.ACL, ACL_MARKER);
bucketEndpoint.queryParamsForTest().set(QueryParams.ACL, ACL_MARKER);
}

@AfterEach
Expand All @@ -82,8 +82,7 @@ public void clean() throws IOException {
@Test
public void testGetAcl() throws Exception {
when(parameterMap.containsKey(ACL_MARKER)).thenReturn(true);
Response response =
bucketEndpoint.get(BUCKET_NAME, 0, 0);
Response response = bucketEndpoint.get(BUCKET_NAME);
assertEquals(HTTP_OK, response.getStatus());
System.out.println(response.getEntity());
}
Expand Down
Loading