Safer URI building.

+package pl.com.it_crowd.youtrack.api;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+
+public class URIUtils {
+// -------------------------- STATIC METHODS --------------------------
+
+  public static URI buildURI(URI base, String path)
+  {
+    return buildURI(base, path, null);
+  }
+
+  public static URI buildURI(URI base, String path, String query)
+  {
+    try {
+      return new URI(base.getScheme(), base.getHost(), base.getPath().replaceAll("/+$", "") + path, query, null);
+    } catch (URISyntaxException e) {
+      throw new RuntimeException(e);
+    }
+  }
+}

 package pl.com.it_crowd.youtrack.api;
 package pl.com.it_crowd.youtrack.api;
 
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.http.Header;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpHeaders;
 import org.apache.http.HttpHeaders;
@@ -17,7 +19,6 @@ import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.conn.ClientConnectionManager;
 import org.apache.http.conn.ClientConnectionManager;
 import org.apache.http.conn.scheme.Scheme;
 import org.apache.http.conn.scheme.Scheme;
 import org.apache.http.conn.scheme.SchemeRegistry;
 import org.apache.http.conn.scheme.SchemeRegistry;
@@ -60,6 +61,8 @@ import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.regex.Pattern;
 
 
+import static pl.com.it_crowd.youtrack.api.URIUtils.buildURI;
+
 public class YoutrackAPI {
 public class YoutrackAPI {
 // ------------------------------ FIELDS ------------------------------
 // ------------------------------ FIELDS ------------------------------
 
 
@@ -67,10 +70,14 @@ public class YoutrackAPI {
 
 
   private final static QName Issue_QNAME = new QName("", "issue");
   private final static QName Issue_QNAME = new QName("", "issue");
 
 
+  private static Log LOG = LogFactory.getLog(YoutrackAPI.class);
+
   private HttpClient httpClient;
   private HttpClient httpClient;
 
 
   private String serviceLocation;
   private String serviceLocation;
 
 
+  private URI serviceLocationURI;
+
 // -------------------------- STATIC METHODS --------------------------
 // -------------------------- STATIC METHODS --------------------------
 
 
   private static HttpClient getDefaultHttpClient()
   private static HttpClient getDefaultHttpClient()
@@ -134,7 +141,15 @@ public class YoutrackAPI {
 
 
   public YoutrackAPI(String serviceLocation, HttpClient httpClient)
   public YoutrackAPI(String serviceLocation, HttpClient httpClient)
   {
   {
+    if (serviceLocation == null) {
+      throw new IllegalArgumentException("serviceLocation cannot be null");
+    }
     this.serviceLocation = serviceLocation;
     this.serviceLocation = serviceLocation;
+    try {
+      serviceLocationURI = new URI(this.serviceLocation);
+    } catch (URISyntaxException e) {
+      throw new RuntimeException(e);
+    }
     this.httpClient = httpClient == null ? getDefaultHttpClient() : httpClient;
     this.httpClient = httpClient == null ? getDefaultHttpClient() : httpClient;
   }
   }
 
 
@@ -170,7 +185,7 @@ public class YoutrackAPI {
 
 
   public void command(String issueId, String command, String comment, String group, Boolean disableNotifications, String runAs) throws IOException
   public void command(String issueId, String command, String comment, String group, Boolean disableNotifications, String runAs) throws IOException
   {
   {
-    final HttpPost request = new HttpPost(serviceLocation + "/rest/issue/" + issueId + "/execute");
+    final HttpPost request = new HttpPost(buildURI(serviceLocationURI, "/rest/issue/" + issueId + "/execute"));
     final List<BasicNameValuePair> parameters = new ArrayList<BasicNameValuePair>();
     final List<BasicNameValuePair> parameters = new ArrayList<BasicNameValuePair>();
     parameters.add(new BasicNameValuePair("command", command));
     parameters.add(new BasicNameValuePair("command", command));
     if (!isBlank(comment)) {
     if (!isBlank(comment)) {
@@ -202,14 +217,8 @@ public class YoutrackAPI {
    */
    */
   public String createIssue(String project, String summary, String description) throws IOException
   public String createIssue(String project, String summary, String description) throws IOException
   {
   {
-    final URI uri;
-    try {
-      uri = new URIBuilder(serviceLocation + "/rest/issue").build();
-    } catch (URISyntaxException e) {
-      throw new RuntimeException(e);
-    }
-    final HttpPut request = createPutRequest(uri, new BasicNameValuePair("project", project), new BasicNameValuePair("summary", summary),
-        new BasicNameValuePair("description", description));
+    final HttpPut request = createPutRequest(buildURI(serviceLocationURI, "/rest/issue"), new BasicNameValuePair("project", project),
+        new BasicNameValuePair("summary", summary), new BasicNameValuePair("description", description));
     final HttpResponse response = httpClient.execute(request);
     final HttpResponse response = httpClient.execute(request);
     final StatusLine statusLine = response.getStatusLine();
     final StatusLine statusLine = response.getStatusLine();
     final HttpEntity entity = response.getEntity();
     final HttpEntity entity = response.getEntity();
@@ -230,24 +239,12 @@ public class YoutrackAPI {
 
 
   public void deleteIssue(String issueId) throws IOException
   public void deleteIssue(String issueId) throws IOException
   {
   {
-    final URI uri;
-    try {
-      uri = new URIBuilder(serviceLocation + "/rest/issue/" + issueId).build();
-    } catch (URISyntaxException e) {
-      throw new RuntimeException(e);
-    }
-    execute(new HttpDelete(uri));
+    execute(new HttpDelete(buildURI(serviceLocationURI, "/rest/issue/" + issueId)));
   }
   }
 
 
   public AssigneeList getAssignees(String project) throws IOException, JAXBException
   public AssigneeList getAssignees(String project) throws IOException, JAXBException
   {
   {
-    final URI uri;
-    try {
-      uri = new URIBuilder(serviceLocation + "/rest/admin/project/" + project + "/assignee").build();
-    } catch (URISyntaxException e) {
-      throw new RuntimeException(e);
-    }
-    final String responseString = execute(new HttpGet(uri));
+    final String responseString = execute(new HttpGet(buildURI(serviceLocationURI, "/rest/admin/project/" + project + "/assignee")));
     final Object result = YoutrackUnmarshaller.unmarshall(responseString);
     final Object result = YoutrackUnmarshaller.unmarshall(responseString);
     if (result instanceof AssigneeList) {
     if (result instanceof AssigneeList) {
       return (AssigneeList) result;
       return (AssigneeList) result;
@@ -258,13 +255,7 @@ public class YoutrackAPI {
 
 
   public Enumeration getBundle(String customField) throws IOException, JAXBException
   public Enumeration getBundle(String customField) throws IOException, JAXBException
   {
   {
-    final URI uri;
-    try {
-      uri = new URIBuilder(serviceLocation + "/rest/admin/customfield/bundle/" + customField).build();
-    } catch (URISyntaxException e) {
-      throw new RuntimeException(e);
-    }
-    final Object result = YoutrackUnmarshaller.unmarshall(execute(new HttpGet(uri)));
+    final Object result = YoutrackUnmarshaller.unmarshall(execute(new HttpGet(buildURI(serviceLocationURI, "/rest/admin/customfield/bundle/" + customField))));
     if (result instanceof Enumeration) {
     if (result instanceof Enumeration) {
       return (Enumeration) result;
       return (Enumeration) result;
     } else if (result instanceof JAXBElement) {
     } else if (result instanceof JAXBElement) {
@@ -281,13 +272,7 @@ public class YoutrackAPI {
 
 
   public List<User> getIndividualAssignees(String project) throws IOException, JAXBException
   public List<User> getIndividualAssignees(String project) throws IOException, JAXBException
   {
   {
-    final URI uri;
-    try {
-      uri = new URIBuilder(serviceLocation + "/rest/admin/project/" + project + "/assignee/individual").build();
-    } catch (URISyntaxException e) {
-      throw new RuntimeException(e);
-    }
-    final String responseString = execute(new HttpGet(uri));
+    final String responseString = execute(new HttpGet(buildURI(serviceLocationURI, "/rest/admin/project/" + project + "/assignee/individual")));
     final Object result = YoutrackUnmarshaller.unmarshall(responseString);
     final Object result = YoutrackUnmarshaller.unmarshall(responseString);
     if (result instanceof UserRefs) {
     if (result instanceof UserRefs) {
       return ((UserRefs) result).getUsers();
       return ((UserRefs) result).getUsers();
@@ -298,15 +283,9 @@ public class YoutrackAPI {
 
 
   public IssueWrapper getIssue(String issueId) throws IOException, JAXBException
   public IssueWrapper getIssue(String issueId) throws IOException, JAXBException
   {
   {
-    final URI uri;
-    try {
-      uri = new URIBuilder(serviceLocation + "/rest/issue/" + issueId).build();
-    } catch (URISyntaxException e) {
-      throw new RuntimeException(e);
-    }
     final String responseString;
     final String responseString;
     try {
     try {
-      responseString = execute(new HttpGet(uri));
+      responseString = execute(new HttpGet(buildURI(serviceLocationURI, "/rest/issue/" + issueId)));
     } catch (YoutrackErrorException e) {
     } catch (YoutrackErrorException e) {
       if (e.getStatusCode() == HttpStatus.SC_NOT_FOUND) {
       if (e.getStatusCode() == HttpStatus.SC_NOT_FOUND) {
         throw new NoResultFoundException(e.getMessage(), e);
         throw new NoResultFoundException(e.getMessage(), e);
@@ -331,15 +310,15 @@ public class YoutrackAPI {
 
 
   public void login(String username, String password) throws IOException, JAXBException
   public void login(String username, String password) throws IOException, JAXBException
   {
   {
-    final HttpPost request = new HttpPost(serviceLocation + "/rest/user/login");
+    final HttpPost request = new HttpPost(buildURI(serviceLocationURI, "/rest/user/login"));
     request.setEntity(new UrlEncodedFormEntity(Arrays.asList(new BasicNameValuePair("login", username), new BasicNameValuePair("password", password))));
     request.setEntity(new UrlEncodedFormEntity(Arrays.asList(new BasicNameValuePair("login", username), new BasicNameValuePair("password", password))));
     execute(request);
     execute(request);
   }
   }
 
 
   public List<IssueWrapper> searchIssuesByProject(String project, Object filter) throws JAXBException, IOException
   public List<IssueWrapper> searchIssuesByProject(String project, Object filter) throws JAXBException, IOException
   {
   {
-    final String url = serviceLocation + "/rest/issue/byproject/" + project + "?filter=" + (filter == null ? "" : filter);
-    final Object result = YoutrackUnmarshaller.unmarshall(execute(new HttpGet(url)));
+    final Object result = YoutrackUnmarshaller.unmarshall(
+        execute(new HttpGet(buildURI(serviceLocationURI, "/rest/issue/byproject/" + project, "filter=" + (filter == null ? "" : filter)))));
     if (!(result instanceof Issues)) {
     if (!(result instanceof Issues)) {
       throw new YoutrackAPIException("Unmarshalling problem. Expected Issues, received: " + result.getClass() + " " + result);
       throw new YoutrackAPIException("Unmarshalling problem. Expected Issues, received: " + result.getClass() + " " + result);
     }
     }
@@ -353,13 +332,7 @@ public class YoutrackAPI {
 
 
   public void updateIssue(String issueId, String summary, String description) throws IOException
   public void updateIssue(String issueId, String summary, String description) throws IOException
   {
   {
-    final URI uri;
-    try {
-      uri = new URIBuilder(serviceLocation + "/rest/issue/" + issueId).build();
-    } catch (URISyntaxException e) {
-      throw new RuntimeException(e);
-    }
-    final HttpPost request = createPostRequest(uri, new BasicNameValuePair(Fields.summary.name(), summary),
+    final HttpPost request = createPostRequest(buildURI(serviceLocationURI, "/rest/issue/" + issueId), new BasicNameValuePair(Fields.summary.name(), summary),
         new BasicNameValuePair(Fields.description.name(), description));
         new BasicNameValuePair(Fields.description.name(), description));
     final HttpResponse response = httpClient.execute(request);
     final HttpResponse response = httpClient.execute(request);
     final StatusLine statusLine = response.getStatusLine();
     final StatusLine statusLine = response.getStatusLine();
@@ -389,7 +362,7 @@ public class YoutrackAPI {
         final String error = ErrorUnmarshaller.unmarshal(responseText);
         final String error = ErrorUnmarshaller.unmarshal(responseText);
         throw new YoutrackErrorException(error, statusLine.getStatusCode());
         throw new YoutrackErrorException(error, statusLine.getStatusCode());
       } catch (JAXBException e) {
       } catch (JAXBException e) {
-        //                    TODO we should log on debug level the JAXBException
+        LOG.error("Cannot unmarshal following response text:\n" + responseText, e);
         throw new HttpResponseException(statusLine.getStatusCode(), responseText);
         throw new HttpResponseException(statusLine.getStatusCode(), responseText);
       }
       }
     }
     }

+package pl.com.it_crowd.youtrack.api.rest;
+
+import org.junit.Test;
+import pl.com.it_crowd.youtrack.api.Filter;
+import pl.com.it_crowd.youtrack.api.URIUtils;
+import pl.com.it_crowd.youtrack.api.defaults.StateValues;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.junit.Assert.assertEquals;
+
+public class URIUtilsTest {
+// -------------------------- OTHER METHODS --------------------------
+
+  @Test
+  public void buildURI() throws URISyntaxException
+  {
+//      Given
+    final URI base = new URI("http://localhost:8080/youtrack/");
+    final String pathA = "/rest/admin/bundle/QA note types";
+    final String queryA = "q=" + Filter.stateFilter(StateValues.NotWontFix).maxResults(20);
+
+//      When
+    final URI uriA = URIUtils.buildURI(base, pathA);
+    final URI uriB = URIUtils.buildURI(base, pathA, queryA);
+
+//      Then
+    assertEquals("http://localhost/youtrack/rest/admin/bundle/QA%20note%20types", uriA.toString());
+    assertEquals("http://localhost/youtrack/rest/admin/bundle/QA%20note%20types?q=state:-%257BWon%2527t+fix%257D&max=20", uriB.toString());
+  }
+}