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

Do not set a guid param during POST. #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

btalbot
Copy link

@btalbot btalbot commented Jul 20, 2016

The API seems to behave the same if a different guid is provided every time (the previous client behavior) or if no guid is provided.

Creating a random guid for every POST request breaks the idempotency intent of the API by making every POST unique and makes creating a reliable application very difficult.

… if a different guid is provided every time (the previous client behavior) or if no guid is provided.
@coveralls
Copy link

coveralls commented Jul 20, 2016

Coverage Status

Coverage decreased (-0.06%) to 91.034% when pulling 5386280 on iJJi:bugs/1 into 0ec4bb1 on bitpay:master.

@kleetus
Copy link
Contributor

kleetus commented Aug 22, 2016

I think I see your point with regard to send a POST request with a newly generated uuid; it really does nothing with respect to using the:

guid(an optional parameter to enforce idempotence for POST requests)

feature supported on the /invoices endpoint, etc. We really should be allowing the callers of the post method in rest_connector to pass in the guid to make full use of this feature. So, in the case of returning a response from process_request to post and then to the caller, I would also include the guid that was either generated or passed in to the post method.

As such, there really is no reason to pluck the guid field out summarily. @btalbot stated:

Creating a random guid for every POST request breaks the idempotency intent of the API by making every POST unique and makes creating a reliable application very difficult.

Yet our documentation stipulates that the guid field would enforce idempotence for POST requests, meaning that subsequent POST requests using the same guid would achieve idempotence. Since the current behavior of the ruby-client is to create a new guid value for each POST request, we are implicitly disregarding this feature altogether.

@btalbot
Copy link
Author

btalbot commented Aug 22, 2016

The only way for a client to ensure that a bitpay resources is created exactly once for a given client-side resource (say to create a bitpay invoice for a client order) is for the client to generate the guid, persist it with the client order, and then submit the request with the guid to bitpay invoice creation.

This way, if the client disconnects or crashes before it is able to process the (successful) response from bitpay, to recover, it can simply find the order, and resubmit it again using the guid already saved with the order. This can be repeated until the client receives a response from bitpay invoice creation and then save the invoice data.

Having the sdk generate a guid and include it in the request provides (almost) no benefit over not including a guid at all. At best, an auto-generated guid is ineffective. The force-generated guid (without this PR) completely breaks idempotency feature. If a guid value is auto-generated if one is not provided will only be confusing to naive implementations that think its presence will provide them sufficient idempotency and will lead to over creation of bitpay objects like invoice, payroll payouts, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants