[tzatziki-spring-jpa] Britle repository fetch by table name

[z06lsouc]

I found an inconsistent bug when it comes to mocking a table with the guard the *** table will contain:.

Context:

I am using polymorphic entities (sharing a common table and an extension) with a common Spring CrudRepository to query all polymorphic variations at once in my main code. The common repo is defined like that: public interface MyBaseRepository<T extends MyBaseEntity> extends CrudRepository<T, MyId>

When I am testing, I am trying to mock one of those entities table with the guard the *** table will contain:. To do so I created in my test classpath a specific CrudRepository for the corresponding entity. However the following code will fail when evaluating the common repo because it's parametrization is still a TypeVariable: https://github.com/Decathlon/tzatziki/blob/5cae3967c28c744e37b03c95d252f37dd0908c47/tzatziki-spring-jpa/src/main/java/com/decathlon/tzatziki/steps/SpringJPASteps.java#L178

Note that this code fails depending on the order of the beans that are filtered by the method: https://github.com/Decathlon/tzatziki/blob/5cae3967c28c744e37b03c95d252f37dd0908c47/tzatziki-spring-jpa/src/main/java/com/decathlon/tzatziki/steps/SpringJPASteps.java#L173

This affects the stability of all call to the guard the *** table will contain: during Tzatziki runtime.

Analysis:

The issue comes from the following util code that assumes a rawType is actually available: https://github.com/Decathlon/tzatziki/blob/5cae3967c28c744e37b03c95d252f37dd0908c47/tzatziki-common/src/main/java/com/decathlon/tzatziki/utils/Types.java#L53

For generic type resolution, I would advice on using TypeUtils from Apache commons lang3 instead of homebrew code, but this is beyond the scope of this issue.

Solutions:

I don't like any of my solutions so I offer for debate those two:

1. Change the expectation/behaviour of Types#rawTypeOf and handle TypeVariable instances. Not sure how it should behave.
2. Add a try catch around getEntityType in SpringJPASteps#getRepositoryForTable

[z06lsouc]

Temporary solution: use the guard: the *** repository will contain:

[hqrd]

I got a similar issue when using a repository that extends another repository (which is the one that extends JpaRepository). The getEntityType(r) method in SpringJPASteps returns a null and every step that uses getRepositoryForTable() fail because of NullPointerException.

Step failed
java.lang.NullPointerException: Cannot invoke "java.lang.Class.isAnnotationPresent(java.lang.Class)" because "e" is null
    at com.decathlon.tzatziki.steps.SpringJPASteps.lambda$getRepositoryForTable$12(SpringJPASteps.java:179)
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:178)
    at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
    at java.base/java.util.Spliterators$IteratorSpliterator.tryAdvance(Spliterators.java:1856)
    at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129)
    at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:647)
    at com.decathlon.tzatziki.steps.SpringJPASteps.getRepositoryForTable(SpringJPASteps.java:182)
    at com.decathlon.tzatziki.steps.SpringJPASteps.the_table_will_contain(SpringJPASteps.java:79)

It looks like this part can only work for repositories that have no special ihneritance : repository.getClass().getInterfaces()[0].getGenericInterfaces()[0]

[brian-mulier]

Sorry I didn't see the issue. I haven't been able to experiment it yet but I will try to replicate the issue and see if I can get a bit more understanding against your solutions.

[z06lsouc]

Sorry. I should have added my stacktrace. Here it is:

Step failed
java.lang.ClassCastException: class sun.reflect.generics.reflectiveObjects.TypeVariableImpl cannot be cast to class java.lang.Class (sun.reflect.generics.reflectiveObjects.TypeVariableImpl and java.lang.Class are in module java.base of loader 'bootstrap')
    at com.decathlon.tzatziki.utils.Types.rawTypeOf(Types.java:53)
    at com.decathlon.tzatziki.utils.Types.rawTypeArgumentOf(Types.java:34)
    at com.decathlon.tzatziki.steps.SpringJPASteps.getEntityType(SpringJPASteps.java:207)
    at com.decathlon.tzatziki.steps.SpringJPASteps.lambda$getRepositoryForTable$12(SpringJPASteps.java:178)
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:178)
    at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
    at java.base/java.util.Spliterators$IteratorSpliterator.tryAdvance(Spliterators.java:1856)
    at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129)
    at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:647)
    at com.decathlon.tzatziki.steps.SpringJPASteps.getRepositoryForTable(SpringJPASteps.java:182)
    at com.decathlon.tzatziki.steps.SpringJPASteps.the_table_will_contain(SpringJPASteps.java:79)

[brian-mulier]

I'll try to check these this week

[brian-mulier]

Hello @z06lsouc, I was able to reproduce the issue. I have some idea around how to solve it but I have to get a bit more context. Are you actually saving base entity & extended one through the same repository or is there 1 repository / base - extension class ? In every case I think we have to handle the TypeVariable and resolve it onto an actual class. I feel like it cannot harm at all since it would otherwise crash since T (or whatever) is not an actual resolvable class by itself so we should probably get the base class from it (see below):

Now the fun part:

• If you are saving through a single common repository for every inherited class, we will probably need to do some reflections in order to get every inherited class and resolve them by type (probably by doing some bruteforce until we find out a suitable class to fit the entity into ?). We can think of some tweaks later on like grouping by key before trying to bruteforce in order to Mapper.read multiple entities at once.
• If you are saving with 1 repository / inherited class, I think we should get every repository fitting an inherited class and then try to resolve through bruteforce (as above) every entity before saving them through the proper repository.

The bruteforce part comes in action if we would like to allow users to specify any subclass pattern into a single "the table will contain" statement like that:

But maybe it would be overkilled for an isolated (for now) usecase like that and we could simply admit that the user will have to split into 1 statement for a given subtype.

Let me know what you think but those are my best bet on how to solve it.

[z06lsouc]

Hello @brian-mulier ,

First of all I'm sorry, I should have given you a code sample and some context because what you found is a more complex variation of the issue that didn't come to my mind. So just to give you my context and share with you my simpler variation, here is my code sample:

@Entity
@Table(name = "my_base_entity")
@Inheritance(strategy = InheritanceType.JOINED)
public abstract class MyBaseEntity {
    @Id
    private Long id;
    // ....
}

@Entity
@Table(name = "my_extended_entity")
public class MyExtendedEntity extends MyBaseEntity {
    // Some extended fields
}

public interface MyBaseEntitySpringRepository<T extends MyBaseEntity> extends CrudRepository<T, Long> {
}

You will see two things:

• My base entity is abstract as it is actually polymorphic.
• The extended entity uses another table to store the extended data.

As for the workaround I talked about before, I created the following repo in my test classpath which is directly used with the guard the MyExtendedEntitySpringRepository repository will contain: :

public interface MyExtendedEntitySpringRepository extends MyBaseEntitySpringRepository<MyExtendedEntity> {
}

Now that this is cleared out:

I was able to reproduce the issue. I have some idea around how to solve it but I have to get a bit more context. Are you actually saving base entity & extended one through the same repository or is there 1 repository / base - extension class ?

I kind of replied to it by finally giving you my code sample. In my case there is no base class saved. In my main classpath I might in the future save more that one form of my base entity at the same time, but I don't think it is a must have in Tzatziki.

In every case I think we have to handle the TypeVariable and resolve it onto an actual class. I feel like it cannot harm at all since it would otherwise crash since T (or whatever) is not an actual resolvable class by itself so we should probably get the base class from it (see below):

Yes, your code seems neat except for the limitation to the first bound. Even though I find it difficult to think of parametrizations with multiple bounds these days, it's a possibility given by java.

Now the fun part:

• If you are saving through a single common repository for every inherited class, we will probably need to do some reflections in order to get every inherited class and resolve them by type (probably by doing some bruteforce until we find out a suitable class to fit the entity into ?). We can think of some tweaks later on like grouping by key before trying to bruteforce in order to Mapper.read multiple entities at once.
• If you are saving with 1 repository / inherited class, I think we should get every repository fitting an inherited class and then try to resolve through bruteforce (as above) every entity before saving them through the proper repository.

Well in my case using the table name constraints to a single type, so it's a non question. But in yours it surely is dicier, but I think limiting the guard to a single type is not too much of a constraint. With this constraintI could see a variation on the guard itself to specify the extended entity to be mapped like the <table name> table will contain <extends type>(s):

Anyway, I think the way to go is to get the base entity/table that can be served by each repo from it parametrized type or type bounds. I don't think a repo can serve more than one. Then:

First solution: we require a single type in the guard for extended entities. If a type is provided in the guard: get the entity and match repos with entities that are assignableFrom. If no type is provided in the guard: match directly repos with the table name. In both cases we should end up with the entity type and the repo needed to map and save.

Second solution: we don't require any input for extended entities (hence possibility of multiple ones). Scan the classpath for any extension of the base entities and, like you said, bruteforce map until it fits for each line. In the end you should have entities that all extend the same base one and you get your repo from it.

The second solution is more generic but also messier and less efficient for nominal case

[z06lsouc]

@brian-mulier I think you've passed a fix for this issue. I'll have to try it to confirm and close it.

[brian-mulier]

@z06lsouc Is it properly fixed ? If it is can you close the issue ? Thanks

[z06lsouc]

@brian-mulier The issue is fixed for me. I roll backed to having only my base repo and using the table variant.