From dbbdad2ef088ee84bd1089aaa77605a04977ce96 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 4 Sep 2015 13:46:48 -0400 Subject: [PATCH] Sort download schemes and command names Map iteration order is not defined either on the server side or in the JSON spec. Make the order stable by simply sorting; the documentation does not specify a particular order, so this is not a compatibility problem. Some users were seeing the first element in the dropdown vary when reloading the app. Combined with the inability to save the preference for custom schemes, this was frustrating both for the users and for finding the core issue. Change-Id: I13f56a32c2e362e08360e7b7a26f1bee3920b723 --- .../gerrit/client/info/DownloadInfo.java | 19 ++++++++++--------- .../google/gerrit/client/rpc/NativeMap.java | 10 ++++++++++ .../client/admin/ProjectInfoScreen.java | 3 +-- .../gerrit/client/change/DownloadBox.java | 5 ++--- .../gerrit/client/download/DownloadPanel.java | 4 ++-- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java index 660ad90338..eee2847722 100644 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java +++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java @@ -26,8 +26,8 @@ import java.util.List; import java.util.Set; public class DownloadInfo extends JavaScriptObject { - public final Set schemes() { - return Natives.keys(_schemes()); + public final List schemes() { + return _schemes().sortedKeys(); } public final List archives() { @@ -46,8 +46,8 @@ public class DownloadInfo extends JavaScriptObject { } public static class DownloadSchemeInfo extends JavaScriptObject { - public final Set commandNames() { - return Natives.keys(_commands()); + public final List commandNames() { + return _commands().sortedKeys(); } public final Set commands(String project) { @@ -67,13 +67,14 @@ public class DownloadInfo extends JavaScriptObject { return project.substring(project.lastIndexOf('/') + 1); } - public final Set cloneCommandNames() { - return Natives.keys(_cloneCommands()); + public final List cloneCommandNames() { + return _cloneCommands().sortedKeys(); } - public final Set cloneCommands(String project) { - Set commands = new HashSet<>(); - for (String commandName : cloneCommandNames()) { + public final List cloneCommands(String project) { + List commandNames = cloneCommandNames(); + List commands = new ArrayList<>(commandNames.size()); + for (String commandName : commandNames) { commands.add(new DownloadCommandInfo(commandName, cloneCommand( commandName, project))); } diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/rpc/NativeMap.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/rpc/NativeMap.java index 2e407d419d..1f003de605 100644 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/rpc/NativeMap.java +++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/rpc/NativeMap.java @@ -18,6 +18,9 @@ import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.core.client.JsArray; import com.google.gwt.user.client.rpc.AsyncCallback; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Set; /** A map of native JSON objects, keyed by a string. */ @@ -58,6 +61,13 @@ public class NativeMap extends JavaScriptObject { return Natives.keys(this); } + public final List sortedKeys() { + Set keys = keySet(); + List sorted = new ArrayList<>(keys); + Collections.sort(sorted); + return sorted; + } + public final native JsArray values() /*-{ var s = this; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java index cb87fc0fcd..eefd19968c 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java @@ -69,7 +69,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.Set; public class ProjectInfoScreen extends ProjectScreen { private boolean isOwner; @@ -710,7 +709,7 @@ public class ProjectInfoScreen extends ProjectScreen { } @Override - protected Set getCommands(DownloadSchemeInfo schemeInfo) { + protected List getCommands(DownloadSchemeInfo schemeInfo) { return schemeInfo.cloneCommands(project); } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java index c90547f9ef..c8326cc69d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java @@ -23,7 +23,6 @@ import com.google.gerrit.client.info.ChangeInfo; import com.google.gerrit.client.info.ChangeInfo.EditInfo; import com.google.gerrit.client.info.ChangeInfo.FetchInfo; import com.google.gerrit.client.rpc.NativeMap; -import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.client.rpc.RestApi; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.reviewdb.client.PatchSet; @@ -119,7 +118,7 @@ class DownloadBox extends VerticalPanel { if (scheme.getItemCount() > 0) { FetchInfo fetchInfo = fetch.get(scheme.getValue(scheme.getSelectedIndex())); - for (String commandName : Natives.keys(fetchInfo.commands())) { + for (String commandName : fetchInfo.commands().sortedKeys()) { CopyableLabel copyLabel = new CopyableLabel(fetchInfo.command(commandName)); copyLabel.setStyleName(Gerrit.RESOURCES.css().downloadBoxCopyLabel()); @@ -209,7 +208,7 @@ class DownloadBox extends VerticalPanel { } private void renderScheme() { - for (String id : fetch.keySet()) { + for (String id : fetch.sortedKeys()) { scheme.addItem(id); } if (scheme.getItemCount() == 0) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/download/DownloadPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/download/DownloadPanel.java index ea5df4cb21..7119bd2ee8 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/download/DownloadPanel.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/download/DownloadPanel.java @@ -21,7 +21,7 @@ import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.InlineLabel; import com.google.gwtexpui.clippy.client.CopyableLabel; -import java.util.Set; +import java.util.List; public abstract class DownloadPanel extends FlowPanel { protected final String project; @@ -63,6 +63,6 @@ public abstract class DownloadPanel extends FlowPanel { commands.select(); } - protected abstract Set getCommands( + protected abstract List getCommands( DownloadSchemeInfo schemeInfo); }