Inject the canonicalweburl rather than using GerritServer

This avoids the dependency on the catch-all kitchen sink of the
GerritServer object, making it available only to those blocks of
code that want to display a URL.

Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/git/MergeOp.java

@@ -28,6 +28,7 @@ import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.GerritServer;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.mail.MergeFailSender;
 import com.google.gerrit.server.mail.MergedSender;
@@ -107,6 +108,8 @@ public class MergeOp {
   private final ReplicationQueue replication;
   private final MergedSender.Factory mergedSenderFactory;
   private final MergeFailSender.Factory mergeFailSenderFactory;
+  private final String canonicalWebUrl;
+
   private final PersonIdent myIdent;
   private final Branch.NameKey destBranch;
   private Project destProject;
@@ -124,12 +127,14 @@ public class MergeOp {
   @Inject
   MergeOp(final GerritServer gs, final SchemaFactory<ReviewDb> sf,
       final ReplicationQueue rq, final MergedSender.Factory msf,
-      final MergeFailSender.Factory mfsf, @Assisted final Branch.NameKey branch) {
+      final MergeFailSender.Factory mfsf, @CanonicalWebUrl final String cwu,
+      @Assisted final Branch.NameKey branch) {
     server = gs;
     schemaFactory = sf;
     replication = rq;
     mergedSenderFactory = msf;
     mergeFailSenderFactory = mfsf;
+    canonicalWebUrl = cwu;
     myIdent = server.newGerritPersonIdent();
     destBranch = branch;
     toMerge = new ArrayList<CodeReviewCommit>();
@@ -571,9 +576,8 @@ public class MergeOp {
       msgbuf.append('\n');
     }
 
-    if (server.getCanonicalURL() != null) {
-      final String url =
-          server.getCanonicalURL() + n.patchsetId.getParentKey().get();
+    if (canonicalWebUrl != null) {
+      final String url = canonicalWebUrl + n.patchsetId.getParentKey().get();
       if (!contains(footers, REVIEWED_ON, url)) {
         msgbuf.append(REVIEWED_ON.getName());
         msgbuf.append(": ");

src/main/java/com/google/gerrit/server/GerritConfigProvider.java

@@ -21,6 +21,7 @@ import com.google.gerrit.client.reviewdb.ApprovalCategory;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.mail.EmailSender;
 import com.google.gerrit.server.ssh.GerritSshDaemon;
@@ -43,8 +44,8 @@ class GerritConfigProvider implements Provider<GerritConfig> {
   }
 
   private final Config cfg;
+  private final String canonicalWebUrl;
   private final AuthConfig authConfig;
-  private final GerritServer server;
   private final SchemaFactory<ReviewDb> schema;
 
   private GerritSshDaemon sshd;
@@ -53,11 +54,11 @@ class GerritConfigProvider implements Provider<GerritConfig> {
 
   @Inject
   GerritConfigProvider(@GerritServerConfig final Config gsc,
-      final AuthConfig ac, final GerritServer gs,
+      @CanonicalWebUrl final String cwu, final AuthConfig ac,
       final SchemaFactory<ReviewDb> sf) {
     cfg = gsc;
+    canonicalWebUrl = cwu;
     authConfig = ac;
-    server = gs;
     schema = sf;
   }
 
@@ -78,7 +79,7 @@ class GerritConfigProvider implements Provider<GerritConfig> {
 
   private GerritConfig create() throws OrmException {
     final GerritConfig config = new GerritConfig();
-    config.setCanonicalUrl(server.getCanonicalURL());
+    config.setCanonicalUrl(canonicalWebUrl);
     config.setUseContributorAgreements(cfg.getBoolean("auth",
         "contributoragreements", false));
     config.setGitDaemonUrl(cfg.getString("gerrit", null, "canonicalgiturl"));

src/main/java/com/google/gerrit/server/GerritServer.java

@@ -92,15 +92,6 @@ public class GerritServer {
     Common.setGroupCache(new GroupCache(sConfig));
   }
 
-  /** Optional canonical URL for this application. */
-  public String getCanonicalURL() {
-    String u = getGerritConfig().getString("gerrit", null, "canonicalweburl");
-    if (u != null && !u.endsWith("/")) {
-      u += "/";
-    }
-    return u;
-  }
-
   private Config getGerritConfig() {
     return gerritConfigFile;
   }

src/main/java/com/google/gerrit/server/GerritServerModule.java

@@ -26,6 +26,8 @@ import com.google.gerrit.git.PushReplication;
 import com.google.gerrit.git.ReplicationQueue;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.CacheManagerProvider;
+import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.CanonicalWebUrlProvider;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerConfigProvider;
 import com.google.gerrit.server.config.SitePath;
@@ -77,6 +79,8 @@ public class GerritServerModule extends AbstractModule {
         SitePathProvider.class);
     bind(Config.class).annotatedWith(GerritServerConfig.class).toProvider(
         GerritServerConfigProvider.class).in(SINGLETON);
+    bind(String.class).annotatedWith(CanonicalWebUrl.class).toProvider(
+        CanonicalWebUrlProvider.class);
     bind(AuthConfig.class);
 
     bind(CacheManager.class).toProvider(CacheManagerProvider.class).in(

src/main/java/com/google/gerrit/server/HostPageServlet.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server;
 import com.google.gerrit.client.data.GerritConfig;
 import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.rpc.Common;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.SitePath;
 import com.google.gwt.user.server.rpc.RPCServletUtils;
 import com.google.inject.Inject;
@@ -47,30 +48,27 @@ import javax.servlet.http.HttpServletResponse;
 @Singleton
 public class HostPageServlet extends HttpServlet {
   private final Provider<GerritCall> callFactory;
-  private final GerritServer server;
   private final File sitePath;
   private final GerritConfig config;
 
-  private String canonicalUrl;
-  private boolean wantSSL;
+  private final String canonicalUrl;
+  private final boolean wantSSL;
   private Document hostDoc;
 
   @Inject
-  HostPageServlet(final Injector i, final GerritServer gs,
-      @SitePath final File path, final GerritConfig gc) {
+  HostPageServlet(final Injector i, @SitePath final File path,
+      final GerritConfig gc, @CanonicalWebUrl final String cwu) {
     callFactory = i.getProvider(GerritCall.class);
-    server = gs;
+    canonicalUrl = cwu;
     sitePath = path;
     config = gc;
+    wantSSL = canonicalUrl != null && canonicalUrl.startsWith("https:");
   }
 
   @Override
   public void init(ServletConfig config) throws ServletException {
     super.init(config);
 
-    canonicalUrl = server.getCanonicalURL();
-    wantSSL = canonicalUrl != null && canonicalUrl.startsWith("https:");
-
     final String hostPageName = "WEB-INF/Gerrit.html";
     hostDoc = HtmlDomUtil.parseFile(getServletContext(), "/" + hostPageName);
     if (hostDoc == null) {

src/main/java/com/google/gerrit/server/OpenIdServiceImpl.java

@@ -27,6 +27,7 @@ import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.reviewdb.SystemConfig;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtjsonrpc.server.ValidToken;
 import com.google.gwtjsonrpc.server.XsrfException;
@@ -98,18 +99,18 @@ class OpenIdServiceImpl implements OpenIdService {
 
   private final Provider<GerritCall> callFactory;
   private final AuthConfig authConfig;
-  private final GerritServer server;
+  private final String canonicalWebUrl;
   private final SchemaFactory<ReviewDb> schema;
   private final ConsumerManager manager;
   private final SelfPopulatingCache discoveryCache;
 
   @Inject
   OpenIdServiceImpl(final Injector i, final AuthConfig ac,
-      final GerritServer gs, final CacheManager cacheMgr,
+      @CanonicalWebUrl final String cwu, final CacheManager cacheMgr,
       final SchemaFactory<ReviewDb> sf) throws ConsumerException {
     callFactory = i.getProvider(GerritCall.class);
     authConfig = ac;
-    server = gs;
+    canonicalWebUrl = cwu;
     schema = sf;
     manager = new ConsumerManager();
     if (authConfig.getLoginType() == SystemConfig.LoginType.OPENID) {
@@ -586,7 +587,7 @@ class OpenIdServiceImpl implements OpenIdService {
       return null;
     }
 
-    String contextUrl = server.getCanonicalURL();
+    String contextUrl = canonicalWebUrl;
     if (contextUrl == null) {
       contextUrl = GerritServer.serverUrl(httpReq);
     }

src/main/java/com/google/gerrit/server/config/CanonicalWebUrl.java

@@ -0,0 +1,27 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.config;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+
+import java.lang.annotation.Retention;
+
+/** Marker on a {@link String} holding the canonical address for this server. */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface CanonicalWebUrl {
+}

src/main/java/com/google/gerrit/server/config/CanonicalWebUrlProvider.java

@@ -0,0 +1,39 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.config;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import org.spearce.jgit.lib.Config;
+
+/** Provides {@link String} annotated with {@link CanonicalWebUrl}. */
+public class CanonicalWebUrlProvider implements Provider<String> {
+  private final String url;
+
+  @Inject
+  CanonicalWebUrlProvider(@GerritServerConfig final Config config) {
+    String u = config.getString("gerrit", null, "canonicalweburl");
+    if (u != null && !u.endsWith("/")) {
+      u += "/";
+    }
+    url = u;
+  }
+
+  @Override
+  public String get() {
+    return url;
+  }
+}

src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java

@@ -30,7 +30,9 @@ import com.google.gerrit.client.reviewdb.StarredChange;
 import com.google.gerrit.client.reviewdb.UserIdentity;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.server.GerritServer;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gwtorm.client.OrmException;
+import com.google.inject.Inject;
 
 import org.spearce.jgit.lib.PersonIdent;
 import org.spearce.jgit.util.SystemReader;
@@ -77,6 +79,10 @@ public abstract class OutgoingEmail {
   protected ChangeMessage changeMessage;
   protected ReviewDb db;
 
+  @Inject
+  @CanonicalWebUrl
+  private String canonicalWebUrl;
+
   protected OutgoingEmail(final GerritServer gs, final EmailSender es,
       final Change c, final String mc) {
     server = gs;
@@ -110,7 +116,7 @@ public abstract class OutgoingEmail {
 
   /**
    * Format and enqueue the message for delivery.
-   * 
+   *
    * @throws EmailException
    */
   public void send() throws EmailException {
@@ -278,17 +284,9 @@ public abstract class OutgoingEmail {
   }
 
   private String getGerritHost() {
-    if (server.getCanonicalURL() != null) {
+    if (getGerritUrl() != null) {
       try {
-        return new URL(server.getCanonicalURL()).getHost();
-      } catch (MalformedURLException e) {
-        // Try something else.
-      }
-    }
-
-    if (myUrl != null) {
-      try {
-        return new URL(myUrl).getHost();
+        return new URL(getGerritUrl()).getHost();
       } catch (MalformedURLException e) {
         // Try something else.
       }
@@ -327,8 +325,8 @@ public abstract class OutgoingEmail {
   }
 
   private String getGerritUrl() {
-    if (server.getCanonicalURL() != null) {
-      return server.getCanonicalURL();
+    if (canonicalWebUrl != null) {
+      return canonicalWebUrl;
     }
     return myUrl;
   }

src/main/java/com/google/gerrit/server/ssh/Receive.java

@@ -44,6 +44,7 @@ import com.google.gerrit.git.PatchSetImporter;
 import com.google.gerrit.git.ReplicationQueue;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.mail.CreateChangeSender;
 import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.mail.MergedSender;
@@ -148,6 +149,10 @@ class Receive extends AbstractGitCommand {
   @Inject
   private DiffCache diffCache;
 
+  @Inject
+  @CanonicalWebUrl
+  private String canonicalWebUrl;
+
   private ReceivePack rp;
   private PersonIdent refLogIdent;
   private ReceiveCommand newChange;
@@ -221,14 +226,14 @@ class Receive extends AbstractGitCommand {
     });
     rp.receive(in, out, err);
 
-    if (!allNewChanges.isEmpty() && server.getCanonicalURL() != null) {
+    if (!allNewChanges.isEmpty() && canonicalWebUrl != null) {
       // Make sure there isn't anything buffered; we want to give the
       // push client a chance to display its status report before we
       // show our own messages on standard error.
       //
       out.flush();
 
-      final String url = server.getCanonicalURL();
+      final String url = canonicalWebUrl;
       final PrintWriter msg = toPrintWriter(err);
       msg.write("\nNew Changes:\n");
       for (final Change.Id c : allNewChanges) {
@@ -281,10 +286,10 @@ class Receive extends AbstractGitCommand {
       msg.append("\nfatal: ");
       msg.append(bestCla.getShortName());
       msg.append(" contributor agreement is expired.\n");
-      if (server.getCanonicalURL() != null) {
+      if (canonicalWebUrl != null) {
         msg.append("\nPlease complete a new agreement");
         msg.append(":\n\n  ");
-        msg.append(server.getCanonicalURL());
+        msg.append(canonicalWebUrl);
         msg.append("#");
         msg.append(Link.SETTINGS_AGREEMENTS);
         msg.append("\n");
@@ -305,10 +310,10 @@ class Receive extends AbstractGitCommand {
         msg.append(bestCla.getShortName());
         msg.append(" contributor agreement requires");
         msg.append(" current contact information.\n");
-        if (server.getCanonicalURL() != null) {
+        if (canonicalWebUrl != null) {
           msg.append("\nPlease review your contact information");
           msg.append(":\n\n  ");
-          msg.append(server.getCanonicalURL());
+          msg.append(canonicalWebUrl);
           msg.append("#");
           msg.append(Link.SETTINGS_CONTACT);
           msg.append("\n");
@@ -336,9 +341,9 @@ class Receive extends AbstractGitCommand {
     final StringBuilder msg = new StringBuilder();
     msg.append("\nfatal: A Contributor Agreement"
         + " must be completed before uploading");
-    if (server.getCanonicalURL() != null) {
+    if (canonicalWebUrl != null) {
       msg.append(":\n\n  ");
-      msg.append(server.getCanonicalURL());
+      msg.append(canonicalWebUrl);
       msg.append("#");
       msg.append(Link.SETTINGS_AGREEMENTS);
       msg.append("\n");