Skip to content

HDDS-14208. Create constants for query parameter names#9525

Merged
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-14208
Dec 19, 2025
Merged

HDDS-14208. Create constants for query parameter names#9525
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-14208

Conversation

@adoroszlai
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Extract constants for @QueryParam values to S3Consts.QueryParams.
  • Extract constants for @PathParam values to the endpoint class, because these depend on the @Path from the same class.
  • Use constants from HttpHeaders for @HeaderParam values. Some were already used as static imports, replace them with qualified access for clarity and consistency.
  • Attempt to clarify usage of OzoneConsts.ETAG vs. HttpHeaders.ETAG for metadata vs. header purposes. BTW the two values are the same.

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

How was this patch tested?

Ran all S3 unit tests locally.

CI:
https://github.com/adoroszlai/ozone/actions/runs/20345474287

@adoroszlai adoroszlai self-assigned this Dec 18, 2025
@adoroszlai adoroszlai added s3 S3 Gateway code-cleanup Changes that aim to make code better, without changing functionality. labels Dec 18, 2025
Copy link
Copy Markdown
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

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

LGTM +1.

.header("Content-Length", key.getDataSize())
.header("Content-Type", "binary/octet-stream")
.header(HttpHeaders.CONTENT_LENGTH, key.getDataSize())
.header(HttpHeaders.CONTENT_TYPE, "binary/octet-stream")
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.

nit: should we consider extracting "binary/octet-stream" to a constant in S3Consts for consistency with the refactoring?

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.

just a question, overall LGTM

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.

Good point, I'll include it in the next patch.

Copy link
Copy Markdown
Member

@peterxcli peterxcli left a comment

Choose a reason for hiding this comment

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

LGTM +1

@adoroszlai adoroszlai merged commit d7a823f into apache:master Dec 19, 2025
43 checks passed
@adoroszlai adoroszlai deleted the HDDS-14208 branch December 19, 2025 07:13
@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @ivandika3, @peterxcli, @rich7420 for the review.

@rich7420
Copy link
Copy Markdown
Contributor

thanks @adoroszlai !

echonesis pushed a commit to echonesis/ozone that referenced this pull request Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-cleanup Changes that aim to make code better, without changing functionality. s3 S3 Gateway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants