-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Context enabled JPA 2.1 @EntityGraph [DATAJPA-749] #1120
Comments
Ryan Rupp commented This would be a nice feature, I was looking for something like this to support a REST API that allows dynamic response expansion e.g.:
where the goal would be that all relations are lazy fetched by default and then toggling eager fetch with dynamically built entity graphs based on the expand parameter then we're only traversing the entity as far as the client has actually requested. As for an explicit parameter vs thread local I think an explicit parameter would be preferable however I can see this would be painful with all the different permutations of parameters available and that basically any method that returns an entity would benefit from being able to programmatically specify the EntityGraph. So, in that regard the thread local approach would be less intrusive at least |
Réda Housni Alaoui commented Hello, I am very interested in this functionality. I think the argument way is preferable even if it is more intrusive. Can I have the opinion of some Spring Data JPA maintainers before I start? Regards |
Réda Housni Alaoui commented Could we use JpaEntityGraph to type this new argument? |
Oliver Drotbohm commented I am not really keen to add this as it moves store specific knowledge into repository clients. Why would a service have to know about what entity graph is in the first place. The other aspect is that with the projection support for repository methods, there already is a way to define different views that can be returned for the same query. So very little chance that this will ever make it into the codebase |
Réda Housni Alaoui commented Hello Oliver,
Because only the service knows the requested depth. And the service should know that if he wants A.B over many A's with B lazy loaded, that would cause performance issue.
|
Réda Housni Alaoui commented
|
Réda Housni Alaoui commented I want to add that we also use EntityGraph to select which sub entities will be serialized to the browser. |
Oliver Drotbohm commented
To be more precise. The repository client of course has to know what it wants to read. What I am saying though is that must not know about the implementation details. The entity graph is a JPA specific concept, a very poorly designed one beyond that.
Mixing the aspects of persisting object and optimizing on what's read is a bad idea in the first place as it means you compromise the two. I'd argue you want to design your entities to form proper aggregates to express clear consistency boundaries and then those either just fit the read model and you can live with the slight performance decrease as the benefit of being able to just use the type as is outweighs that cost. If your read model diverges from the entity I'd rather tweak my queries to return tailored types in the first place rather than messing with the entities themselves.
I fail to see how JSON Views play into this as we're not talking about the rendering here but what data to read in the first place. I personally consider JSON views a very poor solution, too. The look sort of bearable if you only have one. But they become quite some annotation madness once you have a couple of more of them. Also, I think them having to be expressed at the entity in the first place is fundamentally subverting the point. You don't want the entity to know about different views. or be polluted with a plethora of view specific annotations. Externalizing that information using an interface was a deliberate design choice here. Also, using interfaces has the benefit of composability though (multiple) inheritance.
Right.I'd argue that if you give clients too much control about what needs to be fetched, you dramatically degrade the abstraction level a repository provides. You're basically turning the repository into a low leave data access API again. If you really need that flexibility, then hide that inside the repository, don't put it in front of it. As you mention exposing that functionality via REST, too, let me share some thoughts why that idea might be a not so great one as it appears at first glance. You dramatically increase in the API surface as you basically create endlessly more resources (one per each permutation of properties a client can request to be loaded). This dramatically decreases the server's ability to actually cache the representations. In a lot of scenarios I've seen in that regard the overall system architecture was better off providing less flexible HTTP resources as that caused the clients to hit proxy caches more often so that requests don't even hit the application anymore. So producing more coarse grained results will actually pay off.
Whether caches are hit is determined by the queries executed. I don't see why a query result returning a tuple or DTO should be less cacheable than queries returning entities. Quite to the contrary: the projections or DTOs are usually immutable values which makes them much better candidates for caching in the first place. JPA entities are usually way harder to cache as the need to be mutable.
You'd need to replace |
Réda Housni Alaoui commented
I just performed a test on hibernate first level cache: @Entity
public class FakeEntity extends StdEntity {
private String name;
private String lastName;
} public interface NoLastName {
int getId();
String getName();
} public interface FakeRepository extends CosCrudRepository<FakeEntity, Integer> {
NoLastName findById(int id);
} @Transactional
@Test
public void test(){
FakeEntity entity = new FakeEntity();
entity = fakeRepository.save(entity);
entityManager.flush();
entityManager.clear();
fakeRepository.findById(entity.getId());
fakeRepository.findById(entity.getId());
entityManager.flush();
entityManager.clear();
fakeRepository.findOne(entity.getId());
fakeRepository.findOne(entity.getId());
} The projection query is executed twice. |
Réda Housni Alaoui commented
We have been building (and running on cumulated 1 Petabyte databases) our CRUD application for more than two years with Spring Data JPA and a custom repository to which we can pass EntityGraph as an argument. We don't use any DTO. We pass main entity types in two directions. |
Réda Housni Alaoui commented It seems that beside the current topic, you are against JPA EntityGraph. I agree with the fact that the solution must be the less implementation specific possible. |
Oliver Drotbohm commented What you have tested is not the difference between an entity or an projection but the access via Regarding your custom implementation: that's a perfectly fine approach. It just doesn't mean we should elevate that to something the framework promotes. We neither want to optimize for CRUD scenarios nor for JPA in particular. We need to consider what that feature would mean across all modules. I am not "against" EntityGraphs, I am against exposing a store specific way of implementing how to define what should be read. When I say EntityGraph I mean the JPA notion of EntityGraph (as already stated twice). And again, there is a solution to that problem available that admittedly might have slightly different trade offs. But then again, nothing prevents you from implementing whatever you want though custom repository extensions. :) |
Réda Housni Alaoui commented You are right about the test. Query cache is managed by hibernate second level cache. |
Réda Housni Alaoui commented
In the current solution, we only have more or less static solution:
JPA provides the ability to build them dynamically with level one cache support. Can we find a way to complete it in the official code base without exposing jpa specific way? |
Oliver Drotbohm commented We're running in circles here. There's nothing "incomplete". We support selecting entity graphs per query method. You can use everything of JPA you want in custom implementation code. The point of Spring Data JPA is not to replicate every odd feature JPA has on level above. If we did that, we wouldn't raise the abstraction abstraction level. E.g. there are tons of things you can do with the Criteria API that are not supported in our query methods abstraction. That doesn't mean query methods are incomplete. There's no way to be found as there already multiple ones present: projections that can vary from call to call and a custom implementation as escape hatch to give you all flexibility. And no, there are no plans to remove the entity graph support we already have in place. Not sure why that question would actually be raised. I think I'm gonna leave it at that as I feel I am repeating myself over and over again and there's no point in doing that any further |
Réda Housni Alaoui commented Ok. |
Réda Housni Alaoui commented For people who are still interested in this functionnality. https://github.com/Cosium/spring-data-jpa-entity-graph The project is fully functionnal |
Miguel Cardoso commented That is the kind of stuff that should be included directly into the JpaRepository, maybe Oliver would be interesting on including it? The problem is that the current Reda's project solves just that by extending JpaRepository and allowing you to pass an EntityGraph object to the findOne and findAll methods, this gives you a very clean solution which makes using EntityGraphs very enjoyable under Spring Data. So I really hope this project could be merged into Spring Data because it's definitely a feature that should be included on it. I wouldn't say EntityGraphs are an odd JPA feature, they are actually one of the most useful ones that will avoid you to write a lot of query code if you have a lot of different use cases where you need to return different data. |
Marcel Overdijk commented We also use I also don't see the objection to provide a way to specify the entity graph in repository methods. |
Mike M. commented I have to join the chorus. I'm building a REST API and trying to sail around N+1 issues. Spring Data JPA is a wonderful project, I enjoy using it a lot, and this discussion raises some very interesting points. The issue I currently have is that without the ability to fetch different subgraphs for different REST endpoints (for the same findXYZ query), I'm subject to N+1 issues all over the place. Don't get me wrong: I am not going fully dynamic (where the client can specify whatever entities he wants) here, I am modelling my endpoints and their content myself - it is just that data needs to be fetched with different subgraphs and depths for different endpoints / purposes. The only alternative seems to be copy/pasting a lot of boilerplate code, which feels just wrong, especially given the fact that Spring Data JPA is almost there... I mean: just accept what I agree with |
Josh Wand commented +1 from me... not being able to specify different fetch plans means that Spring Data JPA, using the built-in respository methods, won't be performant in production except for the simplest data models. I suspect most people have worked around this by implementing however many of their own custom Impls, doing all their own entitymanger handling, etc. which is what Spring Data was supposed to free me from (and since I'm using spring-data-rest, it means I have to write my own custom controllers, too, since I don't have direct control over what repository methods get invoked). |
Jens Schauder commented As Oliver explained above:
TL;DR: Won't happen. Closing this |
Marcel Overdijk commented Feeling disappointed here. Now without entity graphs it's quite impossible to make make a big system performant. We are talking not only about simple Hello World cases or a Book/Author example. I hope Spring Data (JPA) also aims for big enterprise projects. That being said, imagine we have:
How can I call such a method with different entity graphs? |
Jens Schauder commented Marcel we sure also want and do support big enterprise systems.
You have multiple options:
If you have further questions how to use Spring Data please ask on StackOverflow. The developers (and many others) are answering questions there, and the answers are better discoverable by other users with similar problems. jira.spring.io is for bugs and feature requests only |
Oliver Drotbohm commented Marcel Overdijk — I think you're mixing up two issues here and I have to admit the pure exietance of JPA entity graphs makes it easy to run into that trap: you mixing up the actual aggregate that's to be transitioned between different states and a lot of custom views onto the You're way better off introducing dedicated DTOs or projection interfaces if you need a variety of views onto your aggregate. I'd even argue a custom class or interface is less hassle to declare than that annotation madness entity graph declaration. As a neat side effect all client code using those view types then clearly know that they're dealing with read-only data. So a <T> Optional<T> findByEmailAddress(EmailAddress emailAddress, Class<T> view); gives you all the flexibility you need while not exposing any store specifics on the repository API. Beyond that, what Jens wrote applies: you can manually implement repository methods any time and resolve the entity graph you want to use. As to the question of why there's |
Marcel Overdijk commented Hi Jens, thx for responding!
If I would choose that option, honestly I could almost remove Spring Data JPA from our projects as well.
I don't understand how to do this, e.g.:
This won't work as the signature is the same.
wouldn't work as well because I don't think Spring Data is able to understand this name of query method. Am I maybe missing some option here?
I use https://github.com/Cosium/spring-data-jpa-entity-graph for some projects. |
Marcel Overdijk commented Thanks Oliver, I probably need to look into these DTOs or projection interfaces a little bit more (again) as what you say makes sense: "I'd even argue a custom class or interface is less hassle to declare than that annotation madness entity graph declaration. As a neat side effect all client code using those view types then clearly know that they're dealing with read-only data." |
Marcel Overdijk commented Oliver, Are you maybe aware of medium sized sample projects or OS projects using this approach, going beyond simple Author/Book cases? |
Miguel Cardoso commented I don't have much hope that this will ever be added, but after reading Oliver and Jens comments I think they are not fully understanding what's being requested here so I'll try again.
This won't work at all as Marcel already stated. In fact the whole
How does this help in any way Oliver? I mean the point of using EntityGraphs is to allow us to easily get data from multiple (different) joined tables without writing any actual queries. I'm not seeing how using projections or DTOs will help with this, it's a completely different matter as we what we want is to JOIN with different tables depending on the DTO/Projection used. The only way your solution works is if you implement your own custom repositories and query the data using JPQL, Query API, QueryDSL or whatever, but in that case you don't even need a Class parameter in your method signature in fact going that route I don't think there's much benefit on using Spring Data at all.
The point here is that we don't want to use annotations at all (as I said I don't even see the point on the |
Jens Schauder commented I'm actually pretty confident that Oliver and myself pretty well understand what you want, and why you want it. And I think that we even agree that this might be super useful for certain scenarios. The point where we disagree is: Should it be part of Spring Data? And Oliver and the rest of the team have a rather strict image of what we want in Spring Data and what not. But this was discussed above and I'm not going to repeat it. Instead, I'd like to propose a thought experiment which might result in a feature that is welcome in Spring Data. If you take two steps back you want to be able to pass an additional parameter which affects how/which queries get executed. With this abstraction in mind, we have a couple of occasions where this is already happening in a sense: I have to admit I don't know if and how something like this could be implemented at the moment. And I don't know how Oliver thinks about it but I'm sure he'll let us know |
Marcel Overdijk commented Hi all, I did some experiments lately with the Projection classes mentioned by Oliver and Jens. I created a set of Projection interfaces including nested properties. I did not yet look into the DTO classes as the guide mention they do not support nesting so this will not fit my models. I extended my repository with methods like:
They both do work. But I'm having troubles with these:
It gives me: Note my repository extends the
UPDATE Based on the https://github.com/spring-projects/spring-data-examples/blob/master/jpa/example/src/main/java/example/springdata/jpa/projections/CustomerRepository.java example I tried:
But calling the first gives the following stracktrace:
So it seems the method names are correct, but fail for some other unexplainable reason |
Robert Hunt commented
|
Marcel Overdijk commented Thx Robert, Looking more closely at the link regarding the query method names I see I only need to concentrate on the So I was able to change my methods to:
Note, both methods fail with a I then also tried:
And that worked! But this again fails with a
Should dynamic projections work with these Paged ( |
Jens Schauder commented I think there exists already a separate issue for this: https://jira.spring.io/browse/DATAJPA-1185 |
Oliver Drotbohm commented In addition to what Jens wrote |
Marcel Overdijk commented PS: I created spring-projects/spring-data-examples#295 which contains an additional (but failing) test that uses dynamic projections in combination with pagination |
Marcel Overdijk commented
|
Marcel Overdijk commented
|
Marcel Overdijk commented Looking at other issues found in jira I think dynamic projections should work with Specifications. At least that's what I understand from https://jira.spring.io/browse/DATAJPA-393. |
dsanicolas commented Look at that https://jitpack.io/p/cosium/spring-data-jpa-entity-graph |
Oliver Drotbohm commented My comment from 2016 still stands |
dsanicolas commented Hi Oliver I have read the whole thread. I ll argument on the purpose of optimizing lazyloading on a adecuatly defined Jpa entity mapping . In that point i ll say, yes the service is responsible for choosing the graph loading, since it s where you define the dtos/entities (dependíng on your structure) you ll serve. Thus whatever argument to decouple it from the service makes no sense and will make more difficult its resolution. In the case on querydslpredicateexecutor repostories (and in a minor way specificatioexecutor ones) where the point is about having the predicate/example (or specification ) defined dinamically and in a simple way, it s even most worthy. Look at the quantity of code you avoid (with the right jpa mapping) using querydslrepository with an entitygraph as attribute compared to pure jpa repositories, or jpa queries with proyection; and all of that with cache capability! We re not talking about a few lines but a really distinct dinamic paragnim that deletes any query-annotated (or namely identified) jparepository interface function or any use of dinamic proyections, and a paragnim that completes with caching requirement . That s really huge. With only two functions, findone and findall (predicate, entitygraph) - omitting pageable argument... With all my respect to you (spring jpa data is marvelous and in a constant positiv evolution), i do see more arguments in the point of https://jira.spring.io/secure/ViewProfile.jspa?name=reda-alaoui. His purpose is more a new paragdim to solve lazyloading complexity than a mere conservativ críticism to spring jpa interfaces. But i did not want re-open the whole discussion, just pointed out that there was that solution to those that need it. I can tell you i didnt define any function in my jpa interfaces, neither wrote a jpa or jpsql query, neither use a proyection, i just used some entitygraph as a parametter and querydsl predicates on desired objects (and my entity model is huge). It s just having a huge mount of code away with optimized lazyload... I do think you should consider to integrate it. Your cost of devlopment for that is quite small and it could turns out lazyloading headache into a simple attribute. I really think you Miss a point there😉 |
Jens Schauder commented Batch closing resolved issue without a fix version and a resolution indicating that there is nothing to release (Won't fix, Invalid ...) |
Some people above asked about naming entity-graph based methods: https://docs.spring.io/spring-data/jpa/docs/current/reference/html/#jpa.entity-graph suggests that you can use @EntityGraph(value = "GroupInfo.detail", type = EntityGraphType.LOAD)
GroupInfo getByGroupName(String name); |
Bartłomiej Mocior opened DATAJPA-749 and commented
Currently It's impossible to enable EntityGraph configuration outside of spring data jpa repository. The only possibility is to use
@EntityGraph
annotation over query method.for example
Now, it's impossible to reuse this method in different contexts where different entity graph aka fetch profile is required.
Example I have entity Customer with 2 relations: Invoices and Addresses (OneToMany)
Now, with spring data jpa, I have to create another (second) method and annotate it with another "context"
@EntityGraph
.What is more, my method's name should generate the same JQL Query, but must have different signature (because it's in the same repository), so I will end up with
@Query
annotation and code duplication.The current implementation don't allow to set QueryHints directly on Query - it's possible to annotate method with
@QueryHint
but it won't solve the problem because it's AFAIK static, and doesn't use SpEL.Jpa21Utils with tryGetFetchGraphHints get's the query hints from annotation only, maybe it would be good idea if it could additionally take from other source ? Maybe ThreadLocale or any context object ?
Maybe, It would be consistent with spring data API if we could provide EntityGraphContext object to have full controll of execution ?
Proposed solution I "EntityGraphContext"- repository example
Client code:
Proposed solution II "EntityGraphThreadLocalContext"- repository example
Client code:
Affects: 1.9 M1 (Gosling)
13 votes, 17 watchers
The text was updated successfully, but these errors were encountered: