From e21a03f7175dd2c563adc252de2574bd82c9e55e Mon Sep 17 00:00:00 2001 From: Aaron Myers Date: Mon, 27 Jan 2014 21:36:42 +0000 Subject: [PATCH] HADOOP-10250. VersionUtil returns wrong value when comparing two versions. Contributed by Yongjun Zhang. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1561860 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../dev-support/findbugsExcludeFile.xml | 7 + .../apache/hadoop/util/ComparableVersion.java | 479 ++++++++++++++++++ .../org/apache/hadoop/util/VersionUtil.java | 106 +--- .../apache/hadoop/util/TestVersionUtil.java | 40 +- 5 files changed, 534 insertions(+), 101 deletions(-) create mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ComparableVersion.java diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 5a1e36b0d5..5c39282e9a 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -539,6 +539,9 @@ Release 2.4.0 - UNRELEASED HADOOP-10203. Connection leak in Jets3tNativeFileSystemStore#retrieveMetadata. (Andrei Savu via atm) + HADOOP-10250. VersionUtil returns wrong value when comparing two versions. + (Yongjun Zhang via atm) + Release 2.3.0 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml b/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml index bf4da979ae..3e5661ea0d 100644 --- a/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml +++ b/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml @@ -364,4 +364,11 @@ + + + + + + + diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ComparableVersion.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ComparableVersion.java new file mode 100644 index 0000000000..a57342fa88 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ComparableVersion.java @@ -0,0 +1,479 @@ +// Code source of this file: +// http://grepcode.com/file/repo1.maven.org/maven2/ +// org.apache.maven/maven-artifact/3.1.1/ +// org/apache/maven/artifact/versioning/ComparableVersion.java/ +// +// Modifications made on top of the source: +// 1. Changed +// package org.apache.maven.artifact.versioning; +// to +// package org.apache.hadoop.util; +// 2. Removed author tags to clear hadoop author tag warning +// author Kenney Westerhof +// author Hervé Boutemy +// +package org.apache.hadoop.util; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.Locale; +import java.util.Properties; +import java.util.Stack; + +/** + * Generic implementation of version comparison. + * + *

Features: + *

+ * + * @see "Versioning" on Maven Wiki + */ +public class ComparableVersion + implements Comparable +{ + private String value; + + private String canonical; + + private ListItem items; + + private interface Item + { + int INTEGER_ITEM = 0; + int STRING_ITEM = 1; + int LIST_ITEM = 2; + + int compareTo( Item item ); + + int getType(); + + boolean isNull(); + } + + /** + * Represents a numeric item in the version item list. + */ + private static class IntegerItem + implements Item + { + private static final BigInteger BIG_INTEGER_ZERO = new BigInteger( "0" ); + + private final BigInteger value; + + public static final IntegerItem ZERO = new IntegerItem(); + + private IntegerItem() + { + this.value = BIG_INTEGER_ZERO; + } + + public IntegerItem( String str ) + { + this.value = new BigInteger( str ); + } + + public int getType() + { + return INTEGER_ITEM; + } + + public boolean isNull() + { + return BIG_INTEGER_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 + } + + switch ( item.getType() ) + { + case INTEGER_ITEM: + return value.compareTo( ( (IntegerItem) 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 value.toString(); + } + } + + /** + * Represents a string in the version item list, usually a qualifier. + */ + private static class StringItem + implements Item + { + private static final String[] QUALIFIERS = { "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" }; + + private static final List _QUALIFIERS = Arrays.asList( QUALIFIERS ); + + private static final Properties ALIASES = new Properties(); + static + { + ALIASES.put( "ga", "" ); + ALIASES.put( "final", "" ); + ALIASES.put( "cr", "rc" ); + } + + /** + * A comparable value for the empty-string qualifier. This one is used to determine if a given qualifier makes + * the version older than one without a qualifier, or more recent. + */ + private static final String RELEASE_VERSION_INDEX = String.valueOf( _QUALIFIERS.indexOf( "" ) ); + + private String value; + + public StringItem( String value, boolean followedByDigit ) + { + if ( followedByDigit && value.length() == 1 ) + { + // a1 = alpha-1, b1 = beta-1, m1 = milestone-1 + switch ( value.charAt( 0 ) ) + { + case 'a': + value = "alpha"; + break; + case 'b': + value = "beta"; + break; + case 'm': + value = "milestone"; + break; + } + } + this.value = ALIASES.getProperty( value , value ); + } + + public int getType() + { + return STRING_ITEM; + } + + public boolean isNull() + { + return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 ); + } + + /** + * Returns a comparable value for a qualifier. + * + * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical ordering. + * + * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1 + * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character, + * so this is still fast. If more characters are needed then it requires a lexical sort anyway. + * + * @param qualifier + * @return an equivalent value that can be used with lexical comparison + */ + public static String comparableQualifier( String qualifier ) + { + int i = _QUALIFIERS.indexOf( qualifier ); + + return i == -1 ? ( _QUALIFIERS.size() + "-" + qualifier ) : String.valueOf( i ); + } + + public int compareTo( Item item ) + { + if ( item == null ) + { + // 1-rc < 1, 1-ga > 1 + return comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ); + } + switch ( item.getType() ) + { + case INTEGER_ITEM: + return -1; // 1.any < 1.1 ? + + case STRING_ITEM: + return comparableQualifier( value ).compareTo( comparableQualifier( ( (StringItem) item ).value ) ); + + case LIST_ITEM: + return -1; // 1.any < 1-1 + + default: + throw new RuntimeException( "invalid item: " + item.getClass() ); + } + } + + public String toString() + { + return value; + } + } + + /** + * Represents a version list item. This class is used both for the global item list and for sub-lists (which start + * with '-(number)' in the version specification). + */ + private static class ListItem + extends ArrayList + implements Item + { + public int getType() + { + return LIST_ITEM; + } + + public boolean isNull() + { + return ( size() == 0 ); + } + + void normalize() + { + for ( ListIterator iterator = listIterator( size() ); iterator.hasPrevious(); ) + { + Item item = iterator.previous(); + if ( item.isNull() ) + { + iterator.remove(); // remove null trailing items: 0, "", empty list + } + else + { + break; + } + } + } + + public int compareTo( Item item ) + { + if ( item == null ) + { + if ( size() == 0 ) + { + return 0; // 1-0 = 1- (normalize) = 1 + } + Item first = get( 0 ); + return first.compareTo( null ); + } + switch ( item.getType() ) + { + case INTEGER_ITEM: + return -1; // 1-1 < 1.0.x + + case STRING_ITEM: + return 1; // 1-1 > 1-sp + + case LIST_ITEM: + Iterator left = iterator(); + Iterator right = ( (ListItem) item ).iterator(); + + while ( left.hasNext() || right.hasNext() ) + { + Item l = left.hasNext() ? left.next() : null; + Item r = right.hasNext() ? right.next() : null; + + // if this is shorter, then invert the compare and mul with -1 + int result = l == null ? -1 * r.compareTo( l ) : l.compareTo( r ); + + if ( result != 0 ) + { + return result; + } + } + + return 0; + + default: + throw new RuntimeException( "invalid item: " + item.getClass() ); + } + } + + public String toString() + { + StringBuilder buffer = new StringBuilder( "(" ); + for ( Iterator iter = iterator(); iter.hasNext(); ) + { + buffer.append( iter.next() ); + if ( iter.hasNext() ) + { + buffer.append( ',' ); + } + } + buffer.append( ')' ); + return buffer.toString(); + } + } + + public ComparableVersion( String version ) + { + parseVersion( version ); + } + + public final void parseVersion( String version ) + { + this.value = version; + + items = new ListItem(); + + version = version.toLowerCase( Locale.ENGLISH ); + + ListItem list = items; + + Stack stack = new Stack(); + stack.push( list ); + + boolean isDigit = false; + + int startIndex = 0; + + for ( int i = 0; i < version.length(); i++ ) + { + char c = version.charAt( i ); + + if ( c == '.' ) + { + if ( i == startIndex ) + { + list.add( IntegerItem.ZERO ); + } + else + { + list.add( parseItem( isDigit, version.substring( startIndex, i ) ) ); + } + startIndex = i + 1; + } + else if ( c == '-' ) + { + if ( i == startIndex ) + { + list.add( IntegerItem.ZERO ); + } + else + { + list.add( parseItem( isDigit, version.substring( startIndex, i ) ) ); + } + startIndex = i + 1; + + if ( isDigit ) + { + list.normalize(); // 1.0-* = 1-* + + if ( ( i + 1 < version.length() ) && Character.isDigit( version.charAt( i + 1 ) ) ) + { + // new ListItem only if previous were digits and new char is a digit, + // ie need to differentiate only 1.1 from 1-1 + list.add( list = new ListItem() ); + + stack.push( list ); + } + } + } + else if ( Character.isDigit( c ) ) + { + if ( !isDigit && i > startIndex ) + { + list.add( new StringItem( version.substring( startIndex, i ), true ) ); + startIndex = i; + } + + isDigit = true; + } + else + { + if ( isDigit && i > startIndex ) + { + list.add( parseItem( true, version.substring( startIndex, i ) ) ); + startIndex = i; + } + + isDigit = false; + } + } + + if ( version.length() > startIndex ) + { + list.add( parseItem( isDigit, version.substring( startIndex ) ) ); + } + + while ( !stack.isEmpty() ) + { + list = (ListItem) stack.pop(); + list.normalize(); + } + + canonical = items.toString(); + } + + private static Item parseItem( boolean isDigit, String buf ) + { + return isDigit ? new IntegerItem( buf ) : new StringItem( buf, false ); + } + + public int compareTo( ComparableVersion o ) + { + return items.compareTo( o.items ); + } + + public String toString() + { + return value; + } + + public boolean equals( Object o ) + { + return ( o instanceof ComparableVersion ) && canonical.equals( ( (ComparableVersion) o ).canonical ); + } + + public int hashCode() + { + return canonical.hashCode(); + } +} diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java index 09a272317f..3e14fa91f6 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java @@ -17,55 +17,17 @@ */ package org.apache.hadoop.util; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - import org.apache.hadoop.classification.InterfaceAudience; -import com.google.common.collect.ComparisonChain; - +/** + * A wrapper class to maven's ComparableVersion class, to comply + * with maven's version name string convention + */ @InterfaceAudience.Private public abstract class VersionUtil { - - private static final Pattern COMPONENT_GROUPS = Pattern.compile("(\\d+)|(\\D+)"); - /** - * Suffix added by maven for nightly builds and other snapshot releases. - * These releases are considered to precede the non-SNAPSHOT version - * with the same version number. - */ - private static final String SNAPSHOT_SUFFIX = "-SNAPSHOT"; - - /** - * This function splits the two versions on "." and performs a - * naturally-ordered comparison of the resulting components. For example, the - * version string "0.3" is considered to precede "0.20", despite the fact that - * lexical comparison would consider "0.20" to precede "0.3". This method of - * comparison is similar to the method used by package versioning systems like - * deb and RPM. - * - * Version components are compared numerically whenever possible, however a - * version component can contain non-numeric characters. When a non-numeric - * group of characters is found in a version component, this group is compared - * with the similarly-indexed group in the other version component. If the - * other group is numeric, then the numeric group is considered to precede the - * non-numeric group. If both groups are non-numeric, then a lexical - * comparison is performed. - * - * If two versions have a different number of components, then only the lower - * number of components are compared. If those components are identical - * between the two versions, then the version with fewer components is - * considered to precede the version with more components. - * - * In addition to the above rules, there is one special case: maven SNAPSHOT - * releases are considered to precede a non-SNAPSHOT release with an - * otherwise identical version number. For example, 2.0-SNAPSHOT precedes - * 2.0. - * - * This function returns a negative integer if version1 precedes version2, a - * positive integer if version2 precedes version1, and 0 if and only if the - * two versions' components are identical in value and cardinality. - * + * Compares two version name strings using maven's ComparableVersion class. + * * @param version1 * the first version to compare * @param version2 @@ -75,58 +37,8 @@ public abstract class VersionUtil { * versions are equal. */ public static int compareVersions(String version1, String version2) { - boolean isSnapshot1 = version1.endsWith(SNAPSHOT_SUFFIX); - boolean isSnapshot2 = version2.endsWith(SNAPSHOT_SUFFIX); - version1 = stripSnapshotSuffix(version1); - version2 = stripSnapshotSuffix(version2); - - String[] version1Parts = version1.split("\\."); - String[] version2Parts = version2.split("\\."); - - for (int i = 0; i < version1Parts.length && i < version2Parts.length; i++) { - String component1 = version1Parts[i]; - String component2 = version2Parts[i]; - if (!component1.equals(component2)) { - Matcher matcher1 = COMPONENT_GROUPS.matcher(component1); - Matcher matcher2 = COMPONENT_GROUPS.matcher(component2); - - while (matcher1.find() && matcher2.find()) { - String group1 = matcher1.group(); - String group2 = matcher2.group(); - if (!group1.equals(group2)) { - if (isNumeric(group1) && isNumeric(group2)) { - return Integer.parseInt(group1) - Integer.parseInt(group2); - } else if (!isNumeric(group1) && !isNumeric(group2)) { - return group1.compareTo(group2); - } else { - return isNumeric(group1) ? -1 : 1; - } - } - } - return component1.length() - component2.length(); - } - } - - return ComparisonChain.start() - .compare(version1Parts.length, version2Parts.length) - .compare(isSnapshot2, isSnapshot1) - .result(); - } - - private static String stripSnapshotSuffix(String version) { - if (version.endsWith(SNAPSHOT_SUFFIX)) { - return version.substring(0, version.length() - SNAPSHOT_SUFFIX.length()); - } else { - return version; - } - } - - private static boolean isNumeric(String s) { - try { - Integer.parseInt(s); - return true; - } catch (NumberFormatException nfe) { - return false; - } + ComparableVersion v1 = new ComparableVersion(version1); + ComparableVersion v2 = new ComparableVersion(version2); + return v1.compareTo(v2); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java index f01ae2f73d..b1737dcb52 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java @@ -28,10 +28,30 @@ public void testCompareVersions() { // Equal versions are equal. assertEquals(0, VersionUtil.compareVersions("2.0.0", "2.0.0")); assertEquals(0, VersionUtil.compareVersions("2.0.0a", "2.0.0a")); - assertEquals(0, VersionUtil.compareVersions("1", "1")); assertEquals(0, VersionUtil.compareVersions( "2.0.0-SNAPSHOT", "2.0.0-SNAPSHOT")); - + + assertEquals(0, VersionUtil.compareVersions("1", "1")); + assertEquals(0, VersionUtil.compareVersions("1", "1.0")); + assertEquals(0, VersionUtil.compareVersions("1", "1.0.0")); + + assertEquals(0, VersionUtil.compareVersions("1.0", "1")); + assertEquals(0, VersionUtil.compareVersions("1.0", "1.0")); + assertEquals(0, VersionUtil.compareVersions("1.0", "1.0.0")); + + assertEquals(0, VersionUtil.compareVersions("1.0.0", "1")); + assertEquals(0, VersionUtil.compareVersions("1.0.0", "1.0")); + assertEquals(0, VersionUtil.compareVersions("1.0.0", "1.0.0")); + + assertEquals(0, VersionUtil.compareVersions("1.0.0-alpha-1", "1.0.0-a1")); + assertEquals(0, VersionUtil.compareVersions("1.0.0-alpha-2", "1.0.0-a2")); + assertEquals(0, VersionUtil.compareVersions("1.0.0-alpha1", "1.0.0-alpha-1")); + + assertEquals(0, VersionUtil.compareVersions("1a0", "1.0.0-alpha-0")); + assertEquals(0, VersionUtil.compareVersions("1a0", "1-a0")); + assertEquals(0, VersionUtil.compareVersions("1.a0", "1-a0")); + assertEquals(0, VersionUtil.compareVersions("1.a0", "1.0.0-alpha-0")); + // Assert that lower versions are lower, and higher versions are higher. assertExpectedValues("1", "2.0.0"); assertExpectedValues("1.0.0", "2"); @@ -51,15 +71,27 @@ public void testCompareVersions() { assertExpectedValues("1.0.2a", "1.0.2ab"); assertExpectedValues("1.0.0a1", "1.0.0a2"); assertExpectedValues("1.0.0a2", "1.0.0a10"); + // The 'a' in "1.a" is not followed by digit, thus not treated as "alpha", + // and treated larger than "1.0", per maven's ComparableVersion class + // implementation. assertExpectedValues("1.0", "1.a"); - assertExpectedValues("1.0", "1.a0"); + //The 'a' in "1.a0" is followed by digit, thus treated as "alpha-" + assertExpectedValues("1.a0", "1.0"); + assertExpectedValues("1a0", "1.0"); + assertExpectedValues("1.0.1-alpha-1", "1.0.1-alpha-2"); + assertExpectedValues("1.0.1-beta-1", "1.0.1-beta-2"); // Snapshot builds precede their eventual releases. assertExpectedValues("1.0-SNAPSHOT", "1.0"); - assertExpectedValues("1.0", "1.0.0-SNAPSHOT"); + assertExpectedValues("1.0.0-SNAPSHOT", "1.0"); assertExpectedValues("1.0.0-SNAPSHOT", "1.0.0"); assertExpectedValues("1.0.0", "1.0.1-SNAPSHOT"); assertExpectedValues("1.0.1-SNAPSHOT", "1.0.1"); + assertExpectedValues("1.0.1-SNAPSHOT", "1.0.2"); + + assertExpectedValues("1.0.1-alpha-1", "1.0.1-SNAPSHOT"); + assertExpectedValues("1.0.1-beta-1", "1.0.1-SNAPSHOT"); + assertExpectedValues("1.0.1-beta-2", "1.0.1-SNAPSHOT"); } private static void assertExpectedValues(String lower, String higher) {