Java version of curl request filter #178
Conversation
| class AhcWSRequestFilterSpec(implicit executionEnv: ExecutionEnv) extends Specification with AfterAll with FutureMatchers { | ||
| val testServerPort = 49134 | ||
|
|
||
| sequential |
There was a problem hiding this comment.
No need to use sequential anymore. :-)
| @@ -76,6 +75,7 @@ trait CurlFormat { | |||
| // XXX Need to escape any quotes within the body of the string. | |||
| b.append(s" --data '${quote(bodyString)}'") | |||
| b.append(" \\\n") | |||
| case EmptyBody => // do nothing | |||
There was a problem hiding this comment.
Just do nothing instead of throwing an exception.
| | --request GET \\ | ||
| | --header "Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=" \\ | ||
| | --header 'My-Header: My-Header-Value' \\ | ||
| | --header 'Content-Type: text/plain' \\ |
build.sbt
Outdated
| lazy val mimaSettings = mimaDefaultSettings ++ Seq( | ||
| mimaBinaryIssueFilters ++= Seq( | ||
| // Added to have better parity between Java and Scala APIs | ||
| ProblemFilters.exclude[ReversedMissingMethodProblem]("play.libs.ws.StandaloneWSRequest.getBody"), |
There was a problem hiding this comment.
I guess the justification for the breaking change is that we don't support custom implementations of the WS API? i.e. you have to use our provided AhcWSRequest and can't write your own.
There was a problem hiding this comment.
@gmethvin this was a change to the interface (StandaloneWSRequest).
In this case, the problem is that Java API was missing some methods/features that are already present in the Scala API. Adding them breaks compatibility in both interface and implementation.
I don't see what would prevent custom implementations of the WS API.
|
This looks OK to me. Are we going to bump versions due to bin compatibility changes or are these changes OK? |
|
@richdougherty we can backport part of this, without breaking compatibility. But in order to have curl logging support for both Java and Scala, we need some new methods that don't exist without this PR. :-) |
|
Rebased against the master branch. This is ready to be reviewed/merged. |
|
Fixed conflicts. I think this is ready to be merged. |
|
@marcospereira: not sure if you find this important for this feature, but you might also want to consider adding the See #232 |
Additionally, by using AkkaServerProvider, we are able to remove the need to run sequentially.
This required some changes in StandaloneWSRequest interface so that we can access more information to create the request filter. The good thing is that we bring Scala and Java APIs closer.
Purpose
This adds a Java version of curl-logger request filter that is present in the Scala APIs.
To do that, a number of changes we made in the Java version of
StandaloneWSRequestso that we can access request information and create thecurlcommand.Also, tests were added to both Java and Scala version of the request filter.