Refactor RestApi to minimize encoding errors

Try to make it harder to forget to encode path components in the
client code by making the RestApi handle this for us. Callers that
need to embed dynamic data into a path should use .id(String) to
make dynamically modify the URL before setting query parameters.
Callers that need to embed a constant projection or view name
should use .view(String) as this avoids the URL encoding step.

Encoding is performed with encodeQueryString() to match what
the server uses. Space is encoded as '+' and not as '%20'.

See 925d36c782 for more details
when dashboard encodings were finally fixed.

Change-Id: I04e4b60bc0af09756d7db06974238e8ba4de004a

gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java

@@ -60,7 +60,7 @@ public class AvatarImage extends Image {
     } else {
       u = id.toString();
     }
-    RestApi api = new RestApi("/accounts/" + u + "/avatar");
+    RestApi api = new RestApi("/accounts/").id(u).view("avatar");
     if (size > 0) {
       api.addParameter("s", size);
     }

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java

@@ -28,26 +28,26 @@ public class ChangeApi {
   public static void abandon(int id, String msg, AsyncCallback<ChangeInfo> cb) {
     Input input = Input.create();
     input.message(emptyToNull(msg));
-    api(id, "abandon").data(input).post(cb);
+    call(id, "abandon").data(input).post(cb);
   }
 
   /** Restore a previously abandoned change to be open again. */
   public static void restore(int id, String msg, AsyncCallback<ChangeInfo> cb) {
     Input input = Input.create();
     input.message(emptyToNull(msg));
-    api(id, "restore").data(input).post(cb);
+    call(id, "restore").data(input).post(cb);
   }
 
   /** Create a new change that reverts the delta caused by this change. */
   public static void revert(int id, String msg, AsyncCallback<ChangeInfo> cb) {
     Input input = Input.create();
     input.message(emptyToNull(msg));
-    api(id, "revert").data(input).post(cb);
+    call(id, "revert").data(input).post(cb);
   }
 
   /** Update the topic of a change. */
   public static void topic(int id, String topic, String msg, AsyncCallback<String> cb) {
-    RestApi call = api(id, "topic");
+    RestApi call = call(id, "topic");
     topic = emptyToNull(topic);
     msg = emptyToNull(msg);
     if (topic != null || msg != null) {
@@ -64,7 +64,7 @@ public class ChangeApi {
   public static void submit(int id, String commit, AsyncCallback<SubmitInfo> cb) {
     SubmitInput in = SubmitInput.create();
     in.wait_for_merge(true);
-    api(id, commit, "submit").data(in).post(cb);
+    call(id, commit, "submit").data(in).post(cb);
   }
 
   private static class Input extends JavaScriptObject {
@@ -90,14 +90,17 @@ public class ChangeApi {
     }
   }
 
-  private static RestApi api(int id, String action) {
-    // TODO Switch to triplet project~branch~id format in URI.
-    return new RestApi("/changes/" + id + "/" + action);
+  private static RestApi call(int id, String action) {
+    return change(id).view(action);
   }
 
-  private static RestApi api(int id, String commit, String action) {
+  private static RestApi call(int id, String commit, String action) {
+    return change(id).view("revisions").id(commit).view(action);
+  }
+
+  private static RestApi change(int id) {
     // TODO Switch to triplet project~branch~id format in URI.
-    return new RestApi("/changes/" + id + "/revisions/" + commit + "/" + action);
+    return new RestApi("/changes/").id(String.valueOf(id));
   }
 
   public static String emptyToNull(String str) {

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java

@@ -368,8 +368,9 @@ public class PublishCommentScreen extends AccountScreen implements
     }
 
     enableForm(false);
-    new RestApi("/changes/" + patchSetId.getParentKey().get()
-        + "/revisions/" + revision + "/review")
+    new RestApi("/changes/")
+      .id(String.valueOf(patchSetId.getParentKey().get()))
+      .view("revisions").id(revision).view("review")
       .data(data)
       .post(new GerritCallback<ReviewInput>() {
           @Override

gerrit-gwtui/src/main/java/com/google/gerrit/client/dashboards/DashboardList.java

@@ -24,38 +24,31 @@ import com.google.gwt.user.client.rpc.AsyncCallback;
 public class DashboardList extends NativeList<DashboardInfo> {
   public static void all(Project.NameKey project,
       AsyncCallback<NativeList<DashboardList>> callback) {
-    new RestApi(base(project))
-      .addParameterTrue("inherited")
-      .get(callback);
+    base(project).addParameterTrue("inherited").get(callback);
   }
 
   public static void getDefault(Project.NameKey project,
       AsyncCallback<DashboardInfo> callback) {
-    new RestApi(base(project) + "default")
-      .addParameterTrue("inherited")
-      .get(callback);
+    base(project).view("default").addParameterTrue("inherited").get(callback);
   }
 
-  public static void get(Project.NameKey project, String dashboardId,
+  public static void get(Project.NameKey project, String id,
       AsyncCallback<DashboardInfo> callback) {
-    new RestApi(base(project) + encodeDashboardId(dashboardId))
-        .get(callback);
+    base(project).idRaw(encodeDashboardId(id)).get(callback);
   }
 
-  private static String base(Project.NameKey project) {
-    String name = URL.encodePathSegment(project.get());
-    return "/projects/" + name + "/dashboards/";
+  private static RestApi base(Project.NameKey project) {
+    return new RestApi("/projects/").id(project.get()).view("dashboards");
   }
 
-  private static String encodeDashboardId(String dashboardId) {
-    int c = dashboardId.indexOf(":");
+  private static String encodeDashboardId(String id) {
+    int c = id.indexOf(':');
     if (0 <= c) {
-      final String ref = URL.encodeQueryString(dashboardId.substring(0, c));
-      final String path = URL.encodeQueryString(dashboardId.substring(c + 1));
-      return ref + ":" + path;
-    } else {
-      return URL.encodeQueryString(dashboardId);
+      String ref = URL.encodeQueryString(id.substring(0, c));
+      String path = URL.encodeQueryString(id.substring(c + 1));
+      return ref + ':' + path;
     }
+    return URL.encodeQueryString(id);
   }
 
   protected DashboardList() {

gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupApi.java

@@ -20,7 +20,6 @@ import com.google.gerrit.client.rpc.RestApi;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gwt.core.client.JavaScriptObject;
-import com.google.gwt.http.client.URL;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 
 import java.util.Set;
@@ -30,27 +29,24 @@ import java.util.Set;
  * groups.
  */
 public class GroupApi {
-
   /** Create a new group */
   public static void createGroup(String groupName, AsyncCallback<GroupInfo> cb) {
     JavaScriptObject in = JavaScriptObject.createObject();
-    String n = URL.encodePathSegment(groupName);
-    new RestApi("/groups/" + n).ifNoneMatch().data(in).put(cb);
+    new RestApi("/groups/").id(groupName).ifNoneMatch().data(in).put(cb);
   }
 
   /** Add member to a group. */
-  public static void addMember(AccountGroup.UUID groupUUID,
-      String member, AsyncCallback<MemberInfo> cb) {
-    String n = URL.encodePathSegment(member);
-    new RestApi(membersBase(groupUUID) + "/" + n).put(cb);
+  public static void addMember(AccountGroup.UUID group, String member,
+      AsyncCallback<MemberInfo> cb) {
+    members(group).id(member).put(cb);
   }
 
   /** Add members to a group. */
-  public static void addMembers(AccountGroup.UUID groupUUID,
+  public static void addMembers(AccountGroup.UUID group,
       Set<String> members,
       final AsyncCallback<NativeList<MemberInfo>> cb) {
     if (members.size() == 1) {
-      addMember(groupUUID,
+      addMember(group,
           members.iterator().next(),
           new AsyncCallback<MemberInfo>() {
             @Override
@@ -68,38 +64,37 @@ public class GroupApi {
       for (String member : members) {
         input.add_member(member);
       }
-      new RestApi(membersBase(groupUUID) + ".add").data(input).post(cb);
+      members(group).data(input).post(cb);
     }
   }
 
   /** Remove members from a group. */
-  public static void removeMembers(AccountGroup.UUID groupUUID,
+  public static void removeMembers(AccountGroup.UUID group,
       Set<Account.Id> ids, final AsyncCallback<VoidResult> cb) {
     if (ids.size() == 1) {
       Account.Id u = ids.iterator().next();
-      new RestApi(membersBase(groupUUID) + "/" + u).delete(cb);
+      members(group).id(u.toString()).delete(cb);
     } else {
       MemberInput in = MemberInput.create();
       for (Account.Id u : ids) {
         in.add_member(u.toString());
       }
-      new RestApi(membersBase(groupUUID) + ".delete").data(in).post(cb);
+      group(group).view("members.delete").data(in).post(cb);
     }
   }
 
   /** Include a group into a group. */
-  public static void addIncludedGroup(AccountGroup.UUID groupUUID,
-      String includedGroup, AsyncCallback<GroupInfo> cb) {
-    String n = URL.encodePathSegment(includedGroup);
-    new RestApi(includedGroupsBase(groupUUID) + "/" + n).put(cb);
+  public static void addIncludedGroup(AccountGroup.UUID group, String include,
+      AsyncCallback<GroupInfo> cb) {
+    groups(group).id(include).put(cb);
   }
 
   /** Include groups into a group. */
-  public static void addIncludedGroups(AccountGroup.UUID groupUUID,
+  public static void addIncludedGroups(AccountGroup.UUID group,
       Set<String> includedGroups,
       final AsyncCallback<NativeList<GroupInfo>> cb) {
     if (includedGroups.size() == 1) {
-      addIncludedGroup(groupUUID,
+      addIncludedGroup(group,
           includedGroups.iterator().next(),
           new AsyncCallback<GroupInfo>() {
             @Override
@@ -117,21 +112,20 @@ public class GroupApi {
       for (String includedGroup : includedGroups) {
         input.add_group(includedGroup);
       }
-      new RestApi(includedGroupsBase(groupUUID) + ".add").data(input).post(cb);
+      groups(group).data(input).post(cb);
     }
   }
 
-  private static String membersBase(AccountGroup.UUID groupUUID) {
-    return base(groupUUID) + "members";
+  private static RestApi members(AccountGroup.UUID group) {
+    return group(group).view("members");
   }
 
-  private static String includedGroupsBase(AccountGroup.UUID groupUUID) {
-    return base(groupUUID) + "groups";
+  private static RestApi groups(AccountGroup.UUID group) {
+    return group(group).view("groups");
   }
 
-  private static String base(AccountGroup.UUID groupUUID) {
-    String id = URL.encodePathSegment(groupUUID.get());
-    return "/groups/" + id + "/";
+  private static RestApi group(AccountGroup.UUID group) {
+    return new RestApi("/groups/").id(group.get());
   }
 
   private static class MemberInput extends JavaScriptObject {

gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java

@@ -203,6 +203,25 @@ public class RestApi {
     url.append(name);
   }
 
+  public RestApi view(String name) {
+    return idRaw(name);
+  }
+
+  public RestApi id(String id) {
+    return idRaw(URL.encodeQueryString(id));
+  }
+
+  public RestApi idRaw(String name) {
+    if (hasQueryParams) {
+      throw new IllegalStateException();
+    }
+    if (url.charAt(url.length() - 1) != '/') {
+      url.append('/');
+    }
+    url.append(name);
+    return this;
+  }
+
   public RestApi addParameter(String name, String value) {
     return addParameterRaw(name, URL.encodeQueryString(value));
   }