You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Paul Lindner <pl...@linkedin.com> on 2009/08/04 02:03:10 UTC

Adding guice-servlet support.

Hi,
Some recent work requires me to decode security tokens one way with SSL and
another way without SSL.

However the SecurityTokenDecoder interface only allows passes a map with the
security token value like so:

  SecurityToken createToken(Map<String, String> tokenParameters)
      throws SecurityTokenException;

Because the API does not pass in the HttpServletRequest it's hard to access.
 One method is to define a filter and a thread local, which is fine, but
seems fragile and hackish.

Enter Guice Servlet, GuiceFilter and friends.

With this in place getting a request into the a SecurityTokenDecoder is as
simple adding something like this:

 Provider<HttpServletRequest> reqProvider;

  @Inject
  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
        this.reqProvider = reqProvider;
    }


GuiceFilter takes care of setting up the context.  This may even allow other
idioms that could greatly simplify the code.  For example the AuthInfo
object used by AuthenticationServletFilter, and GadgetRequestContext would
be candidates for injection.  Later on it would be a simple matter of using
the ServletModule to configure servlets and filters based on the
configuration -- no more modifying values in Guice configs and web.xml.

A very basic patch with an example of injecting HttpRequestServlet into
DefaultSecurityTokenDecoder, and minimal request scope test mocking follows.
 Comments welcome.

diff --git a/java/common/pom.xml b/java/common/pom.xml
index 2e08b69..4be00b7 100644
--- a/java/common/pom.xml
+++ b/java/common/pom.xml
@@ -87,6 +87,10 @@
       <artifactId>guice</artifactId>
     </dependency>
     <dependency>
+      <groupId>com.google.code.guice</groupId>
+      <artifactId>guice-servlet</artifactId>
+    </dependency>
+    <dependency>
       <groupId>aopalliance</groupId>
       <artifactId>aopalliance</artifactId>
     </dependency>
diff --git
a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
index 22e1c8d..b4e29f1 100644
---
a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
+++
b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
@@ -22,8 +22,11 @@ import org.apache.shindig.config.ContainerConfig;

 import com.google.inject.Inject;
 import com.google.inject.Singleton;
+import com.google.inject.Provider;
+import com.google.inject.servlet.RequestScoped;

 import java.util.Map;
+import javax.servlet.http.HttpServletRequest;

 /**
  * Default implementation of security tokens.  Decides based on default
container configuration
@@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder implements
SecurityTokenDecoder {

   private final SecurityTokenDecoder decoder;

+ Provider<HttpServletRequest> reqProvider;
+
+  @Inject
+  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
+        this.reqProvider = reqProvider;
+
+    }
+
   @Inject
   public DefaultSecurityTokenDecoder(ContainerConfig config) {
     String tokenType = config.getString(ContainerConfig.DEFAULT_CONTAINER,
SECURITY_TOKEN_TYPE);
@@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder implements
SecurityTokenDecoder {

   public SecurityToken createToken(Map<String, String> tokenParameters)
       throws SecurityTokenException {
+      System.out.println("found " + reqProvider.get());
+
     return decoder.createToken(tokenParameters);
   }

diff --git
a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
index a999ef4..f1201db 100644
---
a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
+++
b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
@@ -24,6 +24,7 @@ import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.google.inject.Module;
 import com.google.inject.Stage;
+import com.google.inject.servlet.ServletModule;
 import com.google.inject.tools.jmx.Manager;

 import java.util.List;
@@ -45,6 +46,8 @@ public class GuiceServletContextListener implements
ServletContextListener {
     ServletContext context = event.getServletContext();
     String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
     List<Module> modules = Lists.newLinkedList();
+    modules.add(new ServletModule());
+
     if (moduleNames != null) {
       for (String moduleName : moduleNames.split(":")) {
         try {
diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
b/java/server/src/main/webapp/WEB-INF/web.xml
index 9b5b52a..73f0b15 100644
--- a/java/server/src/main/webapp/WEB-INF/web.xml
+++ b/java/server/src/main/webapp/WEB-INF/web.xml
@@ -68,6 +68,15 @@
             </param-value>
         </init-param>
     </filter>
+  <filter>
+    <filter-name>guiceFilter</filter-name>
+    <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
+  </filter>
+
+  <filter-mapping>
+    <filter-name>guiceFilter</filter-name>
+    <url-pattern>/*</url-pattern>
+  </filter-mapping>

   <filter>
     <filter-name>authFilter</filter-name>
diff --git
a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
index 564b1d6..519bb84 100644
---
a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
+++
b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
@@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
 import com.google.common.base.Joiner;
 import com.google.common.collect.Maps;

+import com.google.inject.servlet.ServletModule;
+import com.google.inject.servlet.GuiceFilter;
+
 /**
  * Suite for running the end-to-end tests. The suite is responsible for
starting up and shutting
  * down the server.
  */
 public class EndToEndServer {
   private static final int JETTY_PORT = 9003;
+  private static final String ROOT_BASE = "/*";
   private static final String GADGET_BASE = "/gadgets/ifr";
   private static final String REST_BASE = "/social/rest/*";
   private static final String JSON_RPC_BASE = "/social/rpc/*";
@@ -109,11 +113,16 @@ public class EndToEndServer {
     newServer.addHandler(resources);

     Context context = new Context(newServer, "/", Context.SESSIONS);
+
+      // Add GuiceFilter
+      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
+
+
     context.addEventListener(new GuiceServletContextListener());

     Map<String, String> initParams = Maps.newHashMap();
     String modules = Joiner.on(":")
-        .join(SampleModule.class.getName(),
DefaultGuiceModule.class.getName(),
+        .join(ServletModule.class.getName(), SampleModule.class.getName(),
DefaultGuiceModule.class.getName(),
             PropertiesModule.class.getName(), OAuthModule.class.getName());

     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE, modules);
diff --git
a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
index 4e8b468..a409f3b 100644
---
a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
+++
b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
@@ -36,13 +36,19 @@ import
org.apache.shindig.social.opensocial.spi.AppDataService;
 import org.apache.shindig.social.opensocial.spi.PersonService;
 import org.apache.shindig.social.sample.spi.JsonDbOpensocialService;

+import org.easymock.EasyMock;
+
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.AbstractModule;
 import com.google.inject.TypeLiteral;
+import com.google.inject.Provider;
 import com.google.inject.name.Names;

 import java.util.Set;

+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.ServletRequest;
+
 /**
  * Provides social api component injection for all large tests
  */
@@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
AbstractModule {
     bind(Integer.class).annotatedWith(
         Names.named("shindig.cache.lru.default.capacity"))
         .toInstance(10);
+
+    // Mock HttpServlet / Request Scope
+    //bindScope(RequestScoped.class, REQUEST);
+    //bindScope(SessionScoped.class, SESSION);
+
+    Provider<HttpServletRequest> requestProvider =
+        new Provider<HttpServletRequest>() {
+          public HttpServletRequest get() {
+              return EasyMock.createNiceMock(HttpServletRequest.class);
+          }
+
+          public String toString() {
+            return "RequestProvider";
+          }
+        };
+    bind(HttpServletRequest.class).toProvider(requestProvider);
+    bind(ServletRequest.class).toProvider(requestProvider);
   }
 }
diff --git
a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
index c49ac1f..e6bef88 100644
---
a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
+++
b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
@@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.google.inject.Key;
+import com.google.inject.Provider;
 import com.google.inject.TypeLiteral;
+import com.google.inject.servlet.ServletModule;

 import junit.framework.TestCase;

@@ -33,6 +35,10 @@ import org.easymock.EasyMock;

 import java.util.List;

+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.ServletRequest;
+
+
 public class SocialApiGuiceModuleTest extends TestCase {
   private Injector injector;

@@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends TestCase {
           @Override
           protected void configure() {

bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
+            // hack hack
+    Provider<HttpServletRequest> requestProvider =
+        new Provider<HttpServletRequest>() {
+        public HttpServletRequest get() {
+            return EasyMock.createNiceMock(HttpServletRequest.class);
+        }
+
+        public String toString() {
+            return "RequestProvider";
+        }
+    };
+    bind(HttpServletRequest.class).toProvider(requestProvider);
+    bind(ServletRequest.class).toProvider(requestProvider);
+
           }
     });
   }

Re: Adding guice-servlet support.

Posted by John Hjelmstad <fa...@google.com>.
+1. It's also worth mentioning that GuiceFilter simply uses a ThreadLocal
under the hood anyway:
http://code.google.com/p/google-guice/source/browse/trunk/servlet/src/com/google/inject/servlet/GuiceFilter.java?content-type=text/vnd.viewcvs-markup&root=

Putting this code into Shindig directly
bakes in the (pretty well-hidden) assumption that any code accessing
HttpServlet[Request|Response] context via Provider must do so in the
request's parent thread.

--John

On Mon, Aug 3, 2009 at 5:09 PM, Adam Winer <aw...@google.com> wrote:

> The code is OK, but offhand I don't see a reason why any of this needs
> to live in Shindig.  It could/should go in your implementation of
> SecurityTokenDecoder.
>
> -- Adam
>
> On Mon, Aug 3, 2009 at 5:03 PM, Paul Lindner<pl...@linkedin.com> wrote:
> > Hi,
> > Some recent work requires me to decode security tokens one way with SSL
> and
> > another way without SSL.
> >
> > However the SecurityTokenDecoder interface only allows passes a map with
> the
> > security token value like so:
> >
> >  SecurityToken createToken(Map<String, String> tokenParameters)
> >      throws SecurityTokenException;
> >
> > Because the API does not pass in the HttpServletRequest it's hard to
> access.
> >  One method is to define a filter and a thread local, which is fine, but
> > seems fragile and hackish.
> >
> > Enter Guice Servlet, GuiceFilter and friends.
> >
> > With this in place getting a request into the a SecurityTokenDecoder is
> as
> > simple adding something like this:
> >
> >  Provider<HttpServletRequest> reqProvider;
> >
> >  @Inject
> >  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
> >        this.reqProvider = reqProvider;
> >    }
> >
> >
> > GuiceFilter takes care of setting up the context.  This may even allow
> other
> > idioms that could greatly simplify the code.  For example the AuthInfo
> > object used by AuthenticationServletFilter, and GadgetRequestContext
> would
> > be candidates for injection.  Later on it would be a simple matter of
> using
> > the ServletModule to configure servlets and filters based on the
> > configuration -- no more modifying values in Guice configs and web.xml.
> >
> > A very basic patch with an example of injecting HttpRequestServlet into
> > DefaultSecurityTokenDecoder, and minimal request scope test mocking
> follows.
> >  Comments welcome.
> >
> > diff --git a/java/common/pom.xml b/java/common/pom.xml
> > index 2e08b69..4be00b7 100644
> > --- a/java/common/pom.xml
> > +++ b/java/common/pom.xml
> > @@ -87,6 +87,10 @@
> >       <artifactId>guice</artifactId>
> >     </dependency>
> >     <dependency>
> > +      <groupId>com.google.code.guice</groupId>
> > +      <artifactId>guice-servlet</artifactId>
> > +    </dependency>
> > +    <dependency>
> >       <groupId>aopalliance</groupId>
> >       <artifactId>aopalliance</artifactId>
> >     </dependency>
> > diff --git
> >
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> > index 22e1c8d..b4e29f1 100644
> > ---
> >
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> > +++
> >
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> > @@ -22,8 +22,11 @@ import org.apache.shindig.config.ContainerConfig;
> >
> >  import com.google.inject.Inject;
> >  import com.google.inject.Singleton;
> > +import com.google.inject.Provider;
> > +import com.google.inject.servlet.RequestScoped;
> >
> >  import java.util.Map;
> > +import javax.servlet.http.HttpServletRequest;
> >
> >  /**
> >  * Default implementation of security tokens.  Decides based on default
> > container configuration
> > @@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder implements
> > SecurityTokenDecoder {
> >
> >   private final SecurityTokenDecoder decoder;
> >
> > + Provider<HttpServletRequest> reqProvider;
> > +
> > +  @Inject
> > +  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
> > +        this.reqProvider = reqProvider;
> > +
> > +    }
> > +
> >   @Inject
> >   public DefaultSecurityTokenDecoder(ContainerConfig config) {
> >     String tokenType =
> config.getString(ContainerConfig.DEFAULT_CONTAINER,
> > SECURITY_TOKEN_TYPE);
> > @@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder implements
> > SecurityTokenDecoder {
> >
> >   public SecurityToken createToken(Map<String, String> tokenParameters)
> >       throws SecurityTokenException {
> > +      System.out.println("found " + reqProvider.get());
> > +
> >     return decoder.createToken(tokenParameters);
> >   }
> >
> > diff --git
> >
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> > index a999ef4..f1201db 100644
> > ---
> >
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> > +++
> >
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> > @@ -24,6 +24,7 @@ import com.google.inject.Guice;
> >  import com.google.inject.Injector;
> >  import com.google.inject.Module;
> >  import com.google.inject.Stage;
> > +import com.google.inject.servlet.ServletModule;
> >  import com.google.inject.tools.jmx.Manager;
> >
> >  import java.util.List;
> > @@ -45,6 +46,8 @@ public class GuiceServletContextListener implements
> > ServletContextListener {
> >     ServletContext context = event.getServletContext();
> >     String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
> >     List<Module> modules = Lists.newLinkedList();
> > +    modules.add(new ServletModule());
> > +
> >     if (moduleNames != null) {
> >       for (String moduleName : moduleNames.split(":")) {
> >         try {
> > diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
> > b/java/server/src/main/webapp/WEB-INF/web.xml
> > index 9b5b52a..73f0b15 100644
> > --- a/java/server/src/main/webapp/WEB-INF/web.xml
> > +++ b/java/server/src/main/webapp/WEB-INF/web.xml
> > @@ -68,6 +68,15 @@
> >             </param-value>
> >         </init-param>
> >     </filter>
> > +  <filter>
> > +    <filter-name>guiceFilter</filter-name>
> > +    <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
> > +  </filter>
> > +
> > +  <filter-mapping>
> > +    <filter-name>guiceFilter</filter-name>
> > +    <url-pattern>/*</url-pattern>
> > +  </filter-mapping>
> >
> >   <filter>
> >     <filter-name>authFilter</filter-name>
> > diff --git
> >
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> > index 564b1d6..519bb84 100644
> > ---
> >
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> > +++
> >
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> > @@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
> >  import com.google.common.base.Joiner;
> >  import com.google.common.collect.Maps;
> >
> > +import com.google.inject.servlet.ServletModule;
> > +import com.google.inject.servlet.GuiceFilter;
> > +
> >  /**
> >  * Suite for running the end-to-end tests. The suite is responsible for
> > starting up and shutting
> >  * down the server.
> >  */
> >  public class EndToEndServer {
> >   private static final int JETTY_PORT = 9003;
> > +  private static final String ROOT_BASE = "/*";
> >   private static final String GADGET_BASE = "/gadgets/ifr";
> >   private static final String REST_BASE = "/social/rest/*";
> >   private static final String JSON_RPC_BASE = "/social/rpc/*";
> > @@ -109,11 +113,16 @@ public class EndToEndServer {
> >     newServer.addHandler(resources);
> >
> >     Context context = new Context(newServer, "/", Context.SESSIONS);
> > +
> > +      // Add GuiceFilter
> > +      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
> > +
> > +
> >     context.addEventListener(new GuiceServletContextListener());
> >
> >     Map<String, String> initParams = Maps.newHashMap();
> >     String modules = Joiner.on(":")
> > -        .join(SampleModule.class.getName(),
> > DefaultGuiceModule.class.getName(),
> > +        .join(ServletModule.class.getName(),
> SampleModule.class.getName(),
> > DefaultGuiceModule.class.getName(),
> >             PropertiesModule.class.getName(),
> OAuthModule.class.getName());
> >
> >     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE,
> modules);
> > diff --git
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> > index 4e8b468..a409f3b 100644
> > ---
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> > +++
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> > @@ -36,13 +36,19 @@ import
> > org.apache.shindig.social.opensocial.spi.AppDataService;
> >  import org.apache.shindig.social.opensocial.spi.PersonService;
> >  import org.apache.shindig.social.sample.spi.JsonDbOpensocialService;
> >
> > +import org.easymock.EasyMock;
> > +
> >  import com.google.common.collect.ImmutableSet;
> >  import com.google.inject.AbstractModule;
> >  import com.google.inject.TypeLiteral;
> > +import com.google.inject.Provider;
> >  import com.google.inject.name.Names;
> >
> >  import java.util.Set;
> >
> > +import javax.servlet.http.HttpServletRequest;
> > +import javax.servlet.ServletRequest;
> > +
> >  /**
> >  * Provides social api component injection for all large tests
> >  */
> > @@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
> > AbstractModule {
> >     bind(Integer.class).annotatedWith(
> >         Names.named("shindig.cache.lru.default.capacity"))
> >         .toInstance(10);
> > +
> > +    // Mock HttpServlet / Request Scope
> > +    //bindScope(RequestScoped.class, REQUEST);
> > +    //bindScope(SessionScoped.class, SESSION);
> > +
> > +    Provider<HttpServletRequest> requestProvider =
> > +        new Provider<HttpServletRequest>() {
> > +          public HttpServletRequest get() {
> > +              return EasyMock.createNiceMock(HttpServletRequest.class);
> > +          }
> > +
> > +          public String toString() {
> > +            return "RequestProvider";
> > +          }
> > +        };
> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> > +    bind(ServletRequest.class).toProvider(requestProvider);
> >   }
> >  }
> > diff --git
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> > index c49ac1f..e6bef88 100644
> > ---
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> > +++
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> > @@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
> >  import com.google.inject.Guice;
> >  import com.google.inject.Injector;
> >  import com.google.inject.Key;
> > +import com.google.inject.Provider;
> >  import com.google.inject.TypeLiteral;
> > +import com.google.inject.servlet.ServletModule;
> >
> >  import junit.framework.TestCase;
> >
> > @@ -33,6 +35,10 @@ import org.easymock.EasyMock;
> >
> >  import java.util.List;
> >
> > +import javax.servlet.http.HttpServletRequest;
> > +import javax.servlet.ServletRequest;
> > +
> > +
> >  public class SocialApiGuiceModuleTest extends TestCase {
> >   private Injector injector;
> >
> > @@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends TestCase
> {
> >           @Override
> >           protected void configure() {
> >
> >
> bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
> > +            // hack hack
> > +    Provider<HttpServletRequest> requestProvider =
> > +        new Provider<HttpServletRequest>() {
> > +        public HttpServletRequest get() {
> > +            return EasyMock.createNiceMock(HttpServletRequest.class);
> > +        }
> > +
> > +        public String toString() {
> > +            return "RequestProvider";
> > +        }
> > +    };
> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> > +    bind(ServletRequest.class).toProvider(requestProvider);
> > +
> >           }
> >     });
> >   }
> >
>

Re: Adding guice-servlet support.

Posted by Paul Lindner <li...@inuus.com>.
I sent a note to the guice-users list about this issue.
http://groups.google.com/group/google-guice/browse_thread/thread/803a275463b5c236

Some interesting comments there.


On Tue, Aug 4, 2009 at 8:47 AM, Adam Winer <aw...@gmail.com> wrote:

> On Mon, Aug 3, 2009 at 5:57 PM, Paul Lindner<li...@inuus.com> wrote:
> > Aha -- I understand now.
> > All the Future<?> interface implementations that don't use
> ImmediateFuture
> > would break.  That's ugly, and I suppose there's not a lot one can do
> about
> > that, which is a pity since Request Scope seems like it could simplify a
> > number of things..
> >
> > I suppose that Guice could come up with an implementation that uses
> > InheritableThreadLocal, but that could get messy for mutable objects..
>
> Indeed, mutable objects are (a big part of) the problem.  For example,
> ServletRequest, which is mutable and not thread-safe, so injecting
> that into something outside the main thread is dangerous.  Guice
> actually uses the ServletRequest for storing @RequestScoped instances,
> so instantiating any request scoped instances outside of the main
> thread would break too.
>
> -- Adam
>
> >
> >
> > On Mon, Aug 3, 2009 at 5:26 PM, Adam Winer <aw...@google.com> wrote:
> >
> >> On Mon, Aug 3, 2009 at 5:13 PM, Paul Lindner<li...@inuus.com> wrote:
> >> > yes, that's correct it was only in there to illustrate what how you
> could
> >> > inject a provider.
> >> > The important part is installing ServletModule, and adding
> guiceFilter.
> >> >
> >> > Now this could be done independently by an implementor.  However I can
> >> see
> >> > where it might be advantageous for internal shindig classes to support
> >> > request scoped injection.
> >>
> >> Until we have a direct need in Shindig itself, I see little gained
> >> from forcing installation of ServletModule.
> >>
> >> As John points out, ServletModule breaks for injection outside of the
> >> request thread, so a fair number of Shindig classes cannot safely use
> >> request-scoped injection (and that set isn't fixed either).
> >>
> >> -- Adam
> >>
> >>
> >> > On Mon, Aug 3, 2009 at 5:09 PM, Adam Winer <aw...@google.com> wrote:
> >> >
> >> >> The code is OK, but offhand I don't see a reason why any of this
> needs
> >> >> to live in Shindig.  It could/should go in your implementation of
> >> >> SecurityTokenDecoder.
> >> >>
> >> >> -- Adam
> >> >>
> >> >> On Mon, Aug 3, 2009 at 5:03 PM, Paul Lindner<pl...@linkedin.com>
> >> wrote:
> >> >> > Hi,
> >> >> > Some recent work requires me to decode security tokens one way with
> >> SSL
> >> >> and
> >> >> > another way without SSL.
> >> >> >
> >> >> > However the SecurityTokenDecoder interface only allows passes a map
> >> with
> >> >> the
> >> >> > security token value like so:
> >> >> >
> >> >> >  SecurityToken createToken(Map<String, String> tokenParameters)
> >> >> >      throws SecurityTokenException;
> >> >> >
> >> >> > Because the API does not pass in the HttpServletRequest it's hard
> to
> >> >> access.
> >> >> >  One method is to define a filter and a thread local, which is
> fine,
> >> but
> >> >> > seems fragile and hackish.
> >> >> >
> >> >> > Enter Guice Servlet, GuiceFilter and friends.
> >> >> >
> >> >> > With this in place getting a request into the a
> SecurityTokenDecoder
> >> is
> >> >> as
> >> >> > simple adding something like this:
> >> >> >
> >> >> >  Provider<HttpServletRequest> reqProvider;
> >> >> >
> >> >> >  @Inject
> >> >> >  public void setReqProvider(Provider<HttpServletRequest>
> reqProvider)
> >> {
> >> >> >        this.reqProvider = reqProvider;
> >> >> >    }
> >> >> >
> >> >> >
> >> >> > GuiceFilter takes care of setting up the context.  This may even
> allow
> >> >> other
> >> >> > idioms that could greatly simplify the code.  For example the
> AuthInfo
> >> >> > object used by AuthenticationServletFilter, and
> GadgetRequestContext
> >> >> would
> >> >> > be candidates for injection.  Later on it would be a simple matter
> of
> >> >> using
> >> >> > the ServletModule to configure servlets and filters based on the
> >> >> > configuration -- no more modifying values in Guice configs and
> >> web.xml.
> >> >> >
> >> >> > A very basic patch with an example of injecting HttpRequestServlet
> >> into
> >> >> > DefaultSecurityTokenDecoder, and minimal request scope test mocking
> >> >> follows.
> >> >> >  Comments welcome.
> >> >> >
> >> >> > diff --git a/java/common/pom.xml b/java/common/pom.xml
> >> >> > index 2e08b69..4be00b7 100644
> >> >> > --- a/java/common/pom.xml
> >> >> > +++ b/java/common/pom.xml
> >> >> > @@ -87,6 +87,10 @@
> >> >> >       <artifactId>guice</artifactId>
> >> >> >     </dependency>
> >> >> >     <dependency>
> >> >> > +      <groupId>com.google.code.guice</groupId>
> >> >> > +      <artifactId>guice-servlet</artifactId>
> >> >> > +    </dependency>
> >> >> > +    <dependency>
> >> >> >       <groupId>aopalliance</groupId>
> >> >> >       <artifactId>aopalliance</artifactId>
> >> >> >     </dependency>
> >> >> > diff --git
> >> >> >
> >> >>
> >>
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> >> >
> >> >>
> >>
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> >> > index 22e1c8d..b4e29f1 100644
> >> >> > ---
> >> >> >
> >> >>
> >>
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> >> > +++
> >> >> >
> >> >>
> >>
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> >> > @@ -22,8 +22,11 @@ import
> org.apache.shindig.config.ContainerConfig;
> >> >> >
> >> >> >  import com.google.inject.Inject;
> >> >> >  import com.google.inject.Singleton;
> >> >> > +import com.google.inject.Provider;
> >> >> > +import com.google.inject.servlet.RequestScoped;
> >> >> >
> >> >> >  import java.util.Map;
> >> >> > +import javax.servlet.http.HttpServletRequest;
> >> >> >
> >> >> >  /**
> >> >> >  * Default implementation of security tokens.  Decides based on
> >> default
> >> >> > container configuration
> >> >> > @@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder
> implements
> >> >> > SecurityTokenDecoder {
> >> >> >
> >> >> >   private final SecurityTokenDecoder decoder;
> >> >> >
> >> >> > + Provider<HttpServletRequest> reqProvider;
> >> >> > +
> >> >> > +  @Inject
> >> >> > +  public void setReqProvider(Provider<HttpServletRequest>
> >> reqProvider) {
> >> >> > +        this.reqProvider = reqProvider;
> >> >> > +
> >> >> > +    }
> >> >> > +
> >> >> >   @Inject
> >> >> >   public DefaultSecurityTokenDecoder(ContainerConfig config) {
> >> >> >     String tokenType =
> >> >> config.getString(ContainerConfig.DEFAULT_CONTAINER,
> >> >> > SECURITY_TOKEN_TYPE);
> >> >> > @@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder
> implements
> >> >> > SecurityTokenDecoder {
> >> >> >
> >> >> >   public SecurityToken createToken(Map<String, String>
> >> tokenParameters)
> >> >> >       throws SecurityTokenException {
> >> >> > +      System.out.println("found " + reqProvider.get());
> >> >> > +
> >> >> >     return decoder.createToken(tokenParameters);
> >> >> >   }
> >> >> >
> >> >> > diff --git
> >> >> >
> >> >>
> >>
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> >> >
> >> >>
> >>
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> >> > index a999ef4..f1201db 100644
> >> >> > ---
> >> >> >
> >> >>
> >>
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> >> > +++
> >> >> >
> >> >>
> >>
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> >> > @@ -24,6 +24,7 @@ import com.google.inject.Guice;
> >> >> >  import com.google.inject.Injector;
> >> >> >  import com.google.inject.Module;
> >> >> >  import com.google.inject.Stage;
> >> >> > +import com.google.inject.servlet.ServletModule;
> >> >> >  import com.google.inject.tools.jmx.Manager;
> >> >> >
> >> >> >  import java.util.List;
> >> >> > @@ -45,6 +46,8 @@ public class GuiceServletContextListener
> implements
> >> >> > ServletContextListener {
> >> >> >     ServletContext context = event.getServletContext();
> >> >> >     String moduleNames =
> context.getInitParameter(MODULES_ATTRIBUTE);
> >> >> >     List<Module> modules = Lists.newLinkedList();
> >> >> > +    modules.add(new ServletModule());
> >> >> > +
> >> >> >     if (moduleNames != null) {
> >> >> >       for (String moduleName : moduleNames.split(":")) {
> >> >> >         try {
> >> >> > diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
> >> >> > b/java/server/src/main/webapp/WEB-INF/web.xml
> >> >> > index 9b5b52a..73f0b15 100644
> >> >> > --- a/java/server/src/main/webapp/WEB-INF/web.xml
> >> >> > +++ b/java/server/src/main/webapp/WEB-INF/web.xml
> >> >> > @@ -68,6 +68,15 @@
> >> >> >             </param-value>
> >> >> >         </init-param>
> >> >> >     </filter>
> >> >> > +  <filter>
> >> >> > +    <filter-name>guiceFilter</filter-name>
> >> >> > +
> >>  <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
> >> >> > +  </filter>
> >> >> > +
> >> >> > +  <filter-mapping>
> >> >> > +    <filter-name>guiceFilter</filter-name>
> >> >> > +    <url-pattern>/*</url-pattern>
> >> >> > +  </filter-mapping>
> >> >> >
> >> >> >   <filter>
> >> >> >     <filter-name>authFilter</filter-name>
> >> >> > diff --git
> >> >> >
> >> >>
> >>
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> >> >
> >> >>
> >>
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> >> > index 564b1d6..519bb84 100644
> >> >> > ---
> >> >> >
> >> >>
> >>
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> >> > +++
> >> >> >
> >> >>
> >>
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> >> > @@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
> >> >> >  import com.google.common.base.Joiner;
> >> >> >  import com.google.common.collect.Maps;
> >> >> >
> >> >> > +import com.google.inject.servlet.ServletModule;
> >> >> > +import com.google.inject.servlet.GuiceFilter;
> >> >> > +
> >> >> >  /**
> >> >> >  * Suite for running the end-to-end tests. The suite is responsible
> >> for
> >> >> > starting up and shutting
> >> >> >  * down the server.
> >> >> >  */
> >> >> >  public class EndToEndServer {
> >> >> >   private static final int JETTY_PORT = 9003;
> >> >> > +  private static final String ROOT_BASE = "/*";
> >> >> >   private static final String GADGET_BASE = "/gadgets/ifr";
> >> >> >   private static final String REST_BASE = "/social/rest/*";
> >> >> >   private static final String JSON_RPC_BASE = "/social/rpc/*";
> >> >> > @@ -109,11 +113,16 @@ public class EndToEndServer {
> >> >> >     newServer.addHandler(resources);
> >> >> >
> >> >> >     Context context = new Context(newServer, "/",
> Context.SESSIONS);
> >> >> > +
> >> >> > +      // Add GuiceFilter
> >> >> > +      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
> >> >> > +
> >> >> > +
> >> >> >     context.addEventListener(new GuiceServletContextListener());
> >> >> >
> >> >> >     Map<String, String> initParams = Maps.newHashMap();
> >> >> >     String modules = Joiner.on(":")
> >> >> > -        .join(SampleModule.class.getName(),
> >> >> > DefaultGuiceModule.class.getName(),
> >> >> > +        .join(ServletModule.class.getName(),
> >> >> SampleModule.class.getName(),
> >> >> > DefaultGuiceModule.class.getName(),
> >> >> >             PropertiesModule.class.getName(),
> >> >> OAuthModule.class.getName());
> >> >> >
> >> >> >     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE,
> >> >> modules);
> >> >> > diff --git
> >> >> >
> >> >>
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> >> >
> >> >>
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> >> > index 4e8b468..a409f3b 100644
> >> >> > ---
> >> >> >
> >> >>
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> >> > +++
> >> >> >
> >> >>
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> >> > @@ -36,13 +36,19 @@ import
> >> >> > org.apache.shindig.social.opensocial.spi.AppDataService;
> >> >> >  import org.apache.shindig.social.opensocial.spi.PersonService;
> >> >> >  import
> org.apache.shindig.social.sample.spi.JsonDbOpensocialService;
> >> >> >
> >> >> > +import org.easymock.EasyMock;
> >> >> > +
> >> >> >  import com.google.common.collect.ImmutableSet;
> >> >> >  import com.google.inject.AbstractModule;
> >> >> >  import com.google.inject.TypeLiteral;
> >> >> > +import com.google.inject.Provider;
> >> >> >  import com.google.inject.name.Names;
> >> >> >
> >> >> >  import java.util.Set;
> >> >> >
> >> >> > +import javax.servlet.http.HttpServletRequest;
> >> >> > +import javax.servlet.ServletRequest;
> >> >> > +
> >> >> >  /**
> >> >> >  * Provides social api component injection for all large tests
> >> >> >  */
> >> >> > @@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
> >> >> > AbstractModule {
> >> >> >     bind(Integer.class).annotatedWith(
> >> >> >         Names.named("shindig.cache.lru.default.capacity"))
> >> >> >         .toInstance(10);
> >> >> > +
> >> >> > +    // Mock HttpServlet / Request Scope
> >> >> > +    //bindScope(RequestScoped.class, REQUEST);
> >> >> > +    //bindScope(SessionScoped.class, SESSION);
> >> >> > +
> >> >> > +    Provider<HttpServletRequest> requestProvider =
> >> >> > +        new Provider<HttpServletRequest>() {
> >> >> > +          public HttpServletRequest get() {
> >> >> > +              return
> >> EasyMock.createNiceMock(HttpServletRequest.class);
> >> >> > +          }
> >> >> > +
> >> >> > +          public String toString() {
> >> >> > +            return "RequestProvider";
> >> >> > +          }
> >> >> > +        };
> >> >> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> >> >> > +    bind(ServletRequest.class).toProvider(requestProvider);
> >> >> >   }
> >> >> >  }
> >> >> > diff --git
> >> >> >
> >> >>
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> >> >
> >> >>
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> >> > index c49ac1f..e6bef88 100644
> >> >> > ---
> >> >> >
> >> >>
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> >> > +++
> >> >> >
> >> >>
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> >> > @@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
> >> >> >  import com.google.inject.Guice;
> >> >> >  import com.google.inject.Injector;
> >> >> >  import com.google.inject.Key;
> >> >> > +import com.google.inject.Provider;
> >> >> >  import com.google.inject.TypeLiteral;
> >> >> > +import com.google.inject.servlet.ServletModule;
> >> >> >
> >> >> >  import junit.framework.TestCase;
> >> >> >
> >> >> > @@ -33,6 +35,10 @@ import org.easymock.EasyMock;
> >> >> >
> >> >> >  import java.util.List;
> >> >> >
> >> >> > +import javax.servlet.http.HttpServletRequest;
> >> >> > +import javax.servlet.ServletRequest;
> >> >> > +
> >> >> > +
> >> >> >  public class SocialApiGuiceModuleTest extends TestCase {
> >> >> >   private Injector injector;
> >> >> >
> >> >> > @@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends
> >> TestCase
> >> >> {
> >> >> >           @Override
> >> >> >           protected void configure() {
> >> >> >
> >> >> >
> >> >>
> >>
> bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
> >> >> > +            // hack hack
> >> >> > +    Provider<HttpServletRequest> requestProvider =
> >> >> > +        new Provider<HttpServletRequest>() {
> >> >> > +        public HttpServletRequest get() {
> >> >> > +            return
> EasyMock.createNiceMock(HttpServletRequest.class);
> >> >> > +        }
> >> >> > +
> >> >> > +        public String toString() {
> >> >> > +            return "RequestProvider";
> >> >> > +        }
> >> >> > +    };
> >> >> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> >> >> > +    bind(ServletRequest.class).toProvider(requestProvider);
> >> >> > +
> >> >> >           }
> >> >> >     });
> >> >> >   }
> >> >> >
> >> >>
> >> >
> >>
> >
>

Re: Adding guice-servlet support.

Posted by Adam Winer <aw...@gmail.com>.
On Mon, Aug 3, 2009 at 5:57 PM, Paul Lindner<li...@inuus.com> wrote:
> Aha -- I understand now.
> All the Future<?> interface implementations that don't use ImmediateFuture
> would break.  That's ugly, and I suppose there's not a lot one can do about
> that, which is a pity since Request Scope seems like it could simplify a
> number of things..
>
> I suppose that Guice could come up with an implementation that uses
> InheritableThreadLocal, but that could get messy for mutable objects..

Indeed, mutable objects are (a big part of) the problem.  For example,
ServletRequest, which is mutable and not thread-safe, so injecting
that into something outside the main thread is dangerous.  Guice
actually uses the ServletRequest for storing @RequestScoped instances,
so instantiating any request scoped instances outside of the main
thread would break too.

-- Adam

>
>
> On Mon, Aug 3, 2009 at 5:26 PM, Adam Winer <aw...@google.com> wrote:
>
>> On Mon, Aug 3, 2009 at 5:13 PM, Paul Lindner<li...@inuus.com> wrote:
>> > yes, that's correct it was only in there to illustrate what how you could
>> > inject a provider.
>> > The important part is installing ServletModule, and adding guiceFilter.
>> >
>> > Now this could be done independently by an implementor.  However I can
>> see
>> > where it might be advantageous for internal shindig classes to support
>> > request scoped injection.
>>
>> Until we have a direct need in Shindig itself, I see little gained
>> from forcing installation of ServletModule.
>>
>> As John points out, ServletModule breaks for injection outside of the
>> request thread, so a fair number of Shindig classes cannot safely use
>> request-scoped injection (and that set isn't fixed either).
>>
>> -- Adam
>>
>>
>> > On Mon, Aug 3, 2009 at 5:09 PM, Adam Winer <aw...@google.com> wrote:
>> >
>> >> The code is OK, but offhand I don't see a reason why any of this needs
>> >> to live in Shindig.  It could/should go in your implementation of
>> >> SecurityTokenDecoder.
>> >>
>> >> -- Adam
>> >>
>> >> On Mon, Aug 3, 2009 at 5:03 PM, Paul Lindner<pl...@linkedin.com>
>> wrote:
>> >> > Hi,
>> >> > Some recent work requires me to decode security tokens one way with
>> SSL
>> >> and
>> >> > another way without SSL.
>> >> >
>> >> > However the SecurityTokenDecoder interface only allows passes a map
>> with
>> >> the
>> >> > security token value like so:
>> >> >
>> >> >  SecurityToken createToken(Map<String, String> tokenParameters)
>> >> >      throws SecurityTokenException;
>> >> >
>> >> > Because the API does not pass in the HttpServletRequest it's hard to
>> >> access.
>> >> >  One method is to define a filter and a thread local, which is fine,
>> but
>> >> > seems fragile and hackish.
>> >> >
>> >> > Enter Guice Servlet, GuiceFilter and friends.
>> >> >
>> >> > With this in place getting a request into the a SecurityTokenDecoder
>> is
>> >> as
>> >> > simple adding something like this:
>> >> >
>> >> >  Provider<HttpServletRequest> reqProvider;
>> >> >
>> >> >  @Inject
>> >> >  public void setReqProvider(Provider<HttpServletRequest> reqProvider)
>> {
>> >> >        this.reqProvider = reqProvider;
>> >> >    }
>> >> >
>> >> >
>> >> > GuiceFilter takes care of setting up the context.  This may even allow
>> >> other
>> >> > idioms that could greatly simplify the code.  For example the AuthInfo
>> >> > object used by AuthenticationServletFilter, and GadgetRequestContext
>> >> would
>> >> > be candidates for injection.  Later on it would be a simple matter of
>> >> using
>> >> > the ServletModule to configure servlets and filters based on the
>> >> > configuration -- no more modifying values in Guice configs and
>> web.xml.
>> >> >
>> >> > A very basic patch with an example of injecting HttpRequestServlet
>> into
>> >> > DefaultSecurityTokenDecoder, and minimal request scope test mocking
>> >> follows.
>> >> >  Comments welcome.
>> >> >
>> >> > diff --git a/java/common/pom.xml b/java/common/pom.xml
>> >> > index 2e08b69..4be00b7 100644
>> >> > --- a/java/common/pom.xml
>> >> > +++ b/java/common/pom.xml
>> >> > @@ -87,6 +87,10 @@
>> >> >       <artifactId>guice</artifactId>
>> >> >     </dependency>
>> >> >     <dependency>
>> >> > +      <groupId>com.google.code.guice</groupId>
>> >> > +      <artifactId>guice-servlet</artifactId>
>> >> > +    </dependency>
>> >> > +    <dependency>
>> >> >       <groupId>aopalliance</groupId>
>> >> >       <artifactId>aopalliance</artifactId>
>> >> >     </dependency>
>> >> > diff --git
>> >> >
>> >>
>> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> >> >
>> >>
>> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> >> > index 22e1c8d..b4e29f1 100644
>> >> > ---
>> >> >
>> >>
>> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> >> > +++
>> >> >
>> >>
>> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> >> > @@ -22,8 +22,11 @@ import org.apache.shindig.config.ContainerConfig;
>> >> >
>> >> >  import com.google.inject.Inject;
>> >> >  import com.google.inject.Singleton;
>> >> > +import com.google.inject.Provider;
>> >> > +import com.google.inject.servlet.RequestScoped;
>> >> >
>> >> >  import java.util.Map;
>> >> > +import javax.servlet.http.HttpServletRequest;
>> >> >
>> >> >  /**
>> >> >  * Default implementation of security tokens.  Decides based on
>> default
>> >> > container configuration
>> >> > @@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder implements
>> >> > SecurityTokenDecoder {
>> >> >
>> >> >   private final SecurityTokenDecoder decoder;
>> >> >
>> >> > + Provider<HttpServletRequest> reqProvider;
>> >> > +
>> >> > +  @Inject
>> >> > +  public void setReqProvider(Provider<HttpServletRequest>
>> reqProvider) {
>> >> > +        this.reqProvider = reqProvider;
>> >> > +
>> >> > +    }
>> >> > +
>> >> >   @Inject
>> >> >   public DefaultSecurityTokenDecoder(ContainerConfig config) {
>> >> >     String tokenType =
>> >> config.getString(ContainerConfig.DEFAULT_CONTAINER,
>> >> > SECURITY_TOKEN_TYPE);
>> >> > @@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder implements
>> >> > SecurityTokenDecoder {
>> >> >
>> >> >   public SecurityToken createToken(Map<String, String>
>> tokenParameters)
>> >> >       throws SecurityTokenException {
>> >> > +      System.out.println("found " + reqProvider.get());
>> >> > +
>> >> >     return decoder.createToken(tokenParameters);
>> >> >   }
>> >> >
>> >> > diff --git
>> >> >
>> >>
>> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> >> >
>> >>
>> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> >> > index a999ef4..f1201db 100644
>> >> > ---
>> >> >
>> >>
>> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> >> > +++
>> >> >
>> >>
>> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> >> > @@ -24,6 +24,7 @@ import com.google.inject.Guice;
>> >> >  import com.google.inject.Injector;
>> >> >  import com.google.inject.Module;
>> >> >  import com.google.inject.Stage;
>> >> > +import com.google.inject.servlet.ServletModule;
>> >> >  import com.google.inject.tools.jmx.Manager;
>> >> >
>> >> >  import java.util.List;
>> >> > @@ -45,6 +46,8 @@ public class GuiceServletContextListener implements
>> >> > ServletContextListener {
>> >> >     ServletContext context = event.getServletContext();
>> >> >     String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
>> >> >     List<Module> modules = Lists.newLinkedList();
>> >> > +    modules.add(new ServletModule());
>> >> > +
>> >> >     if (moduleNames != null) {
>> >> >       for (String moduleName : moduleNames.split(":")) {
>> >> >         try {
>> >> > diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
>> >> > b/java/server/src/main/webapp/WEB-INF/web.xml
>> >> > index 9b5b52a..73f0b15 100644
>> >> > --- a/java/server/src/main/webapp/WEB-INF/web.xml
>> >> > +++ b/java/server/src/main/webapp/WEB-INF/web.xml
>> >> > @@ -68,6 +68,15 @@
>> >> >             </param-value>
>> >> >         </init-param>
>> >> >     </filter>
>> >> > +  <filter>
>> >> > +    <filter-name>guiceFilter</filter-name>
>> >> > +
>>  <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
>> >> > +  </filter>
>> >> > +
>> >> > +  <filter-mapping>
>> >> > +    <filter-name>guiceFilter</filter-name>
>> >> > +    <url-pattern>/*</url-pattern>
>> >> > +  </filter-mapping>
>> >> >
>> >> >   <filter>
>> >> >     <filter-name>authFilter</filter-name>
>> >> > diff --git
>> >> >
>> >>
>> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> >> >
>> >>
>> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> >> > index 564b1d6..519bb84 100644
>> >> > ---
>> >> >
>> >>
>> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> >> > +++
>> >> >
>> >>
>> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> >> > @@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
>> >> >  import com.google.common.base.Joiner;
>> >> >  import com.google.common.collect.Maps;
>> >> >
>> >> > +import com.google.inject.servlet.ServletModule;
>> >> > +import com.google.inject.servlet.GuiceFilter;
>> >> > +
>> >> >  /**
>> >> >  * Suite for running the end-to-end tests. The suite is responsible
>> for
>> >> > starting up and shutting
>> >> >  * down the server.
>> >> >  */
>> >> >  public class EndToEndServer {
>> >> >   private static final int JETTY_PORT = 9003;
>> >> > +  private static final String ROOT_BASE = "/*";
>> >> >   private static final String GADGET_BASE = "/gadgets/ifr";
>> >> >   private static final String REST_BASE = "/social/rest/*";
>> >> >   private static final String JSON_RPC_BASE = "/social/rpc/*";
>> >> > @@ -109,11 +113,16 @@ public class EndToEndServer {
>> >> >     newServer.addHandler(resources);
>> >> >
>> >> >     Context context = new Context(newServer, "/", Context.SESSIONS);
>> >> > +
>> >> > +      // Add GuiceFilter
>> >> > +      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
>> >> > +
>> >> > +
>> >> >     context.addEventListener(new GuiceServletContextListener());
>> >> >
>> >> >     Map<String, String> initParams = Maps.newHashMap();
>> >> >     String modules = Joiner.on(":")
>> >> > -        .join(SampleModule.class.getName(),
>> >> > DefaultGuiceModule.class.getName(),
>> >> > +        .join(ServletModule.class.getName(),
>> >> SampleModule.class.getName(),
>> >> > DefaultGuiceModule.class.getName(),
>> >> >             PropertiesModule.class.getName(),
>> >> OAuthModule.class.getName());
>> >> >
>> >> >     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE,
>> >> modules);
>> >> > diff --git
>> >> >
>> >>
>> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> >> >
>> >>
>> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> >> > index 4e8b468..a409f3b 100644
>> >> > ---
>> >> >
>> >>
>> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> >> > +++
>> >> >
>> >>
>> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> >> > @@ -36,13 +36,19 @@ import
>> >> > org.apache.shindig.social.opensocial.spi.AppDataService;
>> >> >  import org.apache.shindig.social.opensocial.spi.PersonService;
>> >> >  import org.apache.shindig.social.sample.spi.JsonDbOpensocialService;
>> >> >
>> >> > +import org.easymock.EasyMock;
>> >> > +
>> >> >  import com.google.common.collect.ImmutableSet;
>> >> >  import com.google.inject.AbstractModule;
>> >> >  import com.google.inject.TypeLiteral;
>> >> > +import com.google.inject.Provider;
>> >> >  import com.google.inject.name.Names;
>> >> >
>> >> >  import java.util.Set;
>> >> >
>> >> > +import javax.servlet.http.HttpServletRequest;
>> >> > +import javax.servlet.ServletRequest;
>> >> > +
>> >> >  /**
>> >> >  * Provides social api component injection for all large tests
>> >> >  */
>> >> > @@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
>> >> > AbstractModule {
>> >> >     bind(Integer.class).annotatedWith(
>> >> >         Names.named("shindig.cache.lru.default.capacity"))
>> >> >         .toInstance(10);
>> >> > +
>> >> > +    // Mock HttpServlet / Request Scope
>> >> > +    //bindScope(RequestScoped.class, REQUEST);
>> >> > +    //bindScope(SessionScoped.class, SESSION);
>> >> > +
>> >> > +    Provider<HttpServletRequest> requestProvider =
>> >> > +        new Provider<HttpServletRequest>() {
>> >> > +          public HttpServletRequest get() {
>> >> > +              return
>> EasyMock.createNiceMock(HttpServletRequest.class);
>> >> > +          }
>> >> > +
>> >> > +          public String toString() {
>> >> > +            return "RequestProvider";
>> >> > +          }
>> >> > +        };
>> >> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
>> >> > +    bind(ServletRequest.class).toProvider(requestProvider);
>> >> >   }
>> >> >  }
>> >> > diff --git
>> >> >
>> >>
>> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> >> >
>> >>
>> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> >> > index c49ac1f..e6bef88 100644
>> >> > ---
>> >> >
>> >>
>> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> >> > +++
>> >> >
>> >>
>> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> >> > @@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
>> >> >  import com.google.inject.Guice;
>> >> >  import com.google.inject.Injector;
>> >> >  import com.google.inject.Key;
>> >> > +import com.google.inject.Provider;
>> >> >  import com.google.inject.TypeLiteral;
>> >> > +import com.google.inject.servlet.ServletModule;
>> >> >
>> >> >  import junit.framework.TestCase;
>> >> >
>> >> > @@ -33,6 +35,10 @@ import org.easymock.EasyMock;
>> >> >
>> >> >  import java.util.List;
>> >> >
>> >> > +import javax.servlet.http.HttpServletRequest;
>> >> > +import javax.servlet.ServletRequest;
>> >> > +
>> >> > +
>> >> >  public class SocialApiGuiceModuleTest extends TestCase {
>> >> >   private Injector injector;
>> >> >
>> >> > @@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends
>> TestCase
>> >> {
>> >> >           @Override
>> >> >           protected void configure() {
>> >> >
>> >> >
>> >>
>> bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
>> >> > +            // hack hack
>> >> > +    Provider<HttpServletRequest> requestProvider =
>> >> > +        new Provider<HttpServletRequest>() {
>> >> > +        public HttpServletRequest get() {
>> >> > +            return EasyMock.createNiceMock(HttpServletRequest.class);
>> >> > +        }
>> >> > +
>> >> > +        public String toString() {
>> >> > +            return "RequestProvider";
>> >> > +        }
>> >> > +    };
>> >> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
>> >> > +    bind(ServletRequest.class).toProvider(requestProvider);
>> >> > +
>> >> >           }
>> >> >     });
>> >> >   }
>> >> >
>> >>
>> >
>>
>

Re: Adding guice-servlet support.

Posted by Paul Lindner <li...@inuus.com>.
Aha -- I understand now.
All the Future<?> interface implementations that don't use ImmediateFuture
would break.  That's ugly, and I suppose there's not a lot one can do about
that, which is a pity since Request Scope seems like it could simplify a
number of things..

I suppose that Guice could come up with an implementation that uses
InheritableThreadLocal, but that could get messy for mutable objects..


On Mon, Aug 3, 2009 at 5:26 PM, Adam Winer <aw...@google.com> wrote:

> On Mon, Aug 3, 2009 at 5:13 PM, Paul Lindner<li...@inuus.com> wrote:
> > yes, that's correct it was only in there to illustrate what how you could
> > inject a provider.
> > The important part is installing ServletModule, and adding guiceFilter.
> >
> > Now this could be done independently by an implementor.  However I can
> see
> > where it might be advantageous for internal shindig classes to support
> > request scoped injection.
>
> Until we have a direct need in Shindig itself, I see little gained
> from forcing installation of ServletModule.
>
> As John points out, ServletModule breaks for injection outside of the
> request thread, so a fair number of Shindig classes cannot safely use
> request-scoped injection (and that set isn't fixed either).
>
> -- Adam
>
>
> > On Mon, Aug 3, 2009 at 5:09 PM, Adam Winer <aw...@google.com> wrote:
> >
> >> The code is OK, but offhand I don't see a reason why any of this needs
> >> to live in Shindig.  It could/should go in your implementation of
> >> SecurityTokenDecoder.
> >>
> >> -- Adam
> >>
> >> On Mon, Aug 3, 2009 at 5:03 PM, Paul Lindner<pl...@linkedin.com>
> wrote:
> >> > Hi,
> >> > Some recent work requires me to decode security tokens one way with
> SSL
> >> and
> >> > another way without SSL.
> >> >
> >> > However the SecurityTokenDecoder interface only allows passes a map
> with
> >> the
> >> > security token value like so:
> >> >
> >> >  SecurityToken createToken(Map<String, String> tokenParameters)
> >> >      throws SecurityTokenException;
> >> >
> >> > Because the API does not pass in the HttpServletRequest it's hard to
> >> access.
> >> >  One method is to define a filter and a thread local, which is fine,
> but
> >> > seems fragile and hackish.
> >> >
> >> > Enter Guice Servlet, GuiceFilter and friends.
> >> >
> >> > With this in place getting a request into the a SecurityTokenDecoder
> is
> >> as
> >> > simple adding something like this:
> >> >
> >> >  Provider<HttpServletRequest> reqProvider;
> >> >
> >> >  @Inject
> >> >  public void setReqProvider(Provider<HttpServletRequest> reqProvider)
> {
> >> >        this.reqProvider = reqProvider;
> >> >    }
> >> >
> >> >
> >> > GuiceFilter takes care of setting up the context.  This may even allow
> >> other
> >> > idioms that could greatly simplify the code.  For example the AuthInfo
> >> > object used by AuthenticationServletFilter, and GadgetRequestContext
> >> would
> >> > be candidates for injection.  Later on it would be a simple matter of
> >> using
> >> > the ServletModule to configure servlets and filters based on the
> >> > configuration -- no more modifying values in Guice configs and
> web.xml.
> >> >
> >> > A very basic patch with an example of injecting HttpRequestServlet
> into
> >> > DefaultSecurityTokenDecoder, and minimal request scope test mocking
> >> follows.
> >> >  Comments welcome.
> >> >
> >> > diff --git a/java/common/pom.xml b/java/common/pom.xml
> >> > index 2e08b69..4be00b7 100644
> >> > --- a/java/common/pom.xml
> >> > +++ b/java/common/pom.xml
> >> > @@ -87,6 +87,10 @@
> >> >       <artifactId>guice</artifactId>
> >> >     </dependency>
> >> >     <dependency>
> >> > +      <groupId>com.google.code.guice</groupId>
> >> > +      <artifactId>guice-servlet</artifactId>
> >> > +    </dependency>
> >> > +    <dependency>
> >> >       <groupId>aopalliance</groupId>
> >> >       <artifactId>aopalliance</artifactId>
> >> >     </dependency>
> >> > diff --git
> >> >
> >>
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> >
> >>
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> > index 22e1c8d..b4e29f1 100644
> >> > ---
> >> >
> >>
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> > +++
> >> >
> >>
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >> > @@ -22,8 +22,11 @@ import org.apache.shindig.config.ContainerConfig;
> >> >
> >> >  import com.google.inject.Inject;
> >> >  import com.google.inject.Singleton;
> >> > +import com.google.inject.Provider;
> >> > +import com.google.inject.servlet.RequestScoped;
> >> >
> >> >  import java.util.Map;
> >> > +import javax.servlet.http.HttpServletRequest;
> >> >
> >> >  /**
> >> >  * Default implementation of security tokens.  Decides based on
> default
> >> > container configuration
> >> > @@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder implements
> >> > SecurityTokenDecoder {
> >> >
> >> >   private final SecurityTokenDecoder decoder;
> >> >
> >> > + Provider<HttpServletRequest> reqProvider;
> >> > +
> >> > +  @Inject
> >> > +  public void setReqProvider(Provider<HttpServletRequest>
> reqProvider) {
> >> > +        this.reqProvider = reqProvider;
> >> > +
> >> > +    }
> >> > +
> >> >   @Inject
> >> >   public DefaultSecurityTokenDecoder(ContainerConfig config) {
> >> >     String tokenType =
> >> config.getString(ContainerConfig.DEFAULT_CONTAINER,
> >> > SECURITY_TOKEN_TYPE);
> >> > @@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder implements
> >> > SecurityTokenDecoder {
> >> >
> >> >   public SecurityToken createToken(Map<String, String>
> tokenParameters)
> >> >       throws SecurityTokenException {
> >> > +      System.out.println("found " + reqProvider.get());
> >> > +
> >> >     return decoder.createToken(tokenParameters);
> >> >   }
> >> >
> >> > diff --git
> >> >
> >>
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> >
> >>
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> > index a999ef4..f1201db 100644
> >> > ---
> >> >
> >>
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> > +++
> >> >
> >>
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >> > @@ -24,6 +24,7 @@ import com.google.inject.Guice;
> >> >  import com.google.inject.Injector;
> >> >  import com.google.inject.Module;
> >> >  import com.google.inject.Stage;
> >> > +import com.google.inject.servlet.ServletModule;
> >> >  import com.google.inject.tools.jmx.Manager;
> >> >
> >> >  import java.util.List;
> >> > @@ -45,6 +46,8 @@ public class GuiceServletContextListener implements
> >> > ServletContextListener {
> >> >     ServletContext context = event.getServletContext();
> >> >     String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
> >> >     List<Module> modules = Lists.newLinkedList();
> >> > +    modules.add(new ServletModule());
> >> > +
> >> >     if (moduleNames != null) {
> >> >       for (String moduleName : moduleNames.split(":")) {
> >> >         try {
> >> > diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
> >> > b/java/server/src/main/webapp/WEB-INF/web.xml
> >> > index 9b5b52a..73f0b15 100644
> >> > --- a/java/server/src/main/webapp/WEB-INF/web.xml
> >> > +++ b/java/server/src/main/webapp/WEB-INF/web.xml
> >> > @@ -68,6 +68,15 @@
> >> >             </param-value>
> >> >         </init-param>
> >> >     </filter>
> >> > +  <filter>
> >> > +    <filter-name>guiceFilter</filter-name>
> >> > +
>  <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
> >> > +  </filter>
> >> > +
> >> > +  <filter-mapping>
> >> > +    <filter-name>guiceFilter</filter-name>
> >> > +    <url-pattern>/*</url-pattern>
> >> > +  </filter-mapping>
> >> >
> >> >   <filter>
> >> >     <filter-name>authFilter</filter-name>
> >> > diff --git
> >> >
> >>
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> >
> >>
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> > index 564b1d6..519bb84 100644
> >> > ---
> >> >
> >>
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> > +++
> >> >
> >>
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> > @@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
> >> >  import com.google.common.base.Joiner;
> >> >  import com.google.common.collect.Maps;
> >> >
> >> > +import com.google.inject.servlet.ServletModule;
> >> > +import com.google.inject.servlet.GuiceFilter;
> >> > +
> >> >  /**
> >> >  * Suite for running the end-to-end tests. The suite is responsible
> for
> >> > starting up and shutting
> >> >  * down the server.
> >> >  */
> >> >  public class EndToEndServer {
> >> >   private static final int JETTY_PORT = 9003;
> >> > +  private static final String ROOT_BASE = "/*";
> >> >   private static final String GADGET_BASE = "/gadgets/ifr";
> >> >   private static final String REST_BASE = "/social/rest/*";
> >> >   private static final String JSON_RPC_BASE = "/social/rpc/*";
> >> > @@ -109,11 +113,16 @@ public class EndToEndServer {
> >> >     newServer.addHandler(resources);
> >> >
> >> >     Context context = new Context(newServer, "/", Context.SESSIONS);
> >> > +
> >> > +      // Add GuiceFilter
> >> > +      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
> >> > +
> >> > +
> >> >     context.addEventListener(new GuiceServletContextListener());
> >> >
> >> >     Map<String, String> initParams = Maps.newHashMap();
> >> >     String modules = Joiner.on(":")
> >> > -        .join(SampleModule.class.getName(),
> >> > DefaultGuiceModule.class.getName(),
> >> > +        .join(ServletModule.class.getName(),
> >> SampleModule.class.getName(),
> >> > DefaultGuiceModule.class.getName(),
> >> >             PropertiesModule.class.getName(),
> >> OAuthModule.class.getName());
> >> >
> >> >     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE,
> >> modules);
> >> > diff --git
> >> >
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> >
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> > index 4e8b468..a409f3b 100644
> >> > ---
> >> >
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> > +++
> >> >
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >> > @@ -36,13 +36,19 @@ import
> >> > org.apache.shindig.social.opensocial.spi.AppDataService;
> >> >  import org.apache.shindig.social.opensocial.spi.PersonService;
> >> >  import org.apache.shindig.social.sample.spi.JsonDbOpensocialService;
> >> >
> >> > +import org.easymock.EasyMock;
> >> > +
> >> >  import com.google.common.collect.ImmutableSet;
> >> >  import com.google.inject.AbstractModule;
> >> >  import com.google.inject.TypeLiteral;
> >> > +import com.google.inject.Provider;
> >> >  import com.google.inject.name.Names;
> >> >
> >> >  import java.util.Set;
> >> >
> >> > +import javax.servlet.http.HttpServletRequest;
> >> > +import javax.servlet.ServletRequest;
> >> > +
> >> >  /**
> >> >  * Provides social api component injection for all large tests
> >> >  */
> >> > @@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
> >> > AbstractModule {
> >> >     bind(Integer.class).annotatedWith(
> >> >         Names.named("shindig.cache.lru.default.capacity"))
> >> >         .toInstance(10);
> >> > +
> >> > +    // Mock HttpServlet / Request Scope
> >> > +    //bindScope(RequestScoped.class, REQUEST);
> >> > +    //bindScope(SessionScoped.class, SESSION);
> >> > +
> >> > +    Provider<HttpServletRequest> requestProvider =
> >> > +        new Provider<HttpServletRequest>() {
> >> > +          public HttpServletRequest get() {
> >> > +              return
> EasyMock.createNiceMock(HttpServletRequest.class);
> >> > +          }
> >> > +
> >> > +          public String toString() {
> >> > +            return "RequestProvider";
> >> > +          }
> >> > +        };
> >> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> >> > +    bind(ServletRequest.class).toProvider(requestProvider);
> >> >   }
> >> >  }
> >> > diff --git
> >> >
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> >
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> > index c49ac1f..e6bef88 100644
> >> > ---
> >> >
> >>
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> > +++
> >> >
> >>
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >> > @@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
> >> >  import com.google.inject.Guice;
> >> >  import com.google.inject.Injector;
> >> >  import com.google.inject.Key;
> >> > +import com.google.inject.Provider;
> >> >  import com.google.inject.TypeLiteral;
> >> > +import com.google.inject.servlet.ServletModule;
> >> >
> >> >  import junit.framework.TestCase;
> >> >
> >> > @@ -33,6 +35,10 @@ import org.easymock.EasyMock;
> >> >
> >> >  import java.util.List;
> >> >
> >> > +import javax.servlet.http.HttpServletRequest;
> >> > +import javax.servlet.ServletRequest;
> >> > +
> >> > +
> >> >  public class SocialApiGuiceModuleTest extends TestCase {
> >> >   private Injector injector;
> >> >
> >> > @@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends
> TestCase
> >> {
> >> >           @Override
> >> >           protected void configure() {
> >> >
> >> >
> >>
> bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
> >> > +            // hack hack
> >> > +    Provider<HttpServletRequest> requestProvider =
> >> > +        new Provider<HttpServletRequest>() {
> >> > +        public HttpServletRequest get() {
> >> > +            return EasyMock.createNiceMock(HttpServletRequest.class);
> >> > +        }
> >> > +
> >> > +        public String toString() {
> >> > +            return "RequestProvider";
> >> > +        }
> >> > +    };
> >> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> >> > +    bind(ServletRequest.class).toProvider(requestProvider);
> >> > +
> >> >           }
> >> >     });
> >> >   }
> >> >
> >>
> >
>

Re: Adding guice-servlet support.

Posted by Adam Winer <aw...@google.com>.
On Mon, Aug 3, 2009 at 5:13 PM, Paul Lindner<li...@inuus.com> wrote:
> yes, that's correct it was only in there to illustrate what how you could
> inject a provider.
> The important part is installing ServletModule, and adding guiceFilter.
>
> Now this could be done independently by an implementor.  However I can see
> where it might be advantageous for internal shindig classes to support
> request scoped injection.

Until we have a direct need in Shindig itself, I see little gained
from forcing installation of ServletModule.

As John points out, ServletModule breaks for injection outside of the
request thread, so a fair number of Shindig classes cannot safely use
request-scoped injection (and that set isn't fixed either).

-- Adam


> On Mon, Aug 3, 2009 at 5:09 PM, Adam Winer <aw...@google.com> wrote:
>
>> The code is OK, but offhand I don't see a reason why any of this needs
>> to live in Shindig.  It could/should go in your implementation of
>> SecurityTokenDecoder.
>>
>> -- Adam
>>
>> On Mon, Aug 3, 2009 at 5:03 PM, Paul Lindner<pl...@linkedin.com> wrote:
>> > Hi,
>> > Some recent work requires me to decode security tokens one way with SSL
>> and
>> > another way without SSL.
>> >
>> > However the SecurityTokenDecoder interface only allows passes a map with
>> the
>> > security token value like so:
>> >
>> >  SecurityToken createToken(Map<String, String> tokenParameters)
>> >      throws SecurityTokenException;
>> >
>> > Because the API does not pass in the HttpServletRequest it's hard to
>> access.
>> >  One method is to define a filter and a thread local, which is fine, but
>> > seems fragile and hackish.
>> >
>> > Enter Guice Servlet, GuiceFilter and friends.
>> >
>> > With this in place getting a request into the a SecurityTokenDecoder is
>> as
>> > simple adding something like this:
>> >
>> >  Provider<HttpServletRequest> reqProvider;
>> >
>> >  @Inject
>> >  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
>> >        this.reqProvider = reqProvider;
>> >    }
>> >
>> >
>> > GuiceFilter takes care of setting up the context.  This may even allow
>> other
>> > idioms that could greatly simplify the code.  For example the AuthInfo
>> > object used by AuthenticationServletFilter, and GadgetRequestContext
>> would
>> > be candidates for injection.  Later on it would be a simple matter of
>> using
>> > the ServletModule to configure servlets and filters based on the
>> > configuration -- no more modifying values in Guice configs and web.xml.
>> >
>> > A very basic patch with an example of injecting HttpRequestServlet into
>> > DefaultSecurityTokenDecoder, and minimal request scope test mocking
>> follows.
>> >  Comments welcome.
>> >
>> > diff --git a/java/common/pom.xml b/java/common/pom.xml
>> > index 2e08b69..4be00b7 100644
>> > --- a/java/common/pom.xml
>> > +++ b/java/common/pom.xml
>> > @@ -87,6 +87,10 @@
>> >       <artifactId>guice</artifactId>
>> >     </dependency>
>> >     <dependency>
>> > +      <groupId>com.google.code.guice</groupId>
>> > +      <artifactId>guice-servlet</artifactId>
>> > +    </dependency>
>> > +    <dependency>
>> >       <groupId>aopalliance</groupId>
>> >       <artifactId>aopalliance</artifactId>
>> >     </dependency>
>> > diff --git
>> >
>> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> >
>> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> > index 22e1c8d..b4e29f1 100644
>> > ---
>> >
>> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> > +++
>> >
>> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
>> > @@ -22,8 +22,11 @@ import org.apache.shindig.config.ContainerConfig;
>> >
>> >  import com.google.inject.Inject;
>> >  import com.google.inject.Singleton;
>> > +import com.google.inject.Provider;
>> > +import com.google.inject.servlet.RequestScoped;
>> >
>> >  import java.util.Map;
>> > +import javax.servlet.http.HttpServletRequest;
>> >
>> >  /**
>> >  * Default implementation of security tokens.  Decides based on default
>> > container configuration
>> > @@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder implements
>> > SecurityTokenDecoder {
>> >
>> >   private final SecurityTokenDecoder decoder;
>> >
>> > + Provider<HttpServletRequest> reqProvider;
>> > +
>> > +  @Inject
>> > +  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
>> > +        this.reqProvider = reqProvider;
>> > +
>> > +    }
>> > +
>> >   @Inject
>> >   public DefaultSecurityTokenDecoder(ContainerConfig config) {
>> >     String tokenType =
>> config.getString(ContainerConfig.DEFAULT_CONTAINER,
>> > SECURITY_TOKEN_TYPE);
>> > @@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder implements
>> > SecurityTokenDecoder {
>> >
>> >   public SecurityToken createToken(Map<String, String> tokenParameters)
>> >       throws SecurityTokenException {
>> > +      System.out.println("found " + reqProvider.get());
>> > +
>> >     return decoder.createToken(tokenParameters);
>> >   }
>> >
>> > diff --git
>> >
>> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> >
>> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> > index a999ef4..f1201db 100644
>> > ---
>> >
>> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> > +++
>> >
>> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
>> > @@ -24,6 +24,7 @@ import com.google.inject.Guice;
>> >  import com.google.inject.Injector;
>> >  import com.google.inject.Module;
>> >  import com.google.inject.Stage;
>> > +import com.google.inject.servlet.ServletModule;
>> >  import com.google.inject.tools.jmx.Manager;
>> >
>> >  import java.util.List;
>> > @@ -45,6 +46,8 @@ public class GuiceServletContextListener implements
>> > ServletContextListener {
>> >     ServletContext context = event.getServletContext();
>> >     String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
>> >     List<Module> modules = Lists.newLinkedList();
>> > +    modules.add(new ServletModule());
>> > +
>> >     if (moduleNames != null) {
>> >       for (String moduleName : moduleNames.split(":")) {
>> >         try {
>> > diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
>> > b/java/server/src/main/webapp/WEB-INF/web.xml
>> > index 9b5b52a..73f0b15 100644
>> > --- a/java/server/src/main/webapp/WEB-INF/web.xml
>> > +++ b/java/server/src/main/webapp/WEB-INF/web.xml
>> > @@ -68,6 +68,15 @@
>> >             </param-value>
>> >         </init-param>
>> >     </filter>
>> > +  <filter>
>> > +    <filter-name>guiceFilter</filter-name>
>> > +    <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
>> > +  </filter>
>> > +
>> > +  <filter-mapping>
>> > +    <filter-name>guiceFilter</filter-name>
>> > +    <url-pattern>/*</url-pattern>
>> > +  </filter-mapping>
>> >
>> >   <filter>
>> >     <filter-name>authFilter</filter-name>
>> > diff --git
>> >
>> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> >
>> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> > index 564b1d6..519bb84 100644
>> > ---
>> >
>> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> > +++
>> >
>> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> > @@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
>> >  import com.google.common.base.Joiner;
>> >  import com.google.common.collect.Maps;
>> >
>> > +import com.google.inject.servlet.ServletModule;
>> > +import com.google.inject.servlet.GuiceFilter;
>> > +
>> >  /**
>> >  * Suite for running the end-to-end tests. The suite is responsible for
>> > starting up and shutting
>> >  * down the server.
>> >  */
>> >  public class EndToEndServer {
>> >   private static final int JETTY_PORT = 9003;
>> > +  private static final String ROOT_BASE = "/*";
>> >   private static final String GADGET_BASE = "/gadgets/ifr";
>> >   private static final String REST_BASE = "/social/rest/*";
>> >   private static final String JSON_RPC_BASE = "/social/rpc/*";
>> > @@ -109,11 +113,16 @@ public class EndToEndServer {
>> >     newServer.addHandler(resources);
>> >
>> >     Context context = new Context(newServer, "/", Context.SESSIONS);
>> > +
>> > +      // Add GuiceFilter
>> > +      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
>> > +
>> > +
>> >     context.addEventListener(new GuiceServletContextListener());
>> >
>> >     Map<String, String> initParams = Maps.newHashMap();
>> >     String modules = Joiner.on(":")
>> > -        .join(SampleModule.class.getName(),
>> > DefaultGuiceModule.class.getName(),
>> > +        .join(ServletModule.class.getName(),
>> SampleModule.class.getName(),
>> > DefaultGuiceModule.class.getName(),
>> >             PropertiesModule.class.getName(),
>> OAuthModule.class.getName());
>> >
>> >     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE,
>> modules);
>> > diff --git
>> >
>> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> >
>> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> > index 4e8b468..a409f3b 100644
>> > ---
>> >
>> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> > +++
>> >
>> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
>> > @@ -36,13 +36,19 @@ import
>> > org.apache.shindig.social.opensocial.spi.AppDataService;
>> >  import org.apache.shindig.social.opensocial.spi.PersonService;
>> >  import org.apache.shindig.social.sample.spi.JsonDbOpensocialService;
>> >
>> > +import org.easymock.EasyMock;
>> > +
>> >  import com.google.common.collect.ImmutableSet;
>> >  import com.google.inject.AbstractModule;
>> >  import com.google.inject.TypeLiteral;
>> > +import com.google.inject.Provider;
>> >  import com.google.inject.name.Names;
>> >
>> >  import java.util.Set;
>> >
>> > +import javax.servlet.http.HttpServletRequest;
>> > +import javax.servlet.ServletRequest;
>> > +
>> >  /**
>> >  * Provides social api component injection for all large tests
>> >  */
>> > @@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
>> > AbstractModule {
>> >     bind(Integer.class).annotatedWith(
>> >         Names.named("shindig.cache.lru.default.capacity"))
>> >         .toInstance(10);
>> > +
>> > +    // Mock HttpServlet / Request Scope
>> > +    //bindScope(RequestScoped.class, REQUEST);
>> > +    //bindScope(SessionScoped.class, SESSION);
>> > +
>> > +    Provider<HttpServletRequest> requestProvider =
>> > +        new Provider<HttpServletRequest>() {
>> > +          public HttpServletRequest get() {
>> > +              return EasyMock.createNiceMock(HttpServletRequest.class);
>> > +          }
>> > +
>> > +          public String toString() {
>> > +            return "RequestProvider";
>> > +          }
>> > +        };
>> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
>> > +    bind(ServletRequest.class).toProvider(requestProvider);
>> >   }
>> >  }
>> > diff --git
>> >
>> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> >
>> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> > index c49ac1f..e6bef88 100644
>> > ---
>> >
>> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> > +++
>> >
>> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
>> > @@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
>> >  import com.google.inject.Guice;
>> >  import com.google.inject.Injector;
>> >  import com.google.inject.Key;
>> > +import com.google.inject.Provider;
>> >  import com.google.inject.TypeLiteral;
>> > +import com.google.inject.servlet.ServletModule;
>> >
>> >  import junit.framework.TestCase;
>> >
>> > @@ -33,6 +35,10 @@ import org.easymock.EasyMock;
>> >
>> >  import java.util.List;
>> >
>> > +import javax.servlet.http.HttpServletRequest;
>> > +import javax.servlet.ServletRequest;
>> > +
>> > +
>> >  public class SocialApiGuiceModuleTest extends TestCase {
>> >   private Injector injector;
>> >
>> > @@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends TestCase
>> {
>> >           @Override
>> >           protected void configure() {
>> >
>> >
>> bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
>> > +            // hack hack
>> > +    Provider<HttpServletRequest> requestProvider =
>> > +        new Provider<HttpServletRequest>() {
>> > +        public HttpServletRequest get() {
>> > +            return EasyMock.createNiceMock(HttpServletRequest.class);
>> > +        }
>> > +
>> > +        public String toString() {
>> > +            return "RequestProvider";
>> > +        }
>> > +    };
>> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
>> > +    bind(ServletRequest.class).toProvider(requestProvider);
>> > +
>> >           }
>> >     });
>> >   }
>> >
>>
>

Re: Adding guice-servlet support.

Posted by Paul Lindner <li...@inuus.com>.
yes, that's correct it was only in there to illustrate what how you could
inject a provider.
The important part is installing ServletModule, and adding guiceFilter.

Now this could be done independently by an implementor.  However I can see
where it might be advantageous for internal shindig classes to support
request scoped injection.


On Mon, Aug 3, 2009 at 5:09 PM, Adam Winer <aw...@google.com> wrote:

> The code is OK, but offhand I don't see a reason why any of this needs
> to live in Shindig.  It could/should go in your implementation of
> SecurityTokenDecoder.
>
> -- Adam
>
> On Mon, Aug 3, 2009 at 5:03 PM, Paul Lindner<pl...@linkedin.com> wrote:
> > Hi,
> > Some recent work requires me to decode security tokens one way with SSL
> and
> > another way without SSL.
> >
> > However the SecurityTokenDecoder interface only allows passes a map with
> the
> > security token value like so:
> >
> >  SecurityToken createToken(Map<String, String> tokenParameters)
> >      throws SecurityTokenException;
> >
> > Because the API does not pass in the HttpServletRequest it's hard to
> access.
> >  One method is to define a filter and a thread local, which is fine, but
> > seems fragile and hackish.
> >
> > Enter Guice Servlet, GuiceFilter and friends.
> >
> > With this in place getting a request into the a SecurityTokenDecoder is
> as
> > simple adding something like this:
> >
> >  Provider<HttpServletRequest> reqProvider;
> >
> >  @Inject
> >  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
> >        this.reqProvider = reqProvider;
> >    }
> >
> >
> > GuiceFilter takes care of setting up the context.  This may even allow
> other
> > idioms that could greatly simplify the code.  For example the AuthInfo
> > object used by AuthenticationServletFilter, and GadgetRequestContext
> would
> > be candidates for injection.  Later on it would be a simple matter of
> using
> > the ServletModule to configure servlets and filters based on the
> > configuration -- no more modifying values in Guice configs and web.xml.
> >
> > A very basic patch with an example of injecting HttpRequestServlet into
> > DefaultSecurityTokenDecoder, and minimal request scope test mocking
> follows.
> >  Comments welcome.
> >
> > diff --git a/java/common/pom.xml b/java/common/pom.xml
> > index 2e08b69..4be00b7 100644
> > --- a/java/common/pom.xml
> > +++ b/java/common/pom.xml
> > @@ -87,6 +87,10 @@
> >       <artifactId>guice</artifactId>
> >     </dependency>
> >     <dependency>
> > +      <groupId>com.google.code.guice</groupId>
> > +      <artifactId>guice-servlet</artifactId>
> > +    </dependency>
> > +    <dependency>
> >       <groupId>aopalliance</groupId>
> >       <artifactId>aopalliance</artifactId>
> >     </dependency>
> > diff --git
> >
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> >
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> > index 22e1c8d..b4e29f1 100644
> > ---
> >
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> > +++
> >
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> > @@ -22,8 +22,11 @@ import org.apache.shindig.config.ContainerConfig;
> >
> >  import com.google.inject.Inject;
> >  import com.google.inject.Singleton;
> > +import com.google.inject.Provider;
> > +import com.google.inject.servlet.RequestScoped;
> >
> >  import java.util.Map;
> > +import javax.servlet.http.HttpServletRequest;
> >
> >  /**
> >  * Default implementation of security tokens.  Decides based on default
> > container configuration
> > @@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder implements
> > SecurityTokenDecoder {
> >
> >   private final SecurityTokenDecoder decoder;
> >
> > + Provider<HttpServletRequest> reqProvider;
> > +
> > +  @Inject
> > +  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
> > +        this.reqProvider = reqProvider;
> > +
> > +    }
> > +
> >   @Inject
> >   public DefaultSecurityTokenDecoder(ContainerConfig config) {
> >     String tokenType =
> config.getString(ContainerConfig.DEFAULT_CONTAINER,
> > SECURITY_TOKEN_TYPE);
> > @@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder implements
> > SecurityTokenDecoder {
> >
> >   public SecurityToken createToken(Map<String, String> tokenParameters)
> >       throws SecurityTokenException {
> > +      System.out.println("found " + reqProvider.get());
> > +
> >     return decoder.createToken(tokenParameters);
> >   }
> >
> > diff --git
> >
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> >
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> > index a999ef4..f1201db 100644
> > ---
> >
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> > +++
> >
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> > @@ -24,6 +24,7 @@ import com.google.inject.Guice;
> >  import com.google.inject.Injector;
> >  import com.google.inject.Module;
> >  import com.google.inject.Stage;
> > +import com.google.inject.servlet.ServletModule;
> >  import com.google.inject.tools.jmx.Manager;
> >
> >  import java.util.List;
> > @@ -45,6 +46,8 @@ public class GuiceServletContextListener implements
> > ServletContextListener {
> >     ServletContext context = event.getServletContext();
> >     String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
> >     List<Module> modules = Lists.newLinkedList();
> > +    modules.add(new ServletModule());
> > +
> >     if (moduleNames != null) {
> >       for (String moduleName : moduleNames.split(":")) {
> >         try {
> > diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
> > b/java/server/src/main/webapp/WEB-INF/web.xml
> > index 9b5b52a..73f0b15 100644
> > --- a/java/server/src/main/webapp/WEB-INF/web.xml
> > +++ b/java/server/src/main/webapp/WEB-INF/web.xml
> > @@ -68,6 +68,15 @@
> >             </param-value>
> >         </init-param>
> >     </filter>
> > +  <filter>
> > +    <filter-name>guiceFilter</filter-name>
> > +    <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
> > +  </filter>
> > +
> > +  <filter-mapping>
> > +    <filter-name>guiceFilter</filter-name>
> > +    <url-pattern>/*</url-pattern>
> > +  </filter-mapping>
> >
> >   <filter>
> >     <filter-name>authFilter</filter-name>
> > diff --git
> >
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> > index 564b1d6..519bb84 100644
> > ---
> >
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> > +++
> >
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> > @@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
> >  import com.google.common.base.Joiner;
> >  import com.google.common.collect.Maps;
> >
> > +import com.google.inject.servlet.ServletModule;
> > +import com.google.inject.servlet.GuiceFilter;
> > +
> >  /**
> >  * Suite for running the end-to-end tests. The suite is responsible for
> > starting up and shutting
> >  * down the server.
> >  */
> >  public class EndToEndServer {
> >   private static final int JETTY_PORT = 9003;
> > +  private static final String ROOT_BASE = "/*";
> >   private static final String GADGET_BASE = "/gadgets/ifr";
> >   private static final String REST_BASE = "/social/rest/*";
> >   private static final String JSON_RPC_BASE = "/social/rpc/*";
> > @@ -109,11 +113,16 @@ public class EndToEndServer {
> >     newServer.addHandler(resources);
> >
> >     Context context = new Context(newServer, "/", Context.SESSIONS);
> > +
> > +      // Add GuiceFilter
> > +      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
> > +
> > +
> >     context.addEventListener(new GuiceServletContextListener());
> >
> >     Map<String, String> initParams = Maps.newHashMap();
> >     String modules = Joiner.on(":")
> > -        .join(SampleModule.class.getName(),
> > DefaultGuiceModule.class.getName(),
> > +        .join(ServletModule.class.getName(),
> SampleModule.class.getName(),
> > DefaultGuiceModule.class.getName(),
> >             PropertiesModule.class.getName(),
> OAuthModule.class.getName());
> >
> >     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE,
> modules);
> > diff --git
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> > index 4e8b468..a409f3b 100644
> > ---
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> > +++
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> > @@ -36,13 +36,19 @@ import
> > org.apache.shindig.social.opensocial.spi.AppDataService;
> >  import org.apache.shindig.social.opensocial.spi.PersonService;
> >  import org.apache.shindig.social.sample.spi.JsonDbOpensocialService;
> >
> > +import org.easymock.EasyMock;
> > +
> >  import com.google.common.collect.ImmutableSet;
> >  import com.google.inject.AbstractModule;
> >  import com.google.inject.TypeLiteral;
> > +import com.google.inject.Provider;
> >  import com.google.inject.name.Names;
> >
> >  import java.util.Set;
> >
> > +import javax.servlet.http.HttpServletRequest;
> > +import javax.servlet.ServletRequest;
> > +
> >  /**
> >  * Provides social api component injection for all large tests
> >  */
> > @@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
> > AbstractModule {
> >     bind(Integer.class).annotatedWith(
> >         Names.named("shindig.cache.lru.default.capacity"))
> >         .toInstance(10);
> > +
> > +    // Mock HttpServlet / Request Scope
> > +    //bindScope(RequestScoped.class, REQUEST);
> > +    //bindScope(SessionScoped.class, SESSION);
> > +
> > +    Provider<HttpServletRequest> requestProvider =
> > +        new Provider<HttpServletRequest>() {
> > +          public HttpServletRequest get() {
> > +              return EasyMock.createNiceMock(HttpServletRequest.class);
> > +          }
> > +
> > +          public String toString() {
> > +            return "RequestProvider";
> > +          }
> > +        };
> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> > +    bind(ServletRequest.class).toProvider(requestProvider);
> >   }
> >  }
> > diff --git
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> > index c49ac1f..e6bef88 100644
> > ---
> >
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> > +++
> >
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> > @@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
> >  import com.google.inject.Guice;
> >  import com.google.inject.Injector;
> >  import com.google.inject.Key;
> > +import com.google.inject.Provider;
> >  import com.google.inject.TypeLiteral;
> > +import com.google.inject.servlet.ServletModule;
> >
> >  import junit.framework.TestCase;
> >
> > @@ -33,6 +35,10 @@ import org.easymock.EasyMock;
> >
> >  import java.util.List;
> >
> > +import javax.servlet.http.HttpServletRequest;
> > +import javax.servlet.ServletRequest;
> > +
> > +
> >  public class SocialApiGuiceModuleTest extends TestCase {
> >   private Injector injector;
> >
> > @@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends TestCase
> {
> >           @Override
> >           protected void configure() {
> >
> >
> bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
> > +            // hack hack
> > +    Provider<HttpServletRequest> requestProvider =
> > +        new Provider<HttpServletRequest>() {
> > +        public HttpServletRequest get() {
> > +            return EasyMock.createNiceMock(HttpServletRequest.class);
> > +        }
> > +
> > +        public String toString() {
> > +            return "RequestProvider";
> > +        }
> > +    };
> > +    bind(HttpServletRequest.class).toProvider(requestProvider);
> > +    bind(ServletRequest.class).toProvider(requestProvider);
> > +
> >           }
> >     });
> >   }
> >
>

Re: Adding guice-servlet support.

Posted by Adam Winer <aw...@google.com>.
The code is OK, but offhand I don't see a reason why any of this needs
to live in Shindig.  It could/should go in your implementation of
SecurityTokenDecoder.

-- Adam

On Mon, Aug 3, 2009 at 5:03 PM, Paul Lindner<pl...@linkedin.com> wrote:
> Hi,
> Some recent work requires me to decode security tokens one way with SSL and
> another way without SSL.
>
> However the SecurityTokenDecoder interface only allows passes a map with the
> security token value like so:
>
>  SecurityToken createToken(Map<String, String> tokenParameters)
>      throws SecurityTokenException;
>
> Because the API does not pass in the HttpServletRequest it's hard to access.
>  One method is to define a filter and a thread local, which is fine, but
> seems fragile and hackish.
>
> Enter Guice Servlet, GuiceFilter and friends.
>
> With this in place getting a request into the a SecurityTokenDecoder is as
> simple adding something like this:
>
>  Provider<HttpServletRequest> reqProvider;
>
>  @Inject
>  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
>        this.reqProvider = reqProvider;
>    }
>
>
> GuiceFilter takes care of setting up the context.  This may even allow other
> idioms that could greatly simplify the code.  For example the AuthInfo
> object used by AuthenticationServletFilter, and GadgetRequestContext would
> be candidates for injection.  Later on it would be a simple matter of using
> the ServletModule to configure servlets and filters based on the
> configuration -- no more modifying values in Guice configs and web.xml.
>
> A very basic patch with an example of injecting HttpRequestServlet into
> DefaultSecurityTokenDecoder, and minimal request scope test mocking follows.
>  Comments welcome.
>
> diff --git a/java/common/pom.xml b/java/common/pom.xml
> index 2e08b69..4be00b7 100644
> --- a/java/common/pom.xml
> +++ b/java/common/pom.xml
> @@ -87,6 +87,10 @@
>       <artifactId>guice</artifactId>
>     </dependency>
>     <dependency>
> +      <groupId>com.google.code.guice</groupId>
> +      <artifactId>guice-servlet</artifactId>
> +    </dependency>
> +    <dependency>
>       <groupId>aopalliance</groupId>
>       <artifactId>aopalliance</artifactId>
>     </dependency>
> diff --git
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> index 22e1c8d..b4e29f1 100644
> ---
> a/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> +++
> b/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenDecoder.java
> @@ -22,8 +22,11 @@ import org.apache.shindig.config.ContainerConfig;
>
>  import com.google.inject.Inject;
>  import com.google.inject.Singleton;
> +import com.google.inject.Provider;
> +import com.google.inject.servlet.RequestScoped;
>
>  import java.util.Map;
> +import javax.servlet.http.HttpServletRequest;
>
>  /**
>  * Default implementation of security tokens.  Decides based on default
> container configuration
> @@ -47,6 +50,14 @@ public class DefaultSecurityTokenDecoder implements
> SecurityTokenDecoder {
>
>   private final SecurityTokenDecoder decoder;
>
> + Provider<HttpServletRequest> reqProvider;
> +
> +  @Inject
> +  public void setReqProvider(Provider<HttpServletRequest> reqProvider) {
> +        this.reqProvider = reqProvider;
> +
> +    }
> +
>   @Inject
>   public DefaultSecurityTokenDecoder(ContainerConfig config) {
>     String tokenType = config.getString(ContainerConfig.DEFAULT_CONTAINER,
> SECURITY_TOKEN_TYPE);
> @@ -63,6 +74,8 @@ public class DefaultSecurityTokenDecoder implements
> SecurityTokenDecoder {
>
>   public SecurityToken createToken(Map<String, String> tokenParameters)
>       throws SecurityTokenException {
> +      System.out.println("found " + reqProvider.get());
> +
>     return decoder.createToken(tokenParameters);
>   }
>
> diff --git
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> index a999ef4..f1201db 100644
> ---
> a/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> +++
> b/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
> @@ -24,6 +24,7 @@ import com.google.inject.Guice;
>  import com.google.inject.Injector;
>  import com.google.inject.Module;
>  import com.google.inject.Stage;
> +import com.google.inject.servlet.ServletModule;
>  import com.google.inject.tools.jmx.Manager;
>
>  import java.util.List;
> @@ -45,6 +46,8 @@ public class GuiceServletContextListener implements
> ServletContextListener {
>     ServletContext context = event.getServletContext();
>     String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
>     List<Module> modules = Lists.newLinkedList();
> +    modules.add(new ServletModule());
> +
>     if (moduleNames != null) {
>       for (String moduleName : moduleNames.split(":")) {
>         try {
> diff --git a/java/server/src/main/webapp/WEB-INF/web.xml
> b/java/server/src/main/webapp/WEB-INF/web.xml
> index 9b5b52a..73f0b15 100644
> --- a/java/server/src/main/webapp/WEB-INF/web.xml
> +++ b/java/server/src/main/webapp/WEB-INF/web.xml
> @@ -68,6 +68,15 @@
>             </param-value>
>         </init-param>
>     </filter>
> +  <filter>
> +    <filter-name>guiceFilter</filter-name>
> +    <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
> +  </filter>
> +
> +  <filter-mapping>
> +    <filter-name>guiceFilter</filter-name>
> +    <url-pattern>/*</url-pattern>
> +  </filter-mapping>
>
>   <filter>
>     <filter-name>authFilter</filter-name>
> diff --git
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> index 564b1d6..519bb84 100644
> ---
> a/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> +++
> b/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> @@ -52,12 +52,16 @@ import javax.servlet.http.HttpServletResponse;
>  import com.google.common.base.Joiner;
>  import com.google.common.collect.Maps;
>
> +import com.google.inject.servlet.ServletModule;
> +import com.google.inject.servlet.GuiceFilter;
> +
>  /**
>  * Suite for running the end-to-end tests. The suite is responsible for
> starting up and shutting
>  * down the server.
>  */
>  public class EndToEndServer {
>   private static final int JETTY_PORT = 9003;
> +  private static final String ROOT_BASE = "/*";
>   private static final String GADGET_BASE = "/gadgets/ifr";
>   private static final String REST_BASE = "/social/rest/*";
>   private static final String JSON_RPC_BASE = "/social/rpc/*";
> @@ -109,11 +113,16 @@ public class EndToEndServer {
>     newServer.addHandler(resources);
>
>     Context context = new Context(newServer, "/", Context.SESSIONS);
> +
> +      // Add GuiceFilter
> +      context.addFilter(GuiceFilter.class, ROOT_BASE, 0);
> +
> +
>     context.addEventListener(new GuiceServletContextListener());
>
>     Map<String, String> initParams = Maps.newHashMap();
>     String modules = Joiner.on(":")
> -        .join(SampleModule.class.getName(),
> DefaultGuiceModule.class.getName(),
> +        .join(ServletModule.class.getName(), SampleModule.class.getName(),
> DefaultGuiceModule.class.getName(),
>             PropertiesModule.class.getName(), OAuthModule.class.getName());
>
>     initParams.put(GuiceServletContextListener.MODULES_ATTRIBUTE, modules);
> diff --git
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> index 4e8b468..a409f3b 100644
> ---
> a/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> +++
> b/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java
> @@ -36,13 +36,19 @@ import
> org.apache.shindig.social.opensocial.spi.AppDataService;
>  import org.apache.shindig.social.opensocial.spi.PersonService;
>  import org.apache.shindig.social.sample.spi.JsonDbOpensocialService;
>
> +import org.easymock.EasyMock;
> +
>  import com.google.common.collect.ImmutableSet;
>  import com.google.inject.AbstractModule;
>  import com.google.inject.TypeLiteral;
> +import com.google.inject.Provider;
>  import com.google.inject.name.Names;
>
>  import java.util.Set;
>
> +import javax.servlet.http.HttpServletRequest;
> +import javax.servlet.ServletRequest;
> +
>  /**
>  * Provides social api component injection for all large tests
>  */
> @@ -79,5 +85,22 @@ public class SocialApiTestsGuiceModule extends
> AbstractModule {
>     bind(Integer.class).annotatedWith(
>         Names.named("shindig.cache.lru.default.capacity"))
>         .toInstance(10);
> +
> +    // Mock HttpServlet / Request Scope
> +    //bindScope(RequestScoped.class, REQUEST);
> +    //bindScope(SessionScoped.class, SESSION);
> +
> +    Provider<HttpServletRequest> requestProvider =
> +        new Provider<HttpServletRequest>() {
> +          public HttpServletRequest get() {
> +              return EasyMock.createNiceMock(HttpServletRequest.class);
> +          }
> +
> +          public String toString() {
> +            return "RequestProvider";
> +          }
> +        };
> +    bind(HttpServletRequest.class).toProvider(requestProvider);
> +    bind(ServletRequest.class).toProvider(requestProvider);
>   }
>  }
> diff --git
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> index c49ac1f..e6bef88 100644
> ---
> a/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> +++
> b/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java
> @@ -21,7 +21,9 @@ import com.google.inject.AbstractModule;
>  import com.google.inject.Guice;
>  import com.google.inject.Injector;
>  import com.google.inject.Key;
> +import com.google.inject.Provider;
>  import com.google.inject.TypeLiteral;
> +import com.google.inject.servlet.ServletModule;
>
>  import junit.framework.TestCase;
>
> @@ -33,6 +35,10 @@ import org.easymock.EasyMock;
>
>  import java.util.List;
>
> +import javax.servlet.http.HttpServletRequest;
> +import javax.servlet.ServletRequest;
> +
> +
>  public class SocialApiGuiceModuleTest extends TestCase {
>   private Injector injector;
>
> @@ -43,6 +49,20 @@ public class SocialApiGuiceModuleTest extends TestCase {
>           @Override
>           protected void configure() {
>
> bind(OAuthDataStore.class).toInstance(EasyMock.createMock(OAuthDataStore.class));
> +            // hack hack
> +    Provider<HttpServletRequest> requestProvider =
> +        new Provider<HttpServletRequest>() {
> +        public HttpServletRequest get() {
> +            return EasyMock.createNiceMock(HttpServletRequest.class);
> +        }
> +
> +        public String toString() {
> +            return "RequestProvider";
> +        }
> +    };
> +    bind(HttpServletRequest.class).toProvider(requestProvider);
> +    bind(ServletRequest.class).toProvider(requestProvider);
> +
>           }
>     });
>   }
>