From 9bf3714d38ac712d4484f215ae73466919f43c12 Mon Sep 17 00:00:00 2001 From: Greg Perkins <165826081+gregrperkins-roblox@users.noreply.github.com> Date: Fri, 14 Feb 2025 11:46:27 -0800 Subject: [PATCH 1/8] Clarify expectations for pagination with stale results on AEP-158 The pagination AIP-158 doesn't give clear recommendations about what to do if the underlying data has changed during the course of a paged request. Normally the recommended cursor-based pagination strategy would continue serving stale data. This PR suggests some language to clarify this expectation, and provide a workaround when data freshness is critical. Specifically, it adds an optional `has_changed` boolean to the response, which clients that need up-to-date data can use to show an indicator and add a refresh listing action to their user interface. (roblox internal DSA-4035) --- aep/general/0158/aep.md.j2 | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/aep/general/0158/aep.md.j2 b/aep/general/0158/aep.md.j2 index aafe36f8..2874167d 100644 --- a/aep/general/0158/aep.md.j2 +++ b/aep/general/0158/aep.md.j2 @@ -148,6 +148,25 @@ used. It is not necessary to document this behavior. **Note:** While a reasonable time may vary between APIs, a good rule of thumb is three days. +### Freshness + +Since a request with the same filter (see [AIP-132][]) and `page_token` +**should** return the same results unless the page token has expired, items +returned **may** have been deleted or modified since the initial request, and +paging through the list **may** not include all current items. + +If the service is capable of detecting underlying changes in the listing since +a given `page_token` was generated: + +- The response **may** include a `has_changed` boolean. If this boolean is + supported but `false` or unset, the data **must** be up to date, and paging + through the list **must** include all current items. +- If no `has_changed` boolean is included, the API **may** expire the page + token. + +If the service is incapable of detecting such changes, it **may** return stale +data or phantom reads without any indication. + ### Backwards compatibility Adding pagination to an existing method is a backwards-incompatible change. This @@ -174,4 +193,4 @@ paginate) is reasonable for initially-small collections. ## Changelog -- **2024-04-14**: Imported from https://google.aip.dev/158. \ No newline at end of file +- **2024-04-14**: Imported from https://google.aip.dev/158. From 428784467c52f55a08e97274a8d6ae8ecf6dde3a Mon Sep 17 00:00:00 2001 From: Greg Perkins <165826081+gregrperkins-roblox@users.noreply.github.com> Date: Fri, 14 Feb 2025 11:52:35 -0800 Subject: [PATCH 2/8] Also add relevant suggestions to AEP-132 listing --- aep/general/0132/aep.md.j2 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/aep/general/0132/aep.md.j2 b/aep/general/0132/aep.md.j2 index 4f9f1653..cb9ccb54 100644 --- a/aep/general/0132/aep.md.j2 +++ b/aep/general/0132/aep.md.j2 @@ -112,6 +112,9 @@ result being a resource. - The response message **must** include a field corresponding to the resources being returned, named for the English plural term for the resource, and **should not** include any other repeated fields. +- The response message **may** include a `has_changed` boolean, to indicate that the + underlying data has changed since the `next_page_token` was generated. + - Fields providing metadata about the list request (such as `string next_page_token` or `int32 total_size`) **must** be included on the response (not as part of the resource itself). From 91c7d13a1d16e6a850104fcb3e6ba729fe7d4209 Mon Sep 17 00:00:00 2001 From: Greg Perkins Date: Fri, 14 Feb 2025 12:13:15 -0800 Subject: [PATCH 3/8] Prettier; field name consistency between AEP-158 and AEP-132 --- aep/general/0132/aep.md.j2 | 37 ++++++++++++++++---------------- aep/general/0158/aep.md.j2 | 44 +++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 40 deletions(-) diff --git a/aep/general/0132/aep.md.j2 b/aep/general/0132/aep.md.j2 index cb9ccb54..32cec724 100644 --- a/aep/general/0132/aep.md.j2 +++ b/aep/general/0132/aep.md.j2 @@ -97,24 +97,25 @@ when the REST/JSON interface is used. `List` operations **must** return a page of results, with each individual result being a resource. + - The array of resources **must** be named `results` and contain resources with no additional wrapping. -- The `string nextPageToken` field **must** be included in the list response +- The `string next_page_token` field **must** be included in the list response schema. It **must** be set if there are subsequent pages, and **must not** be set if the response represents the final page. For more information, see AEP-158. -- The response struct **may** include a `int32 totalSize` (or - `int64 totalSize`) field with the number of items in the collection. +- The response struct **may** include a `int32 total_size` (or + `int64 total_size`) field with the number of items in the collection. - The value **may** be an estimate (the field **should** clearly document this if so). - - If filtering is used, the `totalSize` field **should** reflect the size of + - If filtering is used, the `total_size` field **should** reflect the size of the collection _after_ the filter is applied. - The response message **must** include a field corresponding to the resources being returned, named for the English plural term for the resource, and **should not** include any other repeated fields. -- The response message **may** include a `has_changed` boolean, to indicate that the - underlying data has changed since the `next_page_token` was generated. - +- The response message **may** include a `bool has_changed` field, to indicate + that the underlying data has changed since the `next_page_token` was + generated. - Fields providing metadata about the list request (such as `string next_page_token` or `int32 total_size`) **must** be included on the response (not as part of the resource itself). @@ -129,7 +130,7 @@ result being a resource. - The field "results" **must** be an array of resources, with the schema as a reference to the resource (e.g. `#/components/schemas/Book`). -- The field "nextPageToken" **must** be a string that contains the token to +- The field "next_page_token" **must** be a string that contains the token to retrieve the next page. This **must not** be set if the response represents the final page. @@ -159,18 +160,18 @@ return `200 OK` with an empty results array, and not `404 Not Found`. ### Advanced operations -`List` methods **may** allow an extended set of functionality to allow a user to -specify the resources that are returned in a response. +`List` methods **may** allow an extended set of functionality to allow a user +to specify the resources that are returned in a response. -The following table summarizes the applicable AEPs, ordered by the precedence of -the operation on the results. +The following table summarizes the applicable AEPs, ordered by the precedence +of the operation on the results. -| Operation | AEP | -| ---------------------------- | --------------------------------- | -| filter | [AEP-160](/160) | -| ordering (`orderBy`) | [AEP-132](#ordering) | -| pagination (`nextPageToken`) | [AEP-158](/158) | -| skip | [AEP-158](/158/#skipping-results) | +| Operation | AEP | +| ------------------------------ | --------------------------------- | +| filter | [AEP-160](/160) | +| ordering (`orderBy`) | [AEP-132](#ordering) | +| pagination (`next_page_token`) | [AEP-158](/158) | +| skip | [AEP-158](/158/#skipping-results) | For example, if both the `filter` and `skip` fields are specified, then the filter would be applied first, then the resulting set would be the results that diff --git a/aep/general/0158/aep.md.j2 b/aep/general/0158/aep.md.j2 index 2874167d..cc8b1570 100644 --- a/aep/general/0158/aep.md.j2 +++ b/aep/general/0158/aep.md.j2 @@ -8,9 +8,9 @@ be paginated. ## Guidance -Methods returning collections of data **must** provide pagination _at the outset_, -as it is a [backwards-incompatible change](#backwards-compatibility) to add -pagination to an existing method. +Methods returning collections of data **must** provide pagination _at the +outset_, as it is a [backwards-incompatible change](#backwards-compatibility) +to add pagination to an existing method. {% tab proto %} @@ -50,8 +50,8 @@ message ListBooksResponse { } ``` -- The field containing pagination results **should** be the first field in - the message and have a field number of `1`. +- The field containing pagination results **should** be the first field in the + message and have a field number of `1`. {% tab oas %} @@ -60,39 +60,39 @@ message ListBooksResponse { {% endtabs %} - Request messages for collections **should** define an integer (`int32` for -protobuf) `max_page_size` field, allowing users to specify the maximum number of -results to return. + protobuf) `max_page_size` field, allowing users to specify the maximum number + of results to return. - If the user does not specify `max_page_size` (or specifies `0`), the API chooses an appropriate default, which the API **should** document. The API **must not** return an error. - - If the user specifies `max_page_size` greater than the maximum permitted by the - API, the API **should** coerce down to the maximum permitted page size. - - If the user specifies a negative value for `max_page_size`, the API **must** - send an `INVALID_ARGUMENT` error. + - If the user specifies `max_page_size` greater than the maximum permitted by + the API, the API **should** coerce down to the maximum permitted page size. + - If the user specifies a negative value for `max_page_size`, the API + **must** send an `INVALID_ARGUMENT` error. - The API **may** return fewer results than the number requested (including zero results), even if not at the end of the collection. - Request schemas for collections **should** define a `string page_token` field, allowing users to advance to the next page in the collection. - The `page_token` field **must not** be required. - - If the user changes the `max_page_size` in a request for subsequent pages, the - service **must** honor the new page size. + - If the user changes the `max_page_size` in a request for subsequent pages, + the service **must** honor the new page size. - The user is expected to keep all other arguments to the method the same; if any arguments are different, the API **should** send an `INVALID_ARGUMENT` error. - The response **must not** be a streaming response. -- Response messages for collections **must** define a - `string next_page_token` field, providing the user with a page token that may - be used to retrieve the next page. - - The field containing pagination results **must** be a repeated - field containing a list of resources constituting a single page of results. +- Response messages for collections **must** define a `string next_page_token` + field, providing the user with a page token that may be used to retrieve the + next page. + - The field containing pagination results **must** be a repeated field + containing a list of resources constituting a single page of results. - If the end of the collection has been reached, the `next_page_token` field **must** be empty. This is the _only_ way to communicate "end-of-collection" to users. - If the end of the collection has not been reached (or if the API can not determine in time), the API **must** provide a `next_page_token`. - Response messages for collections **may** provide an integer (`int32` for -protobuf) `total_size` field, providing the user with the total number of items -in the list. + protobuf) `total_size` field, providing the user with the total number of + items in the list. - This total **may** be an estimate. If so, the API **should** explicitly document that. @@ -169,8 +169,8 @@ data or phantom reads without any indication. ### Backwards compatibility -Adding pagination to an existing method is a backwards-incompatible change. This -may seem strange; adding fields to proto messages is generally backwards +Adding pagination to an existing method is a backwards-incompatible change. +This may seem strange; adding fields to proto messages is generally backwards compatible. However, this change is _behaviorally_ incompatible. Consider a user whose collection has 75 resources, and who has already written From a3508ca67b056ebd7f54a1c1fd6a77010012c8b7 Mon Sep 17 00:00:00 2001 From: Greg Perkins Date: Fri, 14 Feb 2025 12:19:06 -0800 Subject: [PATCH 4/8] Add example per style guidelines --- aep/general/0158/aep.md.j2 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aep/general/0158/aep.md.j2 b/aep/general/0158/aep.md.j2 index cc8b1570..a0152dea 100644 --- a/aep/general/0158/aep.md.j2 +++ b/aep/general/0158/aep.md.j2 @@ -164,8 +164,9 @@ a given `page_token` was generated: - If no `has_changed` boolean is included, the API **may** expire the page token. -If the service is incapable of detecting such changes, it **may** return stale -data or phantom reads without any indication. +If the service is incapable of detecting such changes, for example if it is +using an in-memory storage mechanism without snapshots, it **may** return stale +data or phantom reads with no indication. ### Backwards compatibility From e3683bd27c3b89e250a80120c22cbc819b37c313 Mon Sep 17 00:00:00 2001 From: Greg Perkins Date: Fri, 14 Feb 2025 12:24:00 -0800 Subject: [PATCH 5/8] total_size would be a breaking change, gotta normalize it back to totalSize --- aep/general/0132/aep.md.j2 | 8 ++++---- aep/general/0158/aep.md.j2 | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/aep/general/0132/aep.md.j2 b/aep/general/0132/aep.md.j2 index 32cec724..e00bdec7 100644 --- a/aep/general/0132/aep.md.j2 +++ b/aep/general/0132/aep.md.j2 @@ -104,11 +104,11 @@ result being a resource. schema. It **must** be set if there are subsequent pages, and **must not** be set if the response represents the final page. For more information, see AEP-158. -- The response struct **may** include a `int32 total_size` (or - `int64 total_size`) field with the number of items in the collection. +- The response struct **may** include a `int32 totalSize` (or + `int64 totalSize`) field with the number of items in the collection. - The value **may** be an estimate (the field **should** clearly document this if so). - - If filtering is used, the `total_size` field **should** reflect the size of + - If filtering is used, the `totalSize` field **should** reflect the size of the collection _after_ the filter is applied. - The response message **must** include a field corresponding to the resources being returned, named for the English plural term for the resource, and @@ -117,7 +117,7 @@ result being a resource. that the underlying data has changed since the `next_page_token` was generated. - Fields providing metadata about the list request (such as - `string next_page_token` or `int32 total_size`) **must** be included on the + `string next_page_token` or `int32 totalSize`) **must** be included on the response (not as part of the resource itself). {% tab proto %} diff --git a/aep/general/0158/aep.md.j2 b/aep/general/0158/aep.md.j2 index a0152dea..071aa8a9 100644 --- a/aep/general/0158/aep.md.j2 +++ b/aep/general/0158/aep.md.j2 @@ -91,7 +91,7 @@ message ListBooksResponse { - If the end of the collection has not been reached (or if the API can not determine in time), the API **must** provide a `next_page_token`. - Response messages for collections **may** provide an integer (`int32` for - protobuf) `total_size` field, providing the user with the total number of + protobuf) `totalSize` field, providing the user with the total number of items in the list. - This total **may** be an estimate. If so, the API **should** explicitly document that. From 50b635f00d2a595107bd52dbba82f63d376db304 Mon Sep 17 00:00:00 2001 From: Greg Perkins Date: Fri, 14 Feb 2025 12:32:01 -0800 Subject: [PATCH 6/8] Undo irrelevant casing changes, leave inconsistency --- aep/general/0132/aep.md.j2 | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/aep/general/0132/aep.md.j2 b/aep/general/0132/aep.md.j2 index e00bdec7..fafdd3bd 100644 --- a/aep/general/0132/aep.md.j2 +++ b/aep/general/0132/aep.md.j2 @@ -100,7 +100,7 @@ result being a resource. - The array of resources **must** be named `results` and contain resources with no additional wrapping. -- The `string next_page_token` field **must** be included in the list response +- The `string nextPageToken` field **must** be included in the list response schema. It **must** be set if there are subsequent pages, and **must not** be set if the response represents the final page. For more information, see AEP-158. @@ -117,7 +117,7 @@ result being a resource. that the underlying data has changed since the `next_page_token` was generated. - Fields providing metadata about the list request (such as - `string next_page_token` or `int32 totalSize`) **must** be included on the + `string next_page_token` or `int32 total_size`) **must** be included on the response (not as part of the resource itself). {% tab proto %} @@ -166,12 +166,12 @@ to specify the resources that are returned in a response. The following table summarizes the applicable AEPs, ordered by the precedence of the operation on the results. -| Operation | AEP | -| ------------------------------ | --------------------------------- | -| filter | [AEP-160](/160) | -| ordering (`orderBy`) | [AEP-132](#ordering) | -| pagination (`next_page_token`) | [AEP-158](/158) | -| skip | [AEP-158](/158/#skipping-results) | +| Operation | AEP | +| ---------------------------- | --------------------------------- | +| filter | [AEP-160](/160) | +| ordering (`orderBy`) | [AEP-132](#ordering) | +| pagination (`nextPageToken`) | [AEP-158](/158) | +| skip | [AEP-158](/158/#skipping-results) | For example, if both the `filter` and `skip` fields are specified, then the filter would be applied first, then the resulting set would be the results that From f95aef484458ae7c637d2e8d803e8b062e1fbe9b Mon Sep 17 00:00:00 2001 From: Greg Perkins Date: Fri, 14 Feb 2025 12:32:56 -0800 Subject: [PATCH 7/8] Oops missed one --- aep/general/0132/aep.md.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aep/general/0132/aep.md.j2 b/aep/general/0132/aep.md.j2 index fafdd3bd..a8bdd537 100644 --- a/aep/general/0132/aep.md.j2 +++ b/aep/general/0132/aep.md.j2 @@ -130,7 +130,7 @@ result being a resource. - The field "results" **must** be an array of resources, with the schema as a reference to the resource (e.g. `#/components/schemas/Book`). -- The field "next_page_token" **must** be a string that contains the token to +- The field "nextPageToken" **must** be a string that contains the token to retrieve the next page. This **must not** be set if the response represents the final page. From 8b41a1de24dda98cbaafd997901b93e8cfe198e3 Mon Sep 17 00:00:00 2001 From: Greg Perkins Date: Fri, 14 Feb 2025 12:43:13 -0800 Subject: [PATCH 8/8] Remove incorrect filter AEP reference --- aep/general/0158/aep.md.j2 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aep/general/0158/aep.md.j2 b/aep/general/0158/aep.md.j2 index 071aa8a9..44ec2a67 100644 --- a/aep/general/0158/aep.md.j2 +++ b/aep/general/0158/aep.md.j2 @@ -150,10 +150,10 @@ is three days. ### Freshness -Since a request with the same filter (see [AIP-132][]) and `page_token` -**should** return the same results unless the page token has expired, items -returned **may** have been deleted or modified since the initial request, and -paging through the list **may** not include all current items. +Since a request with the same filter and `page_token` **should** return the +same results unless the page token has expired, items returned **may** have +been deleted or modified since the initial request, and paging through the list +**may** not include all current items. If the service is capable of detecting underlying changes in the listing since a given `page_token` was generated: