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

DecoratingListener raises a NullPointerException #5162

Closed
oliviercailloux opened this issue Aug 15, 2020 · 18 comments · Fixed by #5177
Closed

DecoratingListener raises a NullPointerException #5162

oliviercailloux opened this issue Aug 15, 2020 · 18 comments · Fixed by #5177
Assignees

Comments

@oliviercailloux
Copy link

oliviercailloux commented Aug 15, 2020

Jetty version
9.4.31.v20200723 (Maven)

Java version
OpenJDK 11

OS type/version
Debian stable

Description
The sample code from the Weld embedded section in the Jetty Reference raises a NullPointerException.

Here is the stack trace.

Exception in thread "main" java.lang.NullPointerException
	at java.base/java.util.Objects.requireNonNull(Objects.java:221)
	at org.eclipse.jetty.servlet.DecoratingListener.<init>(DecoratingListener.java:67)
	at org.eclipse.jetty.webapp.DecoratingListener.<init>(DecoratingListener.java:49)
	at org.eclipse.jetty.webapp.DecoratingListener.<init>(DecoratingListener.java:39)
	at org.eclipse.jetty.webapp.DecoratingListener.<init>(DecoratingListener.java:34)
	at io.github.oliviercailloux.jetty.App.main(App.java:64)

This could be a usage problem. It is unclear to me, reading the documentation, if I have to apply anything from the section “Weld Setup” (in the same page), when running in embedded mode (and if so, what), or if the Weld embedded section is to be applied to the exclusion of the other part. If I have to include the cdi-decorate module, how can I do this with Maven? In any case, the problem also happens with org.eclipse.jetty:jetty-cdi included in the project dependencies.

The Weld documentation gives almost the same instructions, but not exactly: one line of code, context.addEventListener(new Listener()); has been added. Which version is correct? And, similarly, I ignore whether I am supposed to apply section 18.3.2.4. Binding BeanManager to Jetty JNDI (or others) in supplement to 18.3.2.5. Embedded Jetty or if 18.3.2.5. Embedded Jetty is sufficient.

(I also want to use Jersey, which further complicates the matter, but I suppose that this issue is unrelated as the bug happens with a sample code that does not involve Jersey.)

Possibly related
#4064 #3804 #4121

@gregw gregw self-assigned this Aug 17, 2020
@gregw
Copy link
Contributor

gregw commented Aug 17, 2020

Sorry for the confusion. We are always a bit confused with CDI as well.... the problem being that there is no standard nor agreement between the CDI implementations as to how they should integrate with a servlet container, further complicated by the historic usage of jetty APIs for integration that were not meant to be public and have now changed.

We've tried to push the implementations to a single obvious way to integrate, but failed in that approach. Hence the hodge podge of different mechanisms that are very version dependent.

I'll check our examples and see if we can update our documentation and/or fix anything for the weld integration.

Jersey should not be anywhere as near as complex, as it does not need to plug into the containers decoration mechanism.

@gregw
Copy link
Contributor

gregw commented Aug 17, 2020

So there is definitely at least something wrong with both sets of embedded documentation as DecoratingListener will not work with a default constructor unless called from the scope of a context listener or similar. Thus in the embedded examples, at the very least the constructor that takes a context needs to be used. It may also be the case that CdiDecoratingListener should be used.

This may take me a little while to page all this back into my brain and work out exactly the best way to fix the documentation, but feel free to try either/both of the above suggestions. Standby ...

@oliviercailloux
Copy link
Author

Thanks for investigating!

Jersey should not be anywhere as near as complex, as it does not need to plug into the containers decoration mechanism.

I do not understand this sentence. I thought I’d better first make sure Jetty and Weld work together, before trying to tackle the (or what I thought to be the) additional difficulty of making Jersey also work with Jetty and Weld (meaning, make my Jersey-operated Jax-RS servlet handle dependency injection).

Does your sentence suggest that it is in fact easier to set up Jersey for working with Weld, and that I’d better not insist in making Weld behave well with Jetty (or conversely)?

Specifically, one of the things I am interested in is to have the container react to @Transactional annotations (using JPA and a JTA transactional manager).

@joakime
Copy link
Contributor

joakime commented Aug 18, 2020

"have the container react to @transactional"

Can you explain this in a bit more detail?
Which container?
Jetty (which isn't involved in DataSource behaviors)?
Jersey (which is also a container)?
Weld (which is also a container)?
Or something else?

@oliviercailloux
Copy link
Author

oliviercailloux commented Aug 19, 2020

I should have written: “one of the things I am interested in is to have a container react to @Transactional annotations (using JPA and a JTA transactional manager).” (I also would like to use the other CDI basic stuff such as @Inject, of course.)

AFAIU, this is a job for CDI together with some TransactionManager (which registers CDI interceptors to act upon @Transactional). See this post for example. Hence, my desire to make Jetty interact with Weld properly. Tell me if I am mistaken.

@gregw
Copy link
Contributor

gregw commented Aug 19, 2020

@oliviercailloux I've now found a few niggling problems with the various ways that jetty can integrate with Weld when used in an embedded style, specially following our examples. So I will be doing a PR to fix them up.... mostly the issue is that all the goodness we've written for CDI integration is within a ServletContainerInitializer that may not be triggered by embedded usage.

For now you should be able to write just:

        context.addEventListener(new org.jboss.weld.environment.servlet.Listener());

which should result in the message:

INFO: WELD-ENV-001201: Jetty 7.2+ detected, CDI injection will be available in Servlets and Filters. Injection into Listeners is not supported.

which shows that Welds built in integration is working for Filters and Servlets. If you want Listeners you need to use:

        context.addEventListener(new org.eclipse.jetty.webapp.DecoratingListener(context));
        context.addEventListener(new org.jboss.weld.environment.servlet.Listener());

which when run will produce the message:

WARN: WELD-ENV-001211: Deprecated Jetty DecoratingListener support detected, CDI injection will be available in Listeners, Servlets and Filters.

that at least confirms the integration is done.

Actually you can do a little hack to avoid the deprecated warning, by instead invoking the SCI directly

        new CdiServletContainerInitializer().onStartup(Collections.emptySet(), context.getServletContext());        
        context.addEventListener(new org.jboss.weld.environment.servlet.Listener());

which then reports:

INFO: WELD-ENV-001213: Jetty CDI SPI support detected, CDI injection will be available in Listeners, Servlets and Filters.

We will clean this up in the next release. @janbartel what is the status of calling SCIs from embedded server usage? Should that have been working anyway? Would it just be a mater of making sure jetty-cdi is on the classpath? That doesn't appear to work for me?

@gregw
Copy link
Contributor

gregw commented Aug 19, 2020

Looking for better ways to start CDI embedded.... ultimately it is best to just use the ServletContainerInitializers.

If you are not using a real webapp, ie using ServletContextHandler instead of a WebppContext, then you can achieve this by clearing a little class like:

    private static class SciStarter extends AbstractLifeCycle implements ServletContextHandler.ServletContainerInitializerCaller
    {
        private final ServletContext _context;
        private final List<ServletContainerInitializer> _initializers = new ArrayList<>();

        private SciStarter(ServletContext context)
        {
            _context = context;
        }

        public void add(ServletContainerInitializer initializer)
        {
            _initializers.add(initializer);
        }

        @Override
        protected void doStart() throws Exception
        {
            for (ServletContainerInitializer sci : _initializers)
                sci.onStartup(Collections.emptySet(), _context);
        }
    }

Your startup code then can look like:

        Server server = new Server(8080);
        ServletContextHandler context = new ServletContextHandler();
        context.setContextPath("/");
        server.setHandler(context);

        SciStarter sciStarter = new SciStarter(context.getServletContext());
        context.addBean(sciStarter);
        context.setInitParameter(org.eclipse.jetty.cdi.CdiServletContainerInitializer.CDI_INTEGRATION_ATTRIBUTE, org.eclipse.jetty.cdi.CdiDecoratingListener.MODE);
        sciStarter.add(new org.eclipse.jetty.cdi.CdiServletContainerInitializer());
        sciStarter.add(new org.jboss.weld.environment.servlet.EnhancedListener());

So that could be simplified a little (eg we should provide a utility SciStarter) , but it's not too ugly.

Now the strange thing is that because you are using a WebappContext (or at least the documentation suggest that you should), then the SCI's should have already been found for you? Hmm maybe we need to tell it to search for them??? stand by...

@joakime
Copy link
Contributor

joakime commented Aug 19, 2020

I was able to not use jetty-cdi at all, and rely solely on the weld-servlet-core artifact.

This works for ServletContextHandler just fine.
And isn't appropriate when using WebAppContext.

package org.eclipse.jetty.demo.embedded;

import org.eclipse.jetty.demo.logging.Logging;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.log.JavaUtilLog;
import org.eclipse.jetty.util.log.Log;
import org.jboss.weld.environment.servlet.EnhancedListener;

public class EmbeddedDemo
{
    public static void main(String[] args) throws Exception
    {
        Logging.config();
        Log.setLog(new JavaUtilLog());
        Server server = new Server(8099);

        final ServletContextHandler contextHandler = new ServletContextHandler();
        contextHandler.setContextPath("/");
        contextHandler.addServlet(HelloServlet.class, "/hi/*");
        contextHandler.addServlet(GreetingsServlet.class, "/greetings/*");
        contextHandler.addBean(new WeldInitializer(contextHandler));
        server.setHandler(contextHandler);
        try
        {
            server.start();
            HttpClient.GET(server.getURI().resolve("/hi/"));
            HttpClient.GET(server.getURI().resolve("/greetings/"));
        }
        finally
        {
            LifeCycle.stop(server);
        }
    }

    public static class WeldInitializer extends AbstractLifeCycle
    {
        private final ServletContextHandler contextHandler;

        public WeldInitializer(ServletContextHandler contextHandler)
        {
            this.contextHandler = contextHandler;
        }

        @Override
        protected void doStart() throws Exception
        {
            EnhancedListener enhancedListener = new EnhancedListener();
            enhancedListener.onStartup(null, contextHandler.getServletContext());
            super.doStart();
        }
    }
}

@joakime
Copy link
Contributor

joakime commented Aug 19, 2020

This works too.
For both ServletContextHandler and WebAppContext usages.

package org.eclipse.jetty.demo.embedded;

import org.eclipse.jetty.demo.logging.Logging;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.log.JavaUtilLog;
import org.eclipse.jetty.util.log.Log;

public class EmbeddedListenerDemo
{
    public static void main(String[] args) throws Exception
    {
        Logging.config();
        Log.setLog(new JavaUtilLog());
        Server server = new Server(8099);

        final ServletContextHandler contextHandler = new ServletContextHandler();
        contextHandler.setContextPath("/");
        contextHandler.addServlet(HelloServlet.class, "/hi/*");
        contextHandler.addServlet(GreetingsServlet.class, "/greetings/*");
        contextHandler.addEventListener(new org.jboss.weld.environment.servlet.Listener());
        server.setHandler(contextHandler);
        try
        {
            server.start();
            HttpClient.GET(server.getURI().resolve("/hi/"));
            HttpClient.GET(server.getURI().resolve("/greetings/"));
        }
        finally
        {
            LifeCycle.stop(server);
        }
    }
}

@gregw
Copy link
Contributor

gregw commented Aug 19, 2020

@joakime that method only partially works. It uses the Weld integration which will give you the log message:

WELD-ENV-001201: Jetty 7.2+ detected, CDI injection will be available in Servlets and Filters. Injection into Listeners is not supported

ie Listener injection is not supported.

This way will also not work with jetty-10.

gregw added a commit that referenced this issue Aug 20, 2020
Clean up CDI integration and documentation to better support embedded usage.
 + made listener public
 + added utility class for SCIs
@gregw gregw linked a pull request Aug 24, 2020 that will close this issue
gregw added a commit that referenced this issue Aug 24, 2020
Clean up CDI integration and documentation to better support embedded usage.
 + moved EmbeddedWeldTest to jetty-embedded
gregw added a commit that referenced this issue Aug 24, 2020
gregw added a commit that referenced this issue Aug 25, 2020
Moved tests to jetty-cdi to avoid consequences to other tests in embedded
gregw added a commit that referenced this issue Sep 7, 2020
* Issue #5162 CDI embedded integration improvements

Clean up CDI integration and documentation to better support embedded usage.
 + made listener public
 + added utility class for SCIs

* Issue #5162 CDI embedded integration improvements

Clean up CDI integration and documentation to better support embedded usage.
 + moved EmbeddedWeldTest to jetty-embedded

* fix javadoc

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #5162 CDI embedded integration improvements

ventilated text

* fix test pom

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Fixed javadoc

* Fixed javadoc

* Issue #5162 CDI embedded integration improvements

Moved tests to jetty-cdi to avoid consequences to other tests in embedded

* trailing new line

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* updates from review

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Feedback from review
@oliviercailloux
Copy link
Author

Thanks for your very prompt reaction. I see that the related PR (including changes to the documentation, if I understand correctly) has now been merged into 9.4.x. Any idea when this will be consumable through Maven central and when the documentation changes will be visible online (the “current” documentation is at version 9.4.31.v20200723)? Or can I see it somewhere already?

@joakime
Copy link
Contributor

joakime commented Sep 16, 2020

PR #5177 was merged.

@gregw can we close this issue?

@gregw gregw closed this as completed Sep 16, 2020
@oliviercailloux
Copy link
Author

oliviercailloux commented Sep 17, 2020

I saw that that PR has been merged to 9.4.x, but my question was about concrete availability as a consumable Maven artifact.

Is there some documentation about the release policy? I’d like to know if I can expect the patch to become available in the nearby future. (I am not requesting any specific, precise date; I know that this is not how things work.) The last 9.x release is now relatively long past, compared to the usual schedule (usually a release every month or more); does it mean that the next one is coming soon, or that attention is now shifting to the 10.x branch?

Relatedly, are there some snapshots available, in the meantime?

Sorry if I missed something obvious, but I couldn’t find anything about the release policy on the website or in the documentation.

@joakime
Copy link
Contributor

joakime commented Sep 17, 2020

nightly snapshots are available on oss.sonatype.org

URL - https://oss.sonatype.org/content/repositories/jetty-snapshots/

We are working towards a 9.4.32 release, which will contain this fix.

@oliviercailloux
Copy link
Author

Thanks. Please consider documenting this “nightly snapshot” URL, perhaps in the about section. (In retrospect, I admit I should have thought about searching there for snapshots, but better make it explicit as I suppose others could find it useful.)

@joakime
Copy link
Contributor

joakime commented Sep 17, 2020

We generally encourage people to build and test from the git repository.
Not to rely on nightly snapshots, as those can often contain experimental code, or even failed / partial snapshot deployments.

@oliviercailloux
Copy link
Author

I just built the doc from source. I am able to make injection work by following the new instructions. Great! (I use the jetty-snapshot, contrary to instructions here above, because I did not manage to compile jetty from sources yet, I hope that is okay.)

I have however a few questions because I am not sure I am setting things up as I should. Perhaps the doc could be improved so as to remove any doubt from the reader?

  1. The log is not exactly as documented, letting me wonder whether I am unwillingly using some deprecated access instead of the recommended one. I get “Jetty 7.2+ detected, CDI injection will be available in Servlets and Filters. Injection into Listeners should work on Jetty 9.1.1 and newer.” whereas according to the doc, I should get “INFO: WELD-ENV-001212: Jetty CdiDecoratingListener support detected, CDI injection will be available in Listeners, Servlets and Filters.”
  2. I use org.jboss.weld.servlet:weld-servlet-core:3.1.5.SP1 as Weld dependency, whereas the documentation mentions org.jboss.weld.servlet:weld-servlet (which seems unmaintained since 2014), is this the recommended dependency to use with jetty embedded? Perhaps because of me using an incorrect dependency, Weld warns about an illegal reflective access when starting up, thus, I suppose this would fail under more recent JDKs.
  3. I use the jetty-cdi Maven dependency, and not cdi-decorate as documented (which I can’t find in Maven central), is this all right?

The log is the following when my application starts.

19:28:25.524 [main] INFO  org.eclipse.jetty.util.log - Logging initialized @713ms to org.eclipse.jetty.util.log.Slf4jLog
19:28:25.724 [main] INFO  io.github.oliviercailloux.jetty.App - Set handler: HandlerList@11a9e7c8{STOPPED}.
19:28:25.736 [main] INFO  org.eclipse.jetty.server.Server - jetty-9.4.32-SNAPSHOT; built: 2020-09-18T11:03:24.291Z; git: e3ed05fc1c1dca35add2ff5da29f910c693e23e2; jvm 11.0.8+10-post-Debian-1deb10u1
19:28:25.890 [main] INFO  o.j.w.environment.servletWeldServlet - WELD-ENV-001008: Initialize Weld using ServletContainerInitializer
19:28:25.927 [main] INFO  org.jboss.weld.Version - WELD-000900: 3.1.5 (SP1)
19:28:26.208 [main] INFO  org.jboss.weld.Bootstrap - WELD-ENV-000020: Using jandex for bean discovery
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jboss.weld.util.bytecode.ClassFileUtils$1 (file:/home/olivier/.m2/repository/org/jboss/weld/weld-core-impl/3.1.5.SP1/weld-core-impl-3.1.5.SP1.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int)
WARNING: Please consider reporting this to the maintainers of org.jboss.weld.util.bytecode.ClassFileUtils$1
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
19:28:26.497 [main] INFO  org.jboss.weld.Bootstrap - WELD-000101: Transactional services not available. Injection of @Inject UserTransaction not available. Transactional observers will be invoked synchronously.
19:28:26.814 [main] INFO  o.j.weld.environment.servletJetty - WELD-ENV-001200: Jetty 7.2+ detected, CDI injection will be available in Servlets and Filters. Injection into Listeners should work on Jetty 9.1.1 and newer.
19:28:27.152 [main] INFO  o.e.j.c.CdiServletContainerInitializer - CdiDecoratingListener enabled in ServletContext@o.e.j.s.ServletContextHandler@1dd0e7c4{/api,null,STARTING}
19:28:27.162 [main] INFO  o.e.j.server.handler.ContextHandler - Started o.e.j.s.ServletContextHandler@1dd0e7c4{/api,null,AVAILABLE}
19:28:27.185 [main] INFO  o.e.jetty.server.AbstractConnector - Started ServerConnector@4461c7e3{HTTP/1.1, (http/1.1)}{0.0.0.0:8080}
19:28:27.185 [main] INFO  org.eclipse.jetty.server.Server - Started @2381ms

The full application is here, with the main content here.

@oliviercailloux
Copy link
Author

Perhaps interestingly, the illegal reflective access happens in this class, which seems to mention (if I understand it correctly) that this is because “the integrator does not fully implement ProxyServices”. Is this something that jetty can solve, or due to me not having followed the instructions correctly?

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 a pull request may close this issue.

3 participants