Adding methods to retrieve current HTTP method+cookies in Java API

[AlejandroRivera]

Pull Request Checklist

• Have you read through the contributor guidelines?
• Have you signed the Typesafe CLA?
• Have you squashed your commits?
• Have you added copyright headers to new files?
• Have you checked that both Scala and Java APIs are updated?
• Have you updated the documentation for both Scala and Java sections?
• Have you added tests for any changed functionality?

Fixes

Fixes #231 and #232

Purpose

Adds new methods to the Java API for the StandaloneWSRequest to retrieve the request method and cookies

[AlejandroRivera]

FYI, the CI build broke because:

[error]  * abstract method getCookies()java.util.List in interface play.libs.ws.StandaloneWSRequest is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("play.libs.ws.StandaloneWSRequest.getCookies")
[error]  * abstract method getMethod()java.lang.String in interface play.libs.ws.StandaloneWSRequest is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("play.libs.ws.StandaloneWSRequest.getMethod")

Since these are new methods in the API, i assume it's safe to ignore those errors?

[gmethvin]

Since these are new methods in the API, i assume it's safe to ignore those errors?

Adding methods to interfaces is a backwards-incompatible change because any code that depends on this library and uses the StandaloneWSRequest interface now has to implement the new methods.

One thing we could do is define a default method and throw an UnsupportedOperationException. That's not quite so nice but would allow us to break things in a technically binary compatible way. We would need to document the method for now as being optional to implement by third party impls, then we can make the final breaking change on master (since 2.x.x can have breaking changes).

wdyt @marcospereira?

Also is there a reason why this was submitted to 1.x.x rather than master? It might be easier to design the right API on master first and then figure out what we need to do for compatibility to backport that change.

[AlejandroRivera]

Adding methods to interfaces is a backwards-incompatible change because any code that depends on this library and uses the StandaloneWSRequest interface now has to implement the new methods.

You're right. I assumed the StandaloneWSRequest was more of an internal API whose supported implementations were controlled by this project and there was no reasonable risk of breaking other people's code. I'll add the default implementation throwing the UnsupportedOperationException.

Also is there a reason why this was submitted to 1.x.x rather than master? It might be easier to design the right API on master first and then figure out what we need to do for compatibility to backport that change.

I had originally made the change on top of master branch but i got confused when i saw that in master the version was 1.0.1-SNAPSHOT and i expected to see 1.1.4-SNAPSHOT. I then searched where the version 1.1.3 was created/tagged and i found the 1.X.X branch. I assumed that was the place to introduce the change to have this be released as part of the 1.1.4 version. (I did try to search for any documents/guidelines on the development model to follow, but i failed at that).

Anyway, based on this comment, i'll "migrate" the code changes to the master branch.

[AlejandroRivera]

I'm done performing changes (default implementation + PR base). Awaiting for further comments.

[marcospereira]

Closing and reopening to Travis build it again.

[AlejandroRivera]

@marcospereira anything else i can do to see this PR progress (either to get it merged or get it closed)?

[marcospereira]

Hi @AlejandroRivera,

First of all, sorry for the long time to give you a proper answer.

As @gmethvin said before, when submitting pull requests to the master branch, you can break binary compatibility without a problem. We usually try to have a balance between breaking the API and offer a smooth migration path for users, but you can break binary compatibility. When doing so, add the MiMa filters so that we can better track where the API is changing.

Given that, you don't need to provide the default implementation throwing UnsupportedOperationException but instead, provide a proper implementation. When backporting this change to 1.x.x branch, then we avoid breaking binary compatibility, and you can open a new PR to add the default implementation throwing UnsupportedOperationException.

Thank you and again, sorry for the long time to reply.

[marcospereira]

👍

[marcospereira]

I'm merging this and we can later change the defaults to not throw UnsupportedOperationException.