Skip to content

Spring Reactive Web uses kotlin serialization over jackson for its own classes, and it's very hard to change #28856

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

Closed
martypitt opened this issue Jul 22, 2022 · 12 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support

Comments

@martypitt
Copy link

Affects: Spring 5.3.21

This is somewhat related to #26321 - Spring using kotlin serialization over jackson (but not the same, as that's for WebMVC).

In Reactive Web, Kotlin classes that are tagged as @Serializable use Kotlin Serializers, not Jackson.
This is a reasonable default, but changing the behaviour is very difficult, and has a few surprising side effects.

In Spring WebMVC, we could re-order the HttpMessageConverter<>, and put Jackson first:

// The approach we've used for WebMVC - there's no analogus support in WebFlux.
@Configuration
class WebConfig : WebMvcConfigurationSupport() {
  override fun configureMessageConverters(converters: MutableList<HttpMessageConverter<*>?>) {
    super.addDefaultHttpMessageConverters(converters)
    converters.sortBy { converter -> if (converter is KotlinSerializationJsonHttpMessageConverter) 1000 else 0 }
  }
}

The contract of WebFluxConfigurer doesn't allow modification of the list - because the BaseCodecConfigurer returns a new list each time:

	@Override
	public List<HttpMessageWriter<?>> getWriters() {
		this.defaultCodecs.applyDefaultConfig(this.customCodecs);

		List<HttpMessageWriter<?>> result = new ArrayList<>();
		result.addAll(this.customCodecs.getTypedWriters().keySet());
		result.addAll(this.defaultCodecs.getTypedWriters());
		result.addAll(this.customCodecs.getObjectWriters().keySet());
		result.addAll(this.defaultCodecs.getObjectWriters());
		result.addAll(this.defaultCodecs.getCatchAllWriters());
		return result;
	}

Therefore, adding any sort like in WebMVC has no effect.

Changing the Kotlin encoder to null (to try to disable), doesn't work, as BaseDefaultCodecs simply adds it back:

	@Override
	public void kotlinSerializationJsonEncoder(Encoder<?> encoder) {
		this.kotlinSerializationJsonEncoder = encoder;
		initObjectWriters(); // triggers a call to getBaseObjectWriters()
	}

       final List<HttpMessageWriter<?>> getBaseObjectWriters() {
		List<HttpMessageWriter<?>> writers = new ArrayList<>();
		if (kotlinSerializationJsonPresent) {
			addCodec(writers, new EncoderHttpMessageWriter<>(getKotlinSerializationJsonEncoder()));
		}
		...snip...
		return writers;
	}

The workaround I've used is to put a decorator around the configurer to re-order every single time. However, this seems awkward.

@Configuration
class CustomerWebFluxConfigSupport : WebFluxConfigurationSupport() {
   override fun serverCodecConfigurer(): ServerCodecConfigurer {
      return ReOrderingServerCodecConfigurer(super.serverCodecConfigurer())
   }

   class ReOrderingServerCodecConfigurer(private val configurer: ServerCodecConfigurer) :
      ServerCodecConfigurer by configurer {

      override fun getWriters(): MutableList<HttpMessageWriter<*>> {
         val writers = configurer.writers
         val jacksonWriterIndex =
            configurer.writers.indexOfFirst { it is EncoderHttpMessageWriter && it.encoder is Jackson2JsonEncoder }
         val kotlinSerializationWriterIndex =
            configurer.writers.indexOfFirst { it is EncoderHttpMessageWriter && it.encoder is KotlinSerializationJsonEncoder }

         if (kotlinSerializationWriterIndex == -1 || jacksonWriterIndex == -1) {
            return writers
         }

         if (kotlinSerializationWriterIndex < jacksonWriterIndex) {
            Collections.swap(writers, jacksonWriterIndex, kotlinSerializationWriterIndex)
         }
         return writers
      }
   }
}

Expected / Desired Behaviour

It'd be nice if there was an easier way to configure this.

At the very least, where BaseDefaultCodecs overwrites the changed Kotlin serializer feels like a bug.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 22, 2022
@poutsma poutsma added theme: kotlin An issue related to Kotlin support in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jul 25, 2022
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Aug 1, 2022

Currently Kotlin serialization is based on classpath detection only. I'm just wondering, should it be on the classpath, and is there a way to exclude it? Or otherwise it would make sense to provide a way to disable it. @sdeleuze what do you think?

@martypitt, if you register a custom Encoder or Decoder (e.g. Jackson), it is ahead of default ones in the order, so that provides another option to influence the order.

@rstoyanchev rstoyanchev added this to the Triage Queue milestone Aug 1, 2022
@sdeleuze sdeleuze self-assigned this Aug 8, 2022
@sdeleuze
Copy link
Contributor

sdeleuze commented Aug 8, 2022

@martypitt Could you please share more about your use case for using Jackson on classes annotated with @Serializable?

@martypitt
Copy link
Author

@martypitt Could you please share more about your use case for using Jackson on classes annotated with @Serializable?

Sure. We use JSON for responses out to web requests, as part of our "public" api.
We use CBOR for serializing objects to put onto Kafka messages, or other downstream internal services.

Several classes are intended for serializaton in both scenarios.
We also have a large number of custom Jackson serialization adaptors / converters, and didn't see a need to migrate away to the less mature Kotlin Serialization for JSON.

@sdeleuze
Copy link
Contributor

sdeleuze commented Aug 9, 2022

Could you please clarify where the Kotlinx serialization dependency come from? Declared directly in your project (for which use case) or via a third party dependency (which one)?

@sdeleuze sdeleuze added status: waiting-for-feedback We need additional information before we can continue and removed for: team-attention labels Aug 9, 2022
@martypitt
Copy link
Author

martypitt commented Aug 9, 2022

Sure.

In our spring boot app, we have the following:

<dependency>
   <groupId>org.jetbrains.kotlinx</groupId>
   <artifactId>kotlinx-serialization-cbor</artifactId>
</dependency>

We use this for serializing efficient messages between our own components.

Not sure if I've answered the question you're asking - please let me know if I can provide any other info.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 9, 2022
@sdeleuze
Copy link
Contributor

Yes you did, but our classpath detection is based on kotlinx.serialization.json.Json which is expected to be a class specific to kotlinx-serialization-json dependency. So I am not sure why kotlinx-serialization-cbor triggers it, could you please check on your project?

@sdeleuze sdeleuze added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Aug 10, 2022
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Aug 17, 2022
@spring-projects-issues
Copy link
Collaborator

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 24, 2022
@snicoll snicoll removed this from the Triage Queue milestone Sep 7, 2022
@volkert-fastned
Copy link

volkert-fastned commented Mar 7, 2024

In response to the following reply by @rstoyanchev :

Currently Kotlin serialization is based on classpath detection only. I'm just wondering, should it be on the classpath, and is there a way to exclude it? Or otherwise it would make sense to provide a way to disable it. @sdeleuze what do you think?

@martypitt, if you register a custom Encoder or Decoder (e.g. Jackson), it is ahead of default ones in the order, so that provides another option to influence the order.

@sdeleuze As you can see, I'm not the only one who's having problem with this default and unexpected classpath-dependent behavior. This is seems like the same problem I described in #32382 and #32384.

Could we maybe have a wider discussion about this problem? I would also very much appreciate some input here from your fellow developers. Thanks.

@volkert-fastned
Copy link

For anybody stumbling upon this thread through a search engine while looking for a solution to this problem in Spring Boot, see these answers for a workaround:

@rocketraman
Copy link
Contributor

I have the opposite problem -- Jackson is always used in favor of kotlinx-serialization. Man, Spring is... frustrating.

@hantsy
Copy link
Contributor

hantsy commented Sep 30, 2024

@rocketraman Check here, to use kotlinx serialization:

  • Add kotlinx-serialiazation-json to dependencies
  • Annotate your POJOs with @kotlinx.serialization.Serializable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support
Projects
None yet
Development

No branches or pull requests

9 participants