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

[BUG] Next page link missing while listing Storage accounts #1266

Closed
jejner opened this issue Aug 25, 2020 · 9 comments · Fixed by #1269
Closed

[BUG] Next page link missing while listing Storage accounts #1266

jejner opened this issue Aug 25, 2020 · 9 comments · Fixed by #1269
Assignees
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-more-info Issue needs more information for the SDK team to take action on it Storage

Comments

@jejner
Copy link

jejner commented Aug 25, 2020

Describe the bug
While listing storage accounts via the list call next page link is missing in the sdk response and hence not able to fetch the remaining storage accounts.
I guess the problem is here:
https://github.com/Azure/azure-libraries-for-java/blob/v1.36.1/azure-mgmt-storage/src/main/java/com/microsoft/azure/management/storage/implementation/StorageAccountsInner.java is using PageImpl as the transformer pojo which has the missing @JsonProperty("nextLink") on nextPageLink attribute. Hence the next page link remains empty.
It is not the same case with DisksInner where PageImpl1 is being used instead of PageImpl as the former class has the correct annotation present.

This issue looks similar to #136.

To Reproduce
Steps to reproduce the behavior:

  1. List storage accounts call was made multiple times in a very short span of time.

Expected behavior
NextPageLink should get filled with nextLink received from the rest api response.

Setup (please complete the following information):

  • JRE Version: Open JDK 8
  • SDK Version: 1.35.0
@yungezz yungezz added customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage labels Aug 27, 2020
@yungezz
Copy link
Member

yungezz commented Aug 27, 2020

hi @xccc-msft could you pls help to have a look? thanks

@xseeseesee
Copy link
Contributor

@jejner I'm not able to reproduce. Please share more info about your scenario. Thanks.

The test I tried to reproduce by listing storage accounts:

        ResourceGroup resourceGroup = azure.resourceGroups().define("RG_NAME")
                .withRegion(Region.US_WEST)
                .create();

        StorageAccount sa1 = azure.storageAccounts().define("SA_1")
                .withRegion(Region.US_WEST)
                .withExistingResourceGroup("RG_NAME")
                .create();

        StorageAccount sa2 = azure.storageAccounts().define("SA_2")
                .withRegion(Region.US_WEST)
                .withExistingResourceGroup("RG_NAME")
                .create();

        StorageAccount sa3 = azure.storageAccounts().define("SA_3")
                .withRegion(Region.US_WEST)
                .withExistingResourceGroup("RG_NAME")
                .create();

        PagedList<StorageAccount> list = azure.storageAccounts().list();
        for (StorageAccount sa : list) {
            System.out.println(sa.name() + " : " + sa.id());
        }

@xseeseesee xseeseesee added the needs-more-info Issue needs more information for the SDK team to take action on it label Aug 27, 2020
@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Aug 27, 2020

@xccc-msft At least you need more than one page? (4 is not likely be the limit on the page)

@jejner
Copy link
Author

jejner commented Aug 27, 2020

@xccc-msft we are having more than 80 storage accounts spread across various regions & resource groups and the same azure account is being used in multiple dev environments. There were multiple calls for listing storage accounts from various envs in a very short span of time and we hit this issue of pagination. I was able to reproduce this by adding the listing storage accounts call in a for loop with more than 100 iterations.

@xseeseesee
Copy link
Contributor

@jejner I tried to create 150 storage accounts with 5 regions/10 resource groups randomly. And then list all storage accounts. It works fine for me. Can you please capture the response when reproducing this issue? Also, do you have any scenario deleting storage accounts in some dev environments? It might possible if some deletion happened during the list operation so the list operation stopped.

@jejner
Copy link
Author

jejner commented Aug 27, 2020

@xccc-msft One time listing of storage accounts works fine, problem is when you fire multiple listing calls together(like more than 100 requests). With the 150 storage accounts you have created, can you try listing all storage accounts like 100 or 120 times via a loop? Then you will be able to see the issue.
Right now i am using another Azure subscription which has 51 storage accounts
Test code:

for (int i=1; i<=120; i++) {
            PagedList<StorageAccountInner> list = azure.storageAccounts().inner().list();
            System.out.println("Iteration Number: " + i);
            System.out.println("size: " + list.currentPage().items().size());
            System.out.println("isNextPagePresent: " + list.hasNextPage());
        }

and this is the response:

Iteration Number: 97
size: 51
isNextPagePresent: false
Iteration Number: 98
size: 51
isNextPagePresent: false
Iteration Number: 99
size: 48
isNextPagePresent: false
Iteration Number: 100
size: 48
isNextPagePresent: false
Iteration Number: 101
size: 48
isNextPagePresent: false

As for the deletion query, no storage accounts are being deleted from any of the environments.

Also when i am using the storage accounts list rest api, i am seeing the nextLink present in the response but when using the sdk this page link goes missing.

@xseeseesee
Copy link
Contributor

@jejner I tried with same scenario as you suggested

        System.out.println("list by StorageAccountInner:");
        for (int i=1; i<=120; i++) {
            PagedList<StorageAccountInner> list = azure.storageAccounts().inner().list();
            System.out.println("Iteration Number: " + i);
            System.out.println("size: " + list.currentPage().items().size());
            System.out.println("isNextPagePresent: " + list.hasNextPage());
        }

        System.out.println("\n\nlist by StorageAccount:");
        for (int i=1; i<=120; i++) {
            PagedList<StorageAccount> list = azure.storageAccounts().list();
            System.out.println("Iteration Number: " + i);
            System.out.println("size: " + list.currentPage().items().size());
            System.out.println("isNextPagePresent: " + list.hasNextPage());
        }

And the responses for me are all the same

Iteration Number: 68
size: 151
isNextPagePresent: false
Iteration Number: 69
size: 151
isNextPagePresent: false
Iteration Number: 70
size: 151
isNextPagePresent: false
Iteration Number: 71
size: 151

Can you please try to capture the response and check if nextPageLink is missing in the response? Thanks.

@jejner
Copy link
Author

jejner commented Aug 28, 2020

@xccc-msft Can you try running the test code it multiple times or increase the number of iterations? It is happening for me consistently.
Screenshots of response i am receiving while running the test code:
responseContentBodyHaving-nextLink
deserializingResponseContentAndNextPageLinkIsNotFilled

I took these screenshots while debugging sdk code and specifically in the ServiceResponseBuilder class. As you can see nextLink is present in responseContent but nextPageLink is null when deserialised.

Also i will share below the rest api response called via postman which has the "nextLink" field present, so i am sure that results are getting divided between pages and is consistent with the test code result.
Screenshot 2020-08-28 at 11 52 50 AM

Problem is when this response is being transformed in the sdk layer. As i had mentioned in the bug description that "nextLink" is not being mapped to "nextPageLink", hence nextPageLink never gets filled.

I found this file: https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/storage/mgmt-v2019_04_01/src/main/java/com/microsoft/azure/management/storage/v2019_04_01/implementation/StorageAccountsInner.java where the issue seems to be fixed as "PageImpl1" is being used as a transformer. There seems to be inconsistency between this file and the file mentioned in the bug description #1266 (comment) with regards to transformer implementation.
Hope this info helps..

@xseeseesee
Copy link
Contributor

@jejner Thanks for sharing. I think for my tests I always get only one page in the response so that I cannot reproduce. And for your case, there are more than one page in the response while the PageImpl is missing property of nextLink. After checking the PageImpl, it's a generated class by our code generator and swagger definitions. I just create a PR to fix this. Please feel free to review and share your comments. Thanks again for reporting this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-more-info Issue needs more information for the SDK team to take action on it Storage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants