-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Convert protocol headers to enum #27169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThis PR refactors ProtocolHeaders by introducing a new Headers enum to encapsulate all protocol header names and updating the ProtocolHeaders constructor to generate header strings via the enum’s withProtocolName method, replacing manual concatenation. Class diagram for refactored ProtocolHeaders and Headers enumclassDiagram
class ProtocolHeaders {
- String name
- String requestUser
- String requestOriginalUser
- String requestOriginalRole
- String requestSource
- String requestCatalog
- String requestSchema
- String requestPath
- String requestTimeZone
- String requestLanguage
- String requestTraceToken
- String requestSession
- String requestRole
- String requestPreparedStatement
- String requestTransactionId
- String requestClientInfo
- String requestClientTags
- String requestClientCapabilities
- String requestResourceEstimate
- String requestExtraCredential
- String requestQueryDataEncoding
- String responseSetCatalog
- String responseSetSchema
- String responseSetPath
- String responseSetSession
- String responseClearSession
- String responseSetRole
- String responseQueryDataEncoding
- String responseAddedPrepare
- String responseDeallocatedPrepare
- String responseStartedTransactionId
- String responseClearTransactionId
- String responseSetAuthorizationUser
- String responseResetAuthorizationUser
- String responseOriginalRole
+ ProtocolHeaders(String name)
+ String getProtocolName()
}
class Headers {
- String headerName
+ Headers(String headerName)
+ String withProtocolName(String protocolName)
<<enum>>
REQUEST_USER
REQUEST_ORIGINAL_USER
REQUEST_ORIGINAL_ROLES
REQUEST_SOURCE
REQUEST_CATALOG
REQUEST_SCHEMA
REQUEST_PATH
REQUEST_TIME_ZONE
REQUEST_LANGUAGE
REQUEST_TRACE_TOKEN
REQUEST_SESSION
REQUEST_ROLE
REQUEST_PREPARED_STATEMENT
REQUEST_TRANSACTION_ID
REQUEST_CLIENT_INFO
REQUEST_CLIENT_TAGS
REQUEST_CLIENT_CAPABILITIES
REQUEST_RESOURCE_ESTIMATE
REQUEST_EXTRA_CREDENTIAL
REQUEST_QUERY_DATA_ENCODING
RESPONSE_SET_CATALOG
RESPONSE_SET_SCHEMA
RESPONSE_SET_PATH
RESPONSE_SET_SESSION
RESPONSE_CLEAR_SESSION
RESPONSE_SET_ROLE
RESPONSE_SET_ORIGINAL_ROLES
RESPONSE_QUERY_DATA_ENCODING
RESPONSE_ADDED_PREPARE
RESPONSE_DEALLOCATED_PREPARE
RESPONSE_STARTED_TRANSACTION_ID
RESPONSE_CLEAR_TRANSACTION_ID
RESPONSE_SET_AUTHORIZATION_USER
RESPONSE_RESET_AUTHORIZATION_USER
}
ProtocolHeaders o-- Headers : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `client/trino-client/src/main/java/io/trino/client/ProtocolHeaders.java:106-109` </location>
<code_context>
+ this.headerName = requireNonNull(headerName, "headerName is null");
+ }
+
+ public String withProtocolName(String protocolName)
+ {
+ return PREFIX + protocolName + "-" + headerName;
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validate protocolName in withProtocolName to avoid malformed headers.
Add a null or empty check for protocolName to prevent malformed headers.
```suggestion
public String withProtocolName(String protocolName)
{
if (protocolName == null || protocolName.isEmpty()) {
throw new IllegalArgumentException("protocolName is null or empty");
}
return PREFIX + protocolName + "-" + headerName;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4a37cb2 to
441310e
Compare
This is a refactor that will allow to validate extra header in #15826
Description
Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:
Summary by Sourcery
Refactor ProtocolHeaders by consolidating header definitions into a Headers enum and simplifying header name construction
Enhancements: