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

Using provider instead of HttpServletRequest in ServletJaxRsContextFactoryProvider #42

Merged
merged 6 commits into from
May 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ You need to add a dependency on:

- jax-rs-pac4j
1. for Jersey (<2.26) : the `jersey-pac4j` library (<em>groupId</em>: **org.pac4j**, *version*: **3.0.0**)
2. for Resteasy : the `resteasy-pac4j` library (<em>groupId</em>: **org.pac4j**, *version*: **3.0.0**)
2. for Resteasy : the `resteasy-pac4j` library (<em>groupId</em>: **org.pac4j**, *version*: **3.0.0**) and `resteasy-cdi` for CDI support
- the appropriate `pac4j` [submodules](http://www.pac4j.org/docs/clients.html) (<em>groupId</em>: **org.pac4j**, *version*: **2.1.0**): `pac4j-oauth` for OAuth support (Facebook, Twitter...), `pac4j-cas` for CAS support, `pac4j-ldap` for LDAP authentication, etc.

All released artifacts are available in the [Maven central repository](http://search.maven.org/#search%7Cga%7C1%7Cpac4j).
Expand Down Expand Up @@ -122,7 +122,7 @@ resourceConfig
For a Jersey-based and Servlet-based (e.g., Jetty or Grizzly Servlet) environment with session management, annotation support and method parameters injection:
```java
resourceConfig
.register(new ServletJaxRsContextFactoryProvider(config))
.register(ServletJaxRsContextFactoryProvider.class)
.register(new Pac4JSecurityFeature(config))
.register(new Pac4JValueFactoryProvider.Binder());
```
Expand All @@ -144,13 +144,20 @@ For a Resteasy-based and Servlet-based (e.g., Undertow) environment with session
public Set<Object> getSingletons() {
Config config = getConfig();
Set<Object> singletons = new HashSet<>();
singletons.add(new ServletJaxRsContextFactoryProvider(config));
singletons.add(new Pac4JSecurityFeature(config));
singletons.add(new Pac4JProfileInjectorFactory());
return singletons;
}

@Override
public Set<Class<?>> getClasses() {
Set<Class<?>> classes = new HashSet<>();
classes.add(ServletJaxRsContextFactoryProvider.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pac4JProfileInjectorFactory needs to be moved too, I tested and it doesn't work without it

classes.add(Pac4JProfileInjectorFactory.class);
return classes;
}
}
```
For RestEasy, you need to include `resteasy-cdi` support to your project to ensure that dependencies are injected at runtime. Refer to the tests in `resteasy-pac4j` module.

Note that a default value for the `clients` parameter of the `@Pac4JSecurity`
annotation can be passed to the constructor of `Pac4JSecurityFeature`.
Expand Down
5 changes: 5 additions & 0 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
<version>1.2</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<version>1</version>
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-core</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package org.pac4j.jax.rs.servlet.features;

import javax.inject.Inject;
import javax.inject.Provider;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.Context;

import org.pac4j.jax.rs.features.JaxRsContextFactoryProvider;
import org.pac4j.jax.rs.servlet.pac4j.ServletJaxRsContext;
Expand All @@ -17,11 +18,11 @@
*/
public class ServletJaxRsContextFactoryProvider extends JaxRsContextFactoryProvider {

@Context
private HttpServletRequest request;
@Inject
private Provider<HttpServletRequest> requestProvider;

@Override
public JaxRsContextFactory getContext(Class<?> type) {
return context -> new ServletJaxRsContext(getProviders(), context, getConfig().getSessionStore(), request);
return context -> new ServletJaxRsContext(getProviders(), context, getConfig().getSessionStore(), requestProvider.get());
}
}
13 changes: 13 additions & 0 deletions core/src/main/resources/META-INF/beans.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this with the test application? To me it has nothing to do here, it is only needed for resteasy with cdi, no? Or explain why it is needed here please

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the CDI specification to discover beans from the archive, we need to have beans.xml under META-INF folder in the archive. I have spent enough time in figuring out a workaround for this but no success yet.

Bean classes of enabled beans must be deployed in bean archives.
A library jar, EJB jar, application client jar or rar archive is a bean archive if it has a file named beans.xml in the META-INF directory.
https://docs.jboss.org/cdi/spec/1.0/html/packagingdeployment.html

In this case, the ServletJaxRsContextFactoryProvider has an @Inject on requestProvider and without the beans.xml in its jar, the requestProvider is not getting injected.

@victornoel I would be happy to fix if a solution is known. I don't have enough bandwidth to search for a solution right now. I completely agree with you that this is an unnecessary addition but definitely is harmless. Would really appreciate if we can go ahead for now and fix when a solution is available.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I agree, thank you for the explanation :)
Could you just add some comment in that file explaining why it is here exactly and maybe saying it is mainly because of resteasy/cdi?

<!-- NOTE: This file is required primarily for resteasy CDI. "Bean classes
of enabled beans must be deployed in bean archives. A library jar, EJB jar,
application client jar or rar archive is a bean archive if it has a file
named beans.xml in the META-INF directory". https://docs.jboss.org/cdi/spec/1.0/html/packagingdeployment.html -->
<beans
xmlns="http://xmlns.jcp.org/xml/ns/javaee"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/beans_1_1.xsd"
bean-discovery-mode="all"
>

</beans>
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ protected DeploymentContext configureDeployment(ResourceConfig config) {
protected ResourceConfig configureResourceConfig(ResourceConfig config) {
return super
.configureResourceConfig(config)
.register(new ServletJaxRsContextFactoryProvider());
.register(ServletJaxRsContextFactoryProvider.class);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ protected DeploymentContext configureDeployment(ResourceConfig config) {
protected ResourceConfig configureResourceConfig(ResourceConfig config) {
return super
.configureResourceConfig(config)
.register(new ServletJaxRsContextFactoryProvider());
.register(ServletJaxRsContextFactoryProvider.class);
}

}
24 changes: 23 additions & 1 deletion resteasy/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@
<groupId>org.pac4j</groupId>
<artifactId>resteasy-pac4j</artifactId>

<properties>
<resteasy.version>3.1.4.Final</resteasy.version>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.jboss.resteasy</groupId>
<artifactId>resteasy-bom</artifactId>
<version>3.1.4.Final</version>
<version>${resteasy.version}</version>
<scope>import</scope>
<type>pom</type>
</dependency>
Expand Down Expand Up @@ -45,11 +49,29 @@
<artifactId>resteasy-undertow</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jboss.resteasy</groupId>
<artifactId>resteasy-cdi</artifactId>
<scope>test</scope>
<version>${resteasy.version}</version>
</dependency>
<dependency>
<groupId>javax</groupId>
<artifactId>javaee-api</artifactId>
<version>8.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.undertow</groupId>
<artifactId>undertow-servlet</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jboss.weld.servlet</groupId>
<artifactId>weld-servlet</artifactId>
<version>2.4.7.Final</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.glassfish.jersey.core</groupId>
<artifactId>jersey-client</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
import org.awaitility.Awaitility;
import org.awaitility.Duration;
import org.glassfish.jersey.client.JerseyClientBuilder;
import org.jboss.resteasy.cdi.CdiInjectorFactory;
import org.jboss.resteasy.plugins.server.undertow.UndertowJaxrsServer;
import org.jboss.resteasy.spi.ResteasyDeployment;
import org.jboss.resteasy.test.TestPortProvider;
import org.jboss.weld.environment.servlet.Listener;
import org.junit.rules.ExternalResource;
import org.pac4j.jax.rs.features.JaxRsConfigProvider;
import org.pac4j.jax.rs.features.Pac4JSecurityFeature;
Expand All @@ -23,6 +26,8 @@
import org.pac4j.jax.rs.servlet.features.ServletJaxRsContextFactoryProvider;

import io.undertow.server.session.SessionCookieConfig;
import io.undertow.servlet.Servlets;
import io.undertow.servlet.api.DeploymentInfo;

public class RestEasyUndertowServletRule extends ExternalResource implements SessionContainerRule {

Expand All @@ -34,15 +39,16 @@ public class MyApp extends Application {

@Override
public Set<Class<?>> getClasses() {
return getResources();
Set<Class<?>> classes = getResources();
classes.add(ServletJaxRsContextFactoryProvider.class);
classes.add(Pac4JProfileInjectorFactory.class);
return classes;
}

@Override
public Set<Object> getSingletons() {
return Sets.newLinkedHashSet(
new JaxRsConfigProvider(getConfig()),
new ServletJaxRsContextFactoryProvider(),
new Pac4JProfileInjectorFactory(),
new Pac4JSecurityFeature());
}
}
Expand All @@ -66,7 +72,15 @@ protected void before() throws Throwable {
System.setProperty("org.jboss.resteasy.port", "24257");
server = new UndertowJaxrsServer().start();

server.deploy(new MyApp());
ResteasyDeployment deployment = new ResteasyDeployment();
deployment.setInjectorFactoryClass(CdiInjectorFactory.class.getName());
deployment.setApplication(new MyApp());
DeploymentInfo di = server.undertowDeployment(deployment)
.setContextPath("/")
.setDeploymentName("DI")
.setClassLoader(getClass().getClassLoader())
.addListeners(Servlets.listener(Listener.class));
server.deploy(di);
}

@Override
Expand Down