Page MenuHomePhabricator

Kotlin, lazy execution & multipart requests
ClosedPublic

Authored by CLOVIS on Sep 26 2018, 1:49 PM.

Details

Reviewers
WyldBot
Info-Screen
Maniphest Tasks
T267: Support more datatypes
T259: Multipart requests
T262: Better exception handling
T266: Execute request at the last moment
Commits
rLIBWFJVA5850e65a37d5: Kotlin, lazy execution & multipart requests
rLIBWFJVAabd9f14aeb30: Merge branch 'master' into T266-lazy-requests
rLIBWFJVA12123a818110: Fixed bug where the JSON data wouldn't be read by the server
rLIBWFJVA7207848919c8: Added ProgrammingException
rLIBWFJVA95f41e2ba415: Added support for "text/plain" in DataType
rLIBWFJVA98eca39ff137: Using Kotlin's 'use' for cleaner code
rLIBWFJVAfc723d6a2785: Adding FindBugs annotations as dependencies and fixing FindBugs warnings
rLIBWFJVAd9332e0fbedc: No linting for IDEA files
rLIBWFJVA5a70c401234b: IDEA setup for encodings, kotlin compiler and name of the project
rLIBWFJVA8fb84c1560ed: Fixed bug where lib sent PATCH as POST+Override, where API expected PUT+Override
rLIBWFJVA78fa4cdd3614: Added missing "bio" field in testMultipart, sending each JSON field as a…
rLIBWFJVAcd0a38de1da2: Removed writeUTF that was not appropriate, replaced by a custom write method
rLIBWFJVA567e99cebd41: Replacing CHARSET by an object instead of a String
rLIBWFJVA060a44e516f1: Throwing custom NPE in Request.multipart to be clearer in case something happens
rLIBWFJVA4a79b06d073c: Image to test the avatar & PATCH/multipart testing
rLIBWFJVA9bb963310b84: Clearing the users when reseting the Cache
rLIBWFJVA40061367ac5b: Logging, JSON field name, refactoring from Request.Companion
rLIBWFJVA97b89444f5a5: Method.java --> Kotlin
rLIBWFJVA4038d34cadf4: Removed unneeded try-catch
rLIBWFJVAe7c8a15dc317: Better documentation in Request & better platform-types handling
rLIBWFJVA22d9f64b22a0: Removed test that couldn't work because of the IDs of posts that change when…
rLIBWFJVAb5a40cd77b6d: Added missing boundary & fixed missing filenames
rLIBWFJVA42ac1ec2c35c: Better code with 1 null-check instead of 2
rLIBWFJVAfc8ea405846a: Less redundant code
rLIBWFJVAcbf95b58ffcb: Fixed missing CHARSET
rLIBWFJVAf7627ad12a44: Added method addFile to upload files
rLIBWFJVA45544fb49e35: Merge branch 'master' into T266-lazy-requests
rLIBWFJVA9653458a5a5f: Fixing bug in tests
rLIBWFJVA5df1b29a89d4: Better code with functions run and let
rLIBWFJVA1b488357f686: Removed empty line
rLIBWFJVA89b1203c5dec: Merged master & fixed database identification bugs
rLIBWFJVA1e7ad44918c1: Added support for Kotlin
rLIBWFJVA781bee0fef11: Added support for Kotlin
Summary

Conversion to Kotlin, multipart support and lazy-execution of requests

Fixes T266
Fixes T259
Fixes T267

Test Plan

Run unit tests

Diff Detail

Repository
rLIBWFJVA WildFyre Java Libary
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Tests are broken until D184 is landed. This is a preview of what's going on.

CLOVIS added inline comments.
src/main/java/net/wildfyre/http/Request.kt
104 ↗(On Diff #678)

@Info-Screen What should I put as the 'name' for the eventual JSON parameters in the multipart request?

113 ↗(On Diff #678)

Same question, what should I put as the 'name' and 'filename' for the file that gets included ?

CLOVIS added inline comments.
src/main/java/net/wildfyre/http/Request.kt
113 ↗(On Diff #678)
  • Removed empty line
  • Merged master & fixed database identification bugs
CLOVIS retitled this revision from Added support for Kotlin to Kotlin, lazy execution & multipart requests.Oct 5 2018, 8:12 PM
CLOVIS marked 3 inline comments as not done.

This revision contains setup modification, and therefore blocks tasks that use Kotlin.

CLOVIS added inline comments.
src/main/java/net/wildfyre/http/Request.kt
117 ↗(On Diff #693)

What should I put as the name of the file @Info-Screen ?

src/main/java/net/wildfyre/http/Request.kt
104 ↗(On Diff #678)

normally you send the name of the field you have the value for here

113 ↗(On Diff #678)

name as above. filename doesn't matter, but normally this is intended to contain the filename (so the name of the file on the local system).

CLOVIS added inline comments.
src/main/java/net/wildfyre/http/Request.kt
104 ↗(On Diff #678)

This contains the JSON parameters, if any. I'm tempted to just put json in there, but is that fine for the server?

113 ↗(On Diff #678)

As an example, to add an image (see doc here), what should I put?

src/main/java/net/wildfyre/http/Request.kt
104 ↗(On Diff #678)

Normally you would put every json parameter into it's own block and have the name of the block be the name of the json part. To my knowledge there is no combined way to send form data + some json. You can always use the api with your webbrowser directly, and see what the browser sends (in the network console) for the different endpoints. The default in the browser is to use form data

113 ↗(On Diff #678)

when the user selects an image you should get the filename of the image. If you don't just put upload.<file_extension>

CLOVIS added inline comments.
src/main/java/net/wildfyre/http/Request.kt
104 ↗(On Diff #678)

Exemple in the Telegram Bot API where you send both a picture and its caption, etc in one multipart request : https://core.telegram.org/bots/api#sendphoto

src/main/java/net/wildfyre/http/Request.kt
104 ↗(On Diff #678)

Is the same here. But you can't (as far as I know) mix json and form-data

  • Added missing boundary & fixed missing filenames
CLOVIS added inline comments.
src/main/java/net/wildfyre/http/Request.kt
104 ↗(On Diff #678)

Fixed, there were missing boundaries; I guess that's what you were talking about?

  • Better documentation in Request & better platform-types handling
  • Removed unneeded try-catch

TODO: fix missing PATCH requests.

  • Method.java --> Kotlin
  • Logging, JSON field name, refactoring from Request.Companion
  • Clearing the users when reseting the Cache
  • Image to test the avatar & PATCH/multipart testing

Need to fix multipart

  • Throwing custom NPE in Request.multipart to be clearer in case something happens
  • Replacing CHARSET by an object instead of a String
  • Removed writeUTF that was not appropriate, replaced by a custom write method
  • Fixed bug where lib sent PATCH as POST+Override, where API expected PUT+Override
  • Added missing "bio" field in testMultipart, sending each JSON field as a multipart part
  • IDEA setup for encodings, kotlin compiler and name of the project
  • No linting for IDEA files
  • Adding FindBugs annonations

@WyldBot @Info-Screen Everything is fixed, multipart works (tested by publishing a new avatar to the user, that's why there's a PNG in the repo). Tell me if there's any edge case I've forgotten, there might be some tests missing. Otherwise, what's tested works :)

  • Using Kotlin's 'use' for cleaner code
  • Added support for "text/plain" in DataType
  • Added ProgrammingException
  • Fixed bug where the JSON data wouldn't be read by the server
This revision is now accepted and ready to land.Jan 2 2019, 7:37 PM
  • Merge branch 'master' into T266-lazy-requests
This revision now requires review to proceed.Jan 4 2019, 12:46 PM

Had to update, because of conflicts with master.

This revision is now accepted and ready to land.Jan 4 2019, 1:23 PM
This revision was automatically updated to reflect the committed changes.