search

Jasonette / JASONETTE-Android

📡 Native App over HTTP, on Android
https://www.jasonette.com
Other
1.6k stars 270 forks source link

Fix network request action POST method with JSON data to work correctly #189

Closed dewalddp closed 7 years ago

dewalddp commented 7 years ago

Previously it defaulted to PUT method and data was encoded.

... I'm quite new to the whole forking&PR process, so please let me know if I should do something differently 👍

gliechtenstein commented 7 years ago

Thanks! I just took a closer look and refreshed my memory. Just jotting down here for future reference. Here's what's going on:

  1. The upload() function makes a request to a backend that generates a signed url for AWS. That's what it's doing at the end of the function when it calls _request() https://github.com/dewalddp/JASONETTE-Android/blob/75527fc0008cd55b9fb0958e0f6baa16bcdc80ea/app/src/main/java/com/jasonette/seed/Action/JasonNetworkAction.java#L266
  2. But before calling the _request() it constructs a callback function so that it will be executed AFTER the server returns response https://github.com/dewalddp/JASONETTE-Android/blob/75527fc0008cd55b9fb0958e0f6baa16bcdc80ea/app/src/main/java/com/jasonette/seed/Action/JasonNetworkAction.java#L245
  3. As can be seen from what's being put on the stack, it's basically going to call JasonNetworkAction.process after your server returns with a signed url.
  4. The process() function again constructs a callback that will be executed later https://github.com/dewalddp/JASONETTE-Android/blob/75527fc0008cd55b9fb0958e0f6baa16bcdc80ea/app/src/main/java/com/jasonette/seed/Action/JasonNetworkAction.java#L245 (This will be called after the upload finishes)
  5. Then process() actually makes the upload call as a regular network.request. You can see that the request method is "put" https://github.com/dewalddp/JASONETTE-Android/blob/75527fc0008cd55b9fb0958e0f6baa16bcdc80ea/app/src/main/java/com/jasonette/seed/Action/JasonNetworkAction.java#L297

Since it's already making a "put" request, there wasn't any additional fix I had to make. I think your bugfix is sufficient.

I also verified that it works for https://imagejason.herokuapp.com which takes a pic and uploads to S3.

Anyway, hope this clarifies a bit of what's going on here. It's a bit convoluted because currently we don't have an official way to trigger actions asynchronously. But I recently figured out a way to handle these things in a simpler manner https://github.com/Jasonette/Jasonette/issues/6#issuecomment-325038968 so hopefully we won't have any more of these complex lines of code anymore in the future.

Thanks for the PR!

dewalddp commented 7 years ago

Cool, thanks for the detailed explanation. Thanks again for building this app framework, I'm really glad I found this project and it is a joy to work with!