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.

Sign up for GitHub

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

JPA specialization [Proposal] #140

Closed
wants to merge 10 commits into from
Closed

JPA specialization [Proposal] #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
876a198
feat: create JPA repository
otaviojava May 4, 2023
f28001b
doc: update documentation at JPA repository
otaviojava May 4, 2023
1b0e6f3
feat: move the whole detailsto JPArepository
otaviojava May 4, 2023
518f0a4
docs: update package-info
otaviojava May 4, 2023
432a310
refactor: move the specialization of SQL
otaviojava May 4, 2023
46af832
docs: update Pageable api
otaviojava May 4, 2023
25d34c4
feat: creates specialization to JPA repository here
otaviojava May 4, 2023
9e75483
docs: add a todo item to repository
otaviojava May 4, 2023
b7eec04
chore: add SQL license header
otaviojava May 4, 2023
986e870
docs: fix repository docs
otaviojava May 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions api/src/main/java/jakarta/data/repository/Pageable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package jakarta.data.repository;


import java.util.Collections;
import java.util.List;

Expand Down Expand Up @@ -81,11 +82,11 @@ static Pageable ofSize(int maxPageSize) {
}

/**
* <p>Requests {@link KeysetAwareSlice keyset pagination} in the forward direction,
* <p>Requests {@link jakarta.data.repository.jpa.KeysetAwareSlice keyset pagination} in the forward direction,
* starting after the specified keyset values.</p>
*
* @param keyset keyset values, the order and number of which must match the
* {@link OrderBy} annotations, {@link Sort} parameters, or
* {@link jakarta.data.repository.jpa.OrderBy} annotations, {@link Sort} parameters, or
* <code>OrderBy</code> name pattern of the repository method to which
* this pagination will be supplied.
* @return a new instance of <code>Pageable</code> with forward keyset pagination.
Expand All @@ -95,11 +96,11 @@ static Pageable ofSize(int maxPageSize) {
Pageable afterKeyset(Object... keyset);

/**
* <p>Requests {@link KeysetAwareSlice keyset pagination} in the reverse direction,
* <p>Requests {@link jakarta.data.repository.jpa.KeysetAwareSlice keyset pagination} in the reverse direction,
* starting after the specified keyset values.</p>
*
* @param keyset keyset values, the order and number of which must match the
* {@link OrderBy} annotations, {@link Sort} parameters, or
* {@link jakarta.data.repository.jpa.OrderBy} annotations, {@link Sort} parameters, or
* <code>OrderBy</code> name pattern of the repository method to which
* this pagination will be supplied.
* @return a new instance of <code>Pageable</code> with reverse keyset pagination.
Expand All @@ -109,11 +110,11 @@ static Pageable ofSize(int maxPageSize) {
Pageable beforeKeyset(Object... keyset);

/**
* <p>Requests {@link KeysetAwareSlice keyset pagination} in the forward direction,
* <p>Requests {@link jakarta.data.repository.jpa.KeysetAwareSlice keyset pagination} in the forward direction,
* starting after the specified keyset values.</p>
*
* @param keysetCursor cursor with keyset values, the order and number of which must match the
* {@link OrderBy} annotations, {@link Sort} parameters, or
* {@link jakarta.data.repository.jpa.OrderBy} annotations, {@link Sort} parameters, or
* <code>OrderBy</code> name pattern of the repository method to which
* this pagination will be supplied.
* @return a new instance of <code>Pageable</code> with forward keyset pagination.
Expand All @@ -123,11 +124,11 @@ static Pageable ofSize(int maxPageSize) {
Pageable afterKeysetCursor(Cursor keysetCursor);

/**
* <p>Requests {@link KeysetAwareSlice keyset pagination} in the reverse direction,
* <p>Requests {@link jakarta.data.repository.jpa.KeysetAwareSlice keyset pagination} in the reverse direction,
* starting after the specified keyset values.</p>
*
* @param keysetCursor cursor with keyset values, the order and number of which must match the
* {@link OrderBy} annotations, {@link Sort} parameters, or
* {@link jakarta.data.repository.jpa.OrderBy} annotations, {@link Sort} parameters, or
* <code>OrderBy</code> name pattern of the repository method to which
* this pagination will be supplied.
* @return a new instance of <code>Pageable</code> with reverse keyset pagination.
Expand Down Expand Up @@ -187,9 +188,9 @@ static Pageable ofSize(int maxPageSize) {
* if using offset pagination.</p>
*
* <p>If using keyset pagination, traversal of pages must only be done
* via the {@link KeysetAwareSlice#nextPageable()},
* {@link KeysetAwareSlice#previousPageable()}, or
* {@link KeysetAwareSlice#getKeysetCursor(int) keyset cursor},
* via the {@link jakarta.data.repository.jpa.KeysetAwareSlice#nextPageable()},
* {@link jakarta.data.repository.jpa.KeysetAwareSlice#previousPageable()}, or
* {@link jakarta.data.repository.jpa.KeysetAwareSlice#getKeysetCursor(int) keyset cursor},
* not with this method.</p>
*
* @return The next pageable.
Expand Down Expand Up @@ -221,7 +222,7 @@ static Pageable ofSize(int maxPageSize) {
* pagination information, except using the specified sort criteria.
* The order of precedence for sort criteria is that of any statically
* specified sort criteria (from the <code>OrderBy</code> keyword,
* {@link OrderBy} annotation or <code>ORDER BY</code> clause of a the
* {@link jakarta.data.repository.jpa.OrderBy} annotation or <code>ORDER BY</code> clause of a the
* {@link Query} annotation) followed by the order of the
* {@link Iterable} that is supplied to this method.</p>
*
Expand All @@ -235,7 +236,7 @@ static Pageable ofSize(int maxPageSize) {
* pagination information, except using the specified sort criteria.
* The order of precedence for sort criteria is that of any statically
* specified sort criteria (from the <code>OrderBy</code> keyword,
* {@link OrderBy} annotation or <code>ORDER BY</code> clause of a the
* {@link jakarta.data.repository.jpa.OrderBy} annotation or <code>ORDER BY</code> clause of a the
* {@link Query} annotation) followed by the order in which the
* {@link Sort} parameters to this method are listed.</p>
*
Expand All @@ -252,7 +253,7 @@ static Pageable ofSize(int maxPageSize) {
enum Mode {
/**
* Indicates forward keyset pagination, which follows the
* direction of the {@link OrderBy} annotations,
* direction of the {@link jakarta.data.repository.jpa.OrderBy} annotations,
* {@link Pageable#sortBy(Sort...)} or {@link Pageable#sortBy(Iterable)}
* parameters, repository method {@link Sort} parameters,
* or <code>OrderBy</code> name pattern of the repository method.
Expand All @@ -261,7 +262,7 @@ enum Mode {

/**
* Indicates a request for a page with keyset pagination
* in the reverse direction of the {@link OrderBy} annotations,
* in the reverse direction of the {@link jakarta.data.repository.jpa.OrderBy} annotations,
* {@link Pageable#sortBy(Sort...)} or {@link Pageable#sortBy(Iterable)}
* parameters, repository method {@link Sort} parameters,
* or <code>OrderBy</code> name pattern of the repository method.
Expand Down
20 changes: 1 addition & 19 deletions api/src/main/java/jakarta/data/repository/Query.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package jakarta.data.repository;



import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
Expand All @@ -43,24 +44,5 @@
* @return the query to be executed when the annotated method is called.
*/
String value();

/**
* <p>Defines an additional query that counts the number of elements that are
* returned by the {@link #value() primary} query. This is used to compute
* the {@link Page#totalElements() total elements}
* and {@link Page#totalPages() total pages}
* for paginated repository queries that are annotated with
* <code>@Query</code> and return a {@link Page} or {@link KeysetAwarePage}.
* Slices do not use a counting query.</p>
*
* <p>The default value of empty string indicates that no counting query
* is provided. A counting query is unnecessary when pagination is
* performed with slices instead of pages and when pagination is
* not used at all.</p>
*
* @return a query for counting the number of elements across all pages.
* Empty string indicates that no counting query is provided.
*/
String count() default "";
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not specific to Jakarta Persistence. Page counts and total element counts across all pages apply to every type of database that supports pagination.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not valid.
You can paginate using a stream or any "pointer" or cursor of this data.

Samples:

⚠️ Neo4J also have offset-based option

Copy link
Contributor

@njr-11 njr-11 May 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Right, I didn't word that very well. The point is that although not all databases can count total results, types other than Jakarta Persistence/JPA do have that ability and it is therefore wrong to restrict it to Jakarta Persistence/JPA alone. We know for a fact that JDBC can do this via SQL independently of Jakarta Persistence. Even MongoDB which you cited as a counter example exposes an API for counting documents so it is not unreasonable to allow for the possibility they might some day want to let the same thing to be done via whatever query language they support. If a count query isn't useful for a particular type that doesn't support it, then just don't use it with those types. But let it remain for all of those where it useful. It seems like you are boxing in and limiting everything that isn't Jakarta Persistence where it isn't necessary and doesn't make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, some databases can support count, but why not use OOP in our favor and create specializations?
It is the beauty of OOP!

You can have generic and specializations instead of having several optional methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

Specializations should be used where they make sense - when there are categories that provide accurate delineation between where a function is applicable versus not applicable. We have been struggling to find that clear separation here. JPA vs everything else and relational vs everything else don't perfectly fit. While we can state that Jakarta Persistence will always have certain capabilities, we cannot accurately state that non-JPA will always not.

}

35 changes: 3 additions & 32 deletions api/src/main/java/jakarta/data/repository/Repository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
* <code>findByAddress_ZipCode</code> or <code>findByAddressZipCode</code>)
* when referred to within repository method names, and delimited by
* <code>.</code> when used within annotation values, such as for
* {@link OrderBy#value} and {@link Query#value},</p>
* {@link jakarta.data.repository.jpa.OrderBy#value} and {@link Query#value},</p>
*
* <pre>
* &#64;Entity
Expand Down Expand Up @@ -528,6 +528,7 @@
// TODO Does Jakarta NoSQL have the same or different wildcard characters? Document this
// under: "Wildcard characters for patterns are determined by the data access provider"
// TODO Ensure we have all required supported return types listed.
//TODO move away all transaction details to keep it more generic as possible
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
Expand All @@ -537,38 +538,8 @@
* available Jakarta Data provider that supports the type of entity
* annotation that is present on the repository's entity class.
*/
static final String ANY_PROVIDER = "";
String ANY_PROVIDER = "";

/**
* Value for the {@link dataStore} attribute that indicates that the
* Jakarta Data provider should choose a default data store to use.
* The default data store might require additional vendor-specific
* configuration, depending on the vendor.
*/
static final String DEFAULT_DATA_STORE = "";

/**
* <p>Optionally indicates the data store to use for the repository.</p>
*
* <p>The Jakarta Data specification does not define a full configuration
* model, and relies upon the Jakarta Data providers to provide configuration.
* This value serves as an identifier linking to vendor-specific configuration
* for each Jakarta Data provider to interpret in a vendor-specific way.</p>
*
* <p>For some Jakarta Data providers, this value could map directly to an
* identifier in vendor-specific configuration. For others, this value could
* map to a base configuration path that forms a configuration hierarchy,
* such as in MicroProfile Config, or possibly Jakarta Config in a future
* version of this specification. For providers backed by Jakarta Persistence,
* it might point to a {@code jakarta.annotation.sql.DataSourceDefinition} name
* or a {@code javax.sql.DataSource} JNDI name or resource reference,
* or other vendor-specific configuration.</p>
*
* <p>The default value of this attribute is {@link #DEFAULT_DATA_STORE}.</p>
*
* @return the name of a data store or {@link #DEFAULT_DATA_STORE}.
*/
String dataStore() default DEFAULT_DATA_STORE;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not specific to Jakarta Persistence. It applies to every type of backend where there can be multiple, which is all types I'm aware of.

Copy link
Contributor Author

@otaviojava otaviojava May 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Usually, we're using convention over configuration allied with The Twelve-Factor App:
https://12factor.net/
E.g.:

That is why I proposed to keep it on JPA subdomain.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't follow this argument at all with respect to what it has to do with JPA being the deciding factor in whether or not you are able to use multiple databases.


/**
* <p>Restricts the repository implementation to that of a specific
Expand Down
3 changes: 2 additions & 1 deletion api/src/main/java/jakarta/data/repository/Sort.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package jakarta.data.repository;


import java.util.Objects;

/**
Expand All @@ -42,7 +43,7 @@
* </pre>
*
* <p>When combined on a method with static sort criteria
* (<code>OrderBy</code> keyword or {@link OrderBy} annotation or
* (<code>OrderBy</code> keyword or {@link jakarta.data.repository.jpa.OrderBy} annotation or
* {@link Query} with an <code>ORDER BY</code> clause), the static
* sort criteria is applied first, followed by the dynamic sort criteria
* that is defined by <code>Sort</code> instances in the order listed.</p>
Expand Down
Loading