go-sdk icon indicating copy to clipboard operation
go-sdk copied to clipboard

HasContinuationToken() methods return true whereas there is no more page

Open le-yams opened this issue 1 year ago • 3 comments

Hello there,

Description

When using "paginable" methods, the HasContinuationToken() methods return true whereas there is no more page.

I expect the HasContinuationToken() method to return false in such cases.

As the openfga "paginable" endpoints always returns the continuation_token field (with an empty string value if there is no more page), the ContinuationToken response field is never nil (and so the HasContinuationToken() implementation always retrurn true).

Version of SDK

v0.2.2

Version of OpenFGA

v1.1.1

Reproduction

This issue can be reproduced consistently invoking any method querying an openfga "paginable" endpoint.

Here is an example using the ListStores method.

  1. Initialize OpenFgaClient to point an openfga instance with N stores
  2. invoke the ListStores()method with a page size > N
  3. the response HasContinuationToken() should return false but it actually returns true

Sample Code that Produces the Issue

Another example relying on the continuation token to do something with all stores that creates an infinite loop because of the HasContinuationToken() method always returning true.

response, err := fgaClient.ListStores(context.Background()).Options(options).Execute()
if err != nil {
    return "", err
}
doSomethingWith(response.GetStores())

// infinite loop here because HasContinuationToken() always return true
for response.HasContinuationToken() {
    options.ContinuationToken = response.ContinuationToken
    response, err = fgaClient.ListStores(context.Background()).Options(options).Execute()
    if err != nil {
        return "", err
    }
    doSomethingWith(response.GetStores())
}

Expected behavior

The HasContinuationToken() method (on any response type) should return false if the response continuation_token field is an empty string

le-yams avatar Jun 28 '23 13:06 le-yams

Hello @le-yams , thank you for raising this issue.

In order to check whether you are done reading, you not only need to check whether has continuation token is true, but also check against whether it is the same as previous continuation token.

Thus, you will need to have something like

continuationToken := ""
options.ContinuationToken = &continuationToken
response, err := fgaClient.ListStores(context.Background()).Options(options).Execute()
if err != nil {
    return "", err
}
doSomethingWith(response.GetStores())

// check whether the continuation token is the same as previous
for {
    options.ContinuationToken = &continuationToken
    response, err = fgaClient.ListStores(context.Background()).Options(options).Execute()
    if err != nil {
        return "", err
    }
    doSomethingWith(response.GetStores())
    if !response.HasContinuationToken() || *response.ContinuationToken == continuationToken {
       break
    }
    continuationToken =  *response.ContinuationToken

}

Let me know if you need further clarification. Thank you.

adriantam avatar Jun 30 '23 16:06 adriantam

Hello @adriantam ,

In order to check whether you are done reading, you not only need to check whether has continuation token is true, but also check against whether it is the same as previous continuation token.

Thank you for pointing that out 🙂.

Could you clarify the case where the returned continuation token would be the same as the one specified in the request? I did some tests against our openfga server for paginated queries (I only tried the "stores list" endpoint assuming the behavior is the same elsewhere). When I reach the last page I get no more continuation token (an empty one actually) which is the behavior I was excepting.

Is the pagination system specified somewhere (I didn't find it in the documentation)?

Thank you for your help.

le-yams avatar Jul 03 '23 07:07 le-yams

Hello @le-yams , thank you for your notes. You are correct in that empty string means the last page (sorry that we were mistaken).

With that in mind, you should have something like this:

continuationToken := ""
options.ContinuationToken = &continuationToken
response, err := fgaClient.ListStores(context.Background()).Options(options).Execute()
if err != nil {
    return "", err
}
doSomethingWith(response.GetStores())

// check whether the continuation token is the same as previous
for {
    options.ContinuationToken = &continuationToken
    response, err = fgaClient.ListStores(context.Background()).Options(options).Execute()
    if err != nil {
        return "", err
    }
    doSomethingWith(response.GetStores())
    if !response.HasContinuationToken() || *response.ContinuationToken == "" {
       break
    }
    continuationToken =  *response.ContinuationToken

}

adriantam avatar Jul 05 '23 18:07 adriantam