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
This commit is contained in:
Dave Borowitz 2015-09-04 13:46:48 -04:00
parent 0c1e240f6e
commit dbbdad2ef0
5 changed files with 25 additions and 16 deletions

View File

@ -26,8 +26,8 @@ import java.util.List;
import java.util.Set; import java.util.Set;
public class DownloadInfo extends JavaScriptObject { public class DownloadInfo extends JavaScriptObject {
public final Set<String> schemes() { public final List<String> schemes() {
return Natives.keys(_schemes()); return _schemes().sortedKeys();
} }
public final List<String> archives() { public final List<String> archives() {
@ -46,8 +46,8 @@ public class DownloadInfo extends JavaScriptObject {
} }
public static class DownloadSchemeInfo extends JavaScriptObject { public static class DownloadSchemeInfo extends JavaScriptObject {
public final Set<String> commandNames() { public final List<String> commandNames() {
return Natives.keys(_commands()); return _commands().sortedKeys();
} }
public final Set<DownloadCommandInfo> commands(String project) { public final Set<DownloadCommandInfo> commands(String project) {
@ -67,13 +67,14 @@ public class DownloadInfo extends JavaScriptObject {
return project.substring(project.lastIndexOf('/') + 1); return project.substring(project.lastIndexOf('/') + 1);
} }
public final Set<String> cloneCommandNames() { public final List<String> cloneCommandNames() {
return Natives.keys(_cloneCommands()); return _cloneCommands().sortedKeys();
} }
public final Set<DownloadCommandInfo> cloneCommands(String project) { public final List<DownloadCommandInfo> cloneCommands(String project) {
Set<DownloadCommandInfo> commands = new HashSet<>(); List<String> commandNames = cloneCommandNames();
for (String commandName : cloneCommandNames()) { List<DownloadCommandInfo> commands = new ArrayList<>(commandNames.size());
for (String commandName : commandNames) {
commands.add(new DownloadCommandInfo(commandName, cloneCommand( commands.add(new DownloadCommandInfo(commandName, cloneCommand(
commandName, project))); commandName, project)));
} }

View File

@ -18,6 +18,9 @@ import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.client.JsArray; import com.google.gwt.core.client.JsArray;
import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwt.user.client.rpc.AsyncCallback;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set; import java.util.Set;
/** A map of native JSON objects, keyed by a string. */ /** A map of native JSON objects, keyed by a string. */
@ -58,6 +61,13 @@ public class NativeMap<T extends JavaScriptObject> extends JavaScriptObject {
return Natives.keys(this); 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() public final native JsArray<T> values()
/*-{ /*-{
var s = this; var s = this;

View File

@ -69,7 +69,6 @@ import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Map.Entry; import java.util.Map.Entry;
import java.util.Set;
public class ProjectInfoScreen extends ProjectScreen { public class ProjectInfoScreen extends ProjectScreen {
private boolean isOwner; private boolean isOwner;
@ -710,7 +709,7 @@ public class ProjectInfoScreen extends ProjectScreen {
} }
@Override @Override
protected Set<DownloadCommandInfo> getCommands(DownloadSchemeInfo schemeInfo) { protected List<DownloadCommandInfo> getCommands(DownloadSchemeInfo schemeInfo) {
return schemeInfo.cloneCommands(project); return schemeInfo.cloneCommands(project);
} }
} }

View File

@ -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.EditInfo;
import com.google.gerrit.client.info.ChangeInfo.FetchInfo; import com.google.gerrit.client.info.ChangeInfo.FetchInfo;
import com.google.gerrit.client.rpc.NativeMap; 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.client.rpc.RestApi;
import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@ -119,7 +118,7 @@ class DownloadBox extends VerticalPanel {
if (scheme.getItemCount() > 0) { if (scheme.getItemCount() > 0) {
FetchInfo fetchInfo = FetchInfo fetchInfo =
fetch.get(scheme.getValue(scheme.getSelectedIndex())); fetch.get(scheme.getValue(scheme.getSelectedIndex()));
for (String commandName : Natives.keys(fetchInfo.commands())) { for (String commandName : fetchInfo.commands().sortedKeys()) {
CopyableLabel copyLabel = CopyableLabel copyLabel =
new CopyableLabel(fetchInfo.command(commandName)); new CopyableLabel(fetchInfo.command(commandName));
copyLabel.setStyleName(Gerrit.RESOURCES.css().downloadBoxCopyLabel()); copyLabel.setStyleName(Gerrit.RESOURCES.css().downloadBoxCopyLabel());
@ -209,7 +208,7 @@ class DownloadBox extends VerticalPanel {
} }
private void renderScheme() { private void renderScheme() {
for (String id : fetch.keySet()) { for (String id : fetch.sortedKeys()) {
scheme.addItem(id); scheme.addItem(id);
} }
if (scheme.getItemCount() == 0) { if (scheme.getItemCount() == 0) {

View File

@ -21,7 +21,7 @@ import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.InlineLabel; import com.google.gwt.user.client.ui.InlineLabel;
import com.google.gwtexpui.clippy.client.CopyableLabel; import com.google.gwtexpui.clippy.client.CopyableLabel;
import java.util.Set; import java.util.List;
public abstract class DownloadPanel extends FlowPanel { public abstract class DownloadPanel extends FlowPanel {
protected final String project; protected final String project;
@ -63,6 +63,6 @@ public abstract class DownloadPanel extends FlowPanel {
commands.select(); commands.select();
} }
protected abstract Set<DownloadCommandInfo> getCommands( protected abstract List<DownloadCommandInfo> getCommands(
DownloadSchemeInfo schemeInfo); DownloadSchemeInfo schemeInfo);
} }