Add Criteria-Based Filtering

[otaviojava]

This PR introduces enhanced attribute-based querying capabilities to Jakarta Data, consolidating query construction into Attribute, SortableAttribute, and TextAttribute interfaces with corresponding implementations. These updates provide:

• A streamlined and type-safe approach to filtering and sorting.
• Flexible criteria-based filtering with support for equality, null checks, range queries, and pattern-based matching.
• Inline declarative filtering and sorting, simplifying query construction.

@Repository
public interface Library {
    List<Book> find(CompositeRestriction<Book> restriction, Sort<Book> order);
}

// Usage in a service or query:
CompositeRestriction<Book> restriction = CompositeRestriction.all(
    _Book.title.like(Pattern.prefixed("Jakarta")),
    _Book.edition.equal(1),
    _Book.publicationDate.between(pastDate, LocalDate.now())
);

Sort<Book> order = _Book.title.asc();

// Executing the query
List<Book> results = library.find(restriction, order);

Criteria nameCriteria = _Book.title.like("Data%");
Criteria dateCriteria = _Book.publicationDate.between(pastDate, LocalDate.now());
List<Book> books = bookRepository.findByCriteria(nameCriteria, dateCriteria);

[otaviojava]

I see both working together:

• Ability to define query restrictions dynamically #829
• Proposal for Criteria Feature in Jakarta Data[Vote]  #460

Instead of having one API for each other,

[gavinking]

Some comments:

1. The Criteria type should be parameterized by the entity type, for type safety.
2. I prefer Restriction and Operator to Criteria and FilterType. In particular, "criteria" is a really badly overloaded word, and potentially very confusing here.
3. I would make not() or negated() a method of Restriction/Criteria, not a separate static factory method.
4. Unless I've missed it, the binary logical operations are missing from this proposal. I suppose they should follow the pattern we already established with Sort and Order.
5. I believe you're going to have warnings if you put varargs on an interface, so it's better if those operations on CriteriaRepository accept a single conjunction of restrictions, instead of multiple restrictions.
6. I'm still extremely unexcited by the idea of embedding an expression language in annotations: @Is(value=LIKE, not=true) doesn't float my boat at all.

Apart from those comments this is pretty much along the lines of what I was hoping for.

[njr-11]

Yes, this is generally the right ideas.

1. The `Criteria` type should be parameterized by the entity type, for type safety.

agree

2. I prefer `Restriction` and `Operator` to `Criteria`  and `FilterType`. In particular, "criteria" is a really badly overloaded word, and potentially very confusing here.

agree (I actually have some of this coded up locally using the name Restriction, but hadn't finished it yet)

3. I would make `not()` or `negated()` a method of `Restriction`/`Criteria`, not a separate static factory method.

I've been going back and forth on how best to handle not/negate. There are other options as well beyond what is in this PR. I need to think about this some more.

4. Unless I've missed it, the binary logical operations are missing from this proposal. I suppose they should follow the pattern we already established with `Sort` and `Order`.

yes

5. I believe you're going to have warnings if you put varargs on an interface, so it's better if those operations on `CriteriaRepository` accept a single conjunction of restrictions, instead of multiple restrictions.

First, I don't think we need a CriteriaRepository. But the same issue would arise regardless when a user composes a method with multiple restrictions. The way I wrote it up (and which I've also tried locally) is to have Restrict.all(Restriction...) and Restrict.any(Restriction...) which basically covers item 4 above (ignoring that I'm still considering options for item 3 on not/negate)

6. I'm still extremely unexcited by the idea of embedding an expression language in annotations: `@Is(value=LIKE, not=true)` doesn't float my boat at all.

I'm very much in favor of @Is and really want to include it. However, for the ideal usability it should only ever have a single field being the enumerated value, so it would be @Is(NOT_LIKE) which reads much better. See PR #892 which I had already written up for it.

[gavinking]

The way I wrote it up (and which I've also tried locally) is to have Restrict.all(Restriction...) and Restrict.any(Restriction...) which basically covers item 4 above

Yup, that's exactly what I meant by following the precedent already established by Sort/Order.

[otaviojava]

1. The Criteria type should be parameterized by the entity type, for type safety.
   Done

2. I prefer Restriction and Operator to Criteria and FilterType. In particular, "criteria" is a really badly overloaded word, and potentially very confusing here.

Done

3. I would make not() or negated() a method of Restriction/Criteria, not a separate static factory method.

Do you have any suggestion of how?

4. Unless I've missed it, the binary logical operations are missing from this proposal. I suppose they should follow the pattern we already established with Sort and Order.

My initial thought was, by default, working only with ANDbut I am fine to allow ORas well.
What do you think now?

6. I believe you're going to have warnings if you put varargs on an interface, so it's better if those operations on CriteriaRepository accept a single conjunction of restrictions, instead of multiple restrictions.

I've created a collection representation, what do you think?

[otaviojava]

I've been going back and forth on how best to handle not/negate. There are other options as well beyond what is in this PR. I need to think about this some more.

@njr-11 could you bring it to this PR?

[otaviojava]

@njr-11 @gavinking does it makes sense?

[gavinking]

OK, so this is quite a bit different now. :) I have two questions:

1. would it be better to implement these new methods as default methods of the Attribute interface?
2. would it have been better to leave Restriction as a record, and the also have Restrict as the composite type, like what we do with Sort and Order, rather than the new design here which makes Restriction an interface?

[otaviojava]

• would it be better to implement these new methods as default methods of the Attribute interface?

1. Greate! I moved it might not break anything
2. I've moved to the interface because the Pattern will implement it as well and there is the CompositeRestriction

For sure, we can change it

[njr-11]

1. would it be better to implement these new methods as default methods of the Attribute interface?

This is an excellent idea. It eliminates the problem that would have occurred if any annotation processors were supplying their own *Attribute implementations rather than using the provided ones. With the default methods in place, even if they were doing that, there will be no breaking change at all.

[njr-11]

feel free to open PR as well.

We do have all of the Restriction changes only (minus JavaDoc which I don't think we should be adding yet while there are so many API changes still going on) split out under #895 which Gavin has reviewed.

Regarding Pattern, we are getting closer. Here is my review of what you have for the Pattern class under this PR. A general comment is that it's important to account for the possibility that the Strings supplied to endsWith, contains, and so forth will include wildcard characters within them and will behave as patterns as well. My code suggestions included covering that possibility.

[njr-11]

You should not have ignoreCase as a field of Pattern. I understand logically it could make sense, but it is already a field of Restriction, and it is confusing to have it in both places where users will wonder which takes precedence,

employees.search(
    Restriction.like(new Pattern(prefixPattern, ignoreCase = false)).ignoreCase());

It would also be confusing in places like this:

employees.findByLastNameIgnoreCaseLike(new Pattern(prefixPattern, ignoreCase = false));

Omit it from Pattern and none of the above will even be possible.

Also, value is confusing. Specifically, this is a pattern for LIKE. If someone does,

String prefix = "Jakarta";
Pattern jakartaPrefixPattern = new Pattern(prefix

it won't actually behave as a prefix. We are going to need to make this very clear so it isn't misused (any maybe we don't even want to use a record with a public constructor)

I'm not sure what best to call it. Here is something I thought of quickly,

Suggested change

	public record Pattern(String value, boolean ignoreCase) {
	public record Pattern(String like) {

[gavinking]

You should not have ignoreCase as a field of Pattern.

I don't agree with this. One of the main nice things about Pattern is that it encapsulates case-sensitivity.

By original proposal was:

public record Pattern(String pattern, boolean caseSensitive) {

    public Pattern ignoringCase() {
        return new Pattern(pattern, false);
    }

    public static Pattern is(String literal) {
        return new Pattern(escape(literal), true);
    }

    public static Pattern matches(String pattern) {
        return new Pattern(pattern, true);
    }

    public static Pattern matches(String pattern,
                                  char characterWildcard, char stringWildcard) {
        final String standardized =
                escape(pattern)
                        .replace(characterWildcard, '_')
                        .replace(stringWildcard, '%');
        return new Pattern(standardized, true);
    }

    public static Pattern startsWith(String prefix) {
        return new Pattern(escape(prefix) + '%', true);
    }

    public static Pattern endsWith(String suffix) {
        return new Pattern('%' + escape(suffix), true);
    }

    public static Pattern contains(String substring) {
        return new Pattern('%' + escape(substring) + '%', true);
    }

    private static String escape(String literal) {
        return literal.replace("_", "\\_").replace("%", "\\%");
    }
}

Where, the idea was that users would not usually call the constructor to obtain a Pattern.

[njr-11]

Irrespective of the discussion on ignoringCase, the automatic escaping that you proposed above seems very useful, and helps users avoid unintentional wildcards in their prefix/suffix/substring, which can be literals now instead of patterns. I like that better than what is under this PR.

[otaviojava]

Nice, I agreed.
I moved this class to our PR.

[otaviojava]

#895

About Pattern, I used the same Gavin.

On your PR, I moved without keeping out the NOT condition, at least for now.

[otaviojava]

@njr-11 @gavinking

Hey, I've covered all the mentioned points, and I believe what we have here is good enough to merge and become our first draft version.

Once merged, we can go more scalable, I mean, create an Epic task where we can work on:

Spec documentation
Test scenarios
Naturally, experiment and refine the API
Have a Track to discuss the "not" operation.
Finally, create the TCK (once we agree with what we have)

Note: I believe that is a long journey. I would like to propose the first step here.

[njr-11]

@njr-11 @gavinking

Hey, I've covered all the mentioned points, and I believe what we have here is good enough to merge and become our first draft version.

Once merged, we can go more scalable, I mean, create an Epic task where we can work on:

Spec documentation Test scenarios Naturally, experiment and refine the API Have a Track to discuss the "not" operation. Finally, create the TCK (once we agree with what we have)

Note: I believe that is a long journey. I would like to propose the first step here.

This isn't making progress because you are trying to do too much at once in a single PR. I stopped reviewing the rest of the code under this PR because I don't agree with what is being done in Pattern and it is unclear to me at this point whether or not we should even have Pattern and Range. Decisions such as whether to have those, or whether or not to have @Is should not be tied to adding the API for Restriction. I don't think we are going to come to consensus on all of these different API proposals all at once. I ended up splitting out the Restriction piece only under #895 which is what I think we can gain agreement on first. Gavin reviewed it, and I addressed all of his comments. If you want to review that PR, we could try to get Restriction in and then come back to some of these other proposals.

[otaviojava]

@njr-11 @gavinking

I got your point. In my opinion, those should work as a single one; they should either go together or not go, mainly, because those new types, Pattern and Range should be integrated with this one as well.

• About Is it is removed.
• About the Range it is the Gavin code with documentation included.
• About your PR, I've moved the code to this one and kept what we agreed on in the last meeting:

https://docs.google.com/document/d/1MQbwPpbEBHiAHes1NaYTJQzEBGUYXxaJYw5K-yj053U/edit?tab=t.0#heading=h.qpadg2lwp2oh

I don't mind working the whole year in a single branch, but those features should work together.

[njr-11]

I got your point. In my opinion, those should work as a single one; they should either go together or not go, mainly, because those new types, Pattern and Range should be integrated with this one as well.

If Pattern and Range end up being introduced, it might make sense to be able to create Restrictions from them. However, the concept of Restriction has no dependency whatsoever on these classes.

#895 shows that to be the case because it has provided Restriction without them. It enables a user to write only,

products.search(_Product.name.endsWith(suffix));

products.search(_Product.name.endsWith(suffix).ignoreCase());

which is much cleaner than what you have them doing in your PR,

products.search(_Product.name.like(new Pattern('%' + suffix, false));

products.search(_Product.name.like(Pattern.endsWith(suffix));

Note that you have ignoreCase as default for Pattern with the only way to avoid it being to construct a Pattern instance. Even if you correct that, you end up with something like,

products.search(_Product.name.like(Pattern.endsWith(suffix));

products.search(_Product.name.like(Pattern.endsWith(suffix).ignoreCase());

Not only is it unnecessary for Restriction and Pattern to go together, in almost all cases where users want Restrictions, adding in Pattern will only be unwanted extra typing that doesn't provide any additional value to the user.

[otaviojava]

I got your point. In my opinion, those should work as a single one; they should either go together or not go, mainly, because those new types, Pattern and Range should be integrated with this one as well.

If Pattern and Range end up being introduced, it might make sense to be able to create Restrictions from them. However, the concept of Restriction has no dependency whatsoever on these classes.

About Pattern I am using exactly the Gavin one with documentation: #894 (comment)

About you said, we have access to exactly same methods, thus:

You can do those as well:

products.search(_Product.name.like(Pattern.endsWith(suffix));

products.search(_Product.name.like(Pattern.endsWith(suffix).ignoreCase());

[njr-11]

I got your point. In my opinion, those should work as a single one; they should either go together or not go, mainly, because those new types, Pattern and Range should be integrated with this one as well.

I don't think you are understanding the point. You seem to be arguing that it would be invalid to add Restriction without Pattern because then you could only do,

products.search(_Product.name.endsWith(suffix));

products.search(_Product.name.endsWith(suffix).ignoreCase());

but not also,

products.search(_Product.name.like(Pattern.endsWith(suffix)));

products.search(_Product.name.like(Pattern.endsWith(suffix).ignoreCase()));

Those latter two examples are redundant, more verbose, and provide no additional value. This is not a valid reason to claim that Pattern must be introduced along with Restriction.

[otaviojava]

I don't think you are understanding the point. You seem to be arguing that it would be invalid to add Restriction without Pattern because then you could only do,

Yeap, I am lost here because at TextAttribute we do have:

    default Restriction<T> like(Pattern pattern)...

Thus, we can do this since name might be a TextAttribute:

products.search(_Product.name.like(Pattern.endsWith(suffix)));
products.search(_Product.name.like(Pattern.endsWith(suffix).ignoreCase()));

We also have default Restriction<T> like(String text) that allows users to set their own terms to like.

Plus, we also have support for Pattern as you said.

Thus, we have those as supported. Right now, TextAttribute can work either Pattern or without Pattern, the same as `Pattern´, which can work alone as well.

Do you want to remove the method?

    default Restriction<T> like(Pattern pattern)...