Skip to content
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

Encode any Enums in Routing Header as an Int #1966

Closed
lqiu96 opened this issue Sep 6, 2023 · 4 comments
Closed

Encode any Enums in Routing Header as an Int #1966

lqiu96 opened this issue Sep 6, 2023 · 4 comments
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@lqiu96
Copy link
Contributor

lqiu96 commented Sep 6, 2023

See go/clientlibs-routine-headers-numeric-enums for more information.

Enums in every implicit/ explicit routing header should be encoded as an Int.

The Routing Headers seem to be encoded here via a ParamsExtractor.
Example:

.setParamsExtractor(
request -> {
RequestParamsBuilder builder = RequestParamsBuilder.create();
builder.add("name", String.valueOf(request.getName()));
return builder.build();
})

This should be modified to get the Int value instead.

Generator changes:
Affect the AbstractTransportServiceStubClassComposer. Specifically around:

if (method.routingHeaderRule() == null) {
createRequestParamsExtractorBodyForHttpBindings(
method, requestVarExpr, bodyStatements, returnExpr);
} else {
createRequestParamsExtractorBodyForRoutingHeaders(
method, requestVarExpr, classStatements, bodyStatements, returnExpr);
}

Both methods should be modified so that the changes are applied for both implicit and explicit routing headers.

@lqiu96 lqiu96 added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Sep 6, 2023
@ddixit14
Copy link
Contributor

ddixit14 commented Jan 16, 2024

Created a new issue for implicit routing headers case - #2368 . Implemenation PR - #2328

@ddixit14
Copy link
Contributor

FYI, only implicit routing headers can be fixed currently because explicit routing headers are implemented as strings by design. See this doc for more details.

@lqiu96
Copy link
Contributor Author

lqiu96 commented Mar 4, 2024

Implicit Routing Headers were implemented. Discussion is pending explicit routing headers and how to properly implement it.

@blakeli0 blakeli0 added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Mar 4, 2024
@blakeli0
Copy link
Collaborator

blakeli0 commented Mar 5, 2025

Closing this issue as there was no progress for how to implement it for explicit routing headers, which was designed to be String in the first place. Will re-open if this becomes a priority.

@blakeli0 blakeli0 closed this as completed Mar 5, 2025
@blakeli0 blakeli0 self-assigned this Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants