smithy-lang / smithy

Smithy is a protocol-agnostic interface definition language and set of tools for generating clients, servers, and documentation for any programming language.
https://smithy.io
Apache License 2.0
1.79k stars 213 forks source link

Pagination inputToken and pageSize Path Support #1994

Closed hunoz closed 3 months ago

hunoz commented 1 year ago

Hello,

I have an operation with this request model:

@input
structure ListSitesRequest {
    @required
    profileFilters: ProfileFilterSettings,
    @required
    rootFilters: FilterSettings,
    @required
    order: Order,
    @required
    pagination: Pagination,
}

and this response model:

@output
structure ListSitesResponse {
    @required
    items: Sites,
    count: Integer,
    totalCount: Integer,
    nextPage: String,
    totalPages: String,
}

and here is the Pagination structure:

structure Pagination {
    @required
    @range(min: 1, max: 1024)
    page: Integer,
    @required
    @range(min: 1, max: 250)
    pageSize: Integer,
}

I am trying to put the following @paginated trait in my operation:

@paginated(
    inputToken: "pagination.page", outputToken: "nextPage", pageSize: "pagination.pageSize", items: "items"
)

However, I am facing the following error:

     [java] ──  ERROR  ────────────────────────────────────────────────────── PaginatedTrait
     [java] Shape: xxxx.xxxx.xxxx.xxxx#ListSites
     [java] File:  model/site/operation-listSites.smithy:8:1
     [java] 
     [java] 8 | @paginated(
     [java]   | ^
     [java] ··|
     [java] 21| operation ListSites{
     [java] 
     [java] paginated trait inputToken does not allow path values
     [java] 
     [java] 
     [java] ──  ERROR  ────────────────────────────────────────────────────── PaginatedTrait
     [java] Shape: xxx.xxxx.xxxx.xxxx#ListSites
     [java] File:  model/site/operation-listSites.smithy:8:1
     [java] 
     [java] 8 | @paginated(
     [java]   | ^
     [java] ··|
     [java] 21| operation ListSites{
     [java] 
     [java] paginated trait pageSize does not allow path values
     [java] 
     [java] FAILURE: Validated 539 shapes (ERROR: 2, WARNING: 2, NOTE: 5)

I understand that this does not seem to be supported, however I would like this request to be seen as a feature request to allow me to use paths in inputToken and pageSize.

rumenvasilev commented 1 year ago

Can you try to bypass the validation with @suppress(["PaginatedTrait"]) and see if that would work for you?

mtdowling commented 1 year ago

Do you really need to make the token nested on input? It seems like a bad idea to me since if you ever wanted to support something like HTTP bindings, you couldn't use a GET request and put the token in the query string (those only support top-level members).

hunoz commented 1 year ago

Can you try to bypass the validation with @suppress(["PaginatedTrait"]) and see if that would work for you?

That seems to result in it failing to build with this error:

 Projection source failed: java.util.NoSuchElementException: No value present
     [java] java.util.NoSuchElementException: No value present

Do you really need to make the token nested on input? It seems like a bad idea to me since if you ever wanted to support something like HTTP bindings, you couldn't use a GET request and put the token in the query string (those only support top-level members).

Yes, we do this to group any current and future pagination attributes, since a request has other attributes. With this operation, we have no plan of supporting GET requests as the filters can get quite large.

haydenbaker commented 3 months ago

Like Michael stated, it's a bad idea. Additionally, if we start adding support for nesting/pathing in more places, we'll get more requests to add it elsewhere (say, in the http traits).

Closing this as not planned.