search

caelum / vraptor4

A web MVC action-based framework, on top of CDI, for fast and maintainable Java development.
http://vraptor.org
Apache License 2.0
350 stars 333 forks source link

Removing ServletContextFactory since Weld already provides ServletCon… #1108

Closed dfbadawi closed 6 years ago

dfbadawi commented 6 years ago

We are migrating our projects from Wildfly 10 to Wildfly 12 and Wildfly 12 has a different approach for initialization components, such as @Startup from EJB, @WebListener from servlets and @Observes @Initialized(ApplicationScoped.class) from CDI.

With this new approach, if the application is deployed together with the application server startup (like restart the server with the application deployed), these "listeners" are triggered when some of the resources are not available (sometimes it is available, maybe Wildfly 12 registering the components asynchronously?).

So, in this case, the ServletContext may not be available while VRaptor is registering the controllers throwing this Exception:

WELD-000052: Cannot return null from a non-dependent producer method: Producer for Producer Method [ServletContext] with qualifiers [@Any @Default] declared as [[BackedAnnotatedMethod] @Produces @ApplicationScoped public br.com.caelum.vraptor.ioc.cdi.ServletContextFactory.getInstance()] declared on Managed Bean [class br.com.caelum.vraptor.ioc.cdi.ServletContextFactory] with qualifiers [@Any @Default] at br.com.caelum.vraptor.ioc.cdi.ServletContextFactory.getInstance(ServletContextFactory.java:41)

Which makes me wonder why VRaptor has a @Provides for ServletContext, Considering that Weld already provides it. Also, the spec defines (from ServletContext's javadoc):

There is one context per "web application" per Java Virtual Machine

There is no need to alternate the provider and define as ApplicationScoped. That leads me to believe it is safe to remove the ServletContextFactory.

Turini commented 6 years ago

@dfbadawi thanks for your pull-request, it sounds great to me. I'm just not sure if it will work in servlet-containers, just like tomcat and jetty. could you test it?

dfbadawi commented 6 years ago

@Turini sure! I'll test tomorrow

dfbadawi commented 6 years ago

@Turini It didn't work in tomcat. Later I will try to fix the PR.

Turini commented 6 years ago

@dfbadawi great! thank you for your test and the time you're spending improving the project. let me know if there's something I can help you with.

dfbadawi commented 6 years ago

Closing this PR because #1113