This is an automated email from the ASF dual-hosted git repository. fschumacher pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/jmeter.git
commit 6e7b3702cac8808937df56409393f42ee9b60e67 Author: Felix Schumacher <[email protected]> AuthorDate: Sat Jan 2 22:36:42 2021 +0100 Sorting in View Results in Table is not correct All string columns will get sorted by human understandable alpha numeric sort Bugzilla Id: 63061 --- .../apache/jmeter/visualizers/TableVisualizer.java | 11 ++++- ...Comparator.java => AlphaNumericComparator.java} | 48 +++++++++++++-------- .../jorphan/util/AlphaNumericKeyComparator.java | 50 ++-------------------- .../util/TestAlphaNumericKeyComparator.java | 18 +++++++- xdocs/changes.xml | 1 + 5 files changed, 60 insertions(+), 68 deletions(-) diff --git a/src/components/src/main/java/org/apache/jmeter/visualizers/TableVisualizer.java b/src/components/src/main/java/org/apache/jmeter/visualizers/TableVisualizer.java index 45afebf..8f3bbbc 100644 --- a/src/components/src/main/java/org/apache/jmeter/visualizers/TableVisualizer.java +++ b/src/components/src/main/java/org/apache/jmeter/visualizers/TableVisualizer.java @@ -54,6 +54,7 @@ import org.apache.jorphan.gui.RendererUtils; import org.apache.jorphan.gui.RightAlignRenderer; import org.apache.jorphan.gui.layout.VerticalLayout; import org.apache.jorphan.reflect.Functor; +import org.apache.jorphan.util.AlphaNumericComparator; /** * This class implements a statistical analyser that calculates both the average @@ -226,7 +227,7 @@ public class TableVisualizer extends AbstractVisualizer implements Clearable { // Set up the table itself table = new JTable(model); - table.setRowSorter(new ObjectTableSorter(model).setValueComparator(5, + final ObjectTableSorter rowSorter = new ObjectTableSorter(model).setValueComparator(5, Comparator.nullsFirst( (ImageIcon o1, ImageIcon o2) -> { if (o1 == o2) { @@ -239,7 +240,13 @@ public class TableVisualizer extends AbstractVisualizer implements Clearable { return 1; } throw new IllegalArgumentException("Only success and failure images can be compared"); - }))); + })); + for (int i=0; i<model.getColumnCount(); i++) { + if (model.getColumnClass(i).equals(String.class)) { + rowSorter.setValueComparator(i, new AlphaNumericComparator<Object>(o -> o.toString())); + } + } + table.setRowSorter(rowSorter); JMeterUtils.applyHiDPI(table); HeaderAsPropertyRendererWrapper.setupDefaultRenderer(table); RendererUtils.applyRenderers(table, RENDERERS); diff --git a/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericKeyComparator.java b/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericComparator.java similarity index 63% copy from src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericKeyComparator.java copy to src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericComparator.java index 9a610eb..9c56ae2 100644 --- a/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericKeyComparator.java +++ b/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericComparator.java @@ -16,24 +16,28 @@ */ package org.apache.jorphan.util; -import java.math.BigInteger; import java.util.Comparator; -import java.util.Map; +import java.util.function.Function; import java.util.regex.Matcher; import java.util.regex.Pattern; /** - * Comparator for {@link Map.Entry} Objects, that compares based on their keys only. The keys + * Comparator for Objects, that compares based on their <em>converted</em> values. The objects will be + * converted to a String value using the given {@link Function}. That value * will be compared in a human readable fashion by trying to parse numbers that appear in * the keys as integers and compare those, too.<p> * Heavily influenced by https://codereview.stackexchange.com/questions/37192/number-aware-string-sorting-with-comparator */ -public class AlphaNumericKeyComparator implements Comparator<Map.Entry<Object, Object>> { - - public static final AlphaNumericKeyComparator INSTANCE = new AlphaNumericKeyComparator(); - - private AlphaNumericKeyComparator() { - // don't instantiate this class on your own. +public class AlphaNumericComparator<T> implements Comparator<T> { + + private Function<T, String> converter; + + /** + * Constructs a comparator with a converter function + * @param converter that generates a String value from the arguments given to {@link Comparator#compare(Object, Object)} + */ + public AlphaNumericComparator(Function<T, String> converter) { + this.converter = converter; } private static final Pattern parts = Pattern.compile("(\\D*)(\\d*)"); @@ -41,9 +45,9 @@ public class AlphaNumericKeyComparator implements Comparator<Map.Entry<Object, O private static final int NUM_PART = 2; @Override - public int compare(Map.Entry<Object, Object> o1, Map.Entry<Object, Object> o2) { - Matcher m1 = parts.matcher(o1.getKey().toString()); - Matcher m2 = parts.matcher(o2.getKey().toString()); + public int compare(T o1, T o2) { + Matcher m1 = parts.matcher(converter.apply(o1)); + Matcher m2 = parts.matcher(converter.apply(o2)); while (m1.find() && m2.find()) { int compareCharGroup = m1.group(ALPHA_PART).compareTo(m2.group(ALPHA_PART)); @@ -60,17 +64,17 @@ public class AlphaNumericKeyComparator implements Comparator<Map.Entry<Object, O } else if (numberPart2.isEmpty()) { return 1; } - int lengthNumber1 = numberPart1.length(); - int lengthNumber2 = numberPart2.length(); + String nonZeroNumberPart1 = trimLeadingZeroes(numberPart1); + String nonZeroNumberPart2 = trimLeadingZeroes(numberPart2); + int lengthNumber1 = nonZeroNumberPart1.length(); + int lengthNumber2 = nonZeroNumberPart2.length(); if (lengthNumber1 != lengthNumber2) { if (lengthNumber1 < lengthNumber2) { return -1; } return 1; } - BigInteger i1 = new BigInteger(numberPart1); - BigInteger i2 = new BigInteger(numberPart2); - int compareNumber = i1.compareTo(i2); + int compareNumber = nonZeroNumberPart1.compareTo(nonZeroNumberPart2); if (compareNumber != 0) { return compareNumber; } @@ -84,4 +88,14 @@ public class AlphaNumericKeyComparator implements Comparator<Map.Entry<Object, O return 1; } + private String trimLeadingZeroes(String numberPart) { + int length = numberPart.length(); + for (int i = 0; i < length; i++) { + if (numberPart.charAt(i) != '0') { + return numberPart.substring(i); + } + } + return ""; + } + } diff --git a/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericKeyComparator.java b/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericKeyComparator.java index 9a610eb..3ed5e54 100644 --- a/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericKeyComparator.java +++ b/src/jorphan/src/main/java/org/apache/jorphan/util/AlphaNumericKeyComparator.java @@ -16,11 +16,8 @@ */ package org.apache.jorphan.util; -import java.math.BigInteger; import java.util.Comparator; import java.util.Map; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * Comparator for {@link Map.Entry} Objects, that compares based on their keys only. The keys @@ -31,57 +28,16 @@ import java.util.regex.Pattern; public class AlphaNumericKeyComparator implements Comparator<Map.Entry<Object, Object>> { public static final AlphaNumericKeyComparator INSTANCE = new AlphaNumericKeyComparator(); + private AlphaNumericComparator<Map.Entry<Object, Object>> comparator; private AlphaNumericKeyComparator() { // don't instantiate this class on your own. + this.comparator = new AlphaNumericComparator<Map.Entry<Object, Object>>(e -> e.getKey().toString()); } - private static final Pattern parts = Pattern.compile("(\\D*)(\\d*)"); - private static final int ALPHA_PART = 1; - private static final int NUM_PART = 2; - @Override public int compare(Map.Entry<Object, Object> o1, Map.Entry<Object, Object> o2) { - Matcher m1 = parts.matcher(o1.getKey().toString()); - Matcher m2 = parts.matcher(o2.getKey().toString()); - - while (m1.find() && m2.find()) { - int compareCharGroup = m1.group(ALPHA_PART).compareTo(m2.group(ALPHA_PART)); - if (compareCharGroup != 0) { - return compareCharGroup; - } - String numberPart1 = m1.group(NUM_PART); - String numberPart2 = m2.group(NUM_PART); - if (numberPart1.isEmpty()) { - if (numberPart2.isEmpty()) { - return 0; - } - return -1; - } else if (numberPart2.isEmpty()) { - return 1; - } - int lengthNumber1 = numberPart1.length(); - int lengthNumber2 = numberPart2.length(); - if (lengthNumber1 != lengthNumber2) { - if (lengthNumber1 < lengthNumber2) { - return -1; - } - return 1; - } - BigInteger i1 = new BigInteger(numberPart1); - BigInteger i2 = new BigInteger(numberPart2); - int compareNumber = i1.compareTo(i2); - if (compareNumber != 0) { - return compareNumber; - } - } - if (m1.hitEnd() && m2.hitEnd()) { - return 0; - } - if (m1.hitEnd()) { - return -1; - } - return 1; + return this.comparator.compare(o1, o2); } } diff --git a/src/jorphan/src/test/java/org/apache/jorphan/util/TestAlphaNumericKeyComparator.java b/src/jorphan/src/test/java/org/apache/jorphan/util/TestAlphaNumericKeyComparator.java index 4f19a41..a626d3c 100644 --- a/src/jorphan/src/test/java/org/apache/jorphan/util/TestAlphaNumericKeyComparator.java +++ b/src/jorphan/src/test/java/org/apache/jorphan/util/TestAlphaNumericKeyComparator.java @@ -13,16 +13,28 @@ class TestAlphaNumericKeyComparator { @ParameterizedTest @ValueSource(strings = { "abc", "", "var_123", "434", "_" }) - void testComparatorWithEqualKeys(String candidate) { + void testComparatorWithSameKeys(String candidate) { Comparator<Map.Entry<Object, Object>> comparator = AlphaNumericKeyComparator.INSTANCE; assertEquals(0, comparator.compare(entry(candidate), entry(candidate))); } @ParameterizedTest + @CsvSource({ "abc-001, abc-1", "007, 7", "0000, 0", "abc|000, abc|0" }) + void testComparatorWithEquivalentKeys(String left, String right) { + Comparator<Map.Entry<Object, Object>> comparator = AlphaNumericKeyComparator.INSTANCE; + assertEquals(0, comparator.compare(entry(left), entry(right))); + assertEquals(0, comparator.compare(entry(right), entry(left))); + } + + @ParameterizedTest @CsvSource({ "a, 1", + "something-0001, 999999999999999999999999999999999999999999999999999999999999999999999999999999", + "abc[23], abc[2]", "a10, a1", "a2, a1", + "2, 01", + "0010, 000005", "a20, a10", "a10, a2", "z, 10000", @@ -32,7 +44,9 @@ class TestAlphaNumericKeyComparator { "abc, ''", "'abc.,${something}1', 'abc.,${something}'", "number1, number", - "789b, 789" + "789b, 789", + "0xcafebabe, 0x8664", + "abc_0000, abc_" }) void testComparatorDifferentKeys(String higher, String lower) { Comparator<Map.Entry<Object, Object>> comparator = AlphaNumericKeyComparator.INSTANCE; diff --git a/xdocs/changes.xml b/xdocs/changes.xml index 64abc0f..2d0c1fd 100644 --- a/xdocs/changes.xml +++ b/xdocs/changes.xml @@ -91,6 +91,7 @@ Summary <h3>Listeners</h3> <ul> <li><bug>64988</bug>Sort properties and variables in a human expected order for DebugPostProcessor and DebugSampler</li> + <li><bug>63061</bug>Sort View Results in Table in a human expected order</li> </ul> <h3>Timers, Assertions, Config, Pre- & Post-Processors</h3>
