Improve message body handlers #11002

dstepanov · 2024-07-22T07:39:21Z

Previous raw handlers didn't allow to define a custom String handler of a different media type
Raw handlers replaced by Type* reader/writer that include an argument.
Extracted raw handlers to a separate writer / reader bean (no need to hide them with @Bean(typed=)
Improved how dynamic body writer works, it will be always included from the registry for object types
Introduced a separate writer/reader for a String and a JSON type. That is simply threads it a plain type.
Small corrections

test-suite/src/test/java/io/micronaut/docs/server/consumes/ConsumesControllerSpec.java

core/src/main/java/io/micronaut/core/type/Argument.java

http-netty/src/test/groovy/io/micronaut/http/netty/body/CustomBodyReaderSpec.groovy

http/src/main/java/io/micronaut/http/body/DynamicMessageBodyWriter.java

http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java

http-client/src/main/java/io/micronaut/http/client/netty/FullNettyClientHttpResponse.java

http-netty/src/main/java/io/micronaut/http/netty/body/NettyByteBufMessageBodyHandler.java

http-netty/src/main/java/io/micronaut/http/netty/body/NettyTextPlainHandler.java

core/src/main/java/io/micronaut/core/type/Argument.java

graemerocher · 2024-07-22T12:02:42Z

core/src/main/java/io/micronaut/core/type/MutableHeaders.java

+     * @return This headers
+     * @since 4.6
+     */
+    default MutableHeaders setIfMissing(CharSequence header, CharSequence value) {


this calls add so should be named addIfMissing. Add nullability annotations to the arguments and null check the header argument

That an alternative to the method above called set which will remove the previous value

http/src/main/java/io/micronaut/http/body/AbstractMessageBodyHandlerRegistry.java

graemerocher · 2024-07-22T12:05:10Z

http/src/main/java/io/micronaut/http/body/ByteBufferBodyHandler.java

+ */
+@Requires(bean = ByteBufferFactory.class)
+@Singleton
+@BootstrapContextCompatible


can we reduce the number of bean definitions with a registrar type pattern?

Don't think so

http/src/main/java/io/micronaut/http/body/MessageBodyHandlerRegistry.java

http/src/main/java/io/micronaut/http/body/TypedMessageBodyHandler.java

http/src/main/java/io/micronaut/http/body/TypedMessageBodyReader.java

http/src/main/java/io/micronaut/http/body/TypedMessageBodyWriter.java

graemerocher · 2024-07-22T12:08:03Z

json-core/src/main/java/io/micronaut/json/body/JsonMessageHandler.java

+        MediaType.APPLICATION_JSON_PATCH,
+        MediaType.APPLICATION_JSON_MERGE_PATCH,
+        MediaType.APPLICATION_JSON_SCHEMA
+    })


should this be configurable?

graemerocher · 2024-07-22T12:08:13Z

json-core/src/main/java/io/micronaut/json/body/JsonMessageHandler.java

+        MediaType.APPLICATION_JSON_PATCH,
+        MediaType.APPLICATION_JSON_MERGE_PATCH,
+        MediaType.APPLICATION_JSON_SCHEMA
+    })


should this be configurable?

Maybe, but for now it's not, I just create this annotation to reduce repeated code

…tPlainHandler.java Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>

…BodyReaderSpec.groovy Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>

yawkat · 2024-07-22T12:51:55Z

http-netty/src/main/java/io/micronaut/http/netty/body/NettyRawJsonStringWriter.java

+@JsonMessageHandler.ProducesJson
+@JsonMessageHandler.ConsumesJson
+@Internal
+public final class NettyRawJsonStringWriter implements MessageBodyWriter<CharSequence>, MessageBodyReader<String>, NettyBodyWriter<CharSequence>, ChunkedMessageBodyReader<String> {


why do we need separate writers per content type for this now? old behavior was that for these "raw types", the body would always be written the same way, regardless of content type

Not sure 100% how did it work before.
Without this the object type JSON writer will pick it up and convert it into the JSON.
This writer is more correct because we actually want to convert strings as a plain text instead of wrapping it.

Hm, you removed RawMessageHandlerRegistry, but didn't replace the lookup logic. I only see it in the ContextlessMessageBodyHandlerRegistry.

The old logic is very simple: If the type has a RawMessageHandler, that handler is used no matter what. This is the behavior from 3.x and it is the behavior we need to keep.

Raw handlers were internal. Why would you disallow custom handlers?

The problem is if you want to handle the type in a different way. I don't think it's a problem if a user defines a custom handler, he chooses to change the behaviour.

why can't we just have the old behavior?

Bacase that is esentially broken for writers that don't use textual formats like protocol buffers.

that's how it's always been. If you want to change it, do so in 5.0.

So actually it was working like that anyway, there is no need for an extra string json handler. Removed unneeded handlers and added tests.

yawkat

see above comment on raw handlers

sonarcloud · 2024-07-22T14:59:06Z

Quality Gate failed

Failed conditions
2 New Bugs (required ≤ 0)
1 New Blocker Issues (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

dstepanov requested review from graemerocher and yawkat July 22, 2024 07:39

dstepanov commented Jul 22, 2024

View reviewed changes

test-suite/src/test/java/io/micronaut/docs/server/consumes/ConsumesControllerSpec.java Outdated Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

core/src/main/java/io/micronaut/core/type/Argument.java Outdated Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

http-netty/src/test/groovy/io/micronaut/http/netty/body/CustomBodyReaderSpec.groovy Outdated Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

http/src/main/java/io/micronaut/http/body/DynamicMessageBodyWriter.java Outdated Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java Outdated Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java Outdated Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

http-client/src/main/java/io/micronaut/http/client/netty/FullNettyClientHttpResponse.java Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

http-netty/src/main/java/io/micronaut/http/netty/body/NettyByteBufMessageBodyHandler.java Outdated Show resolved Hide resolved

yawkat reviewed Jul 22, 2024

View reviewed changes

http-netty/src/main/java/io/micronaut/http/netty/body/NettyTextPlainHandler.java Outdated Show resolved Hide resolved

graemerocher requested changes Jul 22, 2024

View reviewed changes

dstepanov and others added 10 commits July 22, 2024 14:17

Improve message body handlers

00461db

Only allow text plain writer for basic Java objects

198bf0b

Update http-netty/src/main/java/io/micronaut/http/netty/body/NettyTex…

b9afa88

…tPlainHandler.java Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>

Update http-netty/src/test/groovy/io/micronaut/http/netty/body/Custom…

5dc6cf4

…BodyReaderSpec.groovy Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>

CR

4ea6bde

Correct

fdc92a7

Correct

51a9b90

Improve

f7fe6b9

Eliminate DynamicMessageBodyWriter

3317acc

CR

289f691

dstepanov force-pushed the rawhan branch from bb6cc00 to 289f691 Compare July 22, 2024 12:19

graemerocher approved these changes Jul 22, 2024

View reviewed changes

yawkat reviewed Jul 22, 2024

View reviewed changes

yawkat approved these changes Jul 22, 2024

View reviewed changes

Sonar

98b8196

yawkat requested changes Jul 22, 2024

View reviewed changes

Correct

4989eb4

yawkat approved these changes Jul 23, 2024

View reviewed changes

graemerocher merged commit f4a7f9d into 4.6.x Jul 23, 2024
12 of 16 checks passed

graemerocher deleted the rawhan branch July 23, 2024 12:32

graemerocher added the type: improvement A minor improvement to an existing feature label Jul 23, 2024

This was referenced Aug 12, 2024

NotFoundSpec.test 404 handling with Flowable fails since #11002 #11068

Closed

Update versions micronaut-projects/micronaut-jaxrs#458

Closed

sdelamo mentioned this pull request Aug 19, 2024

update to Micronaut 4.6.1 micronaut-projects/micronaut-views#862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve message body handlers #11002

Improve message body handlers #11002

dstepanov commented Jul 22, 2024

graemerocher Jul 22, 2024

dstepanov Jul 22, 2024

graemerocher Jul 22, 2024

dstepanov Jul 22, 2024

graemerocher Jul 22, 2024

graemerocher Jul 22, 2024

dstepanov Jul 22, 2024

yawkat Jul 22, 2024

dstepanov Jul 22, 2024

yawkat Jul 22, 2024

dstepanov Jul 22, 2024

dstepanov Jul 22, 2024

yawkat Jul 23, 2024

dstepanov Jul 23, 2024

yawkat Jul 23, 2024

dstepanov Jul 23, 2024 •

edited

Loading

yawkat Jul 23, 2024

yawkat left a comment

sonarcloud bot commented Jul 22, 2024

Improve message body handlers #11002

Improve message body handlers #11002

Conversation

dstepanov commented Jul 22, 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

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

dstepanov Jul 23, 2024 • edited Loading

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

sonarcloud bot commented Jul 22, 2024

Quality Gate failed

dstepanov Jul 23, 2024 •

edited

Loading