diff --git a/main/src/com/google/refine/importers/tree/TreeImportUtilities.java b/main/src/com/google/refine/importers/tree/TreeImportUtilities.java index c7e37fff8..0ed0beece 100644 --- a/main/src/com/google/refine/importers/tree/TreeImportUtilities.java +++ b/main/src/com/google/refine/importers/tree/TreeImportUtilities.java @@ -73,8 +73,7 @@ public abstract class TreeImportUtilities { return o1.blankOnFirstRow ? 1 : -1; } - int c = o2.nonBlankCount - o1.nonBlankCount; - return c != 0 ? c : (o1.name.length() - o2.name.length()); + return o2.nonBlankCount - o1.nonBlankCount; } }); @@ -85,6 +84,7 @@ public abstract class TreeImportUtilities { project.columnModel.columns.add(column); } + // The LinkedHashMap iterator will guaranteed that the list is arranged in order found List subgroups = new ArrayList(columnGroup.subgroups.values()); Collections.sort(subgroups, new Comparator() { @Override @@ -96,9 +96,8 @@ public abstract class TreeImportUtilities { // TODO: From a human factors point of view, we probably want // to try to preserve the order that we found things in the XML - // Sort by most populated first, then shortest name - int c = o2.nonBlankCount - o1.nonBlankCount; - return c != 0 ? c : (o1.name.length() - o2.name.length()); + // Sort by most populated first, but leave order unchanged if they're equal + return o2.nonBlankCount - o1.nonBlankCount; } }); diff --git a/main/tests/server/src/com/google/refine/importers/XmlImporterTests.java b/main/tests/server/src/com/google/refine/importers/XmlImporterTests.java index 63bb20828..e18441f9d 100644 --- a/main/tests/server/src/com/google/refine/importers/XmlImporterTests.java +++ b/main/tests/server/src/com/google/refine/importers/XmlImporterTests.java @@ -51,6 +51,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.refine.importers.XmlImporter; import com.google.refine.importers.tree.TreeImportingParserBase; import com.google.refine.importing.ImportingJob; +import com.google.refine.model.ColumnGroup; import com.google.refine.model.Row; import com.google.refine.util.JSONUtilities; import com.google.refine.util.ParsingUtilities; @@ -197,11 +198,12 @@ public class XmlImporterTests extends ImporterTest { assertProjectCreated(project, 5, 6); Assert.assertEquals(project.columnModel.columnGroups.size(),1); - Assert.assertEquals(project.columnModel.columnGroups.get(0).keyColumnIndex, 2); - Assert.assertEquals(project.columnModel.columnGroups.get(0).startColumnIndex, 2); - Assert.assertNull(project.columnModel.columnGroups.get(0).parentGroup); - Assert.assertEquals(project.columnModel.columnGroups.get(0).subgroups.size(),0); - Assert.assertEquals(project.columnModel.columnGroups.get(0).columnSpan,2); + ColumnGroup cg0 = project.columnModel.columnGroups.get(0); + Assert.assertEquals(cg0.keyColumnIndex, 1); + Assert.assertEquals(cg0.startColumnIndex, 1); + Assert.assertNull(cg0.parentGroup); + Assert.assertEquals(cg0.subgroups.size(),0); + Assert.assertEquals(cg0.columnSpan,2); } //------------helper methods---------------