[REGAPIC] REST pagination detection is different from GRPC when the repeated field is not a message
dazuma opened this issue · 3 comments
In GRPC firestore-v1, the ListCollectionIds method currently does not get recognized as paginated by the GRPC generator (even though ListDocuments does get recognized). I think the difference is that in the source proto for ListCollectionIdsResponse, the repeated field is a string whereas the generator expects it to be a message. (I am not 100% sure about this, though, and we should verify.) Arguably this is a bug in the generator, but we can't change it without incurring breaking changes at this point.
The REST generator, however, does generate pagination for ListCollectionIds. We need to figure out if this inconsistency is a problem and what to do about it.
I've located the logic in both cases. gRPC does indeed require that the paginated field has a message type (see Gapic::Presenters::MethodPresenter#paged_response?, whereas REST/REGAPIC allows any repeated type or a map (see Gapic::Presenters::Method::RestPaginationInfo#response_results_field).
After reviewing the relevant design documents, it appears that DIREGAPIC/REGAPIC specification is intentionally slightly different from traditional GAPIC. Hence, I'm closing this as WAI.
To be clear (for posterity): The gRPC behavior is correct according to AIP 4223 which insists that the repeated field is non-primitive. (String fields are considered primitive in protobuf.) The DIREGAPIC pagination design (internal) specifies different logic, including insisting that there be exactly one items field, and that maps are allowed and indeed take precedence over repeateds.