spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
3.01k stars 1.42k forks source link

Context enabled JPA 2.1 @EntityGraph [DATAJPA-749] #1120

Closed spring-projects-issues closed 5 years ago

spring-projects-issues commented 9 years ago

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

@EntityGraph("summary")
Optional<Customer> findByEmailAddress(EmailAddress emailAddress);

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

interface SomeRepository ... 
Optional<Customer> findByEmailAddress(EmailAddress emailAddress, EntityGraphContext graphCtx);

Client code:

EntityGraphContext graphCtx = EntityGraphContext.enable("summary").withType(EntityGraphType.FETCH);
Optonal<Customer> customer = someRepository.findByEmailAddress(address, graphCtx);

Proposed solution II "EntityGraphThreadLocalContext"- repository example

interface SomeRepository ... 
Optional<Customer> findByEmailAddress(EmailAddress emailAddress);

Client code:

EntityGraphThreadLocalContext.enable("summary").withType(EntityGraphType.FETCH);
Optonal<Customer> customer = someRepository.findByEmailAddress(address);

Affects: 1.9 M1 (Gosling)

13 votes, 17 watchers

spring-projects-issues commented 8 years ago

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.:

GET /api/customers?expand=address

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

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

Hello,

I am very interested in this functionality. I am ready to start working on it.

I think the argument way is preferable even if it is more intrusive. Since the javax.persistence EntityGraph can only be generated by the EntityManager, I think this argument class will be fully specific to Spring Data JPA. It would be a class holding the EntityGraph name or the EntityGraph modelisation.

Can I have the opinion of some Spring Data JPA maintainers before I start?

Regards

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

Could we use JpaEntityGraph to type this new argument?

spring-projects-issues commented 7 years ago

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

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

Hello Oliver,

Why would a service have to know about what entity graph is in the first place.

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.

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.

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

spring-projects-issues commented 7 years ago

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. Thanks to Jackson Hibernate Module, only loaded entities are serialized, lazy unloaded ones are ignored. In some cases, the browser can make requests using the graph of its choice without having to modify the server side

spring-projects-issues commented 7 years ago

Oliver Drotbohm commented

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.

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.

You can't save back the result of a projection directly.

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.

Building an interface for each combination is very heavy. It looks like what Jackson did with http://wiki.fasterxml.com/JacksonJsonViews. It was not sufficient, so they also provide http://wiki.fasterxml.com/JacksonFeatureJsonFilter which is fully dynamic.

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.

You can't build your selection dynamically. Projection must be defined at compilation time.

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.

I think that using projection bypass Hibernate entity level 1 and 2 cache. I didn't verify this point but it seems logical to me. Or are you building an entity graph behind each projection interface?

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.

And it seems that you can't use projection on standard repository methods

You'd need to replace findOne(…) or findAll(…) by semantically equivalent query methods but that's not a problem at all

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

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.

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. The standard query is executed once. That seems to prove that hibernate level one cache doesn't cache tuples but only entities

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

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.

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. I consider that this has saved us time and made the code more maintenable than multiplying the view objects. Thanks to EntityGraph, we never felt the need to use view objects

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

It seems that beside the current topic, you are against JPA EntityGraph. I think that Spring Data JPA should fully support JPA EntityGraph, including the ability to build or reference them dynamically.

I agree with the fact that the solution must be the less implementation specific possible. But the EntityGraph is not a concept exclusive to JPA, it exists on MongoDb for example and other database client systems that I am not aware of

spring-projects-issues commented 7 years ago

Oliver Drotbohm commented

What you have tested is not the difference between an entity or an projection but the access via EntityManager.find(…) VS issuing a query. The former hits the first level cace, the latter by definition never. You need to setup proper query caches to see proper caching.

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. :)

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

You are right about the test. You can't perform an EntityManager.find with Projection, you can with EntityGraph. So same rules apply about hitting the cache. The conclusion remains true, projection query result can't be cached by first level cache while EntityGraph ones can.

Query cache is managed by hibernate second level cache. My test is about first level cache, the one mainly used at my company

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

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.

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. To me, point 1 is incomplete.

Can we find a way to complete it in the official code base without exposing jpa specific way? Are you planning to deprecate and delete EntityGraph annotation support?

spring-projects-issues commented 7 years ago

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

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

Ok. Thank you for the time you took to answer me

spring-projects-issues commented 7 years ago

Réda Housni Alaoui commented

For people who are still interested in this functionnality. I created a little project, extending spring-data-jpa and allowing to use EntityGraphs as an argument with any repository method:

https://github.com/Cosium/spring-data-jpa-entity-graph

The project is fully functionnal

spring-projects-issues commented 7 years ago

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 @EntityGraph annotation in Spring Data JPA is almost useless because most use cases where you need to use an EntityGraphs you'll want to use different graphs for either the findOne or findAll methods which is something you can't do thus you'll have to resort to @Query annotations that will be exactly the same except for the JOIN parts, this means a lot of boilerplate code and your code starts to look messy.

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.

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

We also use @EntityGraphs's to define fetch plans. We need it to even avoid basic N+1 queries. I also agree that the services most of the time know what data they need and to boost performance (or better to avoid performance drops) only fetch (eager) the data they need.

I also don't see the objection to provide a way to specify the entity graph in repository methods. Imo it would be very useful if Spring Data JPA would understand an EntityGraph argument in repository methods and act upon it.

spring-projects-issues commented 7 years ago

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 @org.springframework.data.jpa.repository.EntityGraph describes as an additional optional argument and we're all set, right? Wouldn't that level of abstraction be comparable with what the optional org.springframework.data.domain.Pageable argument does? It gives the repository client fine grained control over pagination and sorting, because he's the one knowing what data he needs. Even though EntityGraphs are a "performance", not a "feature" thing, I really do think from an idiomatic perspective this is comparable...

I agree with @ogierke that Entity Graphs aren't (by far) the most elegant solution, but they seem to work pretty well in practice. And for reasons given above, I don't think that having such a feature would have a major negative impact on Spring Data JPA's abstraction. I see you're annoyed by this subject, Oliver, and I wouldn't jump on this topic if I didn't think this would be helpful for a lot of people out there, but would you please reconsider including this feature?

spring-projects-issues commented 7 years ago

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).

spring-projects-issues commented 7 years ago

Jens Schauder commented

As Oliver explained above:

TL;DR: Won't happen. Closing this

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

Feeling disappointed here. I understand that Spring Data should not violate the repository abstraction. But on the other hand it should also be useful. Like why do have e.g. the JpaRepository.saveAndFlush method?

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:

Optional<Customer> findByEmailAddress(EmailAddress emailAddress);

How can I call such a method with different entity graphs? If I would create a second method how would that look?

spring-projects-issues commented 7 years ago

Jens Schauder commented

Marcel we sure also want and do support big enterprise systems.

How can I call such a method with different entity graphs? If I would create a second method how would that look?

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

spring-projects-issues commented 7 years ago

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 Customer that you probably need for display purposes. This is effectively mixing up the write model with the read model of an application and I think entity graphs are a very poor means to try to force the latter onto the former.

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 saveAndFlush(…) but us not wanting to introduce the dynamic entity graphs, it's pretty simple. For the former, there's no such mechanism that works across all stores. For dynamic views we actually have one in place (projections, DTOs), that's why we don't want to introduce a similar concept

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

Hi Jens, thx for responding!

Create a custom method, working with the entity manager directly.

If I would choose that option, honestly I could almost remove Spring Data JPA from our projects as well.

Create multiple methods each with its own @EntityGraph annotation.

I don't understand how to do this, e.g.:

@EntityGraph("summary")
Optional<Customer> findByEmailAddress(EmailAddress emailAddress);

@EntityGraph("details")
Optional<Customer> findByEmailAddress(EmailAddress emailAddress);

This won't work as the signature is the same. Simply changing the method name to something like:

@EntityGraph("details")
Optional<Customer> findDetailsByEmailAddress(EmailAddress emailAddress);

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?

There was https://github.com/Cosium/spring-data-jpa-entity-graph mentioned before

I use https://github.com/Cosium/spring-data-jpa-entity-graph for some projects. But off course something part of (official) Spring Data releases would be preferred (especially for enterprise companies avoiding to many 3th party dependencies). Think about supporting changes like Spring Data 2.0 upgrade.... with something this crucial it would have a lot of benefit that there will be no incompatibilities between (major) versions.

spring-projects-issues commented 7 years ago

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." In particular, I have an example system that 95% is read-only, so maybe this is a good one to evaluate such an approach again. And otherwise I will just stick to https://github.com/Cosium/spring-data-jpa-entity-graph.

spring-projects-issues commented 7 years ago

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?

spring-projects-issues commented 7 years ago

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.

Create multiple methods each with its own @EntityGraph annotation.

This won't work at all as Marcel already stated. In fact the whole @EntityGraph annotation is pretty much useless and I don't see how it can be used on any real life project since it's so limiting.

So a \ Optional\ findByEmailAddress(EmailAddress emailAddress, Class\ view); gives you all the flexibility you need while not exposing any store specifics on the repository API

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. This will all mean a lot of boilerplate code which could easily be avoided if EntityGraphs were used.

I'd even argue a custom class or interface is less hassle to declare than that annotation madness entity graph declaration

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 @EntityGraph annotation existance), the only thing needed is support to pass an EntityGraph as a parameter to the findOne, findAll, etc. methods like the Cosium project does.

spring-projects-issues commented 7 years ago

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: Specifications, Pagination. There are other feature requests which might be possible to implement this way like multi tenant support. So if we find a way to put this abstraction into code so that the feature you want becomes implementable by registering a QueryPreprocessor we have something that could be added to Spring Data and your specific feature could be implemented in a third party add on.

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

spring-projects-issues commented 7 years ago

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. The first thing I can conclude is that using these Projections indeed solve the N+1 query issue I have seen without. It nicely generates 1 single query with the needed joins.

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. Downside is that when debugging the Projection interfaces are proxies which are difficult to introspect.

I extended my repository with methods like:

<T> Collection<T> findAllProjectedBy(Class<T> type);

<T> Collection<T> findAllProjectedByNameContainingIgnoreCase(String name, Class<T> type);

They both do work. But I'm having troubles with these:

<T> Page<T> findAllProjected(Pageable pageable, Class<T> type);

<T> Page<T> findAllProjected(Predicate predicate, Pageable pageable, Class<T> type);

It gives me: PropertyReferenceException: No property findAllProjected found for type Person!

Note my repository extends the PagingAndSortingRepository and QuerydslPredicateExecutor so I would hope I can use the findAll methods with Projections.

@Oliver/Jens Am I using wrong names here, or are the findAll methods not supported?

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:

    <T> Page<T> findPagedProjectedBy(Pageable pageable, Class<T> type);

    <T> Page<T> findPagedProjectedBy(Predicate predicate, Pageable pageable, Class<T> type);

But calling the first gives the following stracktrace:

java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
    at java.util.ArrayList.rangeCheck(ArrayList.java:653) ~[na:1.8.0_144]
    at java.util.ArrayList.get(ArrayList.java:429) ~[na:1.8.0_144]
    at java.util.Collections$UnmodifiableList.get(Collections.java:1309) ~[na:1.8.0_144]
    at org.springframework.data.jpa.repository.query.QueryParameterSetterFactory$CriteriaQueryParameterSetterFactory.create(QueryParameterSetterFactory.java:272) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.ParameterBinderFactory.lambda$createQueryParameterSetter$1(ParameterBinderFactory.java:140) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) ~[na:1.8.0_144]
    at java.util.Spliterators$ArraySpliterator.tryAdvance(Spliterators.java:958) ~[na:1.8.0_144]
    at java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:126) ~[na:1.8.0_144]
    at java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:498) ~[na:1.8.0_144]
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:485) ~[na:1.8.0_144]
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) ~[na:1.8.0_144]
    at java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:152) ~[na:1.8.0_144]
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:1.8.0_144]
    at java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:464) ~[na:1.8.0_144]
    at org.springframework.data.jpa.repository.query.ParameterBinderFactory.createQueryParameterSetter(ParameterBinderFactory.java:142) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.ParameterBinderFactory.lambda$createSetters$0(ParameterBinderFactory.java:132) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) ~[na:1.8.0_144]
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374) ~[na:1.8.0_144]
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) ~[na:1.8.0_144]
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) ~[na:1.8.0_144]
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708) ~[na:1.8.0_144]
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:1.8.0_144]
    at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499) ~[na:1.8.0_144]
    at org.springframework.data.jpa.repository.query.ParameterBinderFactory.createSetters(ParameterBinderFactory.java:133) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.ParameterBinderFactory.createSetters(ParameterBinderFactory.java:125) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.ParameterBinderFactory.createCriteriaBinder(ParameterBinderFactory.java:76) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.PartTreeJpaQuery$QueryPreparer.getBinder(PartTreeJpaQuery.java:242) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.PartTreeJpaQuery$QueryPreparer.createQuery(PartTreeJpaQuery.java:156) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.PartTreeJpaQuery.doCreateQuery(PartTreeJpaQuery.java:84) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.AbstractJpaQuery.createQuery(AbstractJpaQuery.java:200) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.JpaQueryExecution$PagedExecution.doExecute(JpaQueryExecution.java:187) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.JpaQueryExecution.execute(JpaQueryExecution.java:87) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.AbstractJpaQuery.doExecute(AbstractJpaQuery.java:126) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.jpa.repository.query.AbstractJpaQuery.execute(AbstractJpaQuery.java:115) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.data.repository.core.support.RepositoryFactorySupport$QueryExecutorMethodInterceptor.doInvoke(RepositoryFactorySupport.java:565) ~[spring-data-commons-2.0.0.RC3.jar:na]
    at org.springframework.data.repository.core.support.RepositoryFactorySupport$QueryExecutorMethodInterceptor.invoke(RepositoryFactorySupport.java:549) ~[spring-data-commons-2.0.0.RC3.jar:na]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.invoke(DefaultMethodInvokingMethodInterceptor.java:60) ~[spring-data-commons-2.0.0.RC3.jar:na]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:294) ~[spring-tx-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:98) ~[spring-tx-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:139) ~[spring-tx-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor$CrudMethodMetadataPopulatingMethodInterceptor.invoke(CrudMethodMetadataPostProcessor.java:135) ~[spring-data-jpa-2.0.0.RC3.jar:na]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.data.repository.core.support.SurroundingTransactionDetectorMethodInterceptor.invoke(SurroundingTransactionDetectorMethodInterceptor.java:61) ~[spring-data-commons-2.0.0.RC3.jar:na]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:212) ~[spring-aop-5.0.0.RC4.jar:5.0.0.RC4]
    at com.sun.proxy.$Proxy100.findPagedProjectedBy(Unknown Source) ~[na:na]
    at org.mycomp.web.controller.PersonController.list(PersonController.java:104) ~[main/:na]
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_144]
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_144]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_144]
    at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_144]
    at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:209) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:136) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:102) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:869) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:775) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:981) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:915) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:978) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:870) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:635) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:855) ~[spring-webmvc-5.0.0.RC4.jar:5.0.0.RC4]
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:742) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52) ~[tomcat-embed-websocket-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:99) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.springframework.web.filter.HttpPutFormContentFilter.doFilterInternal(HttpPutFormContentFilter.java:108) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.springframework.web.filter.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:81) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:200) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-5.0.0.RC4.jar:5.0.0.RC4]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:198) ~[tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:478) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:140) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:80) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:342) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:799) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:868) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1457) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [na:1.8.0_144]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [na:1.8.0_144]
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) [tomcat-embed-core-8.5.20.jar:8.5.20]
    at java.lang.Thread.run(Thread.java:748) [na:1.8.0_144]

So it seems the method names are correct, but fail for some other unexplainable reason

spring-projects-issues commented 7 years ago

Robert Hunt commented

@marceloverdijk Yes you have spotted the naming error, you have to have the "By" in the query name to act as a delimiter:

The mechanism strips the prefixes find…By, read…By, query…By, count…By, and get…By from the method and starts parsing the rest of it.

https://docs.spring.io/spring-data/jpa/docs/current/reference/html/#repositories.query-methods.query-creation

spring-projects-issues commented 7 years ago

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 find..By prefix.

So I was able to change my methods to:

    <T> Page<T> findAllProjectedBy(Pageable pageable, Class<T> type);

    <T> Page<T> findAllProjectedBy(Predicate predicate, Pageable pageable, Class<T> type);

Note, both methods fail with a IndexOutOfBoundsException: Index: 0, Size: 0. Googling a bit, I found that this exception was thrown in the past when the defined arguments were not ok.

I then also tried:

Page<PersonProjection> findAllProjectedBy(Pageable pageable);

And that worked!

But this again fails with a IndexOutOfBoundsException:

Page<PersonProjection> findAllProjectedBy(Predicate predicate, Pageable pageable);

Should dynamic projections work with these Paged (PagingAndSortingRepository) and Predicate (QuerydslPredicateExecutor) methods? The example https://github.com/spring-projects/spring-data-examples/blob/master/jpa/example/src/main/java/example/springdata/jpa/projections/CustomerRepository.java also does't have them. Note I'm on Spring Data 2.0 RC3 (Using latest Boot 2.0 M4)

spring-projects-issues commented 7 years ago

Jens Schauder commented

I think there exists already a separate issue for this: https://jira.spring.io/browse/DATAJPA-1185 The exception is a different one, but that's because it's based on a different version and the underlying code changed a lot in between

spring-projects-issues commented 7 years ago

Oliver Drotbohm commented

In addition to what Jens wrote Predicate cannot be used as parameter type in query methods

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

PS: I created https://github.com/spring-projects/spring-data-examples/pull/295 which contains an additional (but failing) test that uses dynamic projections in combination with pagination

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

@Jens: just noticed the previous comments. Indeed the mentioned PR is exactly what is described in for https://jira.spring.io/browse/DATAJPA-1185.

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

@Oliver: Can Spring Data Specification's used as parameter types in query methods? Then I can better switch from Querydsl Predicates to Specification's.... But without any of the 2 it will be very difficult..... Or is there another way to return a dynamic projection using Sepcification's/Predicate's? There is no way I know up front which filtering needs to be done

spring-projects-issues commented 7 years ago

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. However it does not seem to work yet due to a bug, for which I created the following new ticket: https://jira.spring.io/browse/DATAJPA-1189

spring-projects-issues commented 6 years ago

dsanicolas commented

Look at that https://jitpack.io/p/cosium/spring-data-jpa-entity-graph

spring-projects-issues commented 6 years ago

Oliver Drotbohm commented

My comment from 2016 still stands

spring-projects-issues commented 6 years ago

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😉

spring-projects-issues commented 5 years ago

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 ...)

serv-inc commented 2 years ago

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 get... instead of find..., e.g.

  @EntityGraph(value = "GroupInfo.detail", type = EntityGraphType.LOAD)
  GroupInfo getByGroupName(String name);