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
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 660ad90..eee2847 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.Set;
public class DownloadInfo extends JavaScriptObject {
- public final Set<String> schemes() {
- return Natives.keys(_schemes());
+ public final List<String> schemes() {
+ return _schemes().sortedKeys();
}
public final List<String> archives() {
@@ -46,8 +46,8 @@
}
public static class DownloadSchemeInfo extends JavaScriptObject {
- public final Set<String> commandNames() {
- return Natives.keys(_commands());
+ public final List<String> commandNames() {
+ return _commands().sortedKeys();
}
public final Set<DownloadCommandInfo> commands(String project) {
@@ -67,13 +67,14 @@
return project.substring(project.lastIndexOf('/') + 1);
}
- public final Set<String> cloneCommandNames() {
- return Natives.keys(_cloneCommands());
+ public final List<String> cloneCommandNames() {
+ return _cloneCommands().sortedKeys();
}
- public final Set<DownloadCommandInfo> cloneCommands(String project) {
- Set<DownloadCommandInfo> commands = new HashSet<>();
- for (String commandName : cloneCommandNames()) {
+ public final List<DownloadCommandInfo> cloneCommands(String project) {
+ List<String> commandNames = cloneCommandNames();
+ List<DownloadCommandInfo> 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 2e407d4..1f003de 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.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 @@
return Natives.keys(this);
}
+ public final List<String> sortedKeys() {
+ Set<String> keys = keySet();
+ List<String> sorted = new ArrayList<>(keys);
+ Collections.sort(sorted);
+ return sorted;
+ }
+
public final native JsArray<T> 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 cb87fc0..eefd199 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.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 @@
}
@Override
- protected Set<DownloadCommandInfo> getCommands(DownloadSchemeInfo schemeInfo) {
+ protected List<DownloadCommandInfo> 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 c90547f..c8326cc 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.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 @@
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 @@
}
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 ea5df4c..7119bd2 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.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 @@
commands.select();
}
- protected abstract Set<DownloadCommandInfo> getCommands(
+ protected abstract List<DownloadCommandInfo> getCommands(
DownloadSchemeInfo schemeInfo);
}
