Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
le-yams opened this issue Jun 28, 2023 · 3 comments
Open
Labels
bug Something isn't working

Comments

@le-yams
Copy link
Contributor

le-yams commented Jun 28, 2023

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 le-yams added the bug Something isn't working label Jun 28, 2023
@adriantam
Copy link
Member

adriantam commented Jun 30, 2023

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.

@le-yams
Copy link
Contributor Author

le-yams commented Jul 3, 2023

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.

@adriantam
Copy link
Member

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

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

2 participants