Adds support for Guice's Scopes.NO_SCOPE
Pull Request Checklist
- [x] Have you read How to write the perfect pull request?
- [x] Have you read through the contributor guidelines?
- [x] Have you referenced any issues you're fixing using commit message keywords?
- [x] Have you added copyright headers to new files?
- [x] Have you checked that both Scala and Java APIs are updated?
- [x] Have you updated the documentation for both Scala and Java sections?
- [x] Have you added tests for any changed functionality?
Helpful things
Fixes
Resolves https://github.com/playframework/playframework/issues/10893
Purpose
This PR adds support for Guice's Scopes.NO_SCOPE.
We do this by creating a new @NoScopes annotation, and in GuiceInjectorBuilder, we detect this annotation and forward it along as such:
def guice(bindings: Seq[PlayBinding[?]], binderOptions: Set[BinderOption]): GuiceModule = {
[...]
(binding.scope, binding.eager) match {
case (Some(scope), false) =>
if (scope.getName.equals(classOf[NoScope].getName)) builder.in(Scopes.NO_SCOPE)
else builder.in(scope)
[...]
}
Background Context
Approach suggested by @mkurz in https://github.com/playframework/playframework/issues/10893.
Initial attempt was made in https://github.com/playframework/playframework/pull/11451, however, PR was abandoned.
References
https://github.com/playframework/playframework/issues/10893 https://github.com/orgs/playframework/discussions/12891
Have you updated the documentation for both Scala and Java sections?
Since this is a new annotation, it's unclear to me if I need to add or edit any documentation. If I do, please point me in the right direction and I'd be happy to update it.
@Mergifyio rebase
rebase
❌ Base branch update has failed
Git reported the following error:
Rebasing (1/6)
Rebasing (2/6)
Rebasing (3/6)
Rebasing (4/6)
Rebasing (5/6)
Auto-merging core/play-guice/src/main/scala/play/api/inject/guice/GuiceInjectorBuilder.scala
CONFLICT (content): Merge conflict in core/play-guice/src/main/scala/play/api/inject/guice/GuiceInjectorBuilder.scala
error: could not apply 622423e3b... Updates GuiceInjectorBuilder to support NoScope
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 622423e3b... Updates GuiceInjectorBuilder to support NoScope
Thanks!
However, the tests that you added were not testing what I described in #10893 and what we actually wanted to solve:
You implemented Guice's AbstractModule, however the whole point of this feature is to make Play's play.[api.]inject.Module be able to bind to some kind of "no scope" scope (see Scala / Java doc). If using Guice directly, you should be able to do this already with bind(...)...in(Scopes.NO_SCOPE) anyway (however this is broken in Guice, see below...)
I pushed tests which I think a correct.
However... it turns out that Guice has a bug (at least I currently think it is a bug) and the whole mechanism I described does not even work in Guice itself.
That means we need to put this PR on hold until we know what's going on - I opened:
- https://github.com/google/guice/issues/1869
- Reproducer: https://github.com/mkurz/guice-noscope/