-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Support java.util.Optional<T> passing to @QueryParam/@RestQuery for RestСlient #45672
Conversation
lasteris
commented
Jan 17, 2025
•
edited by geoand
Loading
edited by geoand
- Closes: Support optional @QueryParam on Rest client #42623
Thanks for your pull request! Your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
Map.Entry<String, List<String>> optionalParam = uriInfo.getQueryParameters().entrySet().iterator() | ||
.next(); | ||
|
||
return optionalParam.getValue().get(0); |
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.
I'm not a huge fan of this, I would rather we return some string like key1=value1,value2/key2=value...
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.
I'm not a huge fan of this, I would rather we return some string like key1=value1,value2/key2=value...
No problem) I just decided that it would be more obvious that we are testing not a Map, but an Optional)
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.
I'm not a huge fan of this, I would rather we return some string like key1=value1,value2/key2=value...
fixed, pushed changes
e0722b2
to
4036dab
Compare
} else if (isOptional(type, index)) { | ||
if (type.kind() == PARAMETERIZED_TYPE) { | ||
Type paramType = type.asParameterizedType().arguments().get(0); | ||
if ((paramType.kind() == CLASS) || (paramType.kind() == PARAMETERIZED_TYPE)) { |
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.
Technically the type argument of Optional
could also be an array.
EDIT: And a wildcard. Let's not get too wild with type variables :-)
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.
Sorry, i am tried to understand these mechanisms for hours, but still having some troubles :)
Do you mean, that i can just drop
if ((paramType.kind() == CLASS) || (paramType.kind() == PARAMETERIZED_TYPE)) {
and assign componentType
directly to componentType = paramType.name().toString();
and that's safe ?
or i need to limit support of parameters type (exclude arrays and etc)
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.
I honestly have no idea, I'm not familiar with this code at all. Just pointed out something that I saw while briefly going through this PR for no particular reason :-)
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.
Indeed. At some point we might need to handle all the possible types, but we can keep that change for another day
🙈 The PR is closed and the preview is expired. |
Status for workflow
|
Status for workflow
|