This adds the auto-configuration for Spring Data Envers.
Fixes #21370.
Comment From: scordio
I am not sure how to address testing properly in JpaRepositoriesAutoConfigurationTests. My preference would have been something like:
assertThat(context).getBean(CityRepository.class)
.extracting(AopProxyUtils::getSingletonTarget)
.isInstanceOf(EnversRevisionRepositoryImpl.class); // fails as it's a SimpleJpaRepository.class
but it doesn't work as the bean is always a SimpleJpaRepository.class although with the debugger I can see EnversRevisionRepositoryImpl being used. Any suggestion for a proper assertion?
Otherwise, I could add a CityRevisionRepository and perform verification on that, for both cases when Spring Data Envers is in the classpath or it is not.
@schauder may I ask your input on this?
Comment From: scordio
@wilkinsona sorry for the slow feedback, the PR should now be ready for review. I am wondering if some more test cases could be added, I will continue thinking about that.
I see the build is failing but I guess not because of my changes. I will keep an eye on master to see if a rebase if needed.
Comment From: scordio
I finished to refactor the tests the way I had in mind and I don't plan to have any other change. Please let me know if there is something to be adjusted.
(the build is failing but it doesn't seem related to my changes)
Comment From: scordio
Hi everyone, is there anything else I could do to move this one forward?
Comment From: snicoll
@scordio thank you for your patience and sorry we didn't have the time to get back to you yet. We're busy handling the scheduled backlog for 2.4.x so it may take a bit more time before we can spend time on this one.
Comment From: scordio
@snicoll thanks for the feedback and no problem, it was just a friendly reminder 🙂
Comment From: scordio
Hi @snicoll, just wanted to check if this is blocked for any reason and if I can help with it.
Comment From: snicoll
Hello @scordio thank you for your patience. I did review the proposal during the holidays and wanted to experiment how we could avoid having to subclass JpaRepositoriesRegistrar.
In retrospect, I should have written that down to get some feedback from the team. I've added that to my todo.
Comment From: scordio
I deleted my previous comment, I mixed up different things. Thanks for looking into it!
Comment From: scordio
Out of curiosity, what concerns do you have about subclassing? I had the assumption that Jpa is a prerequisite to use spring-data-envers, therefore any configuration applied by the JpaRepositoriesRegistrar should be also applied with the new one.
Comment From: wilkinsona
I have just discovered that there's no reference documentation for Spring Data Envers. Unfortunately we can't afford the support burden of an undocumented project so I'm marking this one as blocked.
Comment From: scordio
Got it, thanks for looking into it. I'll get in touch with the project to understand if they are willing to improve the docs and accept some help.
Comment From: scordio
Hello team, @schauder has taken care of the Spring Data Envers reference documentation (see spring-projects/spring-data-envers#279). Is that enough to unblock this PR?
Comment From: scordio
spring-projects/spring-data-envers#289 might influence this PR.
Comment From: scordio
I updated the PR to use the composed annotation introduced with spring-projects/spring-data-envers#290.
In case you have any suggestions on how to avoid the subclass, I'm happy to work in that direction.
Comment From: snicoll
@scordio thank you for making your first contribution to Spring Boot.