From dd326e35bdd1fdc797d0a54b51aaab8e361766c2 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 27 Jul 2009 20:34:28 -0700 Subject: [PATCH] Use Guice injection to pass GerritServer to HttpServlets If an HttpServlet requires the GerritServer instance to do its work (and most do) we can pass it through the constructor by way of Guice rather than trying to obtain it during the init method. Signed-off-by: Shawn O. Pearce --- .../server/BecomeAnyAccountLoginServlet.java | 22 ++----- .../com/google/gerrit/server/CatServlet.java | 20 +++---- .../google/gerrit/server/GerritServer.java | 2 + .../gerrit/server/GerritServletConfig.java | 59 +++++++++++++++++-- .../google/gerrit/server/HostPageServlet.java | 18 +++--- .../google/gerrit/server/StaticServlet.java | 26 ++------ .../gerrit/server/UrlRewriteFilter.java | 10 +--- 7 files changed, 86 insertions(+), 71 deletions(-) diff --git a/src/main/java/com/google/gerrit/server/BecomeAnyAccountLoginServlet.java b/src/main/java/com/google/gerrit/server/BecomeAnyAccountLoginServlet.java index 7c87f53720..91a2d88808 100644 --- a/src/main/java/com/google/gerrit/server/BecomeAnyAccountLoginServlet.java +++ b/src/main/java/com/google/gerrit/server/BecomeAnyAccountLoginServlet.java @@ -18,16 +18,14 @@ import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.reviewdb.Account; import com.google.gerrit.client.reviewdb.ReviewDb; import com.google.gerrit.client.rpc.Common; -import com.google.gwtjsonrpc.server.XsrfException; import com.google.gwtorm.client.OrmException; +import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; import java.util.Collections; import java.util.List; -import javax.servlet.ServletConfig; -import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServlet; @@ -45,21 +43,13 @@ public class BecomeAnyAccountLoginServlet extends HttpServlet { } } - private boolean allowed; - private GerritServer server; + private final boolean allowed; + private final GerritServer server; - @Override - public void init(final ServletConfig config) throws ServletException { - super.init(config); + @Inject + BecomeAnyAccountLoginServlet(final GerritServer gs) { + server = gs; allowed = isAllowed(); - - try { - server = GerritServer.getInstance(); - } catch (OrmException e) { - throw new ServletException("Cannot load GerritServer", e); - } catch (XsrfException e) { - throw new ServletException("Cannot load GerritServer", e); - } } @Override diff --git a/src/main/java/com/google/gerrit/server/CatServlet.java b/src/main/java/com/google/gerrit/server/CatServlet.java index b2b2dd56e0..1fe2a2c00b 100644 --- a/src/main/java/com/google/gerrit/server/CatServlet.java +++ b/src/main/java/com/google/gerrit/server/CatServlet.java @@ -24,8 +24,8 @@ import com.google.gerrit.client.reviewdb.PatchSet; import com.google.gerrit.client.reviewdb.Project; import com.google.gerrit.client.reviewdb.ReviewDb; import com.google.gerrit.client.rpc.Common; -import com.google.gwtjsonrpc.server.XsrfException; import com.google.gwtorm.client.OrmException; +import com.google.inject.Inject; import com.google.inject.Singleton; import eu.medsea.mimeutil.MimeType; @@ -66,21 +66,19 @@ import javax.servlet.http.HttpServletResponse; @Singleton public class CatServlet extends HttpServlet { private static final MimeType ZIP = new MimeType("application/zip"); - private GerritServer server; - private SecureRandom rng; + private final GerritServer server; + private final SecureRandom rng; private FileTypeRegistry registry; + @Inject + CatServlet(final GerritServer gs) { + server = gs; + rng = new SecureRandom(); + } + @Override public void init(final ServletConfig config) throws ServletException { super.init(config); - try { - server = GerritServer.getInstance(); - } catch (OrmException e) { - throw new ServletException("Cannot load GerritServer", e); - } catch (XsrfException e) { - throw new ServletException("Cannot load GerritServer", e); - } - rng = new SecureRandom(); registry = FileTypeRegistry.getInstance(); } diff --git a/src/main/java/com/google/gerrit/server/GerritServer.java b/src/main/java/com/google/gerrit/server/GerritServer.java index 05ab2b3c4d..10defa8773 100644 --- a/src/main/java/com/google/gerrit/server/GerritServer.java +++ b/src/main/java/com/google/gerrit/server/GerritServer.java @@ -48,6 +48,7 @@ import com.google.gwtorm.client.OrmException; import com.google.gwtorm.client.Transaction; import com.google.gwtorm.jdbc.Database; import com.google.gwtorm.jdbc.SimpleDataSource; +import com.google.inject.Singleton; import net.sf.ehcache.Cache; import net.sf.ehcache.CacheManager; @@ -95,6 +96,7 @@ import javax.servlet.http.HttpServletRequest; import javax.sql.DataSource; /** Global server-side state for Gerrit. */ +@Singleton public class GerritServer { private static final Logger log = LoggerFactory.getLogger(GerritServer.class); private static DataSource datasource; diff --git a/src/main/java/com/google/gerrit/server/GerritServletConfig.java b/src/main/java/com/google/gerrit/server/GerritServletConfig.java index 629f27bfd3..a8e6c3e115 100644 --- a/src/main/java/com/google/gerrit/server/GerritServletConfig.java +++ b/src/main/java/com/google/gerrit/server/GerritServletConfig.java @@ -15,16 +15,25 @@ package com.google.gerrit.server; import com.google.gerrit.server.ssh.SshServlet; +import com.google.gwtjsonrpc.server.XsrfException; +import com.google.gwtorm.client.OrmException; +import com.google.inject.AbstractModule; +import com.google.inject.ConfigurationException; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.Module; import com.google.inject.servlet.GuiceServletContextListener; import com.google.inject.servlet.ServletModule; +import java.security.ProviderException; + +import javax.servlet.ServletContextEvent; + /** Configures the web application environment for Gerrit Code Review. */ public class GerritServletConfig extends GuiceServletContextListener { - @Override - protected Injector getInjector() { - return Guice.createInjector(new ServletModule() { + private static Module createServletModule() { + return new ServletModule() { @Override protected void configureServlets() { filter("/*").through(UrlRewriteFilter.class); @@ -65,6 +74,48 @@ public class GerritServletConfig extends GuiceServletContextListener { private void rpc(String name, Class clazz) { serve("/gerrit/rpc/" + name).with(clazz); } - }); + }; + } + + private static Module createDatabaseModule() { + return new AbstractModule() { + @Override + protected void configure() { + try { + bind(GerritServer.class).toInstance(GerritServer.getInstance(true)); + } catch (OrmException e) { + addError(e); + } catch (XsrfException e) { + addError(e); + } + } + }; + } + + private final Injector injector = + Guice.createInjector(createDatabaseModule(), createServletModule()); + + @Override + protected Injector getInjector() { + return injector; + } + + @Override + public void contextInitialized(final ServletContextEvent event) { + super.contextInitialized(event); + } + + @Override + public void contextDestroyed(final ServletContextEvent event) { + try { + final GerritServer gs = injector.getInstance(Key.get(GerritServer.class)); + gs.closeDataSource(); + } catch (ConfigurationException ce) { + // Assume it never started. + } catch (ProviderException ce) { + // Assume it never started. + } + + super.contextDestroyed(event); } } diff --git a/src/main/java/com/google/gerrit/server/HostPageServlet.java b/src/main/java/com/google/gerrit/server/HostPageServlet.java index 3d3abcaf99..98b7b5ca29 100644 --- a/src/main/java/com/google/gerrit/server/HostPageServlet.java +++ b/src/main/java/com/google/gerrit/server/HostPageServlet.java @@ -18,8 +18,7 @@ import com.google.gerrit.client.data.GerritConfig; import com.google.gerrit.client.reviewdb.Account; import com.google.gerrit.client.rpc.Common; import com.google.gwt.user.server.rpc.RPCServletUtils; -import com.google.gwtjsonrpc.server.XsrfException; -import com.google.gwtorm.client.OrmException; +import com.google.inject.Inject; import com.google.inject.Singleton; import org.spearce.jgit.lib.Constants; @@ -44,23 +43,20 @@ import javax.servlet.http.HttpServletResponse; @SuppressWarnings("serial") @Singleton public class HostPageServlet extends HttpServlet { - private GerritServer server; + private final GerritServer server; private String canonicalUrl; private boolean wantSSL; private Document hostDoc; + @Inject + HostPageServlet(final GerritServer gs) { + server = gs; + } + @Override public void init(ServletConfig config) throws ServletException { super.init(config); - try { - server = GerritServer.getInstance(); - } catch (OrmException e) { - throw new ServletException("Cannot load GerritServer", e); - } catch (XsrfException e) { - throw new ServletException("Cannot load GerritServer", e); - } - final File sitePath = server.getSitePath(); canonicalUrl = server.getCanonicalURL(); wantSSL = canonicalUrl != null && canonicalUrl.startsWith("https:"); diff --git a/src/main/java/com/google/gerrit/server/StaticServlet.java b/src/main/java/com/google/gerrit/server/StaticServlet.java index 8cb1e59fae..d4fa8cbc88 100644 --- a/src/main/java/com/google/gerrit/server/StaticServlet.java +++ b/src/main/java/com/google/gerrit/server/StaticServlet.java @@ -15,8 +15,7 @@ package com.google.gerrit.server; import com.google.gwt.user.server.rpc.RPCServletUtils; -import com.google.gwtjsonrpc.server.XsrfException; -import com.google.gwtorm.client.OrmException; +import com.google.inject.Inject; import com.google.inject.Singleton; import org.spearce.jgit.util.NB; @@ -29,8 +28,6 @@ import java.io.OutputStream; import java.util.HashMap; import java.util.zip.GZIPOutputStream; -import javax.servlet.ServletConfig; -import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -90,22 +87,11 @@ public class StaticServlet extends HttpServlet { return out.toByteArray(); } - private File staticBase; - - @Override - public void init(final ServletConfig config) throws ServletException { - super.init(config); - - final GerritServer srv; - try { - srv = GerritServer.getInstance(); - } catch (OrmException e) { - throw new ServletException("Cannot load GerritServer", e); - } catch (XsrfException e) { - throw new ServletException("Cannot load GerritServer", e); - } - - final File p = srv.getSitePath(); + private final File staticBase; + + @Inject + StaticServlet(final GerritServer gs) { + final File p = gs.getSitePath(); staticBase = p != null ? new File(p, "static") : null; } diff --git a/src/main/java/com/google/gerrit/server/UrlRewriteFilter.java b/src/main/java/com/google/gerrit/server/UrlRewriteFilter.java index 09d35fd074..5459f807c6 100644 --- a/src/main/java/com/google/gerrit/server/UrlRewriteFilter.java +++ b/src/main/java/com/google/gerrit/server/UrlRewriteFilter.java @@ -20,7 +20,6 @@ import com.google.gerrit.client.reviewdb.Change; import com.google.gerrit.client.reviewdb.RevId; import com.google.gerrit.client.reviewdb.ReviewDb; import com.google.gerrit.client.rpc.Common; -import com.google.gwtjsonrpc.server.XsrfException; import com.google.gwtorm.client.OrmException; import com.google.inject.Singleton; @@ -72,15 +71,8 @@ public class UrlRewriteFilter implements Filter { private FilterConfig config; - public void init(final FilterConfig config) throws ServletException { + public void init(final FilterConfig config) { this.config = config; - try { - GerritServer.getInstance(); - } catch (OrmException e) { - throw new ServletException("Cannot initialize GerritServer", e); - } catch (XsrfException e) { - throw new ServletException("Cannot initialize GerritServer", e); - } } public void destroy() {