jakartaee/data

[clarification]: Restricting Usage of `@OrderBy` Annotation to Enhance Clarity and Consistency in Jakarta Data

otaviojava opened this issue · 3 comments

Specification

https://github.com/jakartaee/data/blob/main/spec/src/main/asciidoc/repository.asciidoc

I need clarification on ...

Restricting Usage of @OrderBy Annotation to Enhance Clarity and Consistency in Jakarta Data

The @OrderBy annotation in Jakarta Data is currently flexible, allowing it to be combined with various types of query definitions, including the @Query annotation, method query names, and the @Find annotation. While this flexibility offers a broad range of application, it introduces ambiguity, particularly regarding the precedence and interpretation of ordering instructions when multiple sources of order definitions are present. This ambiguity is manageable when thoroughly documented; however, it compromises intuitiveness and clarity in practice, making the code harder to read and understand at a glance, especially for new developers or those not intimately familiar with the specific documentation.

Even with the clear documentation, reading the code is not clear what is the proper result or which one goes first in this case:

Examples of Current Implementation:

@OrderBy("author.name")
@Query("where title like :pattern and type = :type order by lower(title) desc, isbn")
List<Book> booksByTitle(Type type, String pattern);

@Query("where title like :pattern and type = :type order by lower(title) desc, isbn")
@OrderBy("author.name")
List<Book> booksByTitle2(Type type, String pattern);

@OrderBy("author.name")
List<Book> findByTitleOrderByTitleDesc(String title);

Proposed Implementation:

Under the proposed change, @OrderBy would be restricted to use with the @Find annotation only, as demonstrated below:

@OrderBy("author.name")
@Find
List<Book> getBooksByTitle(@By("title") String title);

Rationale:

  • Clarity and Consistency: Limiting @OrderBy to @Find annotations simplifies understanding and implementing ordered queries, reducing potential errors and confusion.
  • Simplicity: Narrowing the scope of @OrderBy usage makes it easier for developers to predict how their queries will behave and in what order results will be returned.
  • Alignment with Best Practices: This change encourages a more annotation-driven approach to query definition, aligning with contemporary practices in Java and database interaction frameworks.
  • Future: This links with Gavin proposal as well: #546

Additional information

No response

I don't agree with this. OrderBy is very useful for these other scenarios which is why it was written that way and @Find is currently too limiting because it currently only supports equality comparisons. Your 3 examples are incorrect because the spec already doesn't let you do any of this things.

I had actually forgotten that @Query @OrderBy is a combination blessed by the spec.

And as the spec is set up today, this combination is actually really useful since we disallow a @Query with an order by clause in some circumstances where @OrderBy is allowed. So disallowing this combination would have an impact on other things. So I would prefer to continue allowing it.

On the other hand, I have no opinion on whether @OrderBy should be allowed with Query by Method Name. It doesn't seem especially useful there.

[I was initially a skeptic of this annotation, by the way, but after actually implementing and using it, I'm not very happy with it (subject to the caveats I already outlined in #546).]

duplicated: #603