[Modular] Implement Pagination

[qiaozha]

Say we have below tsp definition

  @doc("List Widget resources")
  listWidgets is Operations.ResourceList<
    Widget,
    ListQueryParametersTrait<StandardListQueryParameters & SelectQueryParameter>
  >;

We would expect to generate modular operation

  public listWidgets(
    options?: ListWidgetsOptionalParams
  ): PagedAsyncIterableIterator<Widget> 

Option 1: One paging helper per operation

public listWidgets(
    options?: ListWidgetsOptionalParams
  ): PagedAsyncIterableIterator<Widget> {
    const iter = this.__listWidgetsPagingAll(options);
    return {
      next() {
        return iter.next();
      },
      [Symbol.asyncIterator]() {
        return this;
      },
      byPage: (settings?: PageSettings) => {
        if (settings?.maxPageSize) {
          throw new Error("maxPageSize is not supported by this operation.");
        }
        return this.getEmptyNextLinkNamePagesPagingPage(options, settings);
      }
    };
  }


private async __listWidgetsPagingAll(
    options?: ListWidgetsOptionalParams
  ): AsyncIterableIterator<Widget> {
    ....
  }

Option 2: Leverage pagination helper in RLC

  public listWidgets(
    options?: ListWidgetsOptionalParams
  ): PagedAsyncIterableIterator<Widget> {
    const initialResponse = await _listWidgetsSend(
        context,
        options
      );
    return paginate(client, initialResponse, {
         customGetPage: () => { ... // deserilized logic}
   });
}

Option 3: A modular generator

See details as below.

[MaryGao]

@xirzec Could you help review the byPage design in modular? /cc @deyaaeldeen @joheredi @qiaozha Feel free to comment if I missed something.

Background

For list operations result sets may be massive so customers may retrieve them page by page according to continuation token. Currently our byPage interface only offers the page data without any continuation token info and we provide the helper getContinuationToken to cache these details. In modular we could adjust the interface to better facilitate this feature but this could cause breaking changes.

byPage interface change

• Current interface in HLC

operations.ts

import { PagedAsyncIterableIterator } from "@azure/core-paging";
list(
  options?: ListOptionalParams
): PagedAsyncIterableIterator<Product>;

models.ts in core-paging code

export interface PagedAsyncIterableIterator
 //...
 byPage: (settings?: TPageSettings) => AsyncIterableIterator<TPage>; 
}

export interface PageSettings {
  continuationToken?: string;
  maxPageSize?: number;
}

• New interface in modular

operations.ts

// 1. Model name is the same as core-paging but it is defined in SDK side
import { PagedAsyncIterableIterator } from "../models/index.js";
list(
  options?: ListOptionalParams
): PagedAsyncIterableIterator<Product>;

models.ts in SDK

// 2. A new model used as the return type of byPage
export interface ContinuablePage<TPage, TLink = string> {
  /**
   * Gets a list of elements in the page.
   */
  page: TPage;
  /**
   * The token that keeps track of where to continue the iterator
   */
  continuationToken?: TLink;
}

export interface PagedAsyncIterableIterator
 //...
 byPage: (
    settings?: TPageSettings
 // 3. Return type is changed from TPage to ContinuablePage, from AsyncIterableIterator to AsyncIterator
  ) => AsyncIterator<ContinuablePage<TPage>>;
}

// 4. maxPageSize is removed directly
export interface PageSettings {
  continuationToken?: string;
}

Breaking Change Considerations

• Getting rid of maxPageSize could be safe breaking becuase we don't really support this param in HLC
• New design could make the relationship between page and continuation token explicit and close by and would be easier to use and discover.
• Codegen would leverage getPagedAsyncIterator in core by adding some adapter logic to new interfaces. But directly changing the core-paging interfaces could be chanlleging considering other downstreams may not have same requirement e.g service bus.

The whole discussion is under the pr.

[xirzec]

I like removing maxPageSize since any such parameter can still be part of the original list call if the service supports it without needing to be included in a call to byPage.

I also like ContinuablePage as it makes the continuationToken much more discoverable. Can we remove the TLink parameter and enforce that continuationToken is simply an optional string instead? Otherwise, the consumer would have to somehow turn it into a string in order to pass it into PageSettings.

One thing I'm curious about is point 3 in your example, changing from AsyncIterableIterator to AsyncIterator. The main difference between these types being that the AsyncIterableIterator is compatible with anything that expects an interable rather than an iterator, that is to say, I'm not sure if for/of will work correctly if you pass it an iterator directly. The main advantage of iterable iterators is that they're useful by constructors like for/of as well as directly consumable by a developer who wishes to call next() manually.

I have also been thinking about the compatibility problem with previously shipped clients. I wonder if we could include some way to opt into a ContinuablePage result, perhaps through another page setting, though I think writing the TypeScript for this is difficult. Failing that, we could imagine a companion function to byPage like byContinuablePage or some such.

[MaryGao]

Can we remove the TLink parameter and enforce that continuationToken is simply an optional string instead?

Sure let's keep it simple with string.

Changing from AsyncIterableIterator to AsyncIterator

Yeah, I think we should keep AsyncIterableIterator and I may mis-understand this part previously.

I wonder if we could include some way to opt into a ContinuablePage result.

I am also thinking about this and I think adding a companion function would work for me. Another immature idea is that ContinuablePage could extend TPage so that we could keep the backward compatible.

[MaryGao]

Offline confirmed we could look at having it extend the page type so someone returning Array could return Array & { continuationToken: string } but should be careful for the performance impact.