Using named queries fails for methods not returning an entity [DATAJPA-44] about spring-data-jpa HOT 23 CLOSED

Andres March commented

Also get a related exception when the entity returned from the query is different than the one that it is defined in

from spring-data-jpa.

Oliver Drotbohm commented

What do you mean with the last comment? Are you sure this works in Hades? Just curious because the code detecting the entity class for a query method hasn't changed really. In the current state we inspect the return type and unpack Page and Collection definitions and prefer the component type then. As you use Long the named query lookup fails. I think we can work around this by inspecting the type you type the repository to in cases you return a numeric type or even void.

For now you should be able to workaround this issue by renaming the named query to Long.getLatestAgreementVersion or use the @Query annotation

from spring-data-jpa.

Andres March commented

Thanks for cleaning up the description and responding quickly. My previous comment was basically the same as the original. The only difference being that Long is obviously not an Entity. Even when the return type is an entity (but not the same Entity as the class defining the query), I have the same exception. As an example:

@NamedQueries(
{
        @NamedQuery(name = "Country.getPricing",
                query = "select c.pricing from Country c where c.appSubmission = true and c.pricing.status = active ") })
@Entity
@Table(name = "COUNTRY", uniqueConstraints = @UniqueConstraint(name = "unique_country", columnNames =
{ "COUNTRY_CODE" }))
public class Country {

Yes, I'm sure this works in Hades because I just switched the library. I can debug the code a bit if you at least confirm that it should work. Hades definitely "failed back" to the Entity name when doing the named query look up.

I can work around this issue using @Query or renaming the queries as you suggest but there are a ton of them. For the time being, I have turned off CREATE_IF_NOT_FOUND query generation. The USE_DECLARED_QUERY strategy doesn't seem to have this problem. In fact, this last point may be the biggest clue as to where the bug is

from spring-data-jpa.

Andres March commented

I'm sorry. I misspoke (or miscommented). The strategy change isn't a complete workaround. I get an IllegalStateException on some queries that appears to be related:

@NamedQuery(name = "ContentFlag.getDateClearedForTitleId", query = "select max(c.content.statistics.dateFlagCleared)"
                + "from ContentFlag c where c.content.title.id = ?1")

and the exception:

Caused by: java.lang.IllegalStateException: Did neither find a NamedQuery nor an annotated query for method public abstract java.util.Date com.qualcomm.qis.plaza.repo.ContentFlagRepository.getDateClearedForTitleId(java.lang.Long)!
	at org.springframework.data.jpa.repository.query.JpaQueryLookupStrategy$DeclaredQueryLookupStrategy.resolveQuery(JpaQueryLookupStrategy.java:138)
	at org.springframework.data.jpa.repository.query.JpaQueryLookupStrategy$AbstractQueryLookupStrategy.resolveQuery(JpaQueryLookupStrategy.java:73)
	at org.springframework.data.repository.support.RepositoryFactorySupport$QueryExecuterMethodInterceptor.<init>(RepositoryFactorySupport.java:259)
	at org.springframework.data.repository.support.RepositoryFactorySupport.getRepository(RepositoryFactorySupport.java:143)
	at org.springframework.data.repository.support.RepositoryFactoryBeanSupport.getObject(RepositoryFactoryBeanSupport.java:107)
	at org.springframework.data.repository.support.RepositoryFactoryBeanSupport.getObject(RepositoryFactoryBeanSupport.java:36)
	at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:142)
	... 36 more

from spring-data-jpa.

Andres March commented

Ok. I think I might have to explain a bit more. As I am migrating to @Query just to see if there are other issues. I'm hitting other things. I have had trouble parameterized types before but I thought what we had done under Hades was sound.

We don't extend JpaRepository directly. We have intermediate interfaces to insulate our code. Ironically, this should have allowed us to switch from Hades to Spring Data JPA with a simple change:

@NoRepositoryBean
public interface GenericRepository<T extends Entity<PK>, PK extends Serializable> extends JpaRepository<T, PK>

public interface Entity<PK extends Serializable> extends org.springframework.data.domain.Persistable<PK>

However, I'm guessing this might be causing my current exception. This thread is a bit long, so I can move it to the forums even though I think the reported issue is valid.

Caused by: java.lang.ClassCastException: sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl cannot be cast to java.lang.Class
	at org.springframework.data.repository.util.ClassUtils.getReturnedDomainClass(ClassUtils.java:63)
	at org.springframework.data.repository.query.QueryMethod.getDomainClass(QueryMethod.java:107)
	at org.springframework.data.jpa.repository.query.JpaQueryMethod.getNamedQueryName(JpaQueryMethod.java:126)
	at org.springframework.data.jpa.repository.query.NamedQuery.lookupFrom(NamedQuery.java:71)
	at org.springframework.data.jpa.repository.query.JpaQueryLookupStrategy$DeclaredQueryLookupStrategy.resolveQuery(JpaQueryLookupStrategy.java:132)
	at org.springframework.data.jpa.repository.query.JpaQueryLookupStrategy$AbstractQueryLookupStrategy.resolveQuery(JpaQueryLookupStrategy.java:73)
	at org.springframework.data.repository.support.RepositoryFactorySupport$QueryExecuterMethodInterceptor.<init>(RepositoryFactorySupport.java:259)
	at org.springframework.data.repository.support.RepositoryFactorySupport.getRepository(RepositoryFactorySupport.java:143)
	at org.springframework.data.repository.support.RepositoryFactoryBeanSupport.getObject(RepositoryFactoryBeanSupport.java:107)
	at org.springframework.data.repository.support.RepositoryFactoryBeanSupport.getObject(RepositoryFactoryBeanSupport.java:36)
	at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:142)
	... 36 more

from spring-data-jpa.

Oliver Drotbohm commented

Okay, things seem to be a bit more complicated :). You we're completely right. The Hades code uses the generic type of the GenericDao interface to determine the name of the named query. In Spring Data we now inspect the return type of the method and use only that, which is of course wrong in terms of calculating the name of the named query. I will take the domain class the Repository interface is typed to into account in the fix.

However, if the method is returning a subtype of the actual entity class, e.g. Contact, Person, I'd rather expect the query to be declared at the Person class and being named Person.myFinderMethodName, wouldn't I?

from spring-data-jpa.

Oliver Drotbohm commented

You should only face the exception thrown in ClassUtils when using something like Page<Entity<?>> or Collection<Entity<?>> right? We're currently not expecting a ParameterizedType inside the Page or Collection, which has to be fixed of course. Could you please open a separate ticket for that simply pasting the stack trace plus the method signature of the method causing this?

Thanks, Ollie

from spring-data-jpa.

Andres March commented

Does it matter in which entity it is defined? Are the names globally unique?

I would think in terms of naming that it should be pre-fixed with the name of the entity that it is both defined in and for which the Repository is parametrized. If that is just the default and you first look for a query named returnType.methodName, that would allow some more flexibility. Of course if the return type is not an Entity, then I would just go for the default.

In any case, the documentation should be clear on the options and precedence. Let me know if I can help. Thanks!

from spring-data-jpa.

Andres March commented

DATAJPA-45 created. I don't have a workaround yet for that one. Just to be clear that it is not a collection of entities but a collection of collections

from spring-data-jpa.

Oliver Drotbohm commented

With "Entity" I meant the type the Repository interface is typed to. Thus, if you have:

public interface ContactRepository extends Repository<Contact, Long> {

  Contact findByAddress(Address address);

  Person findByLastname(String lastname);

  Long countAllByLastname(String lastname);
}

the current Spring Data codebase will require the named queries to be named Contact.findByAddress, Person.findByLastname and Long.countAllByLastname where the last one is the obviously wrong (and thus problematic) one. It actually doesn't matter where the named query is defined as you can effectively only look em up by name anyway.

So I thought about the following expected named query names: Contact.findByAddress, Person.findByLastname and Contact.countAllByLastname. So the algorithm would be as follows:

1. Use the type, the Repository interface is typed to by default
2. Inspect the method's return type
   • If it's a sub-type of the entity type found in 1, use the sub-type.
   • Use the one found in 1 otherwise

This should also cover the scenario that let's you run into DATAJPA-45 as we'd find the Map there and thus fallback to the entity class of the Repository interface.

Agreed that the documentation is not very clear about this yet. Will update this with the fix accordingly.

What do you think?

from spring-data-jpa.

Andres March commented

We are on the same page about Entity. The algorithm sounds like exactly what I would expect. Thanks again

from spring-data-jpa.

Andres March commented

Is there any way I can help with this change? We are blocked until it is fixed. Just need to know where to start

from spring-data-jpa.

Oliver Drotbohm commented

Fixed the necessary parts in Spring Data Commons and added a testcase in JpaQueryMethodUnitTests. Should be available in tonights snapshot so feel free to give it a try

from spring-data-jpa.

Andres March commented

Exception with latest build. I haven't had time to debug yet.

Caused by: java.lang.AbstractMethodError: org.springframework.data.jpa.repository.query.JpaQueryLookupStrategy$DeclaredQueryLookupStrategy.resolveQuery(Ljava/lang/reflect/Method;Lorg/springframework/data/repository/support/RepositoryMetadata;)Lorg/springframework/data/repository/query/RepositoryQuery;
	at org.springframework.data.repository.support.RepositoryFactorySupport$QueryExecuterMethodInterceptor.<init>(RepositoryFactorySupport.java:287)
	at org.springframework.data.repository.support.RepositoryFactorySupport.getRepository(RepositoryFactorySupport.java:143)
	at org.springframework.data.repository.support.RepositoryFactoryBeanSupport.getObject(RepositoryFactoryBeanSupport.java:107)
	at org.springframework.data.repository.support.RepositoryFactoryBeanSupport.getObject(RepositoryFactoryBeanSupport.java:36)
	at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:142)
	... 36 more

from spring-data-jpa.

Oliver Drotbohm commented

When did you try that? We had issues getting the packages for Spring Data Commons Core into our snapshot repository this morning. Be sure you use latest snapshots of both Commons Core and JPA. Relevant CI builds seem to be fine: https://build.springsource.org/browse/SPRINGDATA

from spring-data-jpa.

Andres March commented

Looks like my SNAPSHOTS for both were fetched at 9:22 PDT. Here is the build date from the pom properties:

from spring-data-jpa.

Andres March commented

#Tue Apr 05 05:49:40 PDT 2011

from spring-data-jpa.

Oliver Drotbohm commented

Could you please try building using -U flag? According to your timestamp the correct artifacts have gone up a few minutes after that ;)

from spring-data-jpa.

Andres March commented

DATAJPA-45 is preventing me from fully testing but so far so good. Let me know if I can help with DATAJPA-45

from spring-data-jpa.

Andres March commented

I have worked around DATAJPA-45 by removing the parametrized arguments. I still have one test failure and this looks really bad:

Given the entity with the named queries:

@Entity
@Table(name = "COLLECTION_TYPE")
@NamedQueries(
{ @NamedQuery(name = "CollectionType.findByNameAndAttrGroup",
                query = "select c from CollectionType c inner join c.attributes ca where c.name=?1 and ca.group=?2)"),
        @NamedQuery(name = "CollectionType.findByName",
                query = "select c from CollectionType c inner join c.attributes ca where c.name=?1 )") })
public class CollectionType {

The following readCategoryType() test fails:

    @Test
    public void readAll()
    {
        List<CollectionType> types = typeRepo.findAll();
        assertEquals(types.size(), 1);
        
    }

    @Test
    public void readCategoryType()
    {
        List<CollectionType> types = typeRepo.findByName("CATEGORY");
        assertEquals(types.size(), 1);
        
    }

Read all return 1 result as expected. readCategoryType returns 2 results. I don't know how this could be due to spring data since it should be hibernate's fault. However this works under hades. There is one row in collection_type but multiple rows in coll attribute. The result from CollectionType.findByName is returning 1 row per attibute even though the collection type entity is being selected. I thought this was a simple projection query. Color me confused.

from spring-data-jpa.

Andres March commented

I just noticed that the "inner join c.attributes ca " wasn't being used, so I removed it. The test now passes. I don't think it should make any difference. Am I wrong?

from spring-data-jpa.

Oliver Drotbohm commented

In the case of named queries we're pretty much doing nothing but execute the query on the method call so you should be able to check whether it's us by simply executing the query against the plain EntityManager. Are you maybe missing a distinct that could cause the duplication?

I'll have my hands on that stuff until the end of the week

from spring-data-jpa.

Andres March commented

Makes sense that distinct would be provide but not sure why it was working before the upgrade. In any case, DATAJPA-44 should be resolved. Thanks!

from spring-data-jpa.