Add new `definitionType` parameter for `@Client` annotation #11278

altro3 · 2024-10-27T06:15:55Z

This was asked in the Discord, and I was also interested in this question earlier: is it possible to use a single interface to describe the client and the controller? The feign library was invented for Spring (8-9 years ago), which later became part of Spring Cloud. And so, when I saw that Micronaut has built-in support for a single interface, I was happy, but it turned out that the client and controller do not support the single interface mode.

So, for now, the main problem, in my opinion, is the opposite perception of the Consumes and Produces annotations for the client and the controller. I added the definitionType parameter, which can be one of two values - CLIENT (default) and SERVER. SERVER value means that you use server (controller) interface to describe http client endpoints.

Perhaps there are some other significant differences between the interface for the controller and the client, but I do not know about them.

Why is this important: in general, it is quite common practice to make a microservice with 2 submodules - API and business logic, so that you can write all the endpoints and DTOs in the API module, and then simply inherit from the interface and thus get a ready-made HTTP client.

@graemerocher @yawkat @sdelamo @dstepanov

sdelamo · 2024-10-28T08:39:17Z

I personally don't think we should do this. It adds more complexity.

dstepanov · 2024-10-29T08:08:47Z

The use-case of reusing the same interface for a client and a controller is valid. Maybe the property can be called controllerDefinition to indicate that the interface is mainly used to define the controller. WDYT @graemerocher

graemerocher · 2024-10-29T08:11:39Z

I would say we add kind = ClientKind.SERVER with an enum. Committing to a boolean makes it difficult to evolve and deprecate. We can also mark the enumeration as experimental so we can get more feedback.

altro3 · 2024-10-30T06:42:08Z

@graemerocher Perhaps it would be better to give this parameter another name. It is a bit strange to name the ClientKind - CLIENT and SERVER.

For example, definitionType

graemerocher · 2024-10-30T07:22:09Z

sure

yawkat · 2024-10-30T09:36:47Z

Is there anything that needs to be mirrored except for Consumes/Produces? We could also simply add RequestType and ResponseType annotations that alias to Consumes or Produces depending on use site.

altro3 · 2024-10-30T12:44:51Z

@yawkat In my opinion, adding one additional parameter is much easier than adding new annotations.

New annotations are more likely to confuse users - when what should be used. My personal opinion is that adding new annotations is exactly the same complication of logic and understanding. And an additional parameter in an annotation is quite laconic and is perceived as a setting

graemerocher · 2024-10-30T12:49:08Z

http-client-core/src/main/java/io/micronaut/http/client/annotation/Client.java

+        public boolean isClient() {
+            return this == CLIENT;
+        }
+
+        public boolean isServer() {
+            return this == SERVER;
+        }


these methods need javadoc

graemerocher · 2024-10-30T12:51:51Z

...nt-core/src/main/java/io/micronaut/http/client/interceptor/HttpClientIntroductionAdvice.java

        if (accept.isEmpty()) {
-            String[] consumesMediaType = context.stringValues(Consumes.class);
+            String[] consumesMediaType = context.stringValues(definitionType.isClient() ? Consumes.class : Produces.class);
            if (ArrayUtils.isEmpty(consumesMediaType)) {
                acceptTypes = DEFAULT_ACCEPT_TYPES;


it is likely this default handling needs altering based on the definition type

I don't understand what you're suggesting?

perhaps it is ok

graemerocher · 2024-10-30T13:02:38Z

http-client-core/src/main/java/io/micronaut/http/client/annotation/Client.java

+    /**
+     * The interface definition type. Use {@code DefinitionType.SERVER} type, if you want
+     * to use controller interface to describe HTTP client. Use {@code DefinitionType.CLIENT} type,
+     * if you want to use custom interface to describe HTTP client.
+     *
+     * @return The interface definition type
+     * @since 4.8.0
+     */


Suggested change

/**

* The interface definition type. Use {@code DefinitionType.SERVER} type, if you want

* to use controller interface to describe HTTP client. Use {@code DefinitionType.CLIENT} type,

* if you want to use custom interface to describe HTTP client.

*

* @return The interface definition type

* @since 4.8.0

*/

/**

* The interface definition type. When set to {@link DefinitionType.SERVER} the {@link Produces} and {@link Consumes}

* definition evaluated for each method of the interface will be reversed .

*

* Whilst not necessarily recommended, there are scenarios like testing where it is useful to share a common interface between client and server and use the

* interface to create a new client declarative client. The client typically needs to produce the content type accepted by the server and consume the content type

* produced by the server. In this arrangement using the interface directly will not result in the correct behaviour.

*

* In this scenario you can set {@link DefinitionType} to {@link DefinitionType.SERVER} which will ensure the requests sent by the client use the content type declared by the {@link Consumes}

* annotation of the interface and that responses use the content type declared by the {@link Produces}.

*

* The default behaviour is to use {@link DefinitionType.CLIENT} where the inverse of the above is true.

*

* @return The interface definition type

* @since 4.8.0

* @see Consumes

* @see Produces

*/

you will need to add the imports when merging this

yawkat

I think a new annotation is "simpler" than a parameter on @Client that magically switches around the meaning of method annotations at-a-distance. But this PR is a low maintenance commitment so I'm not opposed

yawkat · 2024-10-30T13:19:31Z

BTW @altro3 you do not need to force-push your changes into a single commit each time, we will squash any commits when merging anyway.

altro3 · 2024-10-30T13:46:48Z

@yawkat
Yes, I understand, but I have a habit. If this is a problem, then I will try not to do it next time (although, it is indeed a habit, and I may do it again)

Is this a bad habit?

graemerocher · 2024-10-30T13:50:11Z

it makes it a bit more difficult to review individual changes that address code review

graemerocher · 2024-10-31T14:04:02Z

http-client-core/src/main/java/io/micronaut/http/client/annotation/Client.java

+     * @see Produces
+     */
+    @NonNull
+    DefinitionType definitionType() default DefinitionType.CLIENT;


can you annotate this with @Experiemental

graemerocher · 2024-10-31T14:04:07Z

http-client-core/src/main/java/io/micronaut/http/client/annotation/Client.java

+     *
+     * @since 4.8.0
+     */
+    enum DefinitionType {


can you annotate this with @Experiemental

fixed. Sorry, again force pushed. Forgot

graemerocher · 2024-11-05T08:32:29Z

Thanks for the contribution!

altro3 force-pushed the client-inverse branch 2 times, most recently from f51fad4 to dc8d058 Compare October 30, 2024 07:54

altro3 changed the title ~~Add new inverse mode for http client~~ Add new definitionType parameter for @Client annotation Oct 30, 2024

graemerocher requested changes Oct 30, 2024

View reviewed changes

altro3 force-pushed the client-inverse branch 2 times, most recently from 03bed67 to 5d7fd77 Compare October 30, 2024 13:16

graemerocher added the type: enhancement label Oct 30, 2024

yawkat approved these changes Oct 30, 2024

View reviewed changes

graemerocher requested changes Oct 31, 2024

View reviewed changes

Add new definitionType parameter for @Client annotation

Loading
Loading status checks…

c7a016f

altro3 force-pushed the client-inverse branch from 5d7fd77 to c7a016f Compare October 31, 2024 17:03

graemerocher approved these changes Oct 31, 2024

View reviewed changes

graemerocher merged commit 8954c74 into micronaut-projects:4.8.x Nov 5, 2024
16 checks passed

sdelamo mentioned this pull request Feb 11, 2025

@PermitAll and @Secured(SecurityRule.IS_ANONYMOUS) not working on methods with any sort of parameter (body, header or query) micronaut-projects/micronaut-security#1926

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new `definitionType` parameter for `@Client` annotation #11278

Add new `definitionType` parameter for `@Client` annotation #11278

altro3 commented Oct 27, 2024 •

edited

Loading

sdelamo commented Oct 28, 2024

dstepanov commented Oct 29, 2024

graemerocher commented Oct 29, 2024

altro3 commented Oct 30, 2024

graemerocher commented Oct 30, 2024

yawkat commented Oct 30, 2024

altro3 commented Oct 30, 2024

graemerocher Oct 30, 2024

altro3 Oct 30, 2024

graemerocher Oct 30, 2024

altro3 Oct 30, 2024

graemerocher Oct 30, 2024

graemerocher Oct 30, 2024

graemerocher Oct 30, 2024

altro3 Oct 30, 2024

yawkat left a comment

yawkat commented Oct 30, 2024

altro3 commented Oct 30, 2024

graemerocher commented Oct 30, 2024

graemerocher Oct 31, 2024

altro3 Oct 31, 2024

graemerocher Oct 31, 2024

altro3 Oct 31, 2024

graemerocher commented Nov 5, 2024

-    /**
-     * The interface definition type. Use {@code DefinitionType.SERVER} type, if you want
-     * to use controller interface to describe HTTP client. Use {@code DefinitionType.CLIENT} type,
-     * if you want to use custom interface to describe HTTP client.
-     *
-     * @return The interface definition type
-     * @since 4.8.0
-     */
+ /**
+     * The interface definition type. When set to {@link DefinitionType.SERVER} the {@link Produces} and {@link Consumes}
+     * definition evaluated for each method of the interface will be reversed .
+     *
+     * <p>Whilst not necessarily recommended, there are scenarios like testing where it is useful to share a common interface between client and server and use the
+     * interface to create a new client declarative client. The client typically needs to produce the content type accepted by the server and consume the content type
+     * produced by the server. In this arrangement using the interface directly will not result in the correct behaviour.</p>
+     *
+     * <p>In this scenario you can set {@link DefinitionType} to {@link DefinitionType.SERVER} which will ensure the requests sent by the client use the content type declared by the {@link Consumes}
+     *  annotation of the interface and that responses use the content type declared by the {@link Produces}.</p>
+     *
+     * <p>The default behaviour is to use {@link DefinitionType.CLIENT} where the inverse of the above is true.</p>
+     *
+     * @return The interface definition type
+     * @since 4.8.0
+     * @see Consumes
+     * @see Produces
+     */

Add new definitionType parameter for @Client annotation #11278

Add new definitionType parameter for @Client annotation #11278

Conversation

altro3 commented Oct 27, 2024 • edited Loading

sdelamo commented Oct 28, 2024

dstepanov commented Oct 29, 2024

graemerocher commented Oct 29, 2024

altro3 commented Oct 30, 2024

graemerocher commented Oct 30, 2024

yawkat commented Oct 30, 2024

altro3 commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yawkat left a comment

Choose a reason for hiding this comment

yawkat commented Oct 30, 2024

altro3 commented Oct 30, 2024

graemerocher commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graemerocher commented Nov 5, 2024

Add new `definitionType` parameter for `@Client` annotation #11278

Add new `definitionType` parameter for `@Client` annotation #11278

altro3 commented Oct 27, 2024 •

edited

Loading