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

Iterable serialization breaks when adding @JsonFilter annotation #2390

Closed
cmercer opened this issue Jul 25, 2019 · 9 comments
Closed

Iterable serialization breaks when adding @JsonFilter annotation #2390

cmercer opened this issue Jul 25, 2019 · 9 comments
Milestone

Comments

@cmercer
Copy link

cmercer commented Jul 25, 2019

Version: 2.9.9

If an class is Iterable it will serialize correctly with Jackson. However, if I apply a filter at all (even one that does no filtering at all) the Iterable class is turned into an JSON object and contains no data.

We were attempting to use a filter to filter responses back to the user using Spring. Spring PageImpl as a Sort object which is really an Iterable over Sort.Order. Once we applied the filter it broke the JSON for the Sort object.

I have removed all the Spring-ness from a demo project I will attach to this issue later today that shows the issue.

@cmercer
Copy link
Author

cmercer commented Jul 25, 2019

Here is a demo of the problem. There are two "wrapper" implementation one for a Collection and one for an Iterable. If no filter is applied they are treated the same and the JSON is what was expected. If you apply an filter at all Iterable breaks.

Original source https://github.com/cmercer/jackson-mixin-iterable

jackson-mixin-iterable.zip

@cowtowncoder
Copy link
Member

I think I can speculate on the root cause here...

The big problem with Iterable interface is that it is an add-on interface and it typically can not be relied to determine "main" function of the value: for example, many POJOs also include Iterable for convenience; and thereby serializing only "as iterable" would result in functionality users find essentially wrong.

Because of this, there is simple heuristic: if no logical properties are found to allow serialization as POJO, then (and only then) are add-on/tag interfaces like:

  • Iterator
  • Iterable
  • CharSequence

used to serialize values according to these interfaces.

So what is likely happening is that wrapper types expose at least one "getter" method, and as a result they are serialized as POJOs, and not just as Iterable collections of things.

Note, however, that same limitation does not apply to Collection or Map: these types are taken as the "primary" type.

As to how the specific problem (of forcing wrapper Iterables to be considered "just Iterables"), I am not sure how that could be tackled without breaking other use cases.
One way could be to add a MapperFeature to allow more aggressive introspection of Iterables (and probably Iterators), but that seems like a bit too granular and specific thing to handle as a feature.

@cmercer
Copy link
Author

cmercer commented Jul 26, 2019

Iterables are handled as you say above (and correctly) in the normal/no filter applied cases.
The key difference here is that Iterable test cases work fine UNLESS I apply a filter. When the filter, even the simplest filter, is applied it breaks which points to something in that code path that is working differently than "normal".

I am adding links below to make it easy on your time.
IterableWrapper which only exposes the iterator() method.
https://github.com/cmercer/jackson-mixin-iterable/blob/master/src/main/java/com/github/jackson/mixin/iterable/IterableWrapper.java

This is the point where the two different ObjectMapper instances are created.
https://github.com/cmercer/jackson-mixin-iterable/blob/master/src/test/java/com/github/jackson/mixin/iterable/PersonTests.java#L26

Here is where the filters get applied (only to one of the object mappers)
https://github.com/cmercer/jackson-mixin-iterable/blob/master/src/test/java/com/github/jackson/mixin/iterable/PersonTests.java#L68

This is the specific test case that fails.
https://github.com/cmercer/jackson-mixin-iterable/blob/master/src/test/java/com/github/jackson/mixin/iterable/PersonTests.java#L48

The results look like the following
No filter applied
{"name":"Jim Henson","pets":["Kermit the Frog","Miss Piggy","Sam the Eagle"]}
Filter applied
{"name":"Jim Henson","pets":{}}

@cowtowncoder
Copy link
Member

Ok thank you. I'll need to dig bit deeper then it sounds.... I'll see if I can reproduce this with more info.

@cowtowncoder
Copy link
Member

Ahhhh. So filters here are Jackson filter functionality and... ok. Should have read original notes with more thought. Does not necessarily explain everything but gives some hint.

@cowtowncoder
Copy link
Member

Odd. I can see BeanSerializer being constructed in some cases, instead of returning null... despite no properties being discovered.

@cowtowncoder
Copy link
Member

Ah ha. Found it. So... BeanSerializerFactory properly recognizes no-properties case, does not construct real BeanSerializer. But. In attempt to be... helpful :) ... checks for whether there are "known annotations" which is taken to indicate that we should not fail with "no properties" found. So around line 441 we have:

        if (ser == null) {
            // If we get this far, there were no properties found, so no regular BeanSerializer
            // would be constructed. But, couple of exceptions.
            // First: if there are known annotations, just create 'empty bean' serializer
            if (beanDesc.hasKnownClassAnnotations()) {
                return builder.createDummy();
            }
        }

(method called from findBeanSerializer() on line 279 (branch 2.10))

And in this case, then, it's just that mix-in applied @JsonFilter that inadvertently triggers the condition.

So that is unfortunate. Question then is how to prevent creation of this "helpful" serializer in least unclean fashion. I don't want to add specific check for Iterable here, but may be able to reorder code a bit.
But first will need to simplify the test a bit to reproduce.

@cmercer
Copy link
Author

cmercer commented Jul 29, 2019

Thanks again for digging into this, I know it is an extreme edge case.

@cowtowncoder
Copy link
Member

@cmercer Now that I understand it correctly I think it's perfectly valid bug, and not necessarily super rare (although obv. given it is only now reported is not completely common either :) ).

So thank you again for reporting it.

@cowtowncoder cowtowncoder changed the title Iterable breaks when applying a filter Iterable serialization breaks when adding @JsonFilter annotation Aug 7, 2019
@cowtowncoder cowtowncoder added this to the 2.10.0.pr2 milestone Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants