Play server interpreter impl #291

ghostbuster91 · 2019-11-05T15:52:50Z

Issues that need to be resolved:

Streaming support - https://github.com/softwaremill/tapir/pull/291/files#r342987083
Response as multipart - https://github.com/softwaremill/tapir/pull/291/files#r342987083
Incoming multipart form data - https://github.com/softwaremill/tapir/pull/291/files#r342989503
Multiple headers under the same key - This might be impossible without ingeration into play-server implementations. But I would like to have a confirmation. More details here and here

This closes #65

ghostbuster91 · 2019-11-06T09:18:20Z

server/play-server/src/main/scala/tapir/server/play/PlayDecodeInputContext.scala

+  override def headers: Seq[(String, String)] = request.headers.headers
+  override def queryParameter(name: String): Seq[String] = request.queryString.get(name).toSeq.flatten
+  override def queryParameters: Map[String, Seq[String]] = request.queryString
+  override def bodyStream: Any = ???


How to get a body stream here? I found that there is even an Accumulator.source but every accumulator evaluates to a Future. How to just get a Source[Byte, _]?

Pretty sure that Play framework does not support input body streaming. How should we handle this case, what do we want to return here?

Thanks for a clarification. As discussed with @adamw we will throw an exception here.

server/play-server/src/main/scala/tapir/server/play/OutputToPlayResponse.scala

ghostbuster91 · 2019-11-06T09:23:33Z

server/play-server/src/main/scala/tapir/server/play/PlayRequestToRawBody.scala

+      Multipart.handleFilePartAsTemporaryFile(serverOptions.temporaryFileCreator)
+    )
+    bodyParser.apply(request).run().flatMap {
+      case Left(value) => Future.failed(new RuntimeException(s"TODO handle it $value")) // what to do here?


Why this happens while handling multipart/form-data? Should we use some other BodyParser?

@zZKato any idea how to fix this? From my investigation it seems that play does some magic behind the scenes, before throwing it into multipartFormData body parser.

@ghostbuster91 I think the problem here is that you handle the whole request as a FilePart even though we also have to consider DataParts (afaik).

I'll investigate a bit further

…treaming

vigimite · 2019-11-18T16:48:52Z

I have implemented the Tapir output to Play response part for MultipartFormData, can i have write permissions on this branch? (It is a bit of a hacky implementation but it is the same as PlayServer does it with its BodyWriteable for MultipartFormData)

ghostbuster91 · 2019-11-18T17:23:36Z

I am not sure how to give you permission to only this branch, I don't think that there is such an option. AFAIK you can update your fork, so you will have this branch. And then open a new pull request. That way both of us will have a write permissions.

vigimite · 2019-11-19T10:23:07Z

I created a PR that merges the commits into this branch from the forked repo #314

Implemented mapping from output to multipart form data

# Conflicts: # project/Versions.scala # server/tests/src/main/scala/sttp/tapir/server/tests/ServerTests.scala

…layServer

vigimite · 2020-01-06T15:30:16Z

created PR to merge master into this branch to keep this up2date, there are still 3 tests failing that i will take a look at -> #372

Play server interpreter impl

ghostbuster91 · 2020-01-06T15:52:57Z

Awesome, merged :) Thanks!

adamw · 2020-02-01T11:08:52Z

@zZKato do you think the current state is mergeable? Or are you maybe planning to do some more work on this? Would be great to have at least a basic implementation in the sources :)

vigimite · 2020-02-12T09:43:48Z

@adamw hi Adam, I don't think its mergable yet, i still think it needs a little work, however i have been super busy at work and didn't really take time to take another look at this. After my current work project is done I will take another look. I'm sorry for the delays

…preter_impl # Conflicts: # build.sbt # project/Versions.scala # server/tests/src/main/scala/sttp/tapir/server/tests/ServerTests.scala

vigimite · 2020-03-30T10:58:11Z

@adamw @ghostbuster91 I paired on the last bit of this with a colleague of mine and I believe the implementation is now complete, there is a PR from the fork to this branch which has all the changes (we also pulled in master again) PR -> #502
EDIT: Travis build passes now

vigimite · 2020-03-30T11:03:51Z

The last part was a bit tricky, we ran into a couple of issues:

play server doesnt support multiple headers with the same name as it uses a Map to represent it. We implemented it with the HTTP standard way of using a comma separated list, due to this there is an extra flag in the tests for play specifically
in one of the test endpoints there was a cookie being used with the header name "Cookie" however according to the standard we should be using "Set-Cookie" for the response (because the input headers were returned as the response header by blazeserverbuilder so we updated the test)
Play doesn't have a special request logger but we are instantiating a logger manually within the default PlayServerOptions object, is this ok or should we not log anything?

This reverts commit b4ca594

fix multipart form data implementation and broken tests

adamw · 2020-04-02T10:02:14Z

Released in 0.12.28. Thanks! :)

michele.vigilante and others added 6 commits October 18, 2019 16:58

implemented base interpreter classes

bf2b223

13 to go :)

46ceb24

Fix dynamic content type

41cdb4a

Add multipart handling

c4687ce

Compose routes

054b676

Merge playComponents2 into serverOptions

929f7dd

ghostbuster91 requested a review from adamw November 5, 2019 15:52

ghostbuster91 commented Nov 6, 2019

View reviewed changes

server/play-server/src/main/scala/tapir/server/play/OutputToPlayResponse.scala Outdated Show resolved Hide resolved

ghostbuster91 commented Nov 6, 2019

View reviewed changes

adamw mentioned this pull request Nov 15, 2019

Investigate a play server interpreter #65

Closed

ghostbuster91 and others added 5 commits November 18, 2019 13:24

Throw unsupportedOperationException when trying to use request body s…

d574c2f

…treaming

Sending response body as a multipart is not support in play server

2f376d0

Ignore test which involves setting multiple values for the same header

945e890

Turn off streaming tests for play server interpreter

a7e3eb7

Implemented mapping from output to multipart form data

7c249cd

Michele Vigilante added 2 commits November 19, 2019 11:05

Implemented mapping from output to multipart form data

393e0f6

minor change regarding formatting

72d80ec

ghostbuster91 and others added 5 commits November 19, 2019 11:56

Merge pull request #314 from fg-devs/play_server_interpreter_impl

11af702

Implemented mapping from output to multipart form data

fixed merge conflicts

40781eb

Merge branch 'master' into play_server_interpreter_impl

3b76726

# Conflicts: # project/Versions.scala # server/tests/src/main/scala/sttp/tapir/server/tests/ServerTests.scala

merged master into this PR and some changes made because of that

13a8123

added playServer to project aggregate, excluded streaming tests for P…

049a043

…layServer

Merge pull request #372 from fgrutsch/play_server_interpreter_impl

63ef927

Play server interpreter impl

fgrutsch added 3 commits March 27, 2020 14:59

update missing play server improvements

9568736

Merge remote-tracking branch 'upstream/master' into play_server_inter…

84bd57a

…preter_impl # Conflicts: # build.sbt # project/Versions.scala # server/tests/src/main/scala/sttp/tapir/server/tests/ServerTests.scala

fix broken client test

319b9b5

fgrutsch mentioned this pull request Mar 30, 2020

fix multipart form data implementation and broken tests #502

Merged

fgrutsch and others added 9 commits March 30, 2020 13:07

fix compilation error

b4ca594

Revert "fix compilation error"

70e1ef6

This reverts commit b4ca594

fix compilation error without reformatting the code

f726271

fix broken client tests

6c65539

remove hardcoded logger name

063169e

Merge pull request #502 from fgrutsch/play_server_interpreter_impl

f2ab8fd

fix multipart form data implementation and broken tests

Remove duplicated test

f170e8e

[Docs] play-server

759d9d7

[Docs] Remove empty lines

48f70c5

ghostbuster91 merged commit 7ef22e1 into master Apr 1, 2020

ghostbuster91 deleted the play_server_interpreter_impl branch April 1, 2020 20:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Play server interpreter impl #291

Play server interpreter impl #291

ghostbuster91 commented Nov 5, 2019 •

edited

ghostbuster91 Nov 6, 2019 •

edited

vigimite Nov 17, 2019

ghostbuster91 Nov 18, 2019

ghostbuster91 Nov 6, 2019

ghostbuster91 Nov 19, 2019

vigimite Nov 19, 2019

vigimite Nov 19, 2019

vigimite commented Nov 18, 2019 •

edited

ghostbuster91 commented Nov 18, 2019

vigimite commented Nov 19, 2019

vigimite commented Jan 6, 2020

ghostbuster91 commented Jan 6, 2020

adamw commented Feb 1, 2020

vigimite commented Feb 12, 2020

vigimite commented Mar 30, 2020 •

edited

vigimite commented Mar 30, 2020

adamw commented Apr 2, 2020

Play server interpreter impl #291

Play server interpreter impl #291

Conversation

ghostbuster91 commented Nov 5, 2019 • edited

ghostbuster91 Nov 6, 2019 • edited

Choose a reason for hiding this comment

vigimite Nov 17, 2019

Choose a reason for hiding this comment

ghostbuster91 Nov 18, 2019

Choose a reason for hiding this comment

ghostbuster91 Nov 6, 2019

Choose a reason for hiding this comment

ghostbuster91 Nov 19, 2019

Choose a reason for hiding this comment

vigimite Nov 19, 2019

Choose a reason for hiding this comment

vigimite Nov 19, 2019

Choose a reason for hiding this comment

vigimite commented Nov 18, 2019 • edited

ghostbuster91 commented Nov 18, 2019

vigimite commented Nov 19, 2019

vigimite commented Jan 6, 2020

ghostbuster91 commented Jan 6, 2020

adamw commented Feb 1, 2020

vigimite commented Feb 12, 2020

vigimite commented Mar 30, 2020 • edited

vigimite commented Mar 30, 2020

adamw commented Apr 2, 2020

ghostbuster91 commented Nov 5, 2019 •

edited

ghostbuster91 Nov 6, 2019 •

edited

vigimite commented Nov 18, 2019 •

edited

vigimite commented Mar 30, 2020 •

edited