Skip to content

Query by type does not accept type constraints [DATAJPA-1257] #1591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spring-projects-issues opened this issue Feb 2, 2018 · 10 comments
Closed
Labels
for: external-project For an external project and not something we can fix

Comments

@spring-projects-issues
Copy link

Thomas Lang opened DATAJPA-1257 and commented

Hey there spring data jpa folks,

I don´t know if this is really a bug.
The stackoverflow link tells basically everything you need.

Here the details:
I have an abstract class with others derived from that.
I use a repository to query the resulting database table for the discriminator type.

When i use this repository:
Please note: the class constraint on the query type

/**
     * Finds the given role by a given discriminator value and a certain application user id.
     *
     * @param abstractRole      a given abstract role
     * @param applicationUserId a given application user id
     * @return AbstractRole
     */
    @Query("select ar from AbstractRole ar where type(ar) = ?1 and ar.applicationUser.applicationUserId = ?2")
    AbstractRole findsProjectedByRoleTypeAndApplicationUserId(Class<? extends AbstractRole> abstractRole, long applicationUserId);

With the following code:

final AbstractRole abstractRole = abstractRoleRepository.findsProjectedByRoleTypeAndApplicationUserId(AdminRole.class, 6074);

I face the following exception:

org.springframework.dao.InvalidDataAccessApiUsageException: Parameter value [6074] did not match expected type [java.lang.Class (n/a)]; nested exception is java.lang.IllegalArgumentException: Parameter value [6074] did not match expected type [java.lang.Class (n/a)]

When i use the solution suggested in the stackoverflow link (leave out the type constraint) it works as expected.

/**
     * Finds the given role by a given discriminator value and a certain application user id.
     *
     * @param abstractRole      a given abstract role
     * @param applicationUserId a given application user id
     * @return AbstractRole
     */
    @Query("select ar from AbstractRole ar where type(ar) = ?1 and ar.applicationUser.applicationUserId = ?2")
    AbstractRole findsProjectedByRoleTypeAndApplicationUserId(Class<?> abstractRole, long applicationUserId);

So basically the constraint would be cool to have, because it makes the usage safer.
But for now, i can life with that.

Please respond if you need some more information about this.

Have a nice day.

By the way:
Keep up the good job with spring data jpa. This framework is outstanding.
Makes me happy to have this project on board for my daily business.

Regards from Lower Bavaria!
Thomas


Affects: 1.11.10 (Ingalls SR10)

Reference URL: https://stackoverflow.com/questions/47052706/spring-data-repository-query-by-class-with-generic-class

Issue Links:

  • DATAJPA-1497 Allow to add a special parameter type from a Spring Data JPA extension

3 votes, 6 watchers

@spring-projects-issues
Copy link
Author

Jens Schauder commented

I'm pretty sure this is a duplicate but I can't find the older issue.

I don't consider it a bug because class parameters are used for dynamic projections. So it does work as currently designed.

Question is: What could be a way of distinguishing a parameter of type Class that is used in the query from one that is used for a dynamic projection, from one that is used for both?

@spring-projects-issues
Copy link
Author

Thomas Lang commented

Hey there Jens, thank you for your quick reply.
I have just seen at your customer github repository would you mean by "dynamic projection".

I haven´t seen that up to now.
You guys at the spring data jpa team taylor a lot of useful stuff in new releases - awesome.
Nice job!

Back to topic:
I can life with the solution of not to restrict the class parameter.
Because i hide the repository layer package-private, only the service can/should use it.
And if one may call it with a wrong class type, nothing happens.
So everything is fine.

For now i could think of the following solution:

  1. Define a dummy interface - called IAbstractRole (no method bodies)
  2. Let AbstractRole implement this IAbstractRole interface, so all extended concrete types inherit this interface
  3. Put a constraint the following way
AbstractRole findsProjectedByRoleTypeAndApplicationUserId(Class<? implements IAbstractRole> abstractRole, long applicationUserId);

But as far as i know, this is not possible with java.

To make a long story short:
I don´t consider this as a bug too. I can only think of the above idea, but as far as i know this is not possible with java.

So as for me you can close the issue.

Thank you very much for your help.
Kind regards from Lower Bavaria
Thomas

@spring-projects-issues
Copy link
Author

Florian Schaetz commented

Why must it be Class<?> at all? Why not simply define another class that takes over the special behavior, for example...

X findXByYProjected(DynamicProjection<?> to, Y y);

Calling it would be simply...

findXByYProjected(DynamicProjection.to( someClass ), myY);

This would "free" that Class parameter type from being used in dynamic projections, so that it now can be used as a normal parameter.

@spring-projects-issues
Copy link
Author

Thomas Lang commented

Hey there Florian,

thank you for your comment. Although i don´t understand your point utterly.

What my query above does is to select only suitable records, matching a certain discriminator value by the hibernate function type.
See here:

@Query("select ar from AbstractRole ar where type(ar) = ?1 and ar.applicationUser.applicationUserId = ?2")

So i want to query a role table which is implemented by Single Table Inheritance. So all kind of role types are stored in that table.
Now i want to get all rows of type CompanyRole only for example. That does the Class parameter.

As icing on the cake so to speak i want a user not to be able to put all kinds of Objects to the query, like String.class or else.
So it would be cool to put a constraint on the method.
But that does not work.

But as Jens said, it is not a bug, because if someone calls the method as following:

final AbstractRole abstractRole = abstractRoleRepository.findsProjectedByRoleTypeAndApplicationUserId(String.class, 6074);

The query gets executed right.

So for my usage, i am cool with the given situation.

How would i adapt your solution to my case?
Thank you all for your help and tips.

Kind regards from Lower Bavaria
Thomas

@spring-projects-issues
Copy link
Author

Jens Schauder commented

Hi Thomas,

I think Florians comment wasn't directed so much at you but a proposal for an improvement.

The current problem is that a parameter of type Class is used for dynamic projection and can't be used as a parameter.

If Spring Data does one of the following the problem would go away:

  • use a special wrapper type for dynamic projections and normal Class parameters as query arguments.
  • use a special wrapper type for classes as query argument and normal Class parameters for dynamic projections.

I'm also wondering if we can just make a Class typed parameter available as a query parameter all the time. The problem with that might only relate to certain checks that are skipped in many cases anyway. I'll have to look into that

@spring-projects-issues
Copy link
Author

Stefan Penndorf commented

Hi Jens,

 

today I experienced this issue as well. I tend towards calling it a bug because it is - at least - absolute unexpected behavior to me. Projections might be a cool feature but I don't expect java.lang or any other native JDK type to have a special meaning when used as a parameter.

I like Florians idea using a special class as you already do with Pageable. Nobody had the idea using one or two random Long parameters as pagination parameters....

If you really insist in keeping this behavior I'd rather suggest improving the error message and error handling here - it took me some ours to find this issue because I'd expected the JPQL query to be wrong in the first place.

 

What is my usage scenario?

We do have an abstract class "Document" with two subclasses "InternalDocument" and "ExternalDocument". Both share common metadata but have 2 resp. 4 individual fields. For some operations it doesn't matter what type of Document you'll have (e.g. automatic information about new documents). But they are shown in different views with different filter options and visualizations (taking the individual fields into account). Additionally there are different interfaces (SOAP based) that use either "InternalDocument" or "ExternalDocument". There is a unique business key (a document id) that is shared with other applications and will be enforced using a unique constraint in the database. The SOAP based webservices do have very much in common, thus there is abstract parent class implementing those services

abstract class AbstractDocumentService<D extends Document, X extends XmlAbstractDocument> {
  private final Class<D> docClazz; //injected in Constructor

  @Override
  public List<T> getDocuments(Account account, String docIdFrom, String docIdTo) {
    
    final Stream<D> docs = this.documentRepository.findeForAccountAndIdRange(account, docClazz, docIdFrom, docIdTo);

    return docs.map(documentConverter::convertToXml).collect(toList());
 }

 

Thus I'd like to issue the following query:

@Query("SELECT d FROM Document d "
 + "WHERE d.account = :account "
 + "AND TYPE(d) = :docClazz "
 + "AND d.docId BETWEEN :docIdFrom AND :docIdTo ")

My current solution is to use the discriminator value / column as a replacement for TYPE(d). Than I do have a String parameter

@Query("SELECT d FROM Dokument d WHERE [...] d.discriminatorColumn = :discriminatorValue [...]")

But this is a hack and I assume the user (implementor) of AbstractDocumentService uses the correct Discriminator String with the corresponding Class value. If not we do not know until the code gets executed at runtime.... It would be nicer and more object oriented to rely on the Class of the entity only

@spring-projects-issues
Copy link
Author

Ruslan Stelmachenko commented

I agree with commenters above.

The JPA discriminator feature is important and spring-data-jpa should not prevent to use it. The hack with adding a discriminator column as a entity's field is a hack, that clutters a model layer and a service layer forcing to pass a discriminator value (an internal DB representation, which should nothing to do with service layer) instead of class.

And projections should use a dedicated type instead of Class.
This would be backward-incompatible change unfortunately. But I think better is do it earlier than later.

I think, this is a design problem of projections feature, as nobody considered a JPA discriminator feature when projections feature was designed. Nobody thought that Class type can be useful in a JPQL query. That was obvious that Long type can't be used for something like offset or limit, as this is ambiguous obviously, and the dedicated Pageable type was created. But I think it was not so obvious for a Class type

@spring-projects-issues
Copy link
Author

Jens Schauder commented

I agree, that it probably would have been better to introduce a special type for dynamic projections.
But we won't change that in the foreseeable future.

In order to use a class as an actual bindable parameter you can wrap it in another class and then use a SpEL expression to unwrap the class object and pass it to the query.

See: https://docs.spring.io/spring-data/jpa/docs/current/reference/html/#jpa.query.spel-expressions

@nseb
Copy link

nseb commented Dec 23, 2022

Hello , the problem is still relevant ? Because the .class in hql query was deprecated and now we need use type(myclass) and pass a class in parameter. This is going to get really blocking

@mp911de
Copy link
Member

mp911de commented Jul 13, 2023

This has been addressed via spring-projects/spring-data-commons#2770

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Jul 13, 2023
@mp911de mp911de added for: external-project For an external project and not something we can fix and removed type: enhancement A general enhancement labels Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix
Projects
None yet
Development

No branches or pull requests

4 participants