eric-maynard commented on code in PR #1938:
URL: https://github.com/apache/polaris/pull/1938#discussion_r2167141665
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java:
##########
@@ -18,82 +18,75 @@
*/
package org.apache.polaris.core.persistence.pagination;
-import java.util.List;
-import java.util.Objects;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import jakarta.annotation.Nullable;
+import java.util.Optional;
+import java.util.OptionalInt;
+import org.apache.polaris.immutables.PolarisImmutable;
-/**
- * Represents a page token that can be used by operations like `listTables`.
Clients that specify a
- * `pageSize` (or a `pageToken`) may receive a `next-page-token` in the
response, the content of
- * which is a serialized PageToken.
- *
- * <p>By providing that in the next query's `pageToken`, the client can resume
listing where they
- * left off. If the client provides a `pageToken` or `pageSize` but
`next-page-token` is null in the
- * response, that means there is no more data to read.
- */
-public abstract class PageToken {
-
- /** Build a new PageToken that reads everything */
- public static PageToken readEverything() {
- return build(null, null);
- }
+/** A wrapper for pagination information passed in as part of a request. */
+@PolarisImmutable
+@JsonSerialize(as = ImmutablePageToken.class)
+@JsonDeserialize(as = ImmutablePageToken.class)
+public interface PageToken {
+ // Serialization property names are intentionally short to reduce the size
of the serialized
+ // paging token.
- /** Build a new PageToken from an input String, without a specified page
size */
- public static PageToken fromString(String token) {
- return build(token, null);
- }
-
- /** Build a new PageToken from a limit */
- public static PageToken fromLimit(Integer pageSize) {
- return build(null, pageSize);
- }
+ /** The requested page size (optional). */
+ @JsonProperty("p")
Review Comment:
ditto the comment -- this is not a good property name.
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java:
##########
@@ -18,82 +18,75 @@
*/
package org.apache.polaris.core.persistence.pagination;
-import java.util.List;
-import java.util.Objects;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import jakarta.annotation.Nullable;
+import java.util.Optional;
+import java.util.OptionalInt;
+import org.apache.polaris.immutables.PolarisImmutable;
-/**
- * Represents a page token that can be used by operations like `listTables`.
Clients that specify a
- * `pageSize` (or a `pageToken`) may receive a `next-page-token` in the
response, the content of
- * which is a serialized PageToken.
- *
- * <p>By providing that in the next query's `pageToken`, the client can resume
listing where they
- * left off. If the client provides a `pageToken` or `pageSize` but
`next-page-token` is null in the
- * response, that means there is no more data to read.
- */
-public abstract class PageToken {
-
- /** Build a new PageToken that reads everything */
- public static PageToken readEverything() {
- return build(null, null);
- }
+/** A wrapper for pagination information passed in as part of a request. */
+@PolarisImmutable
+@JsonSerialize(as = ImmutablePageToken.class)
+@JsonDeserialize(as = ImmutablePageToken.class)
+public interface PageToken {
+ // Serialization property names are intentionally short to reduce the size
of the serialized
+ // paging token.
- /** Build a new PageToken from an input String, without a specified page
size */
- public static PageToken fromString(String token) {
- return build(token, null);
- }
-
- /** Build a new PageToken from a limit */
- public static PageToken fromLimit(Integer pageSize) {
- return build(null, pageSize);
- }
+ /** The requested page size (optional). */
+ @JsonProperty("p")
Review Comment:
ditto the comment on `i` -- this is not a good property name.
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java:
##########
@@ -18,82 +18,75 @@
*/
package org.apache.polaris.core.persistence.pagination;
-import java.util.List;
-import java.util.Objects;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import jakarta.annotation.Nullable;
+import java.util.Optional;
+import java.util.OptionalInt;
+import org.apache.polaris.immutables.PolarisImmutable;
-/**
- * Represents a page token that can be used by operations like `listTables`.
Clients that specify a
- * `pageSize` (or a `pageToken`) may receive a `next-page-token` in the
response, the content of
- * which is a serialized PageToken.
- *
- * <p>By providing that in the next query's `pageToken`, the client can resume
listing where they
- * left off. If the client provides a `pageToken` or `pageSize` but
`next-page-token` is null in the
- * response, that means there is no more data to read.
- */
-public abstract class PageToken {
-
- /** Build a new PageToken that reads everything */
- public static PageToken readEverything() {
- return build(null, null);
- }
+/** A wrapper for pagination information passed in as part of a request. */
+@PolarisImmutable
+@JsonSerialize(as = ImmutablePageToken.class)
+@JsonDeserialize(as = ImmutablePageToken.class)
+public interface PageToken {
+ // Serialization property names are intentionally short to reduce the size
of the serialized
+ // paging token.
- /** Build a new PageToken from an input String, without a specified page
size */
- public static PageToken fromString(String token) {
- return build(token, null);
- }
-
- /** Build a new PageToken from a limit */
- public static PageToken fromLimit(Integer pageSize) {
- return build(null, pageSize);
- }
+ /** The requested page size (optional). */
+ @JsonProperty("p")
+ OptionalInt pageSize();
- /** Build a {@link PageToken} from the input string and page size */
- public static PageToken build(String token, Integer pageSize) {
- if (token == null || token.isEmpty()) {
- if (pageSize != null) {
- return new LimitPageToken(pageSize);
- } else {
- return new ReadEverythingPageToken();
- }
- } else {
- // TODO implement, split out by the token's prefix
- throw new IllegalArgumentException("Unrecognized page token: " + token);
- }
+ /** Convenience for {@code pageSize().isPresent()}. */
+ default boolean paginationRequested() {
+ return pageSize().isPresent();
}
- /** Serialize a {@link PageToken} into a string */
- public abstract String toTokenString();
-
/**
- * Builds a new page token to reflect new data that's been read. If the
amount of data read is
- * less than the pageSize, this will return a {@link DonePageToken}
+ * Paging token value, if present. Serialized paging tokens always have a
value, but "synthetic"
+ * paging tokens like {@link #readEverything()} or {@link #fromLimit(int)}
do not have a token
+ * value.
*/
- protected abstract PageToken updated(List<?> newData);
+ @JsonProperty("v")
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
