jackson-module-kotlin icon indicating copy to clipboard operation
jackson-module-kotlin copied to clipboard

Issue 368: serialize Sequence as Iterable

Open juan-palacios opened this issue 5 years ago • 6 comments

Translating the Sequence into an Iterable and using the IterableSerializer. This will:

  • Include handling for WRITE_SINGLE_ELEM_ARRAYS_UNWRAPPED
  • Iterate over the elements in the Sequence and serialize them one by one

As explained on Issue 368, if WRITE_SINGLE_ELEM_ARRAYS_UNWRAPPED is enabled the first value of the Sequence will be retrieve twice, which means the Sequence will have to produce the first element twice.

juan-palacios avatar Sep 11 '20 04:09 juan-palacios

@juan-palacios Will review soon—would you fill out the CLA and email a scan/photo of the result to info at fasterxml dot com?

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

dinomite avatar Oct 20 '20 10:10 dinomite

Couple of notes: first, use of IterableSerializer for Sequence should work fine, I think. But there are some aspects of achieving this that can get tricky, esp. regarding handling of more complex included values.

First of all, lookup should occur earlier than from serialize(), both from correctness aspect (typing of contents not available any more) and from performance (there is lookup overhead). There are couple of places where this can be done:

  1. When locating serializer, from KotlinSerializers.kt. Has JavaType available, with generic type variables resolved (if available from property definition)
  2. From createContextual() method (need to declare as ContextualSerializer): can locate similarly to proposed code, although need to hold on the original type serializer was constructed with. One specific benefit here is that BeanProperty is available, allowing proper support of annotations on property that may affect serialization.

Second problem is use of findTypedValueSerializer(): this is usually meant to be used for root values only.

To find out issues it may be necessary to add tests that exercise values like POGOs, and especially polymorphic values.

cowtowncoder avatar Oct 20 '20 16:10 cowtowncoder

I think this could use more work in light of @cowtowncoder's comments

dinomite avatar Dec 01 '20 11:12 dinomite

I've been on leave but I intend to look into @cowtowncoder 's comments

juan-palacios avatar Dec 01 '20 22:12 juan-palacios

@juan-palacios Any update?

dinomite avatar Dec 19 '20 21:12 dinomite

Due to item copies and specialized sequences we use, we overrode the default serializer using the followin serialize method:

	override fun serialize(value: Sequence<*>, gen: JsonGenerator, provider: SerializerProvider) {
		gen.writeStartArray()

		value.forEach {
			provider.defaultSerializeValue(it, gen)
		}

		gen.writeEndArray()
	}

We tested this by replacing that with the following...note that we had to add a null for the property on findTypedValueSerializer for Jackson 2.13.1

	override fun serialize(value: Sequence<*>, gen: JsonGenerator, provider: SerializerProvider) {
		provider.findTypedValueSerializer(
			Iterable::class.java,
			true,
			null
		).serialize(
			value.asIterable(),
			gen,
			provider
		)
	}

We are looking forward to seeing better Sequence handling in Kotlin Jackson Module. We do see the benefit of using the iterable serializer to get the handling of unwrapped single values, etc. Is there any way others like us can support getting this into a release beyond the testing I described?

thecoden avatar Jan 23 '22 16:01 thecoden

I noticed I had implemented a similar idea in #675. This PR is closed.

k163377 avatar Jul 08 '23 11:07 k163377