pagination is not automatic

[gabrielsroka]

Describe the bug?

1. The readme says that pagination is automatic, but doesn't provide a complete example. what does "automatic" mean? i don't have to write additional code?

i was able to get this working, but i don't know if it's correct. is there a better/shorter way? does the SDK do some of this for me? (i'm not a Java programmer.)

import com.okta.sdk.client.Clients;
import com.okta.sdk.resource.common.PagedList;

import org.openapitools.client.ApiClient;
import org.openapitools.client.api.UserApi;
import org.openapitools.client.model.User;

class Main {
    public static void main(String[] args)  {
        ApiClient client = Clients.builder().build();
        UserApi userApi = new UserApi(client);
        String filter = "profile.lastName eq \"Doe\"";
        String after = null;
        do {
            PagedList<User> usersPagedList = 
                userApi.listUsersWithPaginationInfo(null, after, 2, filter, null, null, null);
            for (User user : usersPagedList.getItems()) {
                System.out.println(user.getProfile().getLogin());
            }
            after = getAfter(usersPagedList.getNextPage());
        } while (after != null);
    }

    public static String getAfter(String nextPage) {
        String after = null;
        if (nextPage != null) {
            String query = nextPage.split("\\?")[1];
            for (String pair : query.split("&")) {
                String[] nv = pair.split("=", 2);
                if (nv[0].equals("after")) after = nv[1];
            }
        }
        return after;
    }
}

https://replit.com/@gabrielsroka/okta-sdk-java#Main.java

parts of ^^ were stolen from

okta-sdk-java/integration-tests/src/test/groovy/com/okta/sdk/tests/it/UsersIT.groovy

Lines 874 to 880 in cb0a648

	// e.g. https://example.okta.com/api/v1/users?after=000u3pfv9v4SQXvpBB0g7&limit=2
	String nextPageUrl = usersPagedListOne.nextPage
	String after = splitQuery(new URL(nextPageUrl)).get("after")
	assertThat after, notNullValue()
	PagedList usersPagedListTwo = userApi.listUsersWithPaginationInfo(null, after, pageSize, null, null, null, null)

2. Either way, the readme is wrong. after below should be the 2nd argument, not the 1st:

okta-sdk-java/README.md

Lines 423 to 427 in cb0a648

	// e.g. https://example.okta.com/api/v1/users?after=000u3pfv9v4SQXvpBB0g7&limit=2
	String nextPageUrl = usersPagedListOne.getNextPage();
	// replace 'after' with actual cursor from the nextPageUrl
	PagedList<User> usersPagedListTwo = userApi.listUsersWithPaginationInfo("after", null, pageSize, null, null, null, null);

same with

okta-sdk-java/examples/quickstart/src/main/java/quickstart/ReadmeSnippets.java

Lines 282 to 286 in cb0a648

	// e.g. https://example.okta.com/api/v1/users?after=000u3pfv9v4SQXvpBB0g7&limit=2
	String nextPageUrl = usersPagedListOne.getNextPage();
	// replace 'after' with actual cursor from the nextPageUrl
	PagedList<User> usersPagedListTwo = userApi.listUsersWithPaginationInfo("after", null, pageSize, null, null, null, null);

3. You can't use q to paginate users or groups, because the API doesn't return link headers when using q, so this:

String q = "Doe";
PagedList<User> usersPagedList = userApi.listUsersWithPaginationInfo(q, after, 2, null, null, null, null);

returns this exception

Exception in thread "main" java.lang.NullPointerException:
  Cannot invoke "java.util.List.iterator()" because "linkHeaders" is null
    at org.openapitools.client.api.UserApi.listUsersWithPaginationInfo(UserApi.java:3506)

Note that apps, authorizationServers, idps, logs, etc. do return link headers with q, but users, groups, etc. don't.

4. This example doesn't work:

okta-sdk-java/README.md

Lines 184 to 185 in cb0a648

	// search by email
	List<User> users = userApi.listUsers(null, null, 5, null, "[email protected]", null, null);

same with

okta-sdk-java/examples/quickstart/src/main/java/quickstart/ReadmeSnippets.java

Lines 103 to 104 in cb0a648

	// search by email
	List<User> users = userApi.listUsers(null, null, 5, null, "[email protected]", null, null);

it'd need to be one of these:

// q by email
List<User> users = userApi.listUsers("[email protected]", null, 5, null, null, null, null);

// search by email
List<User> users = userApi.listUsers(null, null, 5, null, "profile.email eq \"[email protected]\"", null, null);

What is expected to happen?

automatic pagination

What is the actual behavior?

it doesn't paginate. i had to write the code to figure out after and add the loop.

Reproduction Steps?

see above

Additional Information?

No response

Java Version

openjdk version "17.0.3"

SDK Version

10.0

OS version

NAME="Ubuntu"
VERSION="20.04.2 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.2 LTS"
VERSION_ID="20.04"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal

[gabrielsroka]

btw, where is the source code for userApi.listUsersWithPaginationInfo ?

EDIT: WithPaginationInfo seems to be generated using the mustache template:

okta-sdk-java/api/src/main/resources/custom_templates/api.mustache

Line 328 in 8be490d

public {{#returnType}}PagedList{{/returnType}}{{^returnType}}void{{/returnType}} {{operationId}}WithPaginationInfo({{#allParams}}{{#isFile}}{{#useAbstractionForFiles}}{{#collectionFormat}}java.util.Collection<org.springframework.core.io.Resource>{{/collectionFormat}}{{^collectionFormat}}org.springframework.core.io.Resource{{/collectionFormat}}{{/useAbstractionForFiles}}{{^useAbstractionForFiles}}{{{dataType}}}{{/useAbstractionForFiles}}{{/isFile}}{{^isFile}}{{{dataType}}}{{/isFile}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}) throws RestClientException {

which, for some reason, isn't indented:

okta-sdk-java/api/src/main/resources/custom_templates/api.mustache

Lines 374 to 397 in 8be490d

	HttpHeaders headers = responseEntity.getHeaders();
	PagedList pagedList = new PagedList();
	List<String> linkHeaders = headers.get("link");
	String self = null, nextPage = null;
	for (String link : linkHeaders) {
	if (link.contains("next")) {
	nextPage = link.split(";")[0]
	.replaceAll("<", "")
	.replaceAll(">", "");
	}
	if (link.contains("self")) {
	self = link.split(";")[0]
	.replaceAll("<", "")
	.replaceAll(">", "");
	}
	}
	pagedList.addItems(Arrays.asList(responseEntity.getBody()));
	if (Objects.nonNull(nextPage)) {
	pagedList.setNextPage(nextPage);
	}
	if (Objects.nonNull(self)) {
	pagedList.setSelf(self);
	}
	return pagedList;

[gabrielsroka]

these 2 lines from the mustache template ^^^ look problematic. what if the url itself contains "next" or "self"?

if (link.contains("next")) {
// ...
if (link.contains("self")) {

eg, this fetches the same url over and over again in an endless loop:

String filter = "profile.lastName eq \"next\"";

something like this would be more robust

if (link.contains("rel=\"next\"")) {
// ...
if (link.contains("rel=\"self\"")) {

[gabrielsroka]

actually, ^^^ is really redundant, also, too, as well.

why not something like this:

PagedList pagedList = new PagedList();
pagedList.addItems(Arrays.asList(responseEntity.getBody()));
HttpHeaders headers = responseEntity.getHeaders();
List<String> linkHeaders = headers.get("link");
for (String link : linkHeaders) {
    String[] parts = link.split("; *");
    String url = parts[0]
        .replaceAll("<", "")
        .replaceAll(">", "");
    String rel = parts[1];
    if (rel.equals("rel=\"next\"")) {
        pagedList.setNextPage(url);
    } else if (rel.equals("rel=\"self\"")) {
        pagedList.setSelf(url);
    }
}
return pagedList;

again, i'm not a Java programmer, so there might be a better (or more idiomatic) way to write this.

EDIT:

1. also, the code is repeated over and over again, generated by the template -- wouldn't it be better to factor it out into its own function?
2. the UserApi.java file generated by the template has 24 PaginationInfo methods, include POSTs, etc. i think only GETs of collections (not individual objects) should paginate. and only those collections that actually return link headers, eg:

endpoint	comment
GET /users	✅ should paginate
GET /users/${userId}/groups	✅ should paginate
GET /users/${userId}/appLinks	❌ should not paginate (doesn't return link headers)
GET /users/${userId}	❌ should not paginate (not a collection)
POST /users	❌ should not paginate (not a GET)
etc

[arvindkrishnakumar-okta]

@gabrielsroka thanks for posting! Apologies for the delayed response. Let me review this in detail and get back to you shortly.

[gabrielsroka]

or, use a regex to parse the link header

import java.util.regex.Matcher;
import java.util.regex.Pattern;

// ...
String pattern = "<(.*)>; *rel=\"(.*)\"";
Pattern r = Pattern.compile(pattern);
// ...
for (String link : linkHeaders) {
    Matcher m = r.matcher(link);
    if (m.find()) {
        String url = m.group(1);
        String rel = m.group(2);
        if (rel.equals("next")) {
            pagedList.setNextPage(url);
        } else if (rel.equals("self")) {
            pagedList.setSelf(url);
        }
    }
}

[arvindkrishnakumar-okta]

@gabrielsroka Thanks for your suggestions! I did review the present pagination implementation and realized that the templates need to be reworked to omit redundant code. Your suggestions are noted and will be factored (fixed) in the next revision.

[arvindkrishnakumar-okta]

@gabrielsroka merged PR to master, will cut a release soon. Please check it out and reopen this issue or open a new one if needed.

[arvindkrishnakumar-okta]

Released 10.2.0

[sdrissi]

I was reading through the doc as well and it is not clear if the SDK does go through all pages automatically. In the example of the doc it says "Loop through all of them". What does that mean? All of them in the first page or all of them as in all existing users?

UserApi userApi = new UserApi(client);
int limit = 2;
PagedList<User> pagedUserList = userApi.listUsersWithPaginationInfo(null, null, limit, null, null, null, null);

// loop through all of them
for (User tmpUser : pagedUserList.getItems()) {
    log.info("User: {}", tmpUser.getProfile().getEmail());
}

// or stream
pagedUserList.getItems().forEach(tmpUser -> log.info("User: {}", tmpUser.getProfile().getEmail()));

Let's say there are 10 existing users. Then, is the expected output two info logs (one for each of the 2 users in page 1 because limit is set to 2 users per page) or is it 10 info logs (i.e pagination is done automatically)?

[gabrielsroka]

@sdrissi my expectation/understanding is that it should give you all users. that said, it was broken in 10.0, but should be fixed in 10.2.

did you try it? (i have not)

@arvindkrishnakumar-okta , keep me honest here...

[sdrissi]

@gabrielsroka thanks for your reply. I did not try it yet. Would be great if @arvindkrishnakumar-okta could validate that this is the expected behavior. Will let you know once I upgrade to 10.2.