Need support for Kotlin Iterables
pijusn opened this issue · 12 comments
I just implemented custom Iterable and Iterator classes which I tried passing into a (default) mapper. It serializes it as an object rather than an array. It does, however, serialize it as an array if java.lang.Iterable and java.util.Iterator are implemented.
Java and Kotlin Iterables are pretty much the same. I believe, this module should also add serializer for Kotlin iterables.
Can you tell me more about your use case, because under the covers there is no difference between a Kotlin class implementing the trait Iterator and a Java one implementing java.util.Iterator. The byte code is the same. Even with delegation, it is the same. Jackson cannot serialize a top-level object of unknown types as anything other than a bean unless you tell it otherwise. Many classes implement iterator but are not intended to only be an iterator. I tested in Java and in Kotlin with basically this code for both, and ended with exactly the same results. Is your use case different?
class IteratorTests {
class TinyPerson(val name: String, val age: Int)
class KotlinPersonIterator(private val personList: List<TinyPerson>) : Iterator<TinyPerson> by personList.iterator() {}
val mapper: ObjectMapper = jacksonObjectMapper().configure(SerializationFeature.INDENT_OUTPUT, false)
Test fun testKotlinIterator() {
val expected = """[{"name":"Fred","age":10},{"name":"Max","age":11}]"""
val people = KotlinPersonIterator(listOf(TinyPerson("Fred", 10), TinyPerson("Max", 11)))
val kotlinJson = mapper.writerFor<ObjectWriter>(object : TypeReference<Iterator<TinyPerson>>() {}).writeValueAsString(people)
assertThat(kotlinJson, equalTo(expected))
}
}When inside another class, same issue, needs to know how to serialize it:
class Company(val name: String, [JsonSerialize(`as` = javaClass<java.util.Iterator<TinyPerson>>())] val people: KotlinPersonIterator)
Test fun testKotlinIteratorAsField() {
val expected = """{"name":"KidVille","people":[{"name":"Fred","age":10},{"name":"Max","age":11}]}"""
val people = KotlinPersonIterator(listOf(TinyPerson("Fred", 10), TinyPerson("Max", 11)))
val company = Company("KidVille", people)
val kotlinJson = mapper.writeValueAsString(company)
assertThat(kotlinJson, equalTo(expected))
}Oh wait, the Java test does work as you describe, how odd. Checking more...
Note the same test in Java (this is why I'm so happy in Kotlin, so verbose!):
public class IteratorTestsInJava {
class TinyPerson {
private String name;
private int age;
public TinyPerson(String name, int age) {
this.name = name;
this.age = age;
}
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public int getAge() {
return age;
}
public void setAge(int age) {
this.age = age;
}
}
class JavaPersonIterator implements Iterator<TinyPerson> {
private List<TinyPerson> personList;
private Iterator<TinyPerson> personIterator;
public JavaPersonIterator(List<TinyPerson> personList) {
this.personList = personList;
this.personIterator = personList.iterator();
}
@Override
public boolean hasNext() {
return personIterator.hasNext();
}
@Override
public TinyPerson next() {
return personIterator.next();
}
@Override
public void remove() {
throw new NotImplementedException();
}
}
private ObjectMapper mapper = new ObjectMapper().configure(SerializationFeature.INDENT_OUTPUT, false);
@Test
public void testKotlinIterator() throws Exception {
String expected = "[{\"name\":\"Fred\",\"age\":10},{\"name\":\"Max\",\"age\":11}]";
JavaPersonIterator people1 = new JavaPersonIterator(Arrays.asList(new TinyPerson("Fred", 10), new TinyPerson("Max", 11)));
String javaJsonNoType = mapper.writeValueAsString(people1);
assertEquals(expected, javaJsonNoType);
JavaPersonIterator people2 = new JavaPersonIterator(Arrays.asList(new TinyPerson("Fred", 10), new TinyPerson("Max", 11)));
String javaJsonWithType = mapper.writerFor(new TypeReference<Iterator<TinyPerson>>() {
}).writeValueAsString(people2);
assertEquals(expected, javaJsonWithType);
}
class Company {
private String name;
private JavaPersonIterator people;
public Company(String name, JavaPersonIterator people) {
this.name = name;
this.people = people;
}
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public JavaPersonIterator getPeople() {
return people;
}
public void setPeople(JavaPersonIterator people) {
this.people = people;
}
}
@Test
public void testJavaIteratorAsField() throws Exception {
String expected = "{\"name\":\"KidVille\",[{\"name\":\"Fred\",\"age\":10},{\"name\":\"Max\",\"age\":11}]}";
JavaPersonIterator people = new JavaPersonIterator(Arrays.asList(new TinyPerson("Fred", 10), new TinyPerson("Max", 11)));
Company company = new Company("KidVille", people);
String javaJson = mapper.writeValueAsString(company);
assertEquals(expected, javaJson);
}
}Ah, the problem is in BeanSerializerFactory.java (Jackson) it has this check:
if (beanDesc.hasKnownClassAnnotations()) {
return builder.createDummy();
}
which changes the behaviour when it sees the KotlinClass annotation, it doesn't discriminate against what class annotations it sees... :-(
It does't call the annotation inspector to let it modify or filter the list. then it is used more blindly later.
@cowtowncoder any advice on this one? all Kotlin classes have a KotlinClass annotation with runtime-type information. So when it is seen the BeanSerializerFactory.java calling beanDesc.hasKnownClassAnnotations() returns true, it doesn't care that the annotation is some random thing it does not recognize.
Its good to have the annotation present later, because we use it when looking at constructors and such. But we don't want it counted as something that interferes with Jackson thinking it is a bean, or not. Not sure what the logic for looking at class annotations to create a dummy bean serializer was for, seems like that should be AFTER trying to see if it is an iterator or other known type.
(note the annotation inspector _hasAnnotation method doesn't help either because the check is if the collection of annotations size is > 0.
@PiusLT you have a workaround above by specifying how to serialize either the top level or a field, and I will work on resolving the issue as I can find a way to make it work more the same as java.
@jaysonminard as far as I remember, using @JacksonAnnotation does not have harmful side effects so it should be ok to add. But then again it should not be necessary.
I do recall a few issues with handling of Iterator, and the main problem is this: given class X that implementas Iterator, should it be serialized as Collection? Quite often it should not, as Iterator is commonly used as sort of add-on to augment processing, but the type is more of a POJO.
This is why there are various heuristics to try to reason right approach.
So this is the background on why such checks are done, instead of simply just using Iterator for class by default.
Given the complexity it may make sense for Kotlin module to try to detect these cases when providing Serializers, in its Serializers implementation (if it already registers one), and not relying on core databind.
This is something that may make sense to discuss on dev list. I remember having some trouble with Guava Iterators and/or iterables.
I'll look at how Guava handles it and can cover Kotlin iterators in the same way. Subclasses of the user's, that's another story.
Ok let me know, I may be able to help here. Iterators are nasty as they are essentially mix-ins, secondary interfaces, and quite often it's unclear if it's to be used as the serialization mechanism or not. Many POJOs implement it for convenience. This is the reason why it has such a low precedence if I recall correctly.
I am using Retrofit and Jackson converter. I cannot apply your fix there, as mapping code is incapsulated there.
When are you planning to release the correct fix?