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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

CLOVIS created this revision.Sep 26 2018, 1:49 PM
CLOVIS planned changes to this revision.Sep 26 2018, 1:50 PM

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

CLOVIS marked 2 inline comments as done.Sep 26 2018, 1:52 PM
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 marked an inline comment as done.Sep 27 2018, 3:25 PM
CLOVIS added inline comments.
src/main/java/net/wildfyre/http/Request.kt
113 ↗(On Diff #678)
CLOVIS edited the summary of this revision. (Show Details)Sep 27 2018, 3:27 PM
CLOVIS edited the summary of this revision. (Show Details)
CLOVIS updated this revision to Diff 681.Oct 5 2018, 8:11 PM
  • 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.
CLOVIS edited the summary of this revision. (Show Details)Oct 5 2018, 8:15 PM

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

CLOVIS updated this revision to Diff 693.Oct 12 2018, 2:27 PM
  • Cleaning the code
CLOVIS marked 3 inline comments as done.Oct 12 2018, 3:34 PM
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 ?

Info-Screen added inline comments.Oct 20 2018, 3:07 PM
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 marked 2 inline comments as done.Oct 20 2018, 3:27 PM
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?

Info-Screen added inline comments.Oct 22 2018, 9:47 AM
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 marked 4 inline comments as done.Oct 23 2018, 4:02 PM
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

Info-Screen added inline comments.Oct 23 2018, 8:26 PM
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

CLOVIS marked an inline comment as done.Oct 24 2018, 9:59 AM
CLOVIS updated this revision to Diff 702.Oct 25 2018, 2:01 PM
  • Added missing boundary & fixed missing filenames
CLOVIS marked 2 inline comments as done.Oct 25 2018, 2:03 PM
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?

CLOVIS marked an inline comment as done.Oct 25 2018, 2:05 PM
WyldBot accepted this revision.Nov 9 2018, 6:14 PM
CLOVIS updated this revision to Diff 713.Dec 2 2018, 12:04 PM
  • Better documentation in Request & better platform-types handling
  • Removed unneeded try-catch
CLOVIS planned changes to this revision.Dec 5 2018, 7:02 PM

TODO: fix missing PATCH requests.

CLOVIS updated this revision to Diff 714.Dec 5 2018, 8:32 PM
  • 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

Fix lint errors

CLOVIS planned changes to this revision.Dec 15 2018, 1:32 PM

Need to fix multipart

CLOVIS updated this revision to Diff 715.Dec 23 2018, 3:04 PM
  • 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 :)

CLOVIS updated this revision to Diff 716.Dec 23 2018, 7:12 PM
  • 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
WyldBot accepted this revision.Dec 27 2018, 4:57 AM
This revision is now accepted and ready to land.Jan 2 2019, 7:37 PM
CLOVIS updated this revision to Diff 717.Jan 4 2019, 12:46 PM
  • Merge branch 'master' into T266-lazy-requests
This revision now requires review to proceed.Jan 4 2019, 12:46 PM
Harbormaster completed remote builds in B531: Diff 717.

Had to update, because of conflicts with master.

Info-Screen accepted this revision.Jan 4 2019, 1:23 PM
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.