Unable to serialize top-level Java8 Stream

sunruh commented 6 years ago

A Stream nested in another object is serialized using a value wrapper just as a collection or array, serializing a Stream directly fails when more than one element is available as it tries to write a second root. If the Stream emits only a single element serialization works but without wrapping:

public class StreamTest {
    private static final ObjectMapper OBJECT_MAPPER = new XmlMapper()
            .registerModule(new Jdk8Module());

    private static class StreamContainer {
        @JsonProperty
        private Stream<String> stream;
        public StreamContainer(Stream<String> stream) { this.stream = stream; }
    }

    @Test
    public void testTopLevelOneElement() throws JsonProcessingException {
        assertNotEquals("<Head>a</Head>",
                OBJECT_MAPPER.writeValueAsString(Stream.of("a")));
    }

    @Test
    public void testTopLevelTwoElements() throws JsonProcessingException {
        try {
            assertEquals("<Head><item>a</item><item>b</item></Head>",
                    OBJECT_MAPPER.writeValueAsString(Stream.of("a", "b")));
        } catch (JsonMappingException e) {
            fail("Trying to output second root?", e);
        }
    }

    @Test
    public void testNestedOneElement() throws JsonProcessingException {
        assertEquals("<StreamContainer><stream>a</stream></StreamContainer>",
                OBJECT_MAPPER.writeValueAsString(new StreamContainer(Stream.of("a"))));
    }

    @Test
    public void testNestedTwoElements() throws JsonProcessingException {
        assertEquals("<StreamContainer><stream>a</stream><stream>b</stream></StreamContainer>",
                OBJECT_MAPPER.writeValueAsString(new StreamContainer(Stream.of("a", "b"))));
    }
}

The root element name is Head for Stream serialization as it's determined from the actual class name ReferencePipeline$Head.

The cause lies in TypeUtil.isIndexedType(Class<?>) which just checks for Arrays and Collections and only if this returns true, a field name is written (and _nextName set).

It works for the nested Stream as _nextName is set due to the Stream being written as a field (multiple times).

cowtowncoder commented 5 years ago

Hmmh. Yes, so the work-around(s) used for Collections will not apply to Streams. I suspect same problem would apply to Iterators / Iterables as well... although in most cases objects probably implement Collection.

It'd be nice to figure out something more general for iterable types (since they are not quite what CollectionLikeType covers unfortunately), but on short term (2.10, maybe even 2.9) I guess added check might be acceptable.

cowtowncoder commented 4 years ago

Realized that support probably requires upgrade of JDK baseline for XML module to be Java 8 -- something that is probably reasonable for 2.11, but can not be done in a patch for 2.10.

cowtowncoder commented 2 years ago

At this point Java 8 requirement is no longer blocker, fwtw.

JooHyukKim commented 1 year ago

May I ask what the expected behavior is here? Serialization of both Iterable and Stream interfaces types just like how Collection works?

pjfanning commented 1 year ago

@JooHyukKim I would expect that Streams should be treated similarly to an Iterable - like a Collection but ideally without pulling the full Iterable or Stream into memory at one time.

JooHyukKim commented 1 year ago

I would expect that Streams should be treated similarly to an Iterable - like a Collection but ideally without pulling the full Iterable or Stream into memory at one time.

@pjfanning Hmm, what I can think of, to acheive the goal here is to obtain an instance of Iterator via iterator() method from both Iterable and Stream interface, then serialize the iterator instance.

By using iterator, we can

handle "like" a Collection (since collection also implements iterator) --note "like" because not 100% as Collection
leave memory optimization upto the implementation side.

What do you think?

pjfanning commented 1 year ago

Feels like the TypeUtil method should be changed to something like

    public static boolean isIndexedType(Class<?> cls)
    {
        return (cls.isArray() && cls != byte[].class && cls != char[].class)
                || Iterable.class.isAssignableFrom(cls) || Iterator.class.isAssignableFrom(cls) || Stream.class.isAssignableFrom(cls);
    }

Or it might be possible to get access to the TypeFactory in order to get a JavaType for the class.

I would suggest adding the broken test cases and seeing if the first approach above is enough to fix it.

JooHyukKim commented 1 year ago

Feels like the TypeUtil method should be changed to something like
    public static boolean isIndexedType(Class<?> cls)
    {
        return (cls.isArray() && cls != byte[].class && cls != char[].class)
                || Iterable.class.isAssignableFrom(cls) || Stream.class.isAssignableFrom(cls);
    }
Or it might be possible to get access to the TypeFactory in order to get a JavaType for the class.

I would suggest adding the broken test cases and seeing if the first approach above is enough to fix it.

I did try exactly that with TypeUtil. But only works for Iteror.

pjfanning commented 1 year ago

try getting an iterator for each type then if that seems to be a viable solution
you also need to support arrays

JooHyukKim commented 1 year ago

Modification

TypeUtil : Iterator, not Iterable

public static boolean isIndexedType(Class<?> cls)
    {
        return (cls.isArray() && cls != byte[].class && cls != char[].class)
                || Collection.class.isAssignableFrom(cls) || Iterator.class.isAssignableFrom(cls) || Stream.class.isAssignableFrom(cls);
    }

Used Test case :

only last case fails serializing Stream directly and the error message is (check bottom)


    public void testElement() throws JsonProcessingException {
        List<String> list = new ArrayList<>();
        list.add("a");
        list.add("b");
        assertEquals("<ArrayList><item>a</item><item>b</item></ArrayList>",
            OBJECT_MAPPER.writeValueAsString(list));
    }

    public void testElem() throws JsonProcessingException {
        List<String> list = new ArrayList<>();
        list.add("a");
        list.add("b");
        assertEquals("<Itr><item>a</item><item>b</item></Itr>",
            OBJECT_MAPPER.writeValueAsString(list.iterator()));
    }

    public void testopLevelTwoElements() throws JsonProcessingException {
        assertEquals("<Adapter><item>a</item><item>b</item></Adapter>",
            OBJECT_MAPPER.writeValueAsString(Stream.of("a", "b").iterator()));
    }

    public void testTopLevelTwoElements() throws JsonProcessingException {
        OBJECT_MAPPER.writeValueAsString(Stream.of("a", "b"));
    }

Direct Stream serialization Error message

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid type definition for type `java.util.stream.ReferencePipeline$Head`: Failed to construct BeanSerializer for [simple type, class java.util.stream.ReferencePipeline$Head]: (java.lang.IllegalArgumentException) Failed to call `setAccess()` on Method 'isParallel' (of class `java.util.stream.AbstractPipeline`) due to `java.lang.reflect.InaccessibleObjectException`, problem: Unable to make public final boolean java.util.stream.AbstractPipeline.isParallel() accessible: module java.base does not "opens java.util.stream" to unnamed module @1068e947

    at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:72)
    at com.fasterxml.jackson.databind.SerializerProvider.reportBadTypeDefinition(SerializerProvider.java:1284)
    at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.constructBeanOrAddOnSerializer(BeanSerializerFactory.java:475)
    at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.findBeanOrAddOnSerializer(BeanSerializerFactory.java:295)
    at com.fasterxml.jackson.databind.ser.BeanSerializerFactory._createSerializer2(BeanSerializerFactory.java:240)
    at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.createSerializer(BeanSerializerFactory.java:174)
    at com.fasterxml.jackson.databind.SerializerProvider._createUntypedSerializer(SerializerProvider.java:1507)
    at com.fasterxml.jackson.databind.SerializerProvider._createAndCacheUntypedSerializer(SerializerProvider.java:1455)
    at com.fasterxml.jackson.databind.SerializerProvider.findValueSerializer(SerializerProvider.java:556)
    at com.fasterxml.jackson.databind.SerializerProvider.findTypedValueSerializer(SerializerProvider.java:834)
    at com.fasterxml.jackson.dataformat.xml.ser.XmlSerializerProvider.serializeValue(XmlSerializerProvider.java:106)
    at com.fasterxml.jackson.databind.ObjectMapper._writeValueAndClose(ObjectMapper.java:4719)
    at com.fasterxml.jackson.databind.ObjectMapper.writeValueAsString(ObjectMapper.java:3964)
    at com.fasterxml.jackson.dataformat.xml.ser.Jdk8StreamSerialization302Test.testTopLevelOneElement(Jdk8StreamSerialization302Test.java:26)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at junit.framework.TestCase.runTest(TestCase.java:177)
    at junit.framework.TestCase.runBare(TestCase.java:142)

Further works (if agreed, I will push through)

Getting iterator instance internally
Tag names for iterators -- instead of <Adapter>, should be <Stream> (Atm, I certainly have no idea on which part in the project to touch)

@pjfanning May I ask for you thoughts?

pjfanning commented 1 year ago

use Iterable instead of Collection - Collection is a subclass of Iterable

pjfanning commented 1 year ago

Did you register the Jdk8Module in your Stream test?

JooHyukKim commented 1 year ago

@pjfanning no, just plain init. final ObjectMapper OBJECT_MAPPER = new XmlMapper();Hold on lemme share link to the test class branch.

pjfanning commented 1 year ago

jackson needs Jdk8Module to support Streams - see https://github.com/FasterXML/jackson-modules-java8

JooHyukKim commented 1 year ago

use Iterable instead of Collection - Collection is a subclass of Iterable

Hmm, using Iterable, the generator tries to write multiple roots :/

JooHyukKim commented 1 year ago

jackson needs Jdk8Module to support Streams - see https://github.com/FasterXML/jackson-modules-java8

@pjfanning Oh, thank you! 🙏🏼🙏🏼 I missed to consider backward compatibility and just simply thought it would all work since baseline jdk is raised to 8.

JooHyukKim commented 1 year ago

I will try to tidy up everything that has come up in convos from this issue and #329, then try to come up with something.

FasterXML / jackson-dataformat-xml