[116] New @ClientHeaderParam annotation and TCK tests
#143
Conversation
|
This is part 1 of the solution for issue #116. |
api/src/main/java/org/eclipse/microprofile/rest/client/annotation/ClientHeaderParam.java
Show resolved
Hide resolved
api/src/main/java/org/eclipse/microprofile/rest/client/annotation/ClientHeaderParam.java
Outdated
Show resolved
Hide resolved
andymc12
force-pushed
the
116-propagating
branch
from
23764fe
to
e28a931
Compare
|
@kenfinnigan @michalszynkiewicz - can you take another look at this PR? I've updated it with the review comments, including: Thanks! |
| return "should not be invoked - unexpected Integer arg"; | ||
| } | ||
|
|
||
| default String invalidMethod(ClientRequestContext context, ClientRequestContext someOtherArg) { |
There was a problem hiding this comment.
Should it work with single ClientRequestContext? This tests seems to suggest so (and I think it may be beneficial in some cases).
I cannot find a test that would check it and the documentation doesn't seem to mention it.
There was a problem hiding this comment.
I was originally thinking that we could pass in a ClientRequestContext in addition the header name, but ultimately decided against it since a lot of the info in CRC would be invalid (or at least, not yet determined this early in the flow).
I should remove that method from the TCK interface.
Is there anything in particular that you thought might be useful from the CRC? We could possibly pass in the Method object from the client interface - that might allow compute methods to read other annotations, etc to better determine what the header value should be...
There was a problem hiding this comment.
I didn't have anything specific in mind.
api/src/main/java/org/eclipse/microprofile/rest/client/annotation/ClientHeaderParam.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Andy McCright <j.andrew.mccright@gmail.com>
- Highlighting `required` attribute behavior - Making `value` attribute a String[] - Enabling compute method to be from a different class - Validation failure if same header specified twice on same target Signed-off-by: Andy McCright <j.andrew.mccright@gmail.com>
andymc12
force-pushed
the
116-propagating
branch
from
e28a931
to
9835a9f
Compare
|
@michalszynkiewicz I updated the Javadoc to be consistent with the spec text to indicate that the compute method can be either a default method on the interface or a public static method on another class/interface. And I added a new test for cases where multiple values were specified on the same annotation and at least one uses the curly braces. Thanks again for the review! |
|
@kenfinnigan I assume you are ok with the latest changes (based on discussion in gitter from a couple weeks ago). I'll plan to merge tomorrow unless you have any objections. Thanks (and Happy New Year)! |
Includes spec document changes describing how to use
@ClientHeaderParam, the annotation in the API (including javadoc) and TCK tests that test proper usage and exception conditions.Signed-off-by: Andy McCright j.andrew.mccright@gmail.com