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

Fix usage of @Header(name="..") #11060

Merged
merged 4 commits into from
Aug 13, 2024
Merged

Fix usage of @Header(name="..") #11060

merged 4 commits into from
Aug 13, 2024

Conversation

dstepanov
Copy link
Contributor

Fixes #11053

@dstepanov dstepanov added the type: bug Something isn't working label Aug 7, 2024
@altro3
Copy link
Contributor

altro3 commented Aug 8, 2024

@dstepanov Does this bug only with @Header? Is it posibble the same issue with @PathVariable?

@dstepanov
Copy link
Contributor Author

Actually I’m not sure that’s a bug, the Javadoc clearly says the name peopert is not supposed to be used inn this case

@dstepanov
Copy link
Contributor Author

That’s why the tests fail

@altro3
Copy link
Contributor

altro3 commented Aug 8, 2024

Sorry I do not understand. In my opinion, name is an auxiliary property, just for convenience and is an alias for value . Those. name is convenient to use to understand that you are specifying the name of a header or variable in the path. When you write value = - it's not so obvious.

So I don't understand why I can't write @Header(name = "my header") . Actually, I don’t understand why my values ​​​​change to some crooked strings

@dstepanov
Copy link
Contributor Author

It’s changed because the logic is to use the parameter name if the value is not specified. Please read the Javadoc, it explicitly says that the name is only used when applied on the class

@altro3
Copy link
Contributor

altro3 commented Aug 8, 2024

What can I say, in my opinion this is an extremely bad solution, because it is very confusing for the user. In the case of PathVariable, these two values ​​have the same meaning, but in the case of Header, it turns out that magic happens and is not obvious at all. By the way, it is not written anywhere that the value in the name field will be distorted

@altro3
Copy link
Contributor

altro3 commented Aug 8, 2024

Well, the decision to put different values ​​in these fields depending on the situation, in my opinion, is very ugly. This annotation is very popular. I can imagine the faces of newcomers to Micronaut when they stumble upon this error, I think they’ll just go back to the good old spring

@dstepanov
Copy link
Contributor Author

I will see what can be done

@sdelamo sdelamo added this to the 4.6.0 milestone Aug 12, 2024
Copy link

sonarcloud bot commented Aug 12, 2024

@sdelamo sdelamo merged commit 98371b2 into 4.6.x Aug 13, 2024
17 checks passed
@sdelamo sdelamo deleted the headersann branch August 13, 2024 04:56
sdelamo pushed a commit that referenced this pull request Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

AliasFor in annotation properties modify real values
4 participants