Skip to content
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

Unable to serialize top-level Java8 Stream #302

Open
sunruh opened this issue Jul 20, 2018 · 17 comments
Open

Unable to serialize top-level Java8 Stream #302

sunruh opened this issue Jul 20, 2018 · 17 comments
Labels
2.16 For issues planned for 2.16

Comments

@sunruh
Copy link

sunruh commented Jul 20, 2018

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
Copy link
Member

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 cowtowncoder added 2.10 good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project and removed 2.9 labels Oct 3, 2019
@cowtowncoder cowtowncoder added 2.11 and removed 2.10 labels Oct 3, 2019
@cowtowncoder
Copy link
Member

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 cowtowncoder added 2.12 and removed 2.11 labels May 9, 2020
@cowtowncoder cowtowncoder added 2.13 and removed 2.12 labels Nov 13, 2020
@cowtowncoder
Copy link
Member

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

@cowtowncoder cowtowncoder removed the 2.13 label Jul 25, 2022
@JooHyukKim
Copy link
Member

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

@pjfanning
Copy link
Member

@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
Copy link
Member

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

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

What do you think?

@pjfanning
Copy link
Member

pjfanning commented May 11, 2023

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
Copy link
Member

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
Copy link
Member

pjfanning commented May 11, 2023

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

@JooHyukKim
Copy link
Member

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
Copy link
Member

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

@pjfanning
Copy link
Member

pjfanning commented May 11, 2023

Did you register the Jdk8Module in your Stream test?

@JooHyukKim
Copy link
Member

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

@pjfanning
Copy link
Member

pjfanning commented May 11, 2023

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

@JooHyukKim
Copy link
Member

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

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

@JooHyukKim
Copy link
Member

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
Copy link
Member

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.

@cowtowncoder cowtowncoder added 2.16 For issues planned for 2.16 and removed good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project labels May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.16 For issues planned for 2.16
Projects
None yet
Development

No branches or pull requests

4 participants