You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by rf...@apache.org on 2019/02/20 19:53:36 UTC
[maven] branch MNG-6572 created (now 9c7774b)
This is an automated email from the ASF dual-hosted git repository.
rfscholte pushed a change to branch MNG-6572
in repository https://gitbox.apache.org/repos/asf/maven.git.
at 9c7774b Optimization of comparison of Items:
This branch includes the following new commits:
new 07baa5b use int or long instead of BigIntegers for little numbers in ComparableVersion
new c600f73 Corrected according to suggestions:
new 01eb2e9 Additional modifications from reviews:
new 9c7774b Optimization of comparison of Items:
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[maven] 02/04: Corrected according to suggestions:
Posted by rf...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rfscholte pushed a commit to branch MNG-6572
in repository https://gitbox.apache.org/repos/asf/maven.git
commit c600f730fa2630b4ea6f829ac1be2ec2d09a961f
Author: Gabriel Belingueres <be...@gmail.com>
AuthorDate: Sat Jan 26 14:44:55 2019 -0300
Corrected according to suggestions:
- Changed LinkedList to ArrayDeque.
- Moved import below license section in test.
---
.../java/org/apache/maven/artifact/versioning/ComparableVersion.java | 4 ++--
.../org/apache/maven/artifact/versioning/ComparableVersionTest.java | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
index ad71ed0..c32760f 100644
--- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
+++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
@@ -20,10 +20,10 @@ package org.apache.maven.artifact.versioning;
*/
import java.math.BigInteger;
+import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
-import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Properties;
@@ -498,7 +498,7 @@ public class ComparableVersion
ListItem list = items;
- LinkedList<Item> stack = new LinkedList<>();
+ ArrayDeque<Item> stack = new ArrayDeque<>();
stack.push( list );
boolean isDigit = false;
diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
index a5a2588..abe7d4a 100644
--- a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
+++ b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
@@ -1,7 +1,5 @@
package org.apache.maven.artifact.versioning;
-import junit.framework.TestCase;
-
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -23,6 +21,8 @@ import junit.framework.TestCase;
import java.util.Locale;
+import junit.framework.TestCase;
+
/**
* Test ComparableVersion.
*
[maven] 03/04: Additional modifications from reviews:
Posted by rf...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rfscholte pushed a commit to branch MNG-6572
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 01eb2e962467b0e3150a5f386e0294e67a306e53
Author: Gabriel Belingueres <be...@gmail.com>
AuthorDate: Sun Jan 27 00:25:28 2019 -0300
Additional modifications from reviews:
- Declare ArrayDeque variable by its interface (Deque).
- Changed throwed RuntimeException by IllegalStateException.
---
.../apache/maven/artifact/versioning/ComparableVersion.java | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
index c32760f..6d190d7 100644
--- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
+++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
@@ -23,6 +23,7 @@ import java.math.BigInteger;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Deque;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
@@ -142,7 +143,7 @@ public class ComparableVersion
return 1; // 1.1 > 1-1
default:
- throw new RuntimeException( "invalid item: " + item.getClass() );
+ throw new IllegalStateException( "invalid item: " + item.getClass() );
}
}
@@ -204,7 +205,7 @@ public class ComparableVersion
return 1; // 1.1 > 1-1
default:
- throw new RuntimeException( "invalid item: " + item.getClass() );
+ throw new IllegalStateException( "invalid item: " + item.getClass() );
}
}
@@ -267,7 +268,7 @@ public class ComparableVersion
return 1; // 1.1 > 1-1
default:
- throw new RuntimeException( "invalid item: " + item.getClass() );
+ throw new IllegalStateException( "invalid item: " + item.getClass() );
}
}
@@ -375,7 +376,7 @@ public class ComparableVersion
return -1; // 1.any < 1-1
default:
- throw new RuntimeException( "invalid item: " + item.getClass() );
+ throw new IllegalStateException( "invalid item: " + item.getClass() );
}
}
@@ -463,7 +464,7 @@ public class ComparableVersion
return 0;
default:
- throw new RuntimeException( "invalid item: " + item.getClass() );
+ throw new IllegalStateException( "invalid item: " + item.getClass() );
}
}
@@ -498,7 +499,7 @@ public class ComparableVersion
ListItem list = items;
- ArrayDeque<Item> stack = new ArrayDeque<>();
+ Deque<Item> stack = new ArrayDeque<>();
stack.push( list );
boolean isDigit = false;
[maven] 04/04: Optimization of comparison of Items:
Posted by rf...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rfscholte pushed a commit to branch MNG-6572
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 9c7774b7f404cd0f20cdbd95604d1a992d93cfa4
Author: Gabriel Belingueres <be...@gmail.com>
AuthorDate: Sun Jan 27 12:12:03 2019 -0300
Optimization of comparison of Items:
- Ensure numeric values don't have leading zeroes, therefore ensuring
that IntItem, LongItem and BigIntItem represent bigger numeric values,
respectively.
- Only compare item value when the other Item is of the same type.
Otherwise infer comparison result from the quantity of digits of the
numerical value representing the other Item.
- Added tests.
---
.../artifact/versioning/ComparableVersion.java | 33 +++++-----
.../artifact/versioning/ComparableVersionTest.java | 72 ++++++++++++++++++++++
2 files changed, 87 insertions(+), 18 deletions(-)
diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
index 6d190d7..f58feb5 100644
--- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
+++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
@@ -29,6 +29,8 @@ import java.util.List;
import java.util.Locale;
import java.util.Properties;
+import org.apache.commons.lang3.StringUtils;
+
/**
* <p>
* Generic implementation of version comparison.
@@ -129,12 +131,8 @@ public class ComparableVersion
return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
}
case LONG_ITEM:
- {
- long itemValue = ( (LongItem) item ).value;
- return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
- }
case BIGINTEGER_ITEM:
- return BigInteger.valueOf( value ).compareTo( ( (BigIntegerItem) item ).value );
+ return -1;
case STRING_ITEM:
return 1; // 1.1 > 1-sp
@@ -186,17 +184,14 @@ public class ComparableVersion
switch ( item.getType() )
{
case INT_ITEM:
- {
- int itemValue = ( (IntItem) item ).value;
- return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
- }
+ return 1;
case LONG_ITEM:
{
long itemValue = ( (LongItem) item ).value;
return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
}
case BIGINTEGER_ITEM:
- return BigInteger.valueOf( value ).compareTo( ( (BigIntegerItem) item ).value );
+ return -1;
case STRING_ITEM:
return 1; // 1.1 > 1-sp
@@ -248,15 +243,8 @@ public class ComparableVersion
switch ( item.getType() )
{
case INT_ITEM:
- {
- int itemValue = ( (IntItem) item ).value;
- return value.compareTo( BigInteger.valueOf( itemValue ) );
- }
case LONG_ITEM:
- {
- long itemValue = ( (LongItem) item ).value;
- return value.compareTo( BigInteger.valueOf( itemValue ) );
- }
+ return 1;
case BIGINTEGER_ITEM:
return value.compareTo( ( (BigIntegerItem) item ).value );
@@ -583,6 +571,7 @@ public class ComparableVersion
{
if ( isDigit )
{
+ buf = stripLeadingZeroes( buf );
if ( buf.length() <= 9 )
{
// lower than 2^31
@@ -598,6 +587,14 @@ public class ComparableVersion
return new StringItem( buf, false );
}
+ private static String stripLeadingZeroes( String buf )
+ {
+ String strippedBuf = StringUtils.stripStart( buf, "0" );
+ if ( strippedBuf.isEmpty() )
+ return "0";
+ return strippedBuf;
+ }
+
public int compareTo( ComparableVersion o )
{
return items.compareTo( o.items );
diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
index abe7d4a..ce7df2d 100644
--- a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
+++ b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
@@ -85,6 +85,14 @@ public class ComparableVersionTest
assertTrue( "expected " + v2 + ".equals( " + v1 + " )", c2.equals( c1 ) );
}
+ private void checkVersionsArrayEqual( String[] array )
+ {
+ // compare against each other (including itself)
+ for ( int i = 0; i < array.length; ++i )
+ for ( int j = i; j < array.length; ++j )
+ checkVersionsEqual( array[i], array[j] );
+ }
+
private void checkVersionsOrder( String v1, String v2 )
{
Comparable c1 = newComparable( v1 );
@@ -219,6 +227,70 @@ public class ComparableVersionTest
checkVersionsOrder( a, d );
}
+ /**
+ * Test all versions are equal when starting with many leading zeroes regardless of string length
+ * (related to MNG-6572 optimization)
+ */
+ public void testVersionEqualWithLeadingZeroes()
+ {
+ // versions with string lengths from 1 to 19
+ String[] arr = new String[] {
+ "0000000000000000001",
+ "000000000000000001",
+ "00000000000000001",
+ "0000000000000001",
+ "000000000000001",
+ "00000000000001",
+ "0000000000001",
+ "000000000001",
+ "00000000001",
+ "0000000001",
+ "000000001",
+ "00000001",
+ "0000001",
+ "000001",
+ "00001",
+ "0001",
+ "001",
+ "01",
+ "1"
+ };
+
+ checkVersionsArrayEqual( arr );
+ }
+
+ /**
+ * Test all "0" versions are equal when starting with many leading zeroes regardless of string length
+ * (related to MNG-6572 optimization)
+ */
+ public void testVersionZeroEqualWithLeadingZeroes()
+ {
+ // versions with string lengths from 1 to 19
+ String[] arr = new String[] {
+ "0000000000000000000",
+ "000000000000000000",
+ "00000000000000000",
+ "0000000000000000",
+ "000000000000000",
+ "00000000000000",
+ "0000000000000",
+ "000000000000",
+ "00000000000",
+ "0000000000",
+ "000000000",
+ "00000000",
+ "0000000",
+ "000000",
+ "00000",
+ "0000",
+ "000",
+ "00",
+ "0"
+ };
+
+ checkVersionsArrayEqual( arr );
+ }
+
public void testLocaleIndependent()
{
Locale orig = Locale.getDefault();
[maven] 01/04: use int or long instead of BigIntegers for little
numbers in ComparableVersion
Posted by rf...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rfscholte pushed a commit to branch MNG-6572
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 07baa5b899a9a23471a4de44d0cef92c32bb06b3
Author: Gabriel Belingueres <be...@gmail.com>
AuthorDate: Sat Jan 26 05:23:54 2019 -0300
use int or long instead of BigIntegers for little numbers in ComparableVersion
- Added class IntItem and LongItem for handling numbers lower than 2^31
and 2^63.
- Renamed IntegerItem to BigIntegerItem for handling larger numbers.
- Changed old Stack implementation to LinkedList.
---
.../artifact/versioning/ComparableVersion.java | 195 ++++++++++++++++++---
.../artifact/versioning/ComparableVersionTest.java | 22 ++-
2 files changed, 194 insertions(+), 23 deletions(-)
diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
index 4c64d2b..ad71ed0 100644
--- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
+++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
@@ -23,10 +23,10 @@ import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
+import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Properties;
-import java.util.Stack;
/**
* <p>
@@ -70,7 +70,9 @@ public class ComparableVersion
private interface Item
{
- int INTEGER_ITEM = 0;
+ int INT_ITEM = 3;
+ int LONG_ITEM = 4;
+ int BIGINTEGER_ITEM = 0;
int STRING_ITEM = 1;
int LIST_ITEM = 2;
@@ -82,48 +84,181 @@ public class ComparableVersion
}
/**
- * Represents a numeric item in the version item list.
+ * Represents a numeric item in the version item list that can be represented with an int.
*/
- private static class IntegerItem
+ private static class IntItem
implements Item
{
- private static final BigInteger BIG_INTEGER_ZERO = new BigInteger( "0" );
+ private final int value;
- private final BigInteger value;
+ public static final IntItem ZERO = new IntItem();
+
+ private IntItem()
+ {
+ this.value = 0;
+ }
+
+ IntItem( String str )
+ {
+ this.value = Integer.parseInt( str );
+ }
+
+ public int getType()
+ {
+ return INT_ITEM;
+ }
+
+ public boolean isNull()
+ {
+ return value == 0;
+ }
+
+ public int compareTo( Item item )
+ {
+ if ( item == null )
+ {
+ return ( value == 0 ) ? 0 : 1; // 1.0 == 1, 1.1 > 1
+ }
+
+ switch ( item.getType() )
+ {
+ case INT_ITEM:
+ {
+ int itemValue = ( (IntItem) item ).value;
+ return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
+ }
+ case LONG_ITEM:
+ {
+ long itemValue = ( (LongItem) item ).value;
+ return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
+ }
+ case BIGINTEGER_ITEM:
+ return BigInteger.valueOf( value ).compareTo( ( (BigIntegerItem) item ).value );
+
+ case STRING_ITEM:
+ return 1; // 1.1 > 1-sp
+
+ case LIST_ITEM:
+ return 1; // 1.1 > 1-1
+
+ default:
+ throw new RuntimeException( "invalid item: " + item.getClass() );
+ }
+ }
+
+ public String toString()
+ {
+ return Integer.toString( value );
+ }
+ }
+
+ /**
+ * Represents a numeric item in the version item list that can be represented with a long.
+ */
+ private static class LongItem
+ implements Item
+ {
+ private final long value;
+
+ LongItem( String str )
+ {
+ this.value = Long.parseLong( str );
+ }
+
+ public int getType()
+ {
+ return LONG_ITEM;
+ }
+
+ public boolean isNull()
+ {
+ return value == 0;
+ }
+
+ public int compareTo( Item item )
+ {
+ if ( item == null )
+ {
+ return ( value == 0 ) ? 0 : 1; // 1.0 == 1, 1.1 > 1
+ }
+
+ switch ( item.getType() )
+ {
+ case INT_ITEM:
+ {
+ int itemValue = ( (IntItem) item ).value;
+ return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
+ }
+ case LONG_ITEM:
+ {
+ long itemValue = ( (LongItem) item ).value;
+ return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
+ }
+ case BIGINTEGER_ITEM:
+ return BigInteger.valueOf( value ).compareTo( ( (BigIntegerItem) item ).value );
+
+ case STRING_ITEM:
+ return 1; // 1.1 > 1-sp
+
+ case LIST_ITEM:
+ return 1; // 1.1 > 1-1
- public static final IntegerItem ZERO = new IntegerItem();
+ default:
+ throw new RuntimeException( "invalid item: " + item.getClass() );
+ }
+ }
- private IntegerItem()
+ public String toString()
{
- this.value = BIG_INTEGER_ZERO;
+ return Long.toString( value );
}
+ }
+
+ /**
+ * Represents a numeric item in the version item list.
+ */
+ private static class BigIntegerItem
+ implements Item
+ {
+ private final BigInteger value;
- IntegerItem( String str )
+ BigIntegerItem( String str )
{
this.value = new BigInteger( str );
}
public int getType()
{
- return INTEGER_ITEM;
+ return BIGINTEGER_ITEM;
}
public boolean isNull()
{
- return BIG_INTEGER_ZERO.equals( value );
+ return BigInteger.ZERO.equals( value );
}
public int compareTo( Item item )
{
if ( item == null )
{
- return BIG_INTEGER_ZERO.equals( value ) ? 0 : 1; // 1.0 == 1, 1.1 > 1
+ return BigInteger.ZERO.equals( value ) ? 0 : 1; // 1.0 == 1, 1.1 > 1
}
switch ( item.getType() )
{
- case INTEGER_ITEM:
- return value.compareTo( ( (IntegerItem) item ).value );
+ case INT_ITEM:
+ {
+ int itemValue = ( (IntItem) item ).value;
+ return value.compareTo( BigInteger.valueOf( itemValue ) );
+ }
+ case LONG_ITEM:
+ {
+ long itemValue = ( (LongItem) item ).value;
+ return value.compareTo( BigInteger.valueOf( itemValue ) );
+ }
+
+ case BIGINTEGER_ITEM:
+ return value.compareTo( ( (BigIntegerItem) item ).value );
case STRING_ITEM:
return 1; // 1.1 > 1-sp
@@ -228,7 +363,9 @@ public class ComparableVersion
}
switch ( item.getType() )
{
- case INTEGER_ITEM:
+ case INT_ITEM:
+ case LONG_ITEM:
+ case BIGINTEGER_ITEM:
return -1; // 1.any < 1.1 ?
case STRING_ITEM:
@@ -297,7 +434,9 @@ public class ComparableVersion
}
switch ( item.getType() )
{
- case INTEGER_ITEM:
+ case INT_ITEM:
+ case LONG_ITEM:
+ case BIGINTEGER_ITEM:
return -1; // 1-1 < 1.0.x
case STRING_ITEM:
@@ -359,7 +498,7 @@ public class ComparableVersion
ListItem list = items;
- Stack<Item> stack = new Stack<>();
+ LinkedList<Item> stack = new LinkedList<>();
stack.push( list );
boolean isDigit = false;
@@ -374,7 +513,7 @@ public class ComparableVersion
{
if ( i == startIndex )
{
- list.add( IntegerItem.ZERO );
+ list.add( IntItem.ZERO );
}
else
{
@@ -386,7 +525,7 @@ public class ComparableVersion
{
if ( i == startIndex )
{
- list.add( IntegerItem.ZERO );
+ list.add( IntItem.ZERO );
}
else
{
@@ -441,7 +580,21 @@ public class ComparableVersion
private static Item parseItem( boolean isDigit, String buf )
{
- return isDigit ? new IntegerItem( buf ) : new StringItem( buf, false );
+ if ( isDigit )
+ {
+ if ( buf.length() <= 9 )
+ {
+ // lower than 2^31
+ return new IntItem( buf );
+ }
+ else if ( buf.length() <= 18 )
+ {
+ // lower than 2^63
+ return new LongItem( buf );
+ }
+ return new BigIntegerItem( buf );
+ }
+ return new StringItem( buf, false );
}
public int compareTo( ComparableVersion o )
diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
index 875b43e..a5a2588 100644
--- a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
+++ b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
@@ -1,5 +1,7 @@
package org.apache.maven.artifact.versioning;
+import junit.framework.TestCase;
+
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -21,8 +23,6 @@ package org.apache.maven.artifact.versioning;
import java.util.Locale;
-import junit.framework.TestCase;
-
/**
* Test ComparableVersion.
*
@@ -201,6 +201,24 @@ public class ComparableVersionTest
checkVersionsOrder( a, c );
}
+ /**
+ * Test <a href="https://jira.apache.org/jira/browse/MNG-6572">MNG-6572</a> optimization.
+ */
+ public void testMng6572()
+ {
+ String a = "20190126.230843"; // resembles a SNAPSHOT
+ String b = "1234567890.12345"; // 10 digit number
+ String c = "123456789012345.1H.5-beta"; // 15 digit number
+ String d = "12345678901234567890.1H.5-beta"; // 20 digit number
+
+ checkVersionsOrder( a, b );
+ checkVersionsOrder( b, c );
+ checkVersionsOrder( a, c );
+ checkVersionsOrder( c, d );
+ checkVersionsOrder( b, d );
+ checkVersionsOrder( a, d );
+ }
+
public void testLocaleIndependent()
{
Locale orig = Locale.getDefault();