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

Refine Specification API #3578

Closed
wants to merge 7 commits into from
Closed

Refine Specification API #3578

wants to merge 7 commits into from

Conversation

mp911de
Copy link
Member

@mp911de mp911de commented Aug 13, 2024

Introduce DeleteSpecification and UpdateSpecification. Add PredicateSpecification. Update SpecificationExecutor.

See #3521

@mp911de mp911de added the type: enhancement A general enhancement label Aug 13, 2024
@mp911de mp911de requested review from christophstrobl, schauder and odrotbohm and removed request for christophstrobl August 13, 2024 07:37
@mp911de mp911de force-pushed the main branch 2 times, most recently from 52ee55f to 61e6d36 Compare October 9, 2024 07:35
@mp911de mp911de linked an issue Oct 28, 2024 that may be closed by this pull request
@christophstrobl christophstrobl added this to the 3.5 M1 (2025.0.0) milestone Dec 13, 2024
@mp911de mp911de changed the title Explore refined Specification API Refine Specification API Dec 13, 2024
@christophstrobl
Copy link
Member

LGTM

return (root, query, builder) -> {

Predicate not = spec.toPredicate(root, query, builder);
return not != null ? builder.not(not) : null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's so many not's, it's making knots in my brain. Could we at least reverse the comparison?

static <T> Specification<T> anyOf(Iterable<Specification<T>> specifications) {

return StreamSupport.stream(specifications.spliterator(), false) //
.reduce(Specification.all(), Specification::or);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really wrong to me. Specification.all() suggests a Specification that produces true for all inputs, which would always yield true when combined by OR with anything else.

I'd say the underlying problem is that all() is a misnomer. It should really be Specification.null() or Specification.empty(), shouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the name to unrestricted() to express really what it does.

public interface UpdateSpecification<T> extends Serializable {

/**
* Simple static factory method to create a specification deleting all objects.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that should read "updating all objects"

* @param spec must not be {@literal null}.
* @return guaranteed to be not {@literal null}.
*/
static <T> UpdateOperation<T> update(UpdateOperation<T> spec) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments and error message talk about UpdateSpecification but the types used are UpdateOperation. I'm confused.

mp911de and others added 7 commits January 8, 2025 11:36
Introduce DeleteSpecification and UpdateSpecification. Add PredicateSpecification. Update SpecificationExecutor.
Gentle reminder to deprecate where before we remove it here
Revise nullability requirements around non-nullable specifications.
@mp911de mp911de changed the base branch from main to 4.0.x January 8, 2025 10:40
christophstrobl added a commit that referenced this pull request Jan 8, 2025
The where method will be removed with next major version containing some refinements in the Specification API.

See: #3578
christophstrobl pushed a commit that referenced this pull request Jan 8, 2025
Introduce DeleteSpecification and UpdateSpecification. Add PredicateSpecification. Update SpecificationExecutor.

Closes: #3521
Original Pull Request: #3578
christophstrobl added a commit that referenced this pull request Jan 8, 2025
Also remove serialVersionUID.

Original Pull Request: #3578
christophstrobl pushed a commit that referenced this pull request Jan 8, 2025
Revise nullability requirements around non-nullable specifications.

Original Pull Request: #3578
christophstrobl pushed a commit that referenced this pull request Jan 8, 2025
Original Pull Request: #3578
christophstrobl pushed a commit that referenced this pull request Jan 8, 2025
Original Pull Request: #3578
@christophstrobl christophstrobl deleted the issue/3521 branch January 8, 2025 15:28
mp911de added a commit that referenced this pull request Jan 14, 2025
Introduce DeleteSpecification and UpdateSpecification. Add PredicateSpecification. Update SpecificationExecutor.

Closes: #3521
Original Pull Request: #3578
mp911de pushed a commit that referenced this pull request Jan 14, 2025
Also remove serialVersionUID.

Original Pull Request: #3578
mp911de added a commit that referenced this pull request Jan 14, 2025
Revise nullability requirements around non-nullable specifications.

Original Pull Request: #3578
mp911de added a commit that referenced this pull request Jan 14, 2025
Original Pull Request: #3578
mp911de added a commit that referenced this pull request Jan 14, 2025
Original Pull Request: #3578
mp911de added a commit that referenced this pull request Jan 15, 2025
Fix since versions.

See: #3521
Original Pull Request: #3578
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

Successfully merging this pull request may close these issues.

Revise Specification API
3 participants