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:40 UTC

[maven] 04/04: Optimization of comparison of Items:

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();