Skip serialization of `groovy.lang.MetaClass` values to avoid `StackOverflowError`

[chashnikov]

Try serializing empty Groovy class using Groovy 3.0.9 and jackson-jr-objects:2.13.1:

import com.fasterxml.jackson.jr.ob.JSON
import groovy.transform.CompileStatic

class MyBean {}

@CompileStatic
class JsonTest {
    static void main(String[] args) {
        println JSON.std.asString(new MyBean())
    }
}

It'll fail with StackOverflowError:

	at com.fasterxml.jackson.jr.ob.impl.JSONWriter.writeBeanValue(JSONWriter.java)
	at com.fasterxml.jackson.jr.ob.impl.BeanWriter.writeValue(BeanWriter.java)
	at com.fasterxml.jackson.jr.ob.impl.JSONWriter._writeValue(JSONWriter.java)
	at com.fasterxml.jackson.jr.ob.impl.JSONWriter.writeBeanValue(JSONWriter.java)
	at com.fasterxml.jackson.jr.ob.impl.BeanWriter.writeValue(BeanWriter.java)
	at com.fasterxml.jackson.jr.ob.impl.JSONWriter._writeValue(JSONWriter.java)
	at com.fasterxml.jackson.jr.ob.impl.JSONWriter.writeBeanValue(JSONWriter.java)
	at com.fasterxml.jackson.jr.ob.impl.BeanWriter.writeValue(BeanWriter.java)

This happens because every class file generated from Groovy class contains getMetaClass method, which contains data with meta-information with cyclic references inside. The method is marked with @java.beans.Transient annotation, so if serialization automatically skips such properties, the problem will be solved.

[cowtowncoder]

I think it is possible for users to implement something like this, similar to jr-annotation-support.

Now, this could make useful addition to jr-annotation-support itself, but there is one practical problem: annotation was added in JDK 7, and jackson-jr only requires JDK 6 as of Jackson 2.13. We could consider increasing baseline to JDK 8 in which case it'd be possible to add support.

If anyone wanted to provide a PR for addition here, I'd be happy to help to see if JDK baseline can be improved.

[chashnikov]

annotation was added in JDK 7, and jackson-jr only requires JDK 6 as of Jackson 2.13

It's possible to check if an element is annotated with Transient without referencing the class, you may just iterate over Method::getDeclaredAnnotations and compare annotation.annotationType().getName() with "java.beans.Transient" string.

[cowtowncoder]

True, there are ways around that. And although it's bit brittle (relying on names) it might be fine.
The other possible challenge is testing: is it possible for Maven project to require Java 8 for tests, but not for main code?
If so, that sounds good.

Oh. However... there is a good reason for NOT adding dependency to that class at all: it will bring big non-core Beans module for JDK 9+. So maybe by-name check is the proper way to go.

So, I'd be open for addition if anyone has time to work on PR.

I would also suggest adding a feature to enable/disable recognition of this annotation: I fully expect someone somewhere to NOT want ignoral. This based on adding similar features in the past.

[cowtowncoder]

Quick note: JDK 8 has been baseline since 2.14 (as per #95) so this could just be added now.

[cowtowncoder]

I think this would be good thing to have PR for; marking as such.

[Shounaks]

Okay, this might sound a bad programming practice, but I noticed skipping over groovy MetaClassImpl to be a better alternative than checking for annotations. (I tried first & didnt work).

Adding a simple line of code can fix this.

currType.getName().equals("groovy.lang.MetaClassImpl")

but since this is not optimal solution , i would like to hear your thoughts @cowtowncoder ? Do we really require groovy class metadata?

[cowtowncoder]

I think core jackson-databind does something like this to automatically ignore this problematic type (in... src/main/java/com/fasterxml/jackson/databind/util/BeanUtil.java I think).
So while it's not super-clean approach, I think in this particular case it makes lots of sense.

So I think a PR for such patch would make sense.

One thing that'd be great would be unit test showing this in action, but not sure how easy that'd be to do.