Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add JEE (jakarta.inject) support to Guice #1630

Closed
wants to merge 5 commits into from

Conversation

hgschmie
Copy link

@hgschmie hgschmie commented May 17, 2022

This PR adds support for the JEE (jakarta inject) annotations to Google Guice, in addition to the existing JSR 330 (javax.inject) and Guice native annotations.

This also adds a new extension module guice-jakarta-servlet which supports Servlet API 5.

Support is pretty straightforward for annotations; supporting jakarta.inject.Provider is a bit trickier. The PR changes the public API of Guice minimally:

Visible changes

  • additional dependency: jakarta.inject:jakarta.inject-api This jar is all of 10 kB (!), so this seems acceptable
  • c.g.i.Provider now implements jakarta.inject.Provider in addition to javax.inject.Provider.
  • the LinkedBindingBuilder and its implementations gets a few new methods toJeeProvider, similar to toProvider as the type erasure for Class<? extends javax.inject.Provider> and Class<? extends jakarta.inject.Provider> are the same. LinkedBindingBuilder provides default methods so existing code that implements the interface and its subclasses still work (but they do not support the jakarta annotations right away)

SPI Changes

  • Two new provider binding classes to support jakarta providers: JeeProviderInstanceBinding and JeeProviderKeyBinding, analog to ProviderInstanceBinding and ProviderKeyBinding but using the jakarta provider interface.
  • ProviderWithExtensionVisitor gets an additional acceptExtensionVisitor method to support JeeProviderInstanceBindings.
  • BindingTargetVisitor gets two new visit methods to support the new provider bindings

The code is lightly tested (it works in a medium-size project that now uses a mix of all three injection frameworks (javax.inject, jakarta.inject and native)) and I added some tests.

Known problems

  • needs more tests, especially cross testing (javax/guice annotations as source, jakarta as targets and vice versa). Anecdotally it works, but testing would be good.
  • extensions need to be tested more. I use servlet and a tiny bit of assistedinject.

This is still a draft PR, feedback welcome!

Support for

- jakarta.inject.Inject
- jakarta.inject.Named
- jakarta.inject.Qualifier
- jakarta.inject.Scope
- jakarta.inject.Singleton

These injections are functional equivalent to the javax.inject
counterparts.
@hgschmie hgschmie force-pushed the jakarta-guice branch 2 times, most recently from 56de719 to 540e6c7 Compare May 17, 2022 18:55
Adds support for jakarta providers. These can be used interchangeably
with their native and javax.inject counterparts.
This is a straight port from the guice-servlet code to support the new
jakarta servlet API classes (jakarta.*).
@hgschmie hgschmie marked this pull request as ready for review May 28, 2022 18:02
@hgschmie
Copy link
Author

Could at least someone approve the workflow execution so that we can see whether this would apply to the current code base or not?

@jetztgradnet
Copy link

I would also be interested in seeing this done as we want to move a big application frm JavaEE to JakartaEE to go along with new developments.
Guice is the one central blocker for this migration for us.

@lev-tonkean
Copy link

please add this support!

@GedMarc
Copy link

GedMarc commented Aug 16, 2022

Mmmm,
You're running Guice (DI - JSR-299) on JEE (CDI - JSR-330)? One or the other never both, so pick one and remove the other :)

I've migrated everything to Jakarta namespace already (guicedee.com), and did the persistence and servlet addons as well, you can just use the jars as a replacement,

I get this allows for both javax and jakarta, but personally I don't think that's ever going to fly hey, also the extensions assistedinject, persistence, servlet, etc aren't updated so this PR would break that entire ecosystem :)

I think this is only required if you are using Guice in a very funny manner

@hgschmie
Copy link
Author

Hey Marc. Thank you for your effort with guicedee. My requirements are different, I have large applications that will migrate slowly. A "just everything from A to B" migration is impossible and not feasible. The challenge with sticking with JSR 330 is that most of the JEE ecosystem is moving to Jakarta DI 2.0 (I think JSR 330 is the "old" javax.inject" and Jakarta EE DI 2.0 is the "new" jakarta.inject spec). Making it possible to mix those is crucial for large scale migrations.

I am using this change in a number or projects; I understand that you want to lobby for "your work", however this is a different use case.

As for the other modules; modifying those is a lesser concern at least to me; it should not be hard to do so. This PR is intended as a starting point for a discussion.

@stickfigure
Copy link

This looks really good, and feels like the right direction for Guice. Is there anyone left at Google still working on this?

@hgschmie
Copy link
Author

It seems that within Google, Guice is one of these "works for us and is stable" projects.

@GedMarc
Copy link

GedMarc commented Sep 26, 2022

@hgschmie i suppose, it could also be viewed as ee got thrown in the trash and superseded with native modules (and then ee got picked up out of the trash for $$$), so why bother, or the entire ecosystem of google uses this so any change must be extremely careful and how many of their applications must switch to jakarta in order to make it viable,

either way, it's not your only option, there are guice shaded libraries out there that have already done the move, and enabled modularization and native modules with it as well, you know, that continuous improvement cycle....

I don't think it's fair to say the project isn't moving when clearly it still is, just because it's not entertaining (yet) the idea of managing an ecosystem that was cancelled for a much better solution with Java 9,
You can only be so backwards-compatible before it reaches the level of, well that was a mistake :)

@ben-manes
Copy link

/cc @sameb

@sameb
Copy link
Member

sameb commented Sep 29, 2022

@hgschmie - can you either revise this PR or send a new one that drops support for jakarta.inject.Provider, and drops the new servlet extension? That is, it only recognizes the new jakarta.inject.{Inject,Scope,Qualifier, Named,Singleton} annotations?

Supporting the Provider interface is much trickier and requires a lot more thought as to how to do that correctly. (And supporting a forked servlet extension is its own discussion.)

Thanks.

@Azimuts
Copy link

Azimuts commented Oct 14, 2022

Guice's greatest virtue is its agility and that it is a DI library by itself with no frills added. Other DI and CDI solutions are fullstack If you want to use more modern application servers ( or other solutiones....) , on many occasions are forced to do a full stack migration. In itself a migration to , for example, toSpring, Micronaut, Quarkus is a full stack migration generating complicated dependencies that can condition the evolution of the applications.....

Indirectly, not supporting this small functionality is forcing developers to do full stack migrations and it's not easy to get out of there! Reconsider!

@vsmid
Copy link

vsmid commented Dec 6, 2022

Please support Jakarta namespace. This library can be considered as a dead technology if not supporting Jakarta properly.

@foxpluto
Copy link

foxpluto commented Dec 9, 2022

I need to use Jakarta too. I have a project based on RestEasy and the Guice support was dropped after version 4.7.4.Final (pretty old) because Guice is missing Jakarta support!

@sameb sameb mentioned this pull request Apr 25, 2023
copybara-service bot pushed a commit that referenced this pull request Apr 26, 2023
…rt for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this pull request Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this pull request Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this pull request Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this pull request Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this pull request Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 527349023
@sameb
Copy link
Member

sameb commented Apr 26, 2023

Hi Folks. As of 5eff2ea, Guice Core mostly supports jakarta.inject. What's missing is the toProvider(...) methods. Please see #1383 (comment) for the release plan. The TL;DR is:

  • Guice 6.0 will release with support for both javax.inject and jakarta.inject (as this CL enables it). If you require this mixed javax & jakarta support, and want to use toProvider(..) with jakarta providers, you have two options: 1) Use the new guicify method for provider instance bindings, or 2) Update the Provider to implement com.google.inject.Provider to continue using linked provider bindings (toProvider(Key|TypeLiteral|Class)). (The 6.0 release will not change anything about the servlet nor persist extension -- they'll still only support javax.)
  • Shortly afterwards, we'll release Guice 7.0 which will drop support for javax.inject and only support jakarta.inject. At that point, toProvider(...) will work fine with jakarta.inject (and the persist & servlet extensions will also support jakarta)

As such, I'm going to close this PR. Thank you for your help in making this happen!

If anyone has further comments, please continue the discussion on #1383 (comment).

@sameb sameb closed this Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants