HDDS-14221. Support primitive query params

[adoroszlai]

What changes were proposed in this pull request?

Reuse Ozone configuration's String conversion logic to support primitive types for query parameters.

Update BucketEndpoint#get to use getInt, tests to use setInt.

https://issues.apache.org/jira/browse/HDDS-14221

How was this patch tested?

Ran all S3 unit and integration tests locally.

CI (in progress):
https://github.com/adoroszlai/ozone/actions/runs/20394478143

[rich7420]

@adoroszlai Thanks for the patch!

[rich7420]

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

now:

ozone/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java

Lines 45 to 48 in 049abeb

	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");
  }
}

[adoroszlai]

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

[adoroszlai]

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.

[rich7420]

sure! I think it makes sense to me.

[ivandika3]

LGTM +1.

[ivandika3]

I'm wondering whether instead of a single long method for handling PUT, GET (e.g. ObjectEndpoint#put) we can divide a single endpoint to multiple methods and let the JAX-RS implementation (Jersey) route the requests the most appropriate handlers (similar to other web frameworks like JavaScript's Express router, Minio https://github.com/minio/minio/blob/master/cmd/api-router.go that uses Gorilla router, etc). For example, for ObjectEndpoint#put, we can divide to putObject, putObjectTagging, createMultipartKey based on whether certain headers appear (e.g. ACL header, multipart header).

Not sure if there are any performance implications.

[adoroszlai]

Thanks @ivandika3, @rich7420 for the review. I have updated the patch to translate to WebApplicationException(400), but stopped using ConfigurationSource, has too much unnecessary baggage for this.

I'm wondering whether instead of a single long method for handling PUT, GET (e.g. ObjectEndpoint#put) we can divide a single endpoint to multiple methods and let the JAX-RS implementation (Jersey) matcher router the most appropriate ones ... For example, for ObjectEndpoint#put, we can divide to putObject, putObjectTagging, createMultipartKey based on whether certain headers appear (e.g. ACL header, multipart header).

We do have two handlers for POST based on content type (@Consumes). I'm not very familiar with Jersey, but looking into the docs I haven't found how to do the same based on headers or query params. Would be great if it's possible.

Assuming we have to roll our own, I'd like to:

• define handler for each request as you describe, but in separate objects
• reduce the current long methods into a loop to delegate to the handlers

(See #9516 (review) for details.)

[rich7420]

@adoroszlai thanks for the update! LGTM

[ivandika3]

Thanks for the update. LGTM +1.

[ivandika3]

Thanks @adoroszlai for the patch and @rich7420 for the review.

[rich7420]

@adoroszlai , @ivandika3 thanks!

[adoroszlai]

Thanks @ivandika3, @rich7420 for the review.