Adding in idempotency_key is a breaking change and not backward compatible
PaulB12 opened this issue · 14 comments
SDK you're using (please complete the following information):
- Version 2.23.2 (https://github.com/XeroAPI/xero-php-oauth2/releases/tag/2.23.2)
Describe the bug
With the new addition of idempotency_key for any PUT/POST/PATCH requests is a breaking change to existing applications.
As idempotency_key isn't last in the constructor it is replacing values that were previously there when named parameters have not been used.
To Reproduce
Take a previous version where the function for createInvoicesWithHttpInfo used to be:
createInvoicesWithHttpInfo($xero_tenant_id, $invoices, $summarize_errors = false, $unitdp = null)
It has now become:
createInvoicesWithHttpInfo($xero_tenant_id, $invoices, $idempotency_key = null, $summarize_errors = false, $unitdp = null)
Code that used to call the previous version may look something like this:
$this->api->createInvoicesWithHttpInfo( $tenantId, $invoices, $this->summarizeErrors, $this->unitdp )
Because idempotency_key is now the third slot, the idempotency_key will be set to either 1 or 0 (depending if your previous summarizeErrors is true or false) and requests will end up failing due to duplicate key.
Expected behavior
To have idempotency_key at the end of the constructor so existing applications that do not use name parameters aren't broken upon upgrading.
Additional context
Using named parameters in our own code would prevent this issue however we still have to support legacy applications, if changing the constructor so idempotency_key is last isn't possible I do think it's a good idea to notify that between versions there is a breaking change for users who are upgrading.
PETOSS-370
Thanks for raising an issue, a ticket has been created to track your request
We just had an outage because of this!
This also broke our application, which we had to rollback...
Same here, this broke our application. I don't understand why this change was made on a patch release, it should have been a minor release IMO and of course, should not have broken backwards compatibility.
Rolling back to 2.31.1 resolved the issue.
We ran into this breaking change as well, fixing to 2.23.1 What's the upcoming plan for this SDK? Will this change be reverted, or should we modify our code to handle these changes?
@mbardelmeijer for what it's worth, we switched to using named arguments for all Xero SDK functions so that this doesn't bite us again.
@pumpkinball bumping
Broke our integration. No notice of breaking changes or updates.
Broken our integration too - this should be a major version bump with upgrade advice.
Having to pin version 2.23.1 for now.
Xero support advising to post an issue here but no response, is quite concerning from Xero.
Think it'll just be the case of sorting your own applications out either using named parameters or just swapping the order you're putting the arguments in.
Hi all, sorry for the radio silence on this.
Whilst it's a fairly simple fix within this SDK, it has knock on effects within the other SDKs which we have to consider. For context the SDK is autogenerated from our OpenAPI Specification here: https://github.com/XeroAPI/Xero-OpenAPI so the solution is to adjust the order within that, however the change will also impact the other SDKs that are generated from the same file.
Please bear with us whilst we look in to it.
Hi all, Apologies for the delay. We have fixed the idempotency_key parameter issue in the SDK version 3.0.0.
The idempotency_key is now the last optional parameter in the method definitions
Latest release also includes other changes (check out the release notes for details).
Do revert back to us if you are still facing any issues.
Thank you for your patience!
Closing the issue. Reopen or create new issue incase facing any issues.