Resolve URL syntax at init/config time
Remove all the noise in the code about managing and adjusting the
Jira URL and centralise validation in two points:
- init
- configuration
Once the JiraConfig is resolved, all the rest of the code can assume
that the URL is valid and can be used as-is.
Change-Id: I0ff634216c8df5b60a897e67ff84aad980b56791
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java
index 0f9e19a..77d2b1b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java
@@ -25,6 +25,8 @@
import com.googlesource.gerrit.plugins.its.base.its.InitIts;
import com.googlesource.gerrit.plugins.its.base.validation.ItsAssociationPolicy;
import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URL;
import java.util.Arrays;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -35,7 +37,7 @@
private final Section.Factory sections;
private final InitFlags flags;
private Section jira;
- private String jiraUrl;
+ private URL jiraUrl;
private String jiraUsername;
private String jiraPassword;
@@ -89,13 +91,14 @@
}
}
- private void init() {
+ private void init() throws MalformedURLException {
this.jira = sections.get(pluginName, null);
Section jiraComment = sections.get(COMMENT_LINK_SECTION, pluginName);
do {
enterJiraConnectivity();
- } while (jiraUrl != null && (isConnectivityRequested(jiraUrl) && !isJiraConnectSuccessful()));
+ } while (jiraUrl != null
+ && (isConnectivityRequested(jiraUrl.toString()) && !isJiraConnectSuccessful()));
if (jiraUrl == null) {
return;
@@ -111,9 +114,10 @@
"Issue-id enforced in commit message", "association", ItsAssociationPolicy.SUGGESTED);
}
- public void enterJiraConnectivity() {
- jiraUrl = jira.string("Jira URL (empty to skip)", "url", null);
- if (jiraUrl != null) {
+ public void enterJiraConnectivity() throws MalformedURLException {
+ String jiraUrlString = jira.string("Jira URL (empty to skip)", "url", null);
+ if (jiraUrlString != null) {
+ jiraUrl = new URL(jiraUrlString);
jiraUsername = jira.string("Jira username", "username", "");
jiraPassword = jira.password("username", "password");
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraClient.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraClient.java
index ae2a864..7311a9b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraClient.java
@@ -31,7 +31,7 @@
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraServerInfo;
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraTransition;
import java.io.IOException;
-import java.net.MalformedURLException;
+import java.net.URL;
import java.util.Arrays;
import java.util.List;
import org.slf4j.Logger;
@@ -44,7 +44,7 @@
private final Gson gson;
@Inject
- public JiraClient(JiraConfig jiraConfig) throws MalformedURLException {
+ public JiraClient(JiraConfig jiraConfig) {
this(jiraConfig.getJiraUrl(), jiraConfig.getUsername(), jiraConfig.getPassword());
}
@@ -55,7 +55,7 @@
* @param user username of the jira user
* @param pass password of the jira user
*/
- JiraClient(String url, String user, String pass) throws MalformedURLException {
+ JiraClient(URL url, String user, String pass) {
this.apiBuilder = new JiraRestApiProvider(url, user, pass);
this.gson = new Gson();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java
index b757b65..9ec5d9b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java
@@ -14,23 +14,26 @@
package com.googlesource.gerrit.plugins.its.jira;
+import static com.googlesource.gerrit.plugins.its.jira.UrlHelper.*;
import static java.lang.String.format;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.net.MalformedURLException;
+import java.net.URL;
import org.eclipse.jgit.lib.Config;
/** The JIRA plugin configuration as read from Gerrit config. */
@Singleton
public class JiraConfig {
- static final String ERROR_MSG = "Unable to load plugin %s. Cause: Wrong configuration ";
+ static final String ERROR_MSG = "Unable to load plugin %s because of invalid configuration: %s";
static final String GERRIT_CONFIG_URL = "url";
static final String GERRIT_CONFIG_USERNAME = "username";
static final String GERRIT_CONFIG_PASSWORD = "password";
- private final String jiraUrl;
+ private final URL jiraUrl;
private final String jiraUsername;
private final String jiraPassword;
@@ -42,11 +45,16 @@
*/
@Inject
JiraConfig(@GerritServerConfig Config config, @PluginName String pluginName) {
- jiraUrl = config.getString(pluginName, null, GERRIT_CONFIG_URL);
+ try {
+ jiraUrl = adjustUrlPath(new URL(config.getString(pluginName, null, GERRIT_CONFIG_URL)));
+ } catch (MalformedURLException e) {
+ throw new RuntimeException(format(ERROR_MSG, pluginName, e.getLocalizedMessage()));
+ }
+
jiraUsername = config.getString(pluginName, null, GERRIT_CONFIG_USERNAME);
jiraPassword = config.getString(pluginName, null, GERRIT_CONFIG_PASSWORD);
if (jiraUrl == null || jiraUsername == null || jiraPassword == null) {
- throw new RuntimeException(format(ERROR_MSG, pluginName));
+ throw new RuntimeException(format(ERROR_MSG, pluginName, "missing username/password"));
}
}
@@ -55,7 +63,7 @@
*
* @return the jira url
*/
- public String getJiraUrl() {
+ public URL getJiraUrl() {
return jiraUrl;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/UrlHelper.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/UrlHelper.java
new file mode 100644
index 0000000..a6b89ef
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/UrlHelper.java
@@ -0,0 +1,37 @@
+
+package com.googlesource.gerrit.plugins.its.jira;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.Arrays;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class UrlHelper {
+ private static final Logger log = LoggerFactory.getLogger(UrlHelper.class);
+
+ public static URL resolveUrl(URL url, String... paths) {
+ if (url == null) {
+ return url;
+ }
+
+ String relativePath = String.join("", Arrays.asList(paths));
+ try {
+ return new URL(url, relativePath);
+ } catch (MalformedURLException e) {
+ log.error("Unexpected exception while composing URL {} with path {}", url, relativePath, e);
+ throw new IllegalArgumentException(e);
+ }
+ }
+
+ public static URL adjustUrlPath(URL url) {
+ if (url == null) {
+ return url;
+ }
+ try {
+ return url.getPath().endsWith("/") ? url : new URL(url, "/");
+ } catch (MalformedURLException e) {
+ throw new RuntimeException(e);
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java
index aff8428..35f70ce 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java
@@ -14,12 +14,13 @@
package com.googlesource.gerrit.plugins.its.jira.restapi;
+import static com.googlesource.gerrit.plugins.its.jira.UrlHelper.*;
+
import com.google.gson.Gson;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.HttpURLConnection;
-import java.net.MalformedURLException;
import java.net.Proxy;
import java.net.ProxySelector;
import java.net.URL;
@@ -45,10 +46,9 @@
* @param user username of the jira user
* @param pass password of the jira user
*/
- JiraRestApi(URL url, String user, String pass, Class<T> classOfT, String classPrefix)
- throws MalformedURLException {
+ JiraRestApi(URL url, String user, String pass, Class<T> classOfT, String classPrefix) {
this.auth = Base64.getEncoder().encodeToString((user + ":" + pass).getBytes());
- this.baseUrl = new URL(url, BASE_PREFIX + classPrefix + "/");
+ this.baseUrl = resolveUrl(url, BASE_PREFIX, classPrefix, "/");
this.gson = new Gson();
this.classOfT = classOfT;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiProvider.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiProvider.java
index 3277f7f..37534a5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiProvider.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiProvider.java
@@ -14,7 +14,6 @@
package com.googlesource.gerrit.plugins.its.jira.restapi;
-import java.net.MalformedURLException;
import java.net.URL;
public class JiraRestApiProvider {
@@ -22,26 +21,25 @@
private final String user;
private final String pass;
- public JiraRestApiProvider(String url, String user, String pass) throws MalformedURLException {
- this.url = new URL(url + (url.endsWith("/") ? "" : "/"));
+ public JiraRestApiProvider(URL url, String user, String pass) {
+ this.url = url;
this.user = user;
this.pass = pass;
}
- public <T> JiraRestApi<T> get(Class<T> classOfT, String classPrefix)
- throws MalformedURLException {
+ public <T> JiraRestApi<T> get(Class<T> classOfT, String classPrefix) {
return new JiraRestApi<>(url, user, pass, classOfT, classPrefix);
}
- public JiraRestApi<JiraIssue> getIssue() throws MalformedURLException {
+ public JiraRestApi<JiraIssue> getIssue() {
return get(JiraIssue.class, "/issue");
}
- public JiraRestApi<JiraServerInfo> getServerInfo() throws MalformedURLException {
+ public JiraRestApi<JiraServerInfo> getServerInfo() {
return get(JiraServerInfo.class, "/serverInfo");
}
- public JiraRestApi<JiraProject[]> getProjects() throws MalformedURLException {
+ public JiraRestApi<JiraProject[]> getProjects() {
return get(JiraProject[].class, "/project");
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/jira/JiraConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/jira/JiraConfigTest.java
index f7f5a40..fba7db7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/jira/JiraConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/jira/JiraConfigTest.java
@@ -15,13 +15,13 @@
package com.googlesource.gerrit.plugins.its.jira;
import static com.google.common.truth.Truth.assertThat;
-import static com.googlesource.gerrit.plugins.its.jira.JiraConfig.ERROR_MSG;
import static com.googlesource.gerrit.plugins.its.jira.JiraConfig.GERRIT_CONFIG_PASSWORD;
import static com.googlesource.gerrit.plugins.its.jira.JiraConfig.GERRIT_CONFIG_URL;
import static com.googlesource.gerrit.plugins.its.jira.JiraConfig.GERRIT_CONFIG_USERNAME;
-import static java.lang.String.format;
import static org.mockito.Mockito.when;
+import java.net.MalformedURLException;
+import java.net.URL;
import org.eclipse.jgit.lib.Config;
import org.junit.Rule;
import org.junit.Test;
@@ -34,7 +34,7 @@
public class JiraConfigTest {
private static final String PASS = "pass";
- private static final String URL = "http://jira_example.com";
+ private static final URL TEST_URL = newUrl("http://jira_example.com/");
private static final String USER = "user";
private static final String PLUGIN_NAME = "its-jira";
@@ -45,19 +45,26 @@
@Test
public void gerritConfigContainsSaneValues() throws Exception {
- when(cfg.getString(PLUGIN_NAME, null, GERRIT_CONFIG_URL)).thenReturn(URL);
+ when(cfg.getString(PLUGIN_NAME, null, GERRIT_CONFIG_URL)).thenReturn(TEST_URL.toString());
when(cfg.getString(PLUGIN_NAME, null, GERRIT_CONFIG_USERNAME)).thenReturn(USER);
when(cfg.getString(PLUGIN_NAME, null, GERRIT_CONFIG_PASSWORD)).thenReturn(PASS);
jiraConfig = new JiraConfig(cfg, PLUGIN_NAME);
assertThat(jiraConfig.getUsername()).isEqualTo(USER);
assertThat(jiraConfig.getPassword()).isEqualTo(PASS);
- assertThat(jiraConfig.getJiraUrl()).isEqualTo(URL);
+ assertThat(jiraConfig.getJiraUrl()).isEqualTo(TEST_URL);
}
@Test
public void gerritConfigContainsNullValues() throws Exception {
thrown.expect(RuntimeException.class);
- thrown.expectMessage(format(ERROR_MSG, PLUGIN_NAME));
jiraConfig = new JiraConfig(cfg, PLUGIN_NAME);
}
+
+ private static URL newUrl(String url) {
+ try {
+ return new URL(url);
+ } catch (MalformedURLException e) {
+ throw new RuntimeException(e);
+ }
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java
index 24512b5..b87b591 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java
@@ -16,37 +16,38 @@
import static com.google.common.truth.Truth.assertThat;
+import java.net.URL;
import org.junit.Test;
public class JiraRestApiTest {
@Test
public void testJiraServerInfoForNonRootJiraUrl() throws Exception {
- String nonRootJiraUrl = "http://jira.mycompany.com/myroot/";
+ URL nonRootJiraUrl = new URL("http://jira.mycompany.com/myroot/");
JiraRestApi<JiraServerInfo> serverInfo =
new JiraRestApiProvider(nonRootJiraUrl, "", "").getServerInfo();
String jiraApiUrl = serverInfo.getBaseUrl().toString();
- assertThat(jiraApiUrl).startsWith(nonRootJiraUrl);
+ assertThat(jiraApiUrl).startsWith(nonRootJiraUrl.toString());
}
@Test
public void testJiraServerInfoForNonRootJiraUrlNotEndingWithSlash() throws Exception {
- String nonRootJiraUrl = "http://jira.mycompany.com/myroot";
+ URL nonRootJiraUrl = new URL("http://jira.mycompany.com/myroot/");
JiraRestApi<JiraServerInfo> serverInfo =
new JiraRestApiProvider(nonRootJiraUrl, "", "").getServerInfo();
String jiraApiUrl = serverInfo.getBaseUrl().toString();
- assertThat(jiraApiUrl).startsWith(nonRootJiraUrl);
+ assertThat(jiraApiUrl).startsWith(nonRootJiraUrl.toString());
}
@Test
public void testJiraServerInfoForRootJiraUrl() throws Exception {
- String rootJiraUrl = "http://jira.mycompany.com";
+ URL rootJiraUrl = new URL("http://jira.mycompany.com");
JiraRestApi<JiraServerInfo> serverInfo =
new JiraRestApiProvider(rootJiraUrl, "", "").getServerInfo();
String jiraApiUrl = serverInfo.getBaseUrl().toString();
- assertThat(jiraApiUrl).startsWith(rootJiraUrl);
+ assertThat(jiraApiUrl).startsWith(rootJiraUrl.toString());
}
}
