Refreshing cache doesn't work

[yc185050]

Describe the bug?

According to https://github.com/okta/okta-sdk-golang#refreshing-cache-for-specific-call, client.CloneRequestExecutor().RefreshNext() can be used to force refreshing the cache on next call. However, based on testing, it doesn't work. See code snippet below

What is expected to happen?

func main() {
    ctx, client, err := getOktaClient()
    if err != nil {
        return
    }
    // create an app
    basicApplication := okta.NewBasicAuthApplication()
    basicApplication.Settings = &okta.BasicApplicationSettings{
        App: &okta.BasicApplicationSettingsApplication{
            AuthURL: "https://example.com/auth.html",
            Url:     "https://example.com/auth.html",
        },
    }

    myapp, _, err := client.Application.CreateApplication(ctx, basicApplication, nil)
    if err != nil {
        return
    }
    appId := myapp.(*okta.BasicAuthApplication).Id
        // list app
    appList, _, _ := client.Application.ListApplications(ctx, nil)
    fmt.Printf("applist length before delete: %d\n", len(appList))
        // deactivate and delete
    _, err = client.Application.DeactivateApplication(ctx, appId)
    if err != nil {
        return
    }
    _, err = client.Application.DeleteApplication(ctx, appId)
    if err != nil {
        return
    }
    // refresh cache
    client.CloneRequestExecutor().RefreshNext()
        // list app again
    appList, _, _ = client.Application.ListApplications(ctx, nil)
    fmt.Printf("applist length after delete: %d\n", len(appList))
}

expected output:

applist length before delete: 8
applist length after delete: 7

What is the actual behavior?

actual output

applist length before delete: 8
applist length after delete: 8

Reproduction Steps?

see above

Additional Information?

No response

Golang Version

go version go1.18.8 darwin/amd64

SDK Version

github.com/okta/okta-sdk-golang/v2 v2.18.0

OS version

No response

[yc185050]

Based on my investigation, client.CloneRequestExecutor().RefreshNext() creates a clone of request executor and set freshCache to true. However, this copy of request executor is never used as in client.Application.ListApplications it creates another copy and makes the call. That's why it didn't work. If I use the rq returned by client.CloneRequestExecutor().RefreshNext() and make my own call to list application for example

    url := fmt.Sprintf("/api/v1/apps")
    rq := client.CloneRequestExecutor().RefreshNext()
    var application []okta.Application
    req, err := rq.WithAccept("application/json").WithContentType("application/json").NewRequest("GET", url, nil)
    if err != nil {
        return
    }
    _, err = rq.Do(ctx, req, &application)
    if err != nil {
        return
    }
    fmt.Printf("applist length after using rq: %d\n", len(application))

I am getting a fresh call with correct applist

applist length before delete: 8
applist length after delete: 8
applist length after using rq: 7

[monde]

@yc185050 I think the example in the readme is incorrect on the suggestion of using a request executor clone each time. Instead just use the current request executor. Does this code solve your problem?

func main() {
    ctx, client, err := getOktaClient()
    if err != nil {
        return
    }
    // create an app
    basicApplication := okta.NewBasicAuthApplication()
    basicApplication.Settings = &okta.BasicApplicationSettings{
        App: &okta.BasicApplicationSettingsApplication{
            AuthURL: "https://example.com/auth.html",
            Url:     "https://example.com/auth.html",
        },
    }

    myapp, _, err := client.Application.CreateApplication(ctx, basicApplication, nil)
    if err != nil {
        return
    }
    appId := myapp.(*okta.BasicAuthApplication).Id
        // list app
    appList, _, _ := client.Application.ListApplications(ctx, nil)
    fmt.Printf("applist length before delete: %d\n", len(appList))
        // deactivate and delete
    _, err = client.Application.DeactivateApplication(ctx, appId)
    if err != nil {
        return
    }
    _, err = client.Application.DeleteApplication(ctx, appId)
    if err != nil {
        return
    }
    // refresh cache
    client.GetRequestExecutor().RefreshNext()
        // list app again
    appList, _, _ = client.Application.ListApplications(ctx, nil)
    fmt.Printf("applist length after delete: %d\n", len(appList))
}

[monde]

@yc185050 if you confirm this works for you I'll correct the example in the readme.

[yc185050]

No. It will get correct value but that's because

client.GetRequestExecutor().RefreshNext()

will set current rq with freshCache to true but it is never used to make any calls to set freshCache back to false. Any further ListApplications or GetApplications etc will also have freshCache to true which is equivelant to not using cache.

Actually workaround example I provided above is exactly how it is supposed to be used. However, the implementation of ListApplications and any other methods don't really work like this. It is always making a copy of rq and execute. see https://github.com/okta/okta-sdk-golang/blob/758b8c38315d262ab5aedb4694655d48ebb68384/okta/application.go#LL73C2-L73C2

[monde]

Getting this on to our backlog: Okta internal reference: https://oktainc.atlassian.net/browse/OKTA-616665

[github-actions[bot]]

This issue has been marked stale because there has been no activity within the last 14 days. To keep this issue active, remove the stale label.

[github-actions[bot]]

This issue has been marked stale because there has been no activity within the last 14 days. To keep this issue active, remove the stale label.

[codeviking]

Using GetRequestExecutor() instead of CloneExecutor() "works", but isn't safe. Also, it appears the cache will be reset after the first call:

https://github.com/okta/okta-sdk-golang/blob/758b8c38315d262ab5aedb4694655d48ebb68384/okta/requestExecutor.go#L453-L457

So I'm not sure this is correct:

will set current rq with freshCache to true but it is never used to make any calls to set freshCache back to false. Any further ListApplications or GetApplications etc will also have freshCache to true which is equivelant to not using cache.

The significant downside, however, is that there's a race. If another request uses the underlying executor before the call you're intending to cache-bust, then you'll end up with a cached response despite best efforts.

I believe this is the reason the documentation references CloneRequestExecutor(), despite things not working correctly.

[github-actions[bot]]

This issue has been marked stale because there has been no activity within the last 14 days. To keep this issue active, remove the stale label.

[github-actions[bot]]

This issue has been marked stale because there has been no activity within the last 14 days. To keep this issue active, remove the stale label.