You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Kevin Brown <et...@google.com> on 2008/09/03 01:35:44 UTC
Re: svn commit: r691426 - in /incubator/shindig/trunk/java: common/src/main/java/org/apache/shindig/auth/ common/src/test/java/org/apache/shindig/auth/ gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ social-api/src/main/java/org/apache/shin
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, <ev...@apache.org> 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 {@code 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)
>
>
>
Re: svn commit: r691426 - in /incubator/shindig/trunk/java:
common/src/main/java/org/apache/shindig/auth/ common/src/test/java/org/apache/shindig/auth/
gadgets/src/main/java/org/apache/shindig/gadgets/servlet/
social-api/src/main/java/org/apache/shin
Posted by Evan Gilbert <ui...@google.com>.
Yes - this is just preparation for a request scoped AuthInfo.
I believe we should start modifying most classes to not use
HttpServletRequest - classes should be injecting the specific objects they
need. This will cut down on a lot of dependencies and help avoid needing to
mock out HttpServletRequest. Currently for many classes to function
properly, we need to orchestrate a complex set of interactions on the
HttpServletRequest.
On Tue, Sep 2, 2008 at 4:42 PM, Cassie <do...@apache.org> wrote:
> 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 <et...@google.com> 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, <ev...@apache.org> 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 {@code 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)
> > >
> > >
> > >
> >
>
Re: svn commit: r691426 - in /incubator/shindig/trunk/java: common/src/main/java/org/apache/shindig/auth/ common/src/test/java/org/apache/shindig/auth/ gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ social-api/src/main/java/org/apache/shin
Posted by Cassie <do...@apache.org>.
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 <et...@google.com> 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, <ev...@apache.org> 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 {@code 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)
> >
> >
> >
>