For now, because the req is not injectable I was thinking it would be: (authInfo gets injected in the constructor of JsonRpcServlet lets say) authInfo.getSecurityToken(req);
Then you can mock out authInfo when testing to return whatever you want by just stuffing a stub class in the constructor. - Cassie On Tue, Sep 2, 2008 at 4:35 PM, Kevin Brown <[EMAIL PROTECTED]> wrote: > This doesn't give any obvious improvement to testability, since the code is > still executed inline and can't be modified by the callers. I don't see > how: > > new AuthInfo(req).getSecurityToken() > > is any different from: > > AuthInfo.getSecurityToken(req); > > If AuthInfo was RequestScoped or at least passed to relevant ctors here, > that would certainly be an improvement, but here it doesn't seem to be > changing anything. > > On Tue, Sep 2, 2008 at 4:17 PM, <[EMAIL PROTECTED]> wrote: > > > Author: evan > > Date: Tue Sep 2 16:17:39 2008 > > New Revision: 691426 > > > > URL: http://svn.apache.org/viewvc?rev=691426&view=rev > > Log: > > AuthInfo is now instance based for testability and injectability. > > > > Modified: > > > > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthInfo.java > > > > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java > > > > > > incubator/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/AuthInfoTest.java > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpGadgetContext.java > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > > > > > > incubator/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ApiServlet.java > > > > Modified: > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthInfo.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthInfo.java?rev=691426&r1=691425&r2=691426&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthInfo.java > > (original) > > +++ > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthInfo.java > > Tue Sep 2 16:17:39 2008 > > @@ -17,16 +17,33 @@ > > */ > > package org.apache.shindig.auth; > > > > +import com.google.inject.Inject; > > + > > import javax.servlet.http.HttpServletRequest; > > > > /** > > - * Class to get/set authorization information on a servlet request. > > - * Used by auth filters. > > + * Class to get authorization information on a servlet request. > > + * > > + * Information is set by adding an AuthentiationServletFilter, and there > > + * is no way to set in a public API. This can be added in the future for > > testing > > + * purposes. > > */ > > public class AuthInfo { > > + private HttpServletRequest req; > > + > > + /** > > + * Create AuthInfo from a given HttpServletRequest > > + * @param req > > + */ > > + @Inject > > + public AuthInfo(HttpServletRequest req) { > > + this.req = req; > > + } > > > > /** > > * Constants for request attribute keys > > + * > > + * This is only public for testing. > > */ > > public enum Attribute { > > /** The security token */ > > @@ -42,41 +59,41 @@ > > /** > > * Get the security token for this request. > > * > > - * @param req The request > > * @return The security token > > */ > > - public static SecurityToken getSecurityToken(HttpServletRequest req) { > > + public SecurityToken getSecurityToken() { > > return getRequestAttribute(req, Attribute.SECURITY_TOKEN); > > } > > > > /** > > * Get the hosted domain for this request. > > * > > - * @param req The request > > * @return The domain, or [EMAIL PROTECTED] null} if no domain was found > > */ > > - public static String getAuthType(HttpServletRequest req) { > > + public String getAuthType() { > > return getRequestAttribute(req, Attribute.AUTH_TYPE); > > } > > > > /** > > * Set the security token for the request. > > * > > - * @param req The request > > * @param token The security token > > + * @return This object > > */ > > - public static void setSecurityToken(HttpServletRequest req, > > SecurityToken token) { > > + AuthInfo setSecurityToken(SecurityToken token) { > > setRequestAttribute(req, Attribute.SECURITY_TOKEN, token); > > + return this; > > } > > > > /** > > * Set the auth type for the request. > > * > > - * @param req The request > > * @param authType The named auth type > > + * @return This object > > */ > > - public static void setAuthType(HttpServletRequest req, String > authType) > > { > > + AuthInfo setAuthType(String authType) { > > setRequestAttribute(req, Attribute.AUTH_TYPE, authType); > > + return this; > > } > > > > /** > > > > Modified: > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java?rev=691426&r1=691425&r2=691426&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java > > (original) > > +++ > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java > > Tue Sep 2 16:17:39 2008 > > @@ -62,8 +62,7 @@ > > for (AuthenticationHandler handler : handlers) { > > SecurityToken token = handler.getSecurityTokenFromRequest(req); > > if (token != null) { > > - AuthInfo.setAuthType(req, handler.getName()); > > - AuthInfo.setSecurityToken(req, token); > > + new > > AuthInfo(req).setAuthType(handler.getName()).setSecurityToken(token); > > chain.doFilter(req, response); > > return; > > } > > > > Modified: > > > incubator/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/AuthInfoTest.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/AuthInfoTest.java?rev=691426&r1=691425&r2=691426&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/AuthInfoTest.java > > (original) > > +++ > > > incubator/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/AuthInfoTest.java > > Tue Sep 2 16:17:39 2008 > > @@ -17,6 +17,10 @@ > > */ > > package org.apache.shindig.auth; > > > > +import com.google.inject.AbstractModule; > > +import com.google.inject.Guice; > > +import com.google.inject.Injector; > > + > > import org.apache.shindig.common.testing.FakeGadgetToken; > > import org.apache.shindig.common.testing.FakeHttpServletRequest; > > > > @@ -29,13 +33,45 @@ > > public void testToken() throws Exception { > > HttpServletRequest req = new FakeHttpServletRequest(); > > SecurityToken token = new FakeGadgetToken(); > > - AuthInfo.setSecurityToken(req, token); > > - assertEquals(token, AuthInfo.getSecurityToken(req)); > > + > > + AuthInfo info = new AuthInfo(req).setSecurityToken(token); > > + > > + assertEquals(token, info.getSecurityToken()); > > + // This should work when creating a new AuthInfo from the same > request > > + assertEquals(token, new AuthInfo(req).getSecurityToken()); > > } > > > > public void testAuthType() throws Exception { > > HttpServletRequest req = new FakeHttpServletRequest(); > > - AuthInfo.setAuthType(req, "FakeAuth"); > > - assertEquals("FakeAuth", AuthInfo.getAuthType(req)); > > + > > + AuthInfo info = new AuthInfo(req).setAuthType("FakeAuth"); > > + > > + assertEquals("FakeAuth", info.getAuthType()); > > + // This should work when creating a new AuthInfo from the same > request > > + assertEquals("FakeAuth", new AuthInfo(req).getAuthType()); > > + } > > + > > + public void testBinding() throws Exception { > > + HttpServletRequest req = new FakeHttpServletRequest(); > > + SecurityToken token = new FakeGadgetToken(); > > + new AuthInfo(req).setSecurityToken(token).setAuthType("FakeAuth"); > > + > > + Injector injector = Guice.createInjector(new TestModule(req)); > > + AuthInfo injected = injector.getInstance(AuthInfo.class); > > + assertEquals(token, injected.getSecurityToken()); > > + assertEquals("FakeAuth", injected.getAuthType()); > > + } > > + > > + private static class TestModule extends AbstractModule { > > + private HttpServletRequest req; > > + > > + public TestModule(HttpServletRequest req) { > > + this.req = req; > > + } > > + @Override > > + protected void configure() { > > + bind(HttpServletRequest.class).toInstance(req); > > + } > > + > > } > > } > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpGadgetContext.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpGadgetContext.java?rev=691426&r1=691425&r2=691426&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpGadgetContext.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpGadgetContext.java > > Tue Sep 2 16:17:39 2008 > > @@ -121,7 +121,7 @@ > > > > @Override > > public SecurityToken getToken() { > > - return AuthInfo.getSecurityToken(request); > > + return new AuthInfo(request).getSecurityToken(); > > } > > > > @Override > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java?rev=691426&r1=691425&r2=691426&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > > Tue Sep 2 16:17:39 2008 > > @@ -235,7 +235,7 @@ > > * @return A valid token for the given input. > > */ > > private SecurityToken extractAndValidateToken(HttpServletRequest > request) > > throws GadgetException { > > - SecurityToken token = AuthInfo.getSecurityToken(request); > > + SecurityToken token = new AuthInfo(request).getSecurityToken(); > > if (token == null) { > > throw new > > GadgetException(GadgetException.Code.INVALID_SECURITY_TOKEN); > > } > > > > Modified: > > > incubator/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ApiServlet.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ApiServlet.java?rev=691426&r1=691425&r2=691426&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ApiServlet.java > > (original) > > +++ > > > incubator/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ApiServlet.java > > Tue Sep 2 16:17:39 2008 > > @@ -66,7 +66,7 @@ > > } > > > > protected SecurityToken getSecurityToken(HttpServletRequest > > servletRequest) { > > - return AuthInfo.getSecurityToken(servletRequest); > > + return new AuthInfo(servletRequest).getSecurityToken(); > > } > > > > protected abstract void sendError(HttpServletResponse servletResponse, > > ResponseItem responseItem) > > > > > > >
