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

Get Invoices paging with IDs #534

Closed
lancedfr opened this issue Dec 1, 2022 · 6 comments
Closed

Get Invoices paging with IDs #534

lancedfr opened this issue Dec 1, 2022 · 6 comments
Labels
Type: API not Specification This issue can not be solved via the specifications

Comments

@lancedfr
Copy link

lancedfr commented Dec 1, 2022

SDK you're using (please complete the following information):

  • Version 4.23.0

Describe the bug
As per the developer documentation when paging for invoices each page will contain up to 100 invoices. However, when paging using IDs query parameter the maximum number of IDs that we can pass into this query parameter is 52. This limits us to 52 invoices per request instead of 100 invoices per request.

We suspect this is a "max character limit per query parameter" set on your gateway where 53 IDs in the IDs query parameter exceeds this limit. This limitation makes us hit our rate limits quicker than expected.

Endpoint being called (IDs removed as the URL is very long with 53 IDs)

GET https://api.xero.com/api.xro/2.0/Invoices?IDs=61548c60-b5ff-460c-acba-7963b9879adc,[53 invoice IDs]&page=1

To Reproduce
Using an HTTP client create a request to the /Invoices endpoint. Set page query parameter to = 1. Set IDs query parameter = 53 valid invoices IDs

Expected behavior
We expect the API to return 100 invoices per page when being passed 100 invoice IDs in the IDs query parameter

Screenshots
Using 53 invoices IDs API returns 404 (even though these IDs do exist)
image

Using 52 invoices IDs API returns 52 invoices
image

@rdemarco-xero rdemarco-xero transferred this issue from XeroAPI/Xero-Java Jan 26, 2023
@rdemarco-xero
Copy link
Collaborator

Hi @lancedfr, thanks for reporting this. I have transferred the issue over to our Open API Spec since the issue is happening at the API level and not specific to the Java SDK.

@RettBehrens RettBehrens added the Type: API not Specification This issue can not be solved via the specifications label Jan 26, 2023
@rdemarco-xero
Copy link
Collaborator

Hi @lancedfr, since this restriction is due to the overall character length of the request url you could provide the InvoiceNumber instead of the InvoiceId. InvoiceNumbers are typically shorter than InvoiceIds which are a GUID (36 characters).

@RettBehrens
Copy link
Contributor

RettBehrens commented Jan 27, 2023

There is a bit about this in the Developer Centre docs
Retrieving a filtered set of resources
The number of parameters is only limited by the maximum url length. To ensure you receive a timely response from the API, we recommend you look to retrieve in a batch of 40.
https://developer.xero.com/documentation/api/accounting/requests-and-responses#retrieving-a-filtered-set-of-resources

@lancedfr
Copy link
Author

lancedfr commented Feb 1, 2023

Hi @rdemarco-xero Thanks for this suggestion. We have updated our solution to retrieve invoices by InvoiceNumber instead of GUID. This still reaches the character length of the request parameter. We have been debugging the Xero JDK and found that the resolved URL being invoked is

https://api.xero.com/api.xro/2.0/Invoices?InvoiceNumbers=I-3051&InvoiceNumbers=INV-0525&InvoiceNumbers=I-3052&InvoiceNumbers=I-3053&InvoiceNumbers=I-3055&InvoiceNumbers=I-3063&InvoiceNumbers=I-3060&InvoiceNumbers=INV-0530&InvoiceNumbers=I-3065&InvoiceNumbers=INV-0533&InvoiceNumbers=I-3066&InvoiceNumbers=INV-0538&InvoiceNumbers=INV-0539&InvoiceNumbers=INV-0540&InvoiceNumbers=I-3072&InvoiceNumbers=INV-0546&InvoiceNumbers=INV-0547&InvoiceNumbers=INV-0548&InvoiceNumbers=I-3077&InvoiceNumbers=I-3079&InvoiceNumbers=INV-0552&InvoiceNumbers=I-3089&InvoiceNumbers=I-3090&InvoiceNumbers=I-3092&InvoiceNumbers=INV-0554A1&InvoiceNumbers=I-249&InvoiceNumbers=I-251&InvoiceNumbers=I-252&InvoiceNumbers=I-254&InvoiceNumbers=I-256&InvoiceNumbers=I-257&InvoiceNumbers=I-258&InvoiceNumbers=I-259&InvoiceNumbers=I-261&InvoiceNumbers=I-263&InvoiceNumbers=I-264&InvoiceNumbers=I-265&InvoiceNumbers=I-3105&InvoiceNumbers=INV-0562&InvoiceNumbers=I-3106&InvoiceNumbers=INV-0565&InvoiceNumbers=I-3108&InvoiceNumbers=I-3109&InvoiceNumbers=I-3112&InvoiceNumbers=I-1973&InvoiceNumbers=I-3114&InvoiceNumbers=I-3115&InvoiceNumbers=I-3116&InvoiceNumbers=I-3118&InvoiceNumbers=I-3120&InvoiceNumbers=I-3122&InvoiceNumbers=I-3124&InvoiceNumbers=INV-0572&InvoiceNumbers=I-3125&InvoiceNumbers=I-3126&InvoiceNumbers=INV-0576&InvoiceNumbers=INV-0577&InvoiceNumbers=INV-0578&InvoiceNumbers=INV-0579&InvoiceNumbers=INV-0580&InvoiceNumbers=INV-0581&InvoiceNumbers=I-3131&InvoiceNumbers=I-3132&InvoiceNumbers=I-3134&InvoiceNumbers=I-3137&InvoiceNumbers=I-3139&InvoiceNumbers=I-3142&InvoiceNumbers=I-3143&InvoiceNumbers=I-3145&InvoiceNumbers=I-3146&InvoiceNumbers=INV-0596&InvoiceNumbers=INV-0598&InvoiceNumbers=INV-0599&InvoiceNumbers=I-3152&InvoiceNumbers=I-3154&InvoiceNumbers=I-3160&InvoiceNumbers=I-3162&InvoiceNumbers=INV-0603&InvoiceNumbers=INV-0607&InvoiceNumbers=INV-0608&InvoiceNumbers=I-3166&InvoiceNumbers=I-3167&InvoiceNumbers=I-3168&InvoiceNumbers=I-3170&InvoiceNumbers=I-3171&InvoiceNumbers=I-3172&InvoiceNumbers=I-3175&InvoiceNumbers=I-3177&InvoiceNumbers=I-3181&InvoiceNumbers=I-3185&InvoiceNumbers=I-3186&InvoiceNumbers=I-3197&InvoiceNumbers=I-310&InvoiceNumbers=I-311&InvoiceNumbers=I-314&InvoiceNumbers=I-317&InvoiceNumbers=I-323&InvoiceNumbers=I-326&InvoiceNumbers=I-3190&InvoiceNumbers=I-3192&page=1&includeArchived=false&createdByMyApp=false&unitdp=2&summaryOnly=false

As you can see InvoiceNumbers= is being repeated.

When running the below HTTP GET command (after removing the repeated InvoiceNumbers=) we manage to get all 100 invoices by number.

GET https://api.xero.com/api.xro/2.0/Invoices?InvoiceNumbers=I-3051,INV-0525,I-3052,I-3053,I-3055,I-3063,I-3060,INV-0530,I-3065,INV-0533,I-3066,INV-0538,INV-0539,INV-0540,I-3072,INV-0546,INV-0547,INV-0548,I-3077,I-3079,INV-0552,I-3089,I-3090,I-3092,INV-0554A1,I-249,I-251,I-252,I-254,I-256,I-257,I-258,I-259,I-261,I-263,I-264,I-265,I-3105,INV-0562,I-3106,INV-0565,I-3108,I-3109,I-3112,I-1973,I-3114,I-3115,I-3116,I-3118,I-3120,I-3122,I-3124,INV-0572,I-3125,I-3126,INV-0576,INV-0577,INV-0578,INV-0579,INV-0580,INV-0581,I-3131,I-3132,I-3134,I-3137,I-3139,I-3142,I-3143,I-3145,I-3146,INV-0596,INV-0598,INV-0599,I-3152,I-3154,I-3160,I-3162,INV-0603,INV-0607,INV-0608,I-3166,I-3167,I-3168,I-3170,I-3171,I-3172,I-3175,I-3177,I-3181,I-3185,I-3186,I-3197,I-310,I-311,I-314,I-317,I-323,I-326,I-3190,I-3192&page=1&includeArchived=false&createdByMyApp=false&unitdp=2&summaryOnly=false

When running the below HTTP GET command (which is what the Xero JDK URL resolves to) we do not get all 100 invoices by number. Instead we get Response code: 404 (Not Found); Time: 826ms (826 ms); Content length: 47 bytes (47 B)

GET https://api.xero.com/api.xro/2.0/Invoices?InvoiceNumbers=I-3051&InvoiceNumbers=INV-0525&InvoiceNumbers=I-3052&InvoiceNumbers=I-3053&InvoiceNumbers=I-3055&InvoiceNumbers=I-3063&InvoiceNumbers=I-3060&InvoiceNumbers=INV-0530&InvoiceNumbers=I-3065&InvoiceNumbers=INV-0533&InvoiceNumbers=I-3066&InvoiceNumbers=INV-0538&InvoiceNumbers=INV-0539&InvoiceNumbers=INV-0540&InvoiceNumbers=I-3072&InvoiceNumbers=INV-0546&InvoiceNumbers=INV-0547&InvoiceNumbers=INV-0548&InvoiceNumbers=I-3077&InvoiceNumbers=I-3079&InvoiceNumbers=INV-0552&InvoiceNumbers=I-3089&InvoiceNumbers=I-3090&InvoiceNumbers=I-3092&InvoiceNumbers=INV-0554A1&InvoiceNumbers=I-249&InvoiceNumbers=I-251&InvoiceNumbers=I-252&InvoiceNumbers=I-254&InvoiceNumbers=I-256&InvoiceNumbers=I-257&InvoiceNumbers=I-258&InvoiceNumbers=I-259&InvoiceNumbers=I-261&InvoiceNumbers=I-263&InvoiceNumbers=I-264&InvoiceNumbers=I-265&InvoiceNumbers=I-3105&InvoiceNumbers=INV-0562&InvoiceNumbers=I-3106&InvoiceNumbers=INV-0565&InvoiceNumbers=I-3108&InvoiceNumbers=I-3109&InvoiceNumbers=I-3112&InvoiceNumbers=I-1973&InvoiceNumbers=I-3114&InvoiceNumbers=I-3115&InvoiceNumbers=I-3116&InvoiceNumbers=I-3118&InvoiceNumbers=I-3120&InvoiceNumbers=I-3122&InvoiceNumbers=I-3124&InvoiceNumbers=INV-0572&InvoiceNumbers=I-3125&InvoiceNumbers=I-3126&InvoiceNumbers=INV-0576&InvoiceNumbers=INV-0577&InvoiceNumbers=INV-0578&InvoiceNumbers=INV-0579&InvoiceNumbers=INV-0580&InvoiceNumbers=INV-0581&InvoiceNumbers=I-3131&InvoiceNumbers=I-3132&InvoiceNumbers=I-3134&InvoiceNumbers=I-3137&InvoiceNumbers=I-3139&InvoiceNumbers=I-3142&InvoiceNumbers=I-3143&InvoiceNumbers=I-3145&InvoiceNumbers=I-3146&InvoiceNumbers=INV-0596&InvoiceNumbers=INV-0598&InvoiceNumbers=INV-0599&InvoiceNumbers=I-3152&InvoiceNumbers=I-3154&InvoiceNumbers=I-3160&InvoiceNumbers=I-3162&InvoiceNumbers=INV-0603&InvoiceNumbers=INV-0607&InvoiceNumbers=INV-0608&InvoiceNumbers=I-3166&InvoiceNumbers=I-3167&InvoiceNumbers=I-3168&InvoiceNumbers=I-3170&InvoiceNumbers=I-3171&InvoiceNumbers=I-3172&InvoiceNumbers=I-3175&InvoiceNumbers=I-3177&InvoiceNumbers=I-3181&InvoiceNumbers=I-3185&InvoiceNumbers=I-3186&InvoiceNumbers=I-3197&InvoiceNumbers=I-310&InvoiceNumbers=I-311&InvoiceNumbers=I-314&InvoiceNumbers=I-317&InvoiceNumbers=I-323&InvoiceNumbers=I-326&InvoiceNumbers=I-3190&InvoiceNumbers=I-3192&page=1&includeArchived=false&createdByMyApp=false&unitdp=2&summaryOnly=false

We still think this is an issue that should be resolved. Thanks @RettBehrens for the recommendation to fetch 40 invoices at a time, this still reaches the URL length limit for us (as our invoices numbers can be up to 9 chars long) because the Xero JDK makes the URL unnecessarily too long by duplicating InvoiceNumbers= in the URL.

@lancedfr
Copy link
Author

lancedfr commented Feb 1, 2023

When reviewing the java doc for the UriBuilder (the class the Xero JDK uses to build the URL) we can see the queryParam(String name, Object... values) method states "If multiple values are supplied the parameter will be added once per value.". This causes the duplicating of InvoiceNumbers= in the URL.

I managed to update the com.xero.api.client.AccountingApi class to use String.join(",", invoiceNumbers) to join the list of strings to a comma separated string (Which is what the API expects for InvoiceNumbers).

if (invoiceNumbers != null) {
  String key = "InvoiceNumbers";
  Object value = invoiceNumbers;
  if (value instanceof Collection) {
    uriBuilder = uriBuilder.queryParam(key, String.join(",", invoiceNumbers));
  } else if (value instanceof Object[]) {
    uriBuilder = uriBuilder.queryParam(key, (Object[]) value);
  } else {
    uriBuilder = uriBuilder.queryParam(key, value);
  }
}

This works because one comma separated string is being passed into queryParam method rather than multiple values. When testing this the resolved URL is now shorter and all 100 invoices are returned.

https://api.xero.com/api.xro/2.0/Invoices?InvoiceNumbers=I-3051,INV-0525,I-3052,I-3053,I-3055,I-3063,I-3060,INV-0530,I-3065,INV-0533,I-3066,INV-0538,INV-0539,INV-0540,I-3072,INV-0546,INV-0547,INV-0548,I-3077,I-3079,INV-0552,I-3089,I-3090,I-3092,INV-0554A1,I-249,I-251,I-252,I-254,I-256,I-257,I-258,I-259,I-261,I-263,I-264,I-265,I-3105,INV-0562,I-3106,INV-0565,I-3108,I-3109,I-3112,I-1973,I-3114,I-3115,I-3116,I-3118,I-3120,I-3122,I-3124,INV-0572,I-3125,I-3126,INV-0576,INV-0577,INV-0578,INV-0579,INV-0580,INV-0581,I-3131,I-3132,I-3134,I-3137,I-3139,I-3142,I-3143,I-3145,I-3146,INV-0596,INV-0598,INV-0599,I-3152,I-3154,I-3160,I-3162,INV-0603,INV-0607,INV-0608,I-3166,I-3167,I-3168,I-3170,I-3171,I-3172,I-3175,I-3177,I-3181,I-3185,I-3186,I-3197,I-310,I-311,I-314,I-317,I-323,I-326,I-3190,I-3192&page=1&includeArchived=false&createdByMyApp=false&unitdp=2&summaryOnly=false

I believe the com.xero.api.client.AccountingApi class is generated from a template and my fix won't work as is. I'm happy to implement this fix; I'm just not sure where your templates reside for me to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: API not Specification This issue can not be solved via the specifications
Projects
None yet
Development

No branches or pull requests

3 participants