Scala Map considered a PersistentEntity causing NullPointerException in UriToEntityConverter [DATAREST-857]

[spring-projects-issues]

Abhijit Sarkar opened DATAREST-857 and commented

I'm converting a pet project from Java to Scala. It uses the following frameworks:

• Spring Boot
• Spring HATEOAS
• Spring Data Rest
• Spring Data MongoDB
• Embedded MongoDB

I hit a wall with that at startup. org.springframework.data.rest.core.UriToEntityConverter constructor tries to look up an entry previous stored in field persistentEntities of class org.springframework.data.mapping.context.AbstractMappingContext with key as follows:

org.springframework.data.util.ParameterizedTypeInformation
--- resolvedType: scala.collection.immutable.Map
--- typeArguments: java.lang.String,java.lang.String

UriToEntityConverter loses the type info and just looks up by raw type scala.collection.immutable.Map, thus not finding the entry. It then, without checking for null, does a hasIdProperty and blows up in pieces.

The code is available here on branch feign-scala-boot. The integration test FeignSpecP1 is the one that fails.

Stacktrace:

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [org.springframework.format.support.DefaultFormattingConversionService]: Factory method 'defaultConversionService' threw exception; nested exception is java.lang.NullPointerException
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:189)
    at org.springframework.beans.factory.support.ConstructorResolver.instantiateUsingFactoryMethod(ConstructorResolver.java:588)
    ... 107 more
Caused by: java.lang.NullPointerException
    at org.springframework.data.rest.core.UriToEntityConverter.<init>(UriToEntityConverter.java:68)
    at org.springframework.data.rest.webmvc.config.RepositoryRestMvcConfiguration.uriToEntityConverter(RepositoryRestMvcConfiguration.java:593)
    at org.springframework.data.rest.webmvc.config.RepositoryRestMvcConfiguration.defaultConversionService(RepositoryRestMvcConfiguration.java:209)
    at org.springframework.boot.autoconfigure.data.rest.SpringBootRepositoryRestMvcConfiguration$$EnhancerBySpringCGLIB$$c2974a83.CGLIB$defaultConversionService$51(<generated>)
    at org.springframework.boot.autoconfigure.data.rest.SpringBootRepositoryRestMvcConfiguration$$EnhancerBySpringCGLIB$$c2974a83$$FastClassBySpringCGLIB$$334e15b3.invoke(<generated>)
    at org.springframework.cglib.proxy.MethodProxy.invokeSuper(MethodProxy.java:228)
    at org.springframework.context.annotation.ConfigurationClassEnhancer$BeanMethodInterceptor.intercept(ConfigurationClassEnhancer.java:356)
    at org.springframework.boot.autoconfigure.data.rest.SpringBootRepositoryRestMvcConfiguration$$EnhancerBySpringCGLIB$$c2974a83.defaultConversionService(<generated>)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:162)
    ... 108 more

---

Affects: 2.4.4 (Gosling SR4)

[spring-projects-issues]

Oliver Drotbohm commented

Thanks for filing this. Would you mind preparing a more focussed example? The test case is just a high level integration test nothing really testing UriToEntityConverter directly. There's just way too many things in between the problem and the code that you think is causing it.

What I am wondering is why a Map makes it's way into the PersistentEntities in the first place as it by no means really is an entity. We of course could guard against PersistentEntity being null here, but I'd love to get to the root cause of why that particular type appears as persistent entity. I guess that's stemming from the fact that we currently detect maps solely based on the java.util.Map type and Scala's Map is not implementing that, so that the Map handling doesn't even apply.

Looks like we need to fix that first then

[spring-projects-issues]

Abhijit Sarkar commented

The Map is a property of the persistent entity. Based on the following flow(I'm looking at ReflectionUtils.doWithFields), it appears to me that the fields are handled by design.

AbstractMappingContext.getPersistentEntity -> AbstractMappingContext.addPersistentEntity -> ReflectionUtils.doWithFields -> AbstractMappingContext.PersistentPropertyCreator.doWith(scala.collection.immutable.Map) -> AbstractMappingContext.createAndRegisterProperty -> AbstractMappingContext.addPersistentEntity

I agree with you that just checking for null is merely a patch fix.

BTW, this is really a blocking issue by all means. I'd think a major (to which the priority was dropped) is applicable when the issue is serious enough but there's a workaround.

Creating a example just to reproduce the NPE might take a while. I will, if you absolutely cannot proceed without a minimal example but I tried to provide as much details as I found to narrow down the problem area.

[spring-projects-issues]

Oliver Drotbohm commented

I think I understand the core culprit here, no need to take any further action.

Regarding the priority. That's the priority for us as the project. That bug is not fundamentally breaking the library. I guess most of our users stay with Java and the issue just doesn't appear there. I hate to break the news but as Scala's usually doing things differently you shouldn't be surprised about hickups in interaction with Java libraries. That said, I can see that it might be blocking you, but looking at from that point of view basically every ticket we get filed would be blocker (as people wouldn't have filed them if the supposed bug wasn't preventing them from doing something), which doesn't really help prioritizing our work :)

[spring-projects-issues]

Abhijit Sarkar commented

Can you elaborate on the root cause and when do you expect this to be fixed?

[spring-projects-issues]

Oliver Drotbohm commented

The root cause is Scala's ImmutableMap not implementing java.util.Map and thus, our special map handling not kicking in, which makes the metadata subsystem consider it an entity. I guess it's not an easy fix, as detecting the type is not enough — we need to be able to access its elements, create new instances of it. Parts of that are currently delegated to Spring Framework code. We'd either have to add that new behavior to our code, or get Spring Framework to implement it. Also, it feels like we're opening a can of worms here as a fix here would probably cause $otherJvmLanguage users to show up and basically ask for the same thing

[spring-projects-issues]

Abhijit Sarkar commented

Turns out that not only immutable.Map, but other Scala types are a problem too. Scala immutable.List, even Option. I converted all the persistent fields to Java types and the NPE went away. Apparently Spring and Scala are not friends. ConfigurationProperties fails, autowiring collection.Seq fails...the list goes on. I finally got the code working, but the POJOs are using Java types. That's a compromise I can live with.

I'd like to see more Scala compatibility for Spring, why waste such a good framework on Java alone. Spring Scala is sadly abandoned. I can always use native Scala frameworks like Play but what's the fun in that?

[spring-projects-issues]

Andrew Glassman commented

I just ran into this issue as well. I'm assuming because I have an entity being picked up as a PersistentEntity even though it is not intended to be. Note, I'm using pure Java.

My opinion is that this should be fixed regardless of the reasons it is happening. The JavaDoc for PersistentientEntities.getPersistentEntity(Class<?> type) explicitly states that null can be returned. At the very least, the result of entity.hasIdProperty() should be checked for null, then an exception should be thrown with a bit more context. An exception with more context would help the developer understand what happened, and get them to a solution or workaround faster.

Getting a NullPointerException thrown from an unexpected dependency when changing a seemingly unrelated portion of code is a big hurdle when coding. (Perfect Example: I'm reading this bug report rather than working on value added code for my project right now!)

I can create a pull request if desired for this. I can check for null, and throw an exception with more context if it is desired. I'm not sure what exception, or message would be appropriate though