QueryByExamplePredicateBuilder does not work with Optional.empty (vs. straight null) #2176
Assignees
Labels
type: bug
A general bug
Comments
spring-projects-issues
added
the
status: waiting-for-triage
christophstrobl
added
type: bug
|
With a little digging, it appears to be trouble in Hibernate, where it's trying to bind to the field and throws up if you provide it an I ran your example and saw the same failure. But when I changed the getters return type from The question is, does Hibernate handle binding to |
gregturn
changed the title
|
Thanks @gregturn for investigating this. Hibernate in deed does NOT support I'm therefore closing this issue. |
|
@schauder this issue has not been resolved. And the fix is still within the bounds of Spring Data, so I would kindly ask you to reopen this ticket @gregturn thanks for the digging. However I don't think that this means that we need to give up on using Optional. As we can see from the Stacktrace an effort has been made to avoid passing an So the suggestions I made initinally on how I think this situation should be handled within Spring Data still stand I'd appreciate some feedback on those. Alternatively, if we can not use Optional then AbstractAuditable should also not use Optional since then any entity that uses AbstractAuditable will be a pain to use with the QueryByExample API. Causing me to question as to when I would ever seriously consider using it. |
|
While we use I agree that we cause the problem. The persistent property has the appropriate type and |
|
Thanks @mp911de. I'll figure out how to align things properly. |
|
I can't get this to fail anymore. Don't know if something has changed. Basically, I swapped in this on the And I updated the test to look like this: It seems to work now. I also can't seem to find where property vs field access is happening, and whether that is SD JPA or SD Commons that makes such a choice. |
|
Hi @gregturn, thanks for taking the time to look into this. I can still make it fail using the the modified org.springframework.data.jpa.repository.UserRepositoryTests#findAllByExampleWithEmptyProbeAndIgnoringNullValues in this repository https://github.com/pgerhard/spring-data-jpa/tree/query-by-example-and-abstract-auditable-issue When running this test as it is checked in, so with the following code added to I receive this exception If I add the following to I receive the following exception Are you seeing the same thing? |
gregturn
added a commit
that referenced
this issue
May 3, 2021
|
@pgerhard Only when you introduce the change from the internal type being When you have What happens in that scenario if you drop the And what is Spring Data supposed to do in such a scenario anyway? Sounds a bit hard to navigate through such a switch in contexts. Thought @schauder ? |
|
I think the type of the property should depend on the As a side note: I think the only type that should be used for things like |
|
@gregturn the original cause for me creating this issue was that AbstractAuditable was resulting in expcetions when using the QueryByExampleAPI. So while I'd love to use to In regards to your suggestion, I updated the code as follows and still received the exception (see at the end of this comment). I'm not a 100% sure what you mean with the "switch in context". However, from my understanding I'd expect the following
Stacktrace caused by the usage of |
|
Hmm. I switched everything over to Not sure what's happening, considering I haven't changed anything since last week. |
|
Okay, I've figured out why the test case was green last week but red today. Last week, my variant of Today, I had nudged things to If I drop that property from being ignored, then indeed it fails as you have shown. Trying to deduce where Spring Data is making that decision. |
|
If I apply If I apply This implies there may be more to properly configuring this. Again, not sure if this is stuff Spring Data JPA is supposed to work with or if we're going way outside the range of expectations. |
|
@gregturn phew, this issue is giving you more work than I was expecting. Sorry about that and thank you once again for spending time on this. What do you think of the suggested solutions I proposed here and here? Specifically I mean
|
gregturn
added a commit
that referenced
this issue
Jan 7, 2022
The Auditable interface introduces Optional getters, which when combined with Query by Example result in cryptic errors. By ignoring a probe's field that contains an Optional.empty, Query by Example works properly. Closes #2176.
gregturn
added a commit
that referenced
this issue
Jan 7, 2022
The Auditable interface introduces Optional getters, which when combined with Query by Example result in cryptic errors. By ignoring a probe's field that contains an Optional.empty, Query by Example works properly. Closes #2176.
|
@pgerhard Indeed, the issue is Query by Example's failure to properly handle |
gregturn
added a commit
that referenced
this issue
Feb 3, 2022
The Auditable interface introduces Optional getters, which when combined with Query by Example results in cryptic errors. By ignoring a probe's field that contains an Optional.empty, Query by Example works properly. NOTE: This fix actually tests outside the originally detected scope of Auditable, verifying that ALL Optional.empty() fields are properly handled. Closes #2176.
gregturn
added a commit
that referenced
this issue
Feb 3, 2022
The Auditable interface introduces Optional getters, which when combined with Query by Example results in cryptic errors. By ignoring a probe's field that contains an Optional.empty, Query by Example works properly. NOTE: This fix actually tests outside the originally detected scope of Auditable, verifying that ALL Optional.empty() fields are properly handled. Closes #2176.
schauder
pushed a commit
that referenced
this issue
Feb 15, 2022
The Auditable interface introduces Optional getters, which when combined with Query by Example results in cryptic errors. By ignoring a probe's field that contains an Optional.empty, Query by Example works properly. NOTE: This fix actually tests outside the originally detected scope of Auditable, verifying that ALL Optional.empty() fields are properly handled. Closes #2176 Original pull request #2401
AntonMolganov
pushed a commit
to AntonMolganov/spring-data-jpa
that referenced
this issue
Apr 14, 2022
The Auditable interface introduces Optional getters, which when combined with Query by Example results in cryptic errors. By ignoring a probe's field that contains an Optional.empty, Query by Example works properly. NOTE: This fix actually tests outside the originally detected scope of Auditable, verifying that ALL Optional.empty() fields are properly handled. Closes spring-projects#2176 Original pull request spring-projects#2401
AntonMolganov
pushed a commit
to AntonMolganov/spring-data-jpa
that referenced
this issue
Apr 14, 2022
Removed Given/When/Then comments. They are of limited use, especially since it is debatable what belongs in which section. See spring-projects#2176 Original pull request spring-projects#2401
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When using AbstractAuditable with Query By Example the following exception is thrown
and if the
createdByandlastModifiedBypaths are ignored the following exception is caused by thecreatedDateandlastModifiedDateattributesSetting
ExampleMatcher.matching().withIgnoreNullValues()does not change this. The only way to prevent is to exclude all fields provided by AbstractAuditable.I would expect the following
withIgnoreValues()on theExampleMatcherprevents these errors entirelySharedEntityManagerCreator$SharedEntityManagerInvocationHandlershould be able to handle the conversion of anOptional.emptywithout an errorcreatedByandlastModifiedBymentioned the path that caused the error, instead of just reporting that an object may be null. Especially since the usage ofOptionalis to prevent issues with null valuesI modified the User to exhibit similar behaviour to AbstractAuditable and can reproduce the issue in the UserRepositoryTest. The code is available here pgerhard@db65778
The text was updated successfully, but these errors were encountered: