Conversation
| * <pre>{@code | ||
| * @Find | ||
| * List<Car> newVehicles(@By(_Car.MAKE) String make, | ||
| * @By(_Car.MODEL) String model, | ||
| * @By(_Car.PREVIOUSLYOWNED) False preOwned); | ||
| * ... | ||
| * | ||
| * found = cars.newVehciles("Jakarta Motors", | ||
| * "Jakadia", | ||
| * False.instance()); | ||
| * }</pre> |
There was a problem hiding this comment.
This use case seems like a lot of boilerplate without substantial benefit.
If I wanted to restrict this method on the Repository to only return previously owned vehicles, then I would like to write something like this:
@Query("where perviouslyOwned = false")
List<Car> newVehicles(@By(_Car.MAKE) String make,
@By(_Car.MODEL) String model);Whereas if I wanted to allow the repository user to choose new vs. pre-owned, then I would like to write something like this:
@Find
List<Car> search(@By(_Car.MAKE) String make,
@By(_Car.MODEL) String model
@By(_Car.PREVIOUSLYOWNED) Boolean preOwned );There was a problem hiding this comment.
This use case seems like a lot of boilerplate without substantial benefit.
That is something I wanted everyone to think about - whether it is worth it to have True and False constraints like we do for Null and NotNull.
The alternative that you pointed out--mixing the parameter based query pattern and @Query on the same method--is not something that Jakarta Data currently allows. Even if it were valid, involving @Query would not be consistent with the goal of being able to represent the simplest queries without query language.
Here is what I see being the alternative for parameter-based query if there is no True and False constraint and someone wants to require a true or false value,
@Find
List<Car> search(@By(_Car.MAKE) String make,
@By(_Car.MODEL) String model,
@By(_Car.PREVIOUSLYOWNED) boolean preOwned);
default List<Car> newVehicles(String make, String model) {
return search(make, model, false);
}There was a problem hiding this comment.
Yeah I think I had missed when the null/non-null restrictions went in, which is functionally the same as boolean.
I see the value of being able to use a boolean restriction while using the parameter based query pattern, but it seems awkward from a Java language standpoint to have a parameter on a method that can never be variable.
The alternative you presented is how I would have done it as well, but at the same time it exposes a method search the repository writer may not have wanted. I might need to think on this one.
There was a problem hiding this comment.
I think we need to accept that there are limits to this functionality. Which is fine, because when you reach the limits, you have ample much more powerful alternative.
There was a problem hiding this comment.
I had written up the True and False constraints to get an idea of whether they would be useful or not worth it. I don't think they are needed and it looks like others agree, so I have removed them from this PR. They aren't needed for BooleanExpression.isTrue/isFalse which can rely on the existing EqualTo(true/false) constraint instead.
| */ | ||
| package jakarta.data.metamodel; | ||
|
|
||
| import java.util.UUID; |
There was a problem hiding this comment.
once it is only documentation, does it make sense here?
There was a problem hiding this comment.
once it is only documentation, does it make sense here?
Yes, it's nice for usability when our Javadoc can directly link to the Java SE classes like UUID rather than showing it as text only java.util.UUID that customers need to go find themselves.
|
@gavinking , are you ok with moving this one forward? |
|
Well we have not made a decision with respect to the JPA criteria API yet. Now, in principle these don't need to be aligned, but I think it would be cleaner if they were. |
ecdb744 to
606966b
Compare
606966b to
960d708
Compare
|
Soft merge conflicts with the addition of |
gavinking
left a comment
There was a problem hiding this comment.
LGTM, I think we can go ahead with this now, since the similar issue with JPA has been resolved.
There has been some discussion in Query and Persistence about boolean expressions. This PR explores what that might look like in Jakarta Data. (Although none of it requires any changes from Query or Persistence because it is already possible based on
=TRUEand=FALSE)This would enable usage such as:
The alternative to the above that is already possible is to use,
Another possible reason for adding this is the future possibility that we want to do even more with boolean attributes/expressions, which would become automatically available to applications if their generated static metamodel were already using BooleanAttribute.