Add utilities for Pageable to be used with Doma Criteria.

[mazeneko]

Good evening! I'm always grateful for how convenient doma-spring-boot is!

I really like Doma criteria API, and there are times when I wish there were a criteria-based version of Pageables.
So, I tried creating something that can be used like this:

public Page<Task> getPage(Pageable pageable) {
    final var task_ = new Task_();
    final var content = this.queryDsl
        .from(task_)
        .offset(PageablesForCriteria.offset(pageable))
        .limit(PageablesForCriteria.limit(pageable))
        .orderBy(PageablesForCriteria.orderBySingleEntity(pageable, task_))
        .fetch();
    final var total = this.queryDsl
        .from(task_)
        .select(Expressions.count())
        .fetchOne();
    return new PageImpl<>(content, pageable, total);
  }

If you’re interested, please take a look!

(P.S. This is my first time trying to contribute, so I apologize in advance if I’m doing anything inappropriate.)

[backpaper0]

Thank you for your contribution!
I'll review it soon, but could you please format the source code using the following command before that?

./mvnw formatter:format

Note

On Windows, you may need to explicitly pass the current directory using the -DrootDir option.

[mazeneko]

Oh! I’m sorry! I applied the formatting!

[backpaper0]

Please don't worry about it!
We haven't written a contribution guide or integrated it into CI yet, so it's our oversight.
Thank you for taking the time to address it!

I'll be reviewing it over the weekend, so please wait a little longer.

[backpaper0]

@mazeneko
I’ve reviewed your code.
I’m glad to see that you’ve written thorough tests as well — thank you!

However, if you don’t mind, I’d like to start by discussing the API design.

Personally, I prefer aggregating operations related to a class and exposing them as instance methods.
Providing static utility methods is a lower priority for me.

For example, what do you think about writing it like this?

public Page<Task> getPage(Pageable pageable) {
    final var task_ = new Task_();
    final var p = UnifiedQueryPageable.from(pageable, task_);
    final var content = this.queryDsl
        .from(task_)
        .offset(p.offset())
        .limit(p.limit())
        .orderBy(p.sort())
        .fetch();
    final var total = this.queryDsl
        .from(task_)
        .select(Expressions.count())
        .fetchOne();
    return new PageImpl<>(content, pageable, total);
}

I believe this approach is more concise and easier to read compared to something like PageablesForCriteria.<method>(pageable).
Since pagination typically involves all three of offset, limit, and orderBy, being able to write it more concisely is a significant advantage.

What do you think?
I’d love to hear your thoughts.

[mazeneko]

@backpaper0
I see, that sounds good! I'll give it a try.

[mazeneko]

@backpaper0
I’ve updated the code! I think it looks good now. What do you think?

[backpaper0]

I’ve reviewed the code.
Thank you for accommodating my suggestion to introduce the UnifiedQueryPageable class.
I truly enjoyed reading through your code.

I’ve left some comments on points that caught my attention, as well as a few requests. I’d appreciate it if you could take a look.

Note

I expect we’ll go back and forth once or twice more, but please keep in mind that none of this is a criticism of you personally.
I hope you’ll enjoy the discussion with me.

[mazeneko]

Thank you for your thorough review!
I found it really insightful and enjoyable to read.
I'll start working on the changes soon.

[backpaper0]

Thank you for patiently sticking with the review process!
I’m approving and merging this pull request.

[mazeneko]

Thank you so much for all the thoughtful reviews and advice throughout the process!
I really appreciated your guidance.

[backpaper0]

@mazeneko
I owe you an apology.

When transitioning from a static utility to a class, I initially proposed the name UnifiedQueryPageable.
However, the Doma feature I based the name on is actually the Unified Criteria API.
In other words, I intended to propose the name UnifiedCriteriaPageable, but mistakenly suggested UnifiedQueryPageable.

Since it's just a simple rename via the IDE, I considered fixing it myself.
However, I felt it would be disrespectful to make the change without consulting you first, so I'm posting this comment instead.

If you're okay with it, would you mind creating a pull request to rename it?
You can change it to UnifiedCriteriaPageable, or feel free to suggest a better name if you have one.

[backpaper0]

By the way, while I was preparing for the release, I thought it would be a good opportunity to add a simple sample using Query DSL. That’s when I noticed the naming mistake (of course, I’ll be using the feature you created as well!).
I'm really glad I caught it before the release...

[mazeneko]

@backpaper0
Thank you for your thoughtfulness!
I've created a pull request to update it to UnifiedCriteriaPageable.

#280