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

Add support for Spring boot 2.5+/Spring data 2021.0 #1333

Closed
ask4gilles opened this issue Aug 23, 2021 · 24 comments · Fixed by #1336
Closed

Add support for Spring boot 2.5+/Spring data 2021.0 #1333

ask4gilles opened this issue Aug 23, 2021 · 24 comments · Fixed by #1336
Assignees
Labels
Milestone

Comments

@ask4gilles
Copy link

Hi,

I just tried 1.6.1 together with SB 2.5.4 and got several issues regarding java.time module not added in the entityViewObjectMapper and exceptions regarding generated queries.

Do you plan to support this latest version?
Thanks!

@jwgmeligmeyling
Copy link
Collaborator

jwgmeligmeyling commented Aug 25, 2021

The issues regarding the java.time module actually sound like Java 11 module issues rather than Spring Boot 2.5 issues. Are you sure its Spring Boot 2.5 that is causing the issues? Could you also please list one of the stacktraces being generated while executing a query?

We fixed the compatibility for 2.3.x (#1100), 2.4.x (#1223) and even a minor change for 2.4.3 (#1267) so I am sure any 2.5.x issue will be resolved.

@ask4gilles
Copy link
Author

ask4gilles commented Aug 26, 2021

Hey @jwgmeligmeyling,
Thanks for your comment.

	at com.blazebit.persistence.parser.PathTargetResolvingExpressionVisitor.visit(PathTargetResolvingExpressionVisitor.java:243)
	at com.blazebit.persistence.parser.expression.PropertyExpression.accept(PropertyExpression.java:41)
	at com.blazebit.persistence.parser.PathTargetResolvingExpressionVisitor.visit(PathTargetResolvingExpressionVisitor.java:336)
	at com.blazebit.persistence.parser.expression.PathExpression.accept(PathExpression.java:85)
	at com.blazebit.persistence.impl.JpaUtils.getAttributeForJoining(JpaUtils.java:476)
	at com.blazebit.persistence.impl.JoinManager.createOrUpdateNode(JoinManager.java:3619)
	at com.blazebit.persistence.impl.JoinManager.implicitJoinSingle(JoinManager.java:3543)
	at com.blazebit.persistence.impl.JoinManager.implicitJoin(JoinManager.java:3459)
	at com.blazebit.persistence.impl.JoinManager.implicitJoin(JoinManager.java:3285)
	at com.blazebit.persistence.impl.JoinManager.implicitJoin(JoinManager.java:2682)
	at com.blazebit.persistence.impl.JoinVisitor.visit(JoinVisitor.java:268)
	at com.blazebit.persistence.impl.JoinVisitor.visit(JoinVisitor.java:219)
	at com.blazebit.persistence.parser.expression.PathExpression.accept(PathExpression.java:85)
	at com.blazebit.persistence.impl.JoinVisitor.removeAssociationIdIfPossible(JoinVisitor.java:444)
	at com.blazebit.persistence.impl.JoinVisitor.visit(JoinVisitor.java:376)
	at com.blazebit.persistence.parser.predicate.EqPredicate.accept(EqPredicate.java:57)
	at com.blazebit.persistence.parser.expression.VisitorAdapter.visit(VisitorAdapter.java:223)
	at com.blazebit.persistence.parser.predicate.CompoundPredicate.accept(CompoundPredicate.java:77)
	at com.blazebit.persistence.impl.PredicateManager.acceptVisitor(PredicateManager.java:246)
	at com.blazebit.persistence.impl.AbstractCommonQueryBuilder.applyImplicitJoins(AbstractCommonQueryBuilder.java:2664)
	at com.blazebit.persistence.impl.AbstractCommonQueryBuilder.prepareAndCheck(AbstractCommonQueryBuilder.java:3586)
	at com.blazebit.persistence.impl.AbstractCommonQueryBuilder.getBaseQueryStringWithCheck(AbstractCommonQueryBuilder.java:2803)
	at com.blazebit.persistence.impl.AbstractCommonQueryBuilder.getTypedQuery(AbstractCommonQueryBuilder.java:2818)
	at com.blazebit.persistence.impl.AbstractQueryBuilder.getQuery(AbstractQueryBuilder.java:52)
	at com.blazebit.persistence.spring.data.base.query.AbstractPartTreeBlazePersistenceQuery$QueryPreparer.createQuery0(AbstractPartTreeBlazePersistenceQuery.java:243)
	at com.blazebit.persistence.spring.data.base.query.AbstractPartTreeBlazePersistenceQuery$QueryPreparer.createQuery(AbstractPartTreeBlazePersistenceQuery.java:219)
	at com.blazebit.persistence.spring.data.base.query.AbstractPartTreeBlazePersistenceQuery$QueryPreparer.createQuery(AbstractPartTreeBlazePersistenceQuery.java:437)
	at com.blazebit.persistence.spring.data.base.query.AbstractPartTreeBlazePersistenceQuery.doCreateQuery(AbstractPartTreeBlazePersistenceQuery.java:161)
	at com.blazebit.persistence.spring.data.impl.query.PartTreeBlazePersistenceQuery.doCreateQuery(PartTreeBlazePersistenceQuery.java:194)
	at org.springframework.data.jpa.repository.query.AbstractJpaQuery.createQuery(AbstractJpaQuery.java:227)
	at org.springframework.data.jpa.repository.query.JpaQueryExecution$CollectionExecution.doExecute(JpaQueryExecution.java:126)
	at org.springframework.data.jpa.repository.query.JpaQueryExecution.execute(JpaQueryExecution.java:88)
	at org.springframework.data.jpa.repository.query.AbstractJpaQuery.doExecute(AbstractJpaQuery.java:155)
	at org.springframework.data.jpa.repository.query.AbstractJpaQuery.execute(AbstractJpaQuery.java:143)
	at org.springframework.data.repository.core.support.RepositoryMethodInvoker.doInvoke(RepositoryMethodInvoker.java:137)
	at org.springframework.data.repository.core.support.RepositoryMethodInvoker.invoke(RepositoryMethodInvoker.java:121)
	at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.doInvoke(QueryExecutorMethodInterceptor.java:159)
	at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.invoke(QueryExecutorMethodInterceptor.java:138)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.invoke(DefaultMethodInvokingMethodInterceptor.java:80)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at com.blazebit.persistence.spring.data.repository.EntityViewReplacingMethodInterceptor.invoke(EntityViewReplacingMethodInterceptor.java:51)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:123)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:388)
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:137)
	... 96 common frames omitted```

@beikov beikov added this to the 1.6.2 milestone Aug 26, 2021
@beikov
Copy link
Member

beikov commented Aug 26, 2021

Hi @ask4gilles,

could you please debug to AbstractPartTreeBlazePersistenceQuery.java:243 and post the result of getQueryString() here? If this worked before, I suppose some Spring Data JPA internal behavior changed again that might be the cause for this 😞

It would be awesome if you could provide a reproducer for this. You can use the maven archetype quickstart for this purpose if you want and post the project here:

mvn archetype:generate "-DarchetypeGroupId=com.blazebit" "-DarchetypeArtifactId=blaze-persistence-archetype-spring-data-sample" "-DarchetypeVersion=1.6.1"

@ask4gilles
Copy link
Author

Hi @beikov ,

I'm digging into the AbstractPartTreeBlazePersistenceQuery, and the first thing that strikes me is that the entityViewClass is always null (and I'm using the entity view API)
So, I reach line 243 cb.getQuery(), but should'nt I reach the other branch instead?
Isn't it the root cause?

@beikov
Copy link
Member

beikov commented Aug 26, 2021

Sounds like there is something wrong with the integration and the new Spring Data version then. Unfortunately, I don't think I can give you any workaround for now other than sticking to 2.4 for the time. I'll definitely look into this in the coming days though for the next 1.6 release.

@beikov beikov self-assigned this Aug 26, 2021
@ask4gilles
Copy link
Author

Indeed, I can see here
That even though my view is present in the managedViews for this class, it returns null
I'll try to downgrade spring-data-jpa to 2.4.x an retest, hoping that this won't give other side effects.

@beikov
Copy link
Member

beikov commented Aug 26, 2021

Maybe there is some class loader nonesense going on. Did you try to disable the devtools yet? https://persistence.blazebit.com/documentation/1.6/entity-view/manual/en_US/#spring-boot-devtools

@ask4gilles
Copy link
Author

I'm not using it...

@beikov
Copy link
Member

beikov commented Aug 26, 2021

Hmm, can you check if maybe the classloader is different from the class that is passed vs. the one within the map?

@ask4gilles
Copy link
Author

It's the same one

@ask4gilles
Copy link
Author

The issue is that the class used to retrieve from the managedViews map is the actual entity and the key from the map is the entityView interface. If I manually set the FQDN of the view as domainType, then it finds it...

@beikov
Copy link
Member

beikov commented Aug 26, 2021

I see, well in that case there is definitely something messed up with the integration in 2.5. Thanks for all the insights. That will be a great help when I work on this.

@ask4gilles
Copy link
Author

FYI, I tried to downgrade spring data.
The latest version for which it works is:
spring-data-bom.version: 2020.0.5 (spring-data.version:2.4.5) OK
Starting with:
spring-data-bom.version: 2020.0.6 (spring-data.version:2.4.6) KO
I've the same behaviour.
There is, I think, only this fix spring-projects/spring-data-jpa#2111 between both.
Hope it helps...

@beikov
Copy link
Member

beikov commented Aug 26, 2021

This really helps fixing this issue fast, so thanks a lot for the analysis :)

@EugenMayer
Copy link
Contributor

This is not a BP issue at all.

The issues is that Jackson has been updated in SB 2.5 and this jackson version no longer allows silent fallbacks if a type could not have been loaded. This does just uncover missing jackson type serialization/deserialization issues which have been there before 2.4 but have been masked by Jackson.

Mostly you issue here is entirely SB setup related:

  • ensure you use the spring boot DI based objectMapper so you get the auto-configurated one by SB
  • be sure to use public Jackson2ObjectMapperBuilderCustomizer jsonCustomizer() and nothing else so you do no override SB default configuration (which includes the support for java time)

We are using BP 1.6.1 with SB 2.5 since 2.5 was released, including Java time and we had to fix a couple of issues with non DI based objectMappers leading to this

@ask4gilles
Copy link
Author

Thanks for your comment @EugenMayer ,
Never mind about the time issue, I had it once but was not able to reproduce.
This one is mainly regarding spring-data-jpa compatibility as discussed above.
For the rest, it's working fine indeed.

@beikov
Copy link
Member

beikov commented Aug 27, 2021

Hey @ask4gilles, do you think you can create a reproducer for this? I can't reproduce this by just updating the version in the testsuite unfortunately.

@ask4gilles
Copy link
Author

That’s already a good news for you!
I’ll try to do that next week yes. I’ll also provide you with a deps tree.

beikov added a commit to beikov/blaze-persistence that referenced this issue Aug 28, 2021
beikov added a commit to beikov/blaze-persistence that referenced this issue Aug 28, 2021
@ask4gilles
Copy link
Author

Hi @beikov

First, sorry to comment on a closed issue.

In fact, regarding the java.time issue, it is well present, but I should have mention that the issue lies in the spring-hateoas-webmvc module where a new ObjectMapper bean is created at different places without the JavaTimeModule registered...
So, even though in my configuration I don't create ObjectMapper and I define a JavaTimeModule bean,
when performing the keyset pagination with hateoas, I get the following exception:

- com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Java 8 date/time type `java.time.Instant` not supported by default: add Module "com.fasterxml.jackson.datatype:jackson-datatype-jsr310" to enable handling
java.lang.RuntimeException: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Java 8 date/time type `java.time.Instant` not supported by default: add Module "com.fasterxml.jackson.datatype:jackson-datatype-jsr310" to enable handling
        at com.blazebit.persistence.spring.hateoas.webmvc.KeysetAwarePagedResourcesAssembler.serialize(KeysetAwarePagedResourcesAssembler.java:231)
        at com.blazebit.persistence.spring.hateoas.webmvc.KeysetAwarePagedResourcesAssembler.createLink(KeysetAwarePagedResourcesAssembler.java:195)
        at com.blazebit.persistence.spring.hateoas.webmvc.KeysetAwarePagedResourcesAssembler.addPaginationLinks(KeysetAwarePagedResourcesAssembler.java:160)
        at com.blazebit.persistence.spring.hateoas.webmvc.KeysetAwarePagedResourcesAssembler.createModel(KeysetAwarePagedResourcesAssembler.java:138)
        at com.blazebit.persistence.spring.hateoas.webmvc.KeysetAwarePagedResourcesAssembler.toModel(KeysetAwarePagedResourcesAssembler.java:88)
        at com.blazebit.persistence.spring.hateoas.webmvc.KeysetAwarePagedResourcesAssembler.toModel(KeysetAwarePagedResourcesAssembler.java:80)
[...]

If I downgrade jackson to 2.11.4 (still with BP 1.6.1), then it's fine.
Do you see any other workaround for now?

@beikov
Copy link
Member

beikov commented Sep 3, 2021

I can look at it tomorrow and I plan to do a release this weekend anyway, so maybe you don't need a workaround :)

Do you have an idea how this could be fixed?

@ask4gilles
Copy link
Author

Wow, it would be awesome since we had to cancel today's release because of that :/
I think that we should either just rely on the SB autoconfigured ObjectMapper either register the time module when you instantiate it:

objectMapper.registerModule(new JavaTimeModule());

@beikov
Copy link
Member

beikov commented Sep 4, 2021

Let's move the discussion to this issue: #1348

@beikov
Copy link
Member

beikov commented Sep 4, 2021

Would be cool if you could checkout the following PR and let me know if that works for you @ask4gilles: #1347

@ask4gilles
Copy link
Author

@beikov sure, I’ll have a look at it tomorrow !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants