Skip to content
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

Allow to add a special parameter type from a Spring Data JPA extension [DATAJPA-1497] #1810

Closed
spring-projects-issues opened this issue Jan 18, 2019 · 8 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

Réda Housni Alaoui opened DATAJPA-1497 and commented

I am the maintainer of spring-data-jpa-entity-graph

This extension needs to add a special parameter type to Spring Data JPA.
This special parameter type is EntityGraph

For 2 years, the type has been added by reflection to Parameter.TYPES and Parameters.TYPES.
Since Java 9, the library has been producing illegal access warning.

Would it be possible to add an extension point for special types to avoid the reflection?


Affects: 2.1.4 (Lovelace SR4)

Issue Links:

  • DATAJPA-1257 Query by type does not accept type constraints

Referenced from: pull request #305, and commits 4c6279e, 85e4ffa, 8f5f3af

7 votes, 9 watchers

@spring-projects-issues
Copy link
Author

Réda Housni Alaoui commented

A pull request has been opened: #305

@spring-projects-issues
Copy link
Author

Jens Schauder commented

I'll push this back until we have a solution for DATAJPA-1257 since it affects how we determine bindable parameters in the first place

@spring-projects-issues
Copy link
Author

Jens Schauder commented

We discussed this internally and came to the following conclusion:

We won't accept the PR.
It makes JpaParameters public API for which it never was intended.
Also we are not convinced that the benefits of such a change would support a wide enough audience to be worthwhile.

In order to make this feature happen we would need to see the following:

  1. A clear description what kind of extendability we are aiming for.
  2. A couple of use cases that would benefit from it.
  3. A clear abstraction on which the extensions can be build

@spring-projects-issues
Copy link
Author

Réda Housni Alaoui commented

Hello Jens Schauder,

I can only speak for https://github.com/Cosium/spring-data-jpa-entity-graph needs.

The library allows to pass EntityGraph as arguments of Spring Data JPA repository methods.
To do that:

  1. it flags EntityGraph as Spring Data JPA special parameter type
  2. thanks to addRepositoryProxyPostProcessor, it proxies spring data jpa core repositories.
  3. on method invocation, repositories proxies watch for EntityGraph argument, interpret them and turn them into JPA query hints

Without step 1, running the application fails with exception like the following:

Caused by: java.lang.IllegalArgumentException: At least 2 parameter(s) provided but only 1 parameter(s) present in query.
	at org.springframework.util.Assert.isTrue(Assert.java:136)
	at org.springframework.data.jpa.repository.query.QueryParameterSetterFactory$CriteriaQueryParameterSetterFactory.create(QueryParameterSetterFactory.java:297)

The purpose of my demand is to make it possible to perform step 1 without reflection.

Since addRepositoryProxyPostProcessor already exists, we may be just missing an extension point to add custom special parameter type. This was the purpose of my first PR.

But here are some alternatives.

1. Make org.springframework.data.repository.query.Parameters.TYPES and org.springframework.data.repository.query.Parameter.TYPES mutables

2. Define a SpecialType interface

An interface would identify custom special types.
This could be added to Spring Data JPA or even Spring Data Commons.

// In Spring Data JPA or Commons code
public interface SpecialType{}
// In consumer code
public class MySpecialType implements SpecialType{}
// In consumer code
public interface MyRepository extends CrudRepository<MyEntity, Long> {
  Optional<MyEntity> findByName(String name, MySpecialType mySpecialArgument);
}

3. Define a SpecialType wrapper

A class would embed custom special types.
This could be added to Spring Data JPA or even Spring Data Commons.

// In Spring Data JPA or Commons code
public class SpecialType<T>{
  private final  T value;
  private SpecialType(T value){
    this.value = value;
  }

  public static SpecialType of(T value){
    return new SpecialType<>(value);
  }

  public T value(){
    return value;
  }
}
// In consumer code
public class MySpecialArgument{}
// In consumer code
public interface MyRepository extends CrudRepository<MyEntity, Long> {
  Optional<MyEntity> findByName(String name, SpecialType<MySpecialArgument> mySpecialArgument);
}

4. Go further and allow extensions to alter queries before they make it to the ORM

This is the most ambitious alternative.
I remember an existing ticket about this but could not find it.
Before performing a query, Spring Data would pass the query with the unknown method arguments to the extension. The extension would have to return the altered/unaltered query to let Spring Data execute it.
This alternative allows all use cases requiring systematic query customization

@spring-projects-issues
Copy link
Author

Réda Housni Alaoui commented

I think the current issue relates to DATAJPA-1551 and DATACMNS-293

@spring-projects-issues
Copy link
Author

idkw commented

The library at https://github.com/Cosium/spring-data-jpa-entity-graph/issues/23 is a must have for me for precise per-request and dynamic EntityGraph configuration.

 

Jens Schauder Can you reconsider your position regarding the PR rejection or suggest an alternate way how we could avoid the reflection requirement ?

@spring-projects-issues
Copy link
Author

Jens Schauder commented

We had another discussion and decided to merge the original PR with some little changes that shouldn't hurt the usability of the PR

@spring-projects-issues
Copy link
Author

Réda Housni Alaoui commented

Great, thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants