Author: fschumacher
Date: Wed Oct 14 20:28:55 2015
New Revision: 1708687
URL: http://svn.apache.org/viewvc?rev=1708687&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58489
Comparator should follow the rules. If first object has lastInvocation of zero,
we should compare it to the second objects lastInvocation and vice versa.
When we do that, we can use Long#compare just as well.
Added:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestSlowQueryComparator.java
Modified:
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
Modified:
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java?rev=1708687&r1=1708686&r2=1708687&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
Wed Oct 14 20:28:55 2015
@@ -475,17 +475,7 @@ public class SlowQueryReport extends Abs
@Override
public int compare(QueryStats stats1, QueryStats stats2) {
- if (stats1.lastInvocation == 0) return 1;
- if (stats2.lastInvocation == 0) return -1;
-
- long result = stats1.lastInvocation - stats2.lastInvocation;
- if (result > 0) {
- return 1;
- } else if (result == 0) {
- return 0;
- } else {
- return -1;
- }
+ return Long.compare(stats1.lastInvocation, stats2.lastInvocation);
}
}
Added:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestSlowQueryComparator.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestSlowQueryComparator.java?rev=1708687&view=auto
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestSlowQueryComparator.java
(added)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestSlowQueryComparator.java
Wed Oct 14 20:28:55 2015
@@ -0,0 +1,121 @@
+package org.apache.tomcat.jdbc.test;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+import org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport.QueryStats;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestSlowQueryComparator {
+
+ @Test
+ public void testBug58489() throws ClassNotFoundException,
+ InstantiationException, IllegalAccessException,
+ InvocationTargetException {
+
+ long[] testData = { 0, 0, 0, 1444225382010l, 0, 1444225382011l, 0,
+ 1444225382012l, 0, 1444225382056l, 0, 1444225382014l, 0,
+ 1444225382015l, 0, 1444225382016l, 0, 0, 1444225382017l, 0,
+ 1444225678350l, 0, 1444225680397l, 0, 1444225382018l,
+ 1444225382019l, 1444225382020l, 0, 1444225382021l, 0,
+ 1444225382022l, 1444225382023l
+
+ };
+
+ List<QueryStats> stats = new ArrayList<>();
+
+ for (int i = 0; i < testData.length; i++) {
+ QueryStats qs = new QueryStats(String.valueOf(i));
+ qs.add(0, testData[i]);
+ stats.add(qs);
+ }
+
+ try {
+ Collections.sort(stats, createComparator());
+ } catch (IllegalArgumentException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ @Test
+ public void testEqualQueryStatsWithNoLastInvocation()
+ throws ClassNotFoundException, InstantiationException,
+ IllegalAccessException, IllegalArgumentException,
+ InvocationTargetException {
+ Comparator<QueryStats> queryStatsComparator = createComparator();
+ QueryStats q1 = new QueryStats("abc");
+ Assert.assertEquals(0, queryStatsComparator.compare(q1, q1));
+ }
+
+ @Test
+ public void testEqualQueryStatsWithLastInvocation()
+ throws ClassNotFoundException, InstantiationException,
+ IllegalAccessException, IllegalArgumentException,
+ InvocationTargetException {
+ Comparator<QueryStats> queryStatsComparator = createComparator();
+ QueryStats q1 = new QueryStats("abc");
+ q1.add(0, 100);
+ Assert.assertEquals(0, queryStatsComparator.compare(q1, q1));
+ }
+
+ @Test
+ public void testQueryStatsOneWithLastInvocation()
+ throws ClassNotFoundException, InstantiationException,
+ IllegalAccessException, IllegalArgumentException,
+ InvocationTargetException {
+ Comparator<QueryStats> queryStatsComparator = createComparator();
+ QueryStats q1 = new QueryStats("abc");
+ QueryStats q2 = new QueryStats("def");
+ q2.add(0, 100);
+ Assert.assertEquals(-1, queryStatsComparator.compare(q1, q2));
+ Assert.assertEquals(1, queryStatsComparator.compare(q2, q1));
+ }
+
+ @Test
+ public void testQueryStatsBothWithSameLastInvocation()
+ throws ClassNotFoundException, InstantiationException,
+ IllegalAccessException, IllegalArgumentException,
+ InvocationTargetException {
+ Comparator<QueryStats> queryStatsComparator = createComparator();
+ QueryStats q1 = new QueryStats("abc");
+ QueryStats q2 = new QueryStats("def");
+ q1.add(0, 100);
+ q2.add(0, 100);
+ Assert.assertEquals(0, queryStatsComparator.compare(q1, q2));
+ Assert.assertEquals(0, queryStatsComparator.compare(q2, q1));
+ }
+
+ @Test
+ public void testQueryStatsBothWithSomeLastInvocation()
+ throws ClassNotFoundException, InstantiationException,
+ IllegalAccessException, IllegalArgumentException,
+ InvocationTargetException {
+ Comparator<QueryStats> queryStatsComparator = createComparator();
+ QueryStats q1 = new QueryStats("abc");
+ QueryStats q2 = new QueryStats("abc");
+ q1.add(0, 100);
+ q2.add(0, 150);
+ Assert.assertEquals(-1, queryStatsComparator.compare(q1, q2));
+ Assert.assertEquals(1, queryStatsComparator.compare(q2, q1));
+ }
+
+ private Comparator<QueryStats> createComparator()
+ throws ClassNotFoundException, InstantiationException,
+ IllegalAccessException, InvocationTargetException {
+ Class<?> comparatorClass = Class
+
.forName("org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport$QueryStatsComparator");
+ Constructor<?> comparatorConstructor = comparatorClass
+ .getDeclaredConstructors()[1];
+ comparatorConstructor.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Comparator<QueryStats> queryStatsComparator = (Comparator<QueryStats>)
comparatorConstructor
+ .newInstance();
+ return queryStatsComparator;
+ }
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
