spring-data-commons icon indicating copy to clipboard operation
spring-data-commons copied to clipboard

Null Handling of Repository Methods not implemented correctly for "Containing" kind of methods

Open vgarmash opened this issue 3 years ago • 8 comments

In the documentation we see that it is possible to manage nullability for the repository methods. Look at section 9.4.7 Null Handling of Repository Methods.

image

While it works for simple methods like: User findByEmailAddress(@Nullable EmailAddress emailAdress);

The same approach will cause an error for "Containing" kind of methods: User findByFirstnameContaining(@Nullable String name)

If I call this method with null name I get this error: "message": "Argument for creating $regex pattern for property 'section' must not be null!"

I think the more appropriate solution would be omitting any null input parameter during query building phase.

vgarmash avatar May 03 '21 16:05 vgarmash

Can you help us understand your context? The sample for findByFirstnameContaining refers to Firstname being a singular property. What should a contains query using null even mean? Do you mean In queries that can match null values?

The mentioned sample, findByEmailAddress(@Nullable EmailAddress emailAdress) works using null as the entire property can be compared against null.

mp911de avatar May 10 '21 14:05 mp911de

@mp911de You right: the finder with singular input doesn't make much sense because all you can do in this case is either return nothing or return everything from the collection. Although imagine the table with filter, something like this: image

You can have a filter finder implemented like following:

List<Order> findByDeliveryAddressContainingAndItemInAndCostContainingAndRegionContainingAllIgnoreCase(
                                         @Nullable String deliveryAddress, @Nullable Collection items, 
                                         @Nullable String cost, @Nullable String region);

In cases like this all input fields are optional and when nothing is entered we expect all data to be displayed.

I personally tried to implement finders like that one above but found the problem with nullable String inputs. Therefore I created this ticket to address the issue.

As I said, I think the more appropriate solution would be omitting any null input parameter during query building phase. Although if developers think otherwise they can put a disclosure into the documentation saying what types of queries should not use @Nullable inputs at all. In the current version those would be all queries, implemented using regex. See them all in Table 15. Supported keywords for query methods in section 15.3. Query Methods: Like, StartingWith, EndingWith, NotLike, IsNotLike, Containing on String, NotContaining on String.

I personally have not tested all combinations of queries and null inputs, but I hope that someone at Spring Data team would write more unit tests to find other incompatible queries with @Nullable inputs.

vgarmash avatar May 11 '21 15:05 vgarmash

Thanks for the detail. Query methods aren't designed to cover absence of parameters where null or other means exclude certain parameters from being part of the filter query.

What you're looking for is query by example via QueryByExampleExecutor. With Query by Example you can provide a query object to derive a query based on null-rules for properties. Additionally, you can define matching rules (exact match, contains queries, starts with/ends with).

mp911de avatar May 11 '21 16:05 mp911de

Query methods aren't designed to cover absence of parameters where null or other means exclude certain parameters from being part of the filter query.

If so, then section 9.4.7 need to be updated, because it says that we can just add @Nullable to the input parameter and it should just work. Currently it cause the error. Therefore the documentation need to be updated with more details about when @Nullable inputs expected to work properly.

As far about QueryByExample -you are right. Although to implement special matching rules per collection I have to implement the finder myself (like shown in 11.7.3. Example Matchers). I was hoping to implement all finders using repository queries defined as an interface methods so the framework will take care of the queries. I need so because I use it in combination with Spring Data Rest where REST API is being generated on top of my Spring Data registries.

vgarmash avatar May 11 '21 18:05 vgarmash

We could improve the wording to mention that nullability annotations make only sense for parameters in which it actually makes sense.

mp911de avatar May 12 '21 09:05 mp911de

I moved the ticket to Spring Data Commons where the common documentation fragments are maintained.

mp911de avatar May 12 '21 09:05 mp911de

We could improve the wording to mention that nullability annotations make only sense for parameters in which it actually makes sense.

I would like to insist on more specific wording. "makes sense" is not specific enough. It should specifically mention incompatibility between @Nullable String input and all queries built using keywords: Like, StartingWith, EndingWith, NotLike, IsNotLike, Containing on String, NotContaining on String.

vgarmash avatar May 12 '21 09:05 vgarmash

Sounds that you have already a suggestion on your mind how the docs should look like. I'd be happy to review a pull request that changes https://github.com/spring-projects/spring-data-commons/blob/main/src/main/asciidoc/repository-query-keywords-reference.adoc by introducing another column whether the specific keyword would accept a nullable argument.

mp911de avatar May 12 '21 09:05 mp911de