From 104a0c0df11771ec6add2a3dd46a8352930edea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20Vi=C3=A9?= Date: Sat, 8 Jul 2017 02:36:42 +0200 Subject: [PATCH 1/3] Implement parsing and comparison on SemanticVersion --- .../japicmp/versioning/SemanticVersion.java | 146 +++++++++++++++++- 1 file changed, 145 insertions(+), 1 deletion(-) diff --git a/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java b/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java index 36d362bfe..4fd452f56 100644 --- a/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java +++ b/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java @@ -1,11 +1,15 @@ package japicmp.versioning; import com.google.common.base.Optional; +import java.util.List; +import java.util.ArrayList; -public class SemanticVersion { +public class SemanticVersion implements Comparable { private final int major; private final int minor; private final int patch; + private final List preReleaseIdentifiers; + private final String buildMetadata; public enum ChangeType { MAJOR(3), @@ -28,6 +32,71 @@ public SemanticVersion(int major, int minor, int patch) { this.major = major; this.minor = minor; this.patch = patch; + this.preReleaseIdentifiers = new ArrayList(); + this.buildMetadata = null; + } + + /** + Parses a semantic version string formatted as per http://semver.org/spec/v2.0.0.html + */ + public SemanticVersion(String versionDescription) { + + int firstDotPos = versionDescription.indexOf('.'); + int secondDotPos = versionDescription.indexOf('.', firstDotPos + 1); + int hyphenPos = versionDescription.indexOf('-', secondDotPos + 1); + int plusSignPos = -1; + + int patchEndPos = -1; + int idListEndPos = -1; + + this.major = Integer.valueOf(versionDescription.substring(0, firstDotPos - 1)); + this.minor = Integer.valueOf(versionDescription.substring(firstDotPos + 1, secondDotPos - 1)); + + if (hyphenPos != -1) { + patchEndPos = hyphenPos - 1; + plusSignPos = versionDescription.indexOf('+', hyphenPos + 1); + } + else { + plusSignPos = versionDescription.indexOf('+', secondDotPos + 1); + } + + if (plusSignPos != -1) { + if (hyphenPos != -1) { + idListEndPos = plusSignPos - 1; + } + else { + patchEndPos = plusSignPos - 1; + } + } + else { + if (hyphenPos != -1) { + idListEndPos = versionDescription.length() - 1; + } + else { + patchEndPos = versionDescription.length() - 1; + } + } + + this.patch = Integer.valueOf(versionDescription.substring(secondDotPos + 1, patchEndPos)); + this.preReleaseIdentifiers = (hyphenPos == -1) ? new ArrayList() : parseIdentifierList(versionDescription.substring(hyphenPos + 1, idListEndPos)); + this.buildMetadata = (plusSignPos == -1) ? null : versionDescription.substring(plusSignPos + 1); + } + + private List parseIdentifierList(String idListString) { + List idList = new ArrayList(); + + int currentDotPos = -1; + int nextDotPos = idListString.indexOf('.'); + + while (nextDotPos != -1) { + idList.add(idListString.substring(currentDotPos + 1, nextDotPos - 1)); + + currentDotPos = nextDotPos; + nextDotPos = idListString.indexOf('.', nextDotPos + 1); + } + idList.add(idListString.substring(currentDotPos + 1)); + + return idList; } public int getMajor() { @@ -75,4 +144,79 @@ public int hashCode() { public String toString() { return major + "." + minor + "." + patch; } + + private boolean isInteger(String str) { + for(int i = 0; i < str.length(); i++) + { + if(!Character.isDigit(str.charAt(i))) { + return false; + } + } + return true; + } + + /** + Compares two semantic version following the specifications at http://semver.org/spec/v2.0.0.html#spec-item-11 + */ + @Override + public int compareTo(SemanticVersion other) { + if(this.major == other.major) { + if(this.minor == other.minor) { + if(this.patch == other.patch) { + // "When major, minor, and patch are equal, a pre-release version has lower precedence than a normal version" as per spec. + if(this.preReleaseIdentifiers.size() == 0 && other.preReleaseIdentifiers.size() > 0) { + return 1; + } + else if(this.preReleaseIdentifiers.size() > 0 && other.preReleaseIdentifiers.size() == 0) { + return -1; + } + + // Compare all ids field by field, stopping at first difference. + for(int i = 0; i < Math.min(this.preReleaseIdentifiers.size(), other.preReleaseIdentifiers.size()); i++) { + String thisId = this.preReleaseIdentifiers.get(i); + String otherId = other.preReleaseIdentifiers.get(i); + + if (isInteger(thisId)) { + if (isInteger(otherId)) { + // Two integer identifiers : compare them. + int difference = Integer.valueOf(thisId) - Integer.valueOf(otherId); + if (difference != 0) { + return difference; + } + } + else { + // "Numeric identifiers always have lower precedence than non-numeric identifiers" as per spec. + return 1; + } + } + else { + if (isInteger(otherId)) { + // Same as above : int greater than string. + return -1; + } + else { + // Compare strings in lexicographic order. + int difference = thisId.compareTo(otherId); + if (difference != 0) { + return difference; + } + } + } + } + + // "A larger set of pre-release fields has a higher precedence than a smaller set, if all of the preceding identifiers are equal", as per spec. + return other.preReleaseIdentifiers.size() - this.preReleaseIdentifiers.size(); + } + else { + return this.patch - other.patch; + } + } + else { + return this.minor - other.minor; + } + } + else { + return this.major - other.major; + } + } } From 4a41fc098a6a6713bae8905108d84be70f0aae42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20Vi=C3=A9?= Date: Sat, 8 Jul 2017 20:35:50 +0200 Subject: [PATCH 2/3] Simplify code using Guava functions, Fix bugs, Create JUnit --- japicmp/pom.xml | 6 ++ .../japicmp/versioning/SemanticVersion.java | 45 ++++------- .../versioning/SemanticVersionTest.java | 77 +++++++++++++++++++ 3 files changed, 99 insertions(+), 29 deletions(-) create mode 100644 japicmp/src/test/java/japicmp/versioning/SemanticVersionTest.java diff --git a/japicmp/pom.xml b/japicmp/pom.xml index 626331bce..fa4932969 100644 --- a/japicmp/pom.xml +++ b/japicmp/pom.xml @@ -39,6 +39,12 @@ airline 0.7 + + org.hamcrest + hamcrest-library + 1.3 + test + diff --git a/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java b/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java index 4fd452f56..56d90b711 100644 --- a/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java +++ b/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java @@ -1,6 +1,8 @@ package japicmp.versioning; +import com.google.common.base.Joiner; import com.google.common.base.Optional; +import com.google.common.base.Splitter; import java.util.List; import java.util.ArrayList; @@ -49,11 +51,11 @@ public SemanticVersion(String versionDescription) { int patchEndPos = -1; int idListEndPos = -1; - this.major = Integer.valueOf(versionDescription.substring(0, firstDotPos - 1)); - this.minor = Integer.valueOf(versionDescription.substring(firstDotPos + 1, secondDotPos - 1)); + this.major = Integer.valueOf(versionDescription.substring(0, firstDotPos)); + this.minor = Integer.valueOf(versionDescription.substring(firstDotPos + 1, secondDotPos)); if (hyphenPos != -1) { - patchEndPos = hyphenPos - 1; + patchEndPos = hyphenPos; plusSignPos = versionDescription.indexOf('+', hyphenPos + 1); } else { @@ -62,42 +64,25 @@ public SemanticVersion(String versionDescription) { if (plusSignPos != -1) { if (hyphenPos != -1) { - idListEndPos = plusSignPos - 1; + idListEndPos = plusSignPos; } else { - patchEndPos = plusSignPos - 1; + patchEndPos = plusSignPos; } } else { if (hyphenPos != -1) { - idListEndPos = versionDescription.length() - 1; + idListEndPos = versionDescription.length(); } else { - patchEndPos = versionDescription.length() - 1; + patchEndPos = versionDescription.length(); } } this.patch = Integer.valueOf(versionDescription.substring(secondDotPos + 1, patchEndPos)); - this.preReleaseIdentifiers = (hyphenPos == -1) ? new ArrayList() : parseIdentifierList(versionDescription.substring(hyphenPos + 1, idListEndPos)); + this.preReleaseIdentifiers = (hyphenPos == -1) ? new ArrayList() : Splitter.on('.').splitToList(versionDescription.substring(hyphenPos + 1, idListEndPos)); this.buildMetadata = (plusSignPos == -1) ? null : versionDescription.substring(plusSignPos + 1); } - - private List parseIdentifierList(String idListString) { - List idList = new ArrayList(); - - int currentDotPos = -1; - int nextDotPos = idListString.indexOf('.'); - - while (nextDotPos != -1) { - idList.add(idListString.substring(currentDotPos + 1, nextDotPos - 1)); - - currentDotPos = nextDotPos; - nextDotPos = idListString.indexOf('.', nextDotPos + 1); - } - idList.add(idListString.substring(currentDotPos + 1)); - - return idList; - } public int getMajor() { return major; @@ -142,7 +127,9 @@ public int hashCode() { @Override public String toString() { - return major + "." + minor + "." + patch; + return major + "." + minor + "." + patch + + (preReleaseIdentifiers.size() > 0 ? "-" + Joiner.on('.').join(preReleaseIdentifiers) : "") + + (buildMetadata != null ? "+" + buildMetadata : ""); } private boolean isInteger(String str) { @@ -186,13 +173,13 @@ else if(this.preReleaseIdentifiers.size() > 0 && other.preReleaseIdentifiers.siz } else { // "Numeric identifiers always have lower precedence than non-numeric identifiers" as per spec. - return 1; + return -1; } } else { if (isInteger(otherId)) { // Same as above : int greater than string. - return -1; + return 1; } else { // Compare strings in lexicographic order. @@ -205,7 +192,7 @@ else if(this.preReleaseIdentifiers.size() > 0 && other.preReleaseIdentifiers.siz } // "A larger set of pre-release fields has a higher precedence than a smaller set, if all of the preceding identifiers are equal", as per spec. - return other.preReleaseIdentifiers.size() - this.preReleaseIdentifiers.size(); + return this.preReleaseIdentifiers.size() - other.preReleaseIdentifiers.size(); } else { return this.patch - other.patch; diff --git a/japicmp/src/test/java/japicmp/versioning/SemanticVersionTest.java b/japicmp/src/test/java/japicmp/versioning/SemanticVersionTest.java new file mode 100644 index 000000000..016d7c286 --- /dev/null +++ b/japicmp/src/test/java/japicmp/versioning/SemanticVersionTest.java @@ -0,0 +1,77 @@ +package japicmp.versioning; + +import japicmp.exception.JApiCmpException; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; + +import static org.hamcrest.Matchers.comparesEqualTo; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class SemanticVersionTest { + + @Test + public void testToStringReturnsSourceDescriptionStringIfCorrectlyFormatted() { + assertThat(new SemanticVersion("1.0.0-alpha+test").toString(), is("1.0.0-alpha+test")); + assertThat(new SemanticVersion("1.0.0").toString(), is("1.0.0")); + } + + /** + Validates that the "compareTo" implementation matches the specification for main version numbers. + */ + @Test + public void testVersionOrderMatches() { + String[] orderedExampleVersions = new String[] { + "1.0.0", "2.0.0", "2.1.0", "2.1.1" + }; + + SemanticVersion leftVersion = new SemanticVersion(orderedExampleVersions[0]); + + for(int i = 1; i < orderedExampleVersions.length; i++) { + SemanticVersion rightVersion = new SemanticVersion(orderedExampleVersions[i]); + + assertThat(leftVersion, lessThan(rightVersion)); + + leftVersion = rightVersion; + } + } + + /** + Validates that the "compareTo" implementation matches the order of the example provided in the SemVer specification for build identifiers. + cf. http://semver.org/spec/v2.0.0.html#spec-item-11 + */ + @Test + public void testBuildIdentifierOrderMatchesSpecExample() { + String[] orderedExampleVersions = new String[] { + "1.0.0-alpha", + "1.0.0-alpha.1", + "1.0.0-alpha.beta", + "1.0.0-beta", + "1.0.0-beta.2", + "1.0.0-beta.11", + "1.0.0-rc.1", + "1.0.0" + }; + + SemanticVersion leftVersion = new SemanticVersion(orderedExampleVersions[0]); + + for(int i = 1; i < orderedExampleVersions.length; i++) { + SemanticVersion rightVersion = new SemanticVersion(orderedExampleVersions[i]); + + assertThat(leftVersion, lessThan(rightVersion)); + + leftVersion = rightVersion; + } + } + + @Test + public void testBuildMetadataDoesNotAffectOrdering() { + assertThat(new SemanticVersion("1.0.0-alpha+test"), comparesEqualTo(new SemanticVersion("1.0.0-alpha"))); + assertThat(new SemanticVersion("1.0.0-alpha+test1"), comparesEqualTo(new SemanticVersion("1.0.0-alpha+test2"))); + } +} From d38d3a3b4013386379870bd1c81207ed4ec9e3f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20Vi=C3=A9?= Date: Mon, 10 Jul 2017 00:07:23 +0200 Subject: [PATCH 3/3] Integrate SemanticVersion in maven plugin for resolving previous version --- .../main/java/japicmp/maven/JApiCmpMojo.java | 3 + .../main/java/japicmp/maven/Parameter.java | 10 +++ .../maven/SemanticArtifactVersion.java | 62 +++++++++++++++++++ .../japicmp/maven/SemanticVersionRange.java | 42 +++++++++++++ .../japicmp/versioning/SemanticVersion.java | 4 ++ 5 files changed, 121 insertions(+) create mode 100644 japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticArtifactVersion.java create mode 100644 japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticVersionRange.java diff --git a/japicmp-maven-plugin/src/main/java/japicmp/maven/JApiCmpMojo.java b/japicmp-maven-plugin/src/main/java/japicmp/maven/JApiCmpMojo.java index e972ff43b..fcc098a09 100644 --- a/japicmp-maven-plugin/src/main/java/japicmp/maven/JApiCmpMojo.java +++ b/japicmp-maven-plugin/src/main/java/japicmp/maven/JApiCmpMojo.java @@ -189,6 +189,9 @@ private Artifact getComparisonArtifact(MavenParameters mavenParameters, PluginPa VersionRange versionRange; try { versionRange = VersionRange.createFromVersionSpec(mavenParameters.getVersionRangeWithProjectVersion()); + if (pluginParameters.getParameterParam().isUseSemanticVersioningOrderingToResolvePreviousVersion()) { + versionRange = SemanticVersionRange.getFromExistingVersionRange(versionRange); + } } catch (InvalidVersionSpecificationException e) { throw new MojoFailureException("Invalid version versionRange: " + e.getMessage(), e); } diff --git a/japicmp-maven-plugin/src/main/java/japicmp/maven/Parameter.java b/japicmp-maven-plugin/src/main/java/japicmp/maven/Parameter.java index fb0391df9..bb66fe79e 100644 --- a/japicmp-maven-plugin/src/main/java/japicmp/maven/Parameter.java +++ b/japicmp-maven-plugin/src/main/java/japicmp/maven/Parameter.java @@ -52,6 +52,8 @@ public class Parameter { private List excludeModules; @org.apache.maven.plugins.annotations.Parameter(required = false, defaultValue = "false") private boolean breakBuildBasedOnSemanticVersioningForMajorVersionZero; + @org.apache.maven.plugins.annotations.Parameter(required = false, defaultValue = "false") + private boolean useSemanticVersioningOrderingToResolvePreviousVersion; public String getNoAnnotations() { return noAnnotations; @@ -292,4 +294,12 @@ public boolean isBreakBuildBasedOnSemanticVersioningForMajorVersionZero() { public void setBreakBuildBasedOnSemanticVersioningForMajorVersionZero(boolean breakBuildBasedOnSemanticVersioningForMajorVersionZero) { this.breakBuildBasedOnSemanticVersioningForMajorVersionZero = breakBuildBasedOnSemanticVersioningForMajorVersionZero; } + + public boolean isUseSemanticVersioningOrderingToResolvePreviousVersion() { + return useSemanticVersioningOrderingToResolvePreviousVersion; + } + + public void setUseSemanticVersioningOrderingToResolvePreviousVersion(boolean useSemanticVersioningOrderingToResolvePreviousVersion) { + this.useSemanticVersioningOrderingToResolvePreviousVersion = useSemanticVersioningOrderingToResolvePreviousVersion; + } } diff --git a/japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticArtifactVersion.java b/japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticArtifactVersion.java new file mode 100644 index 000000000..585ba2cb0 --- /dev/null +++ b/japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticArtifactVersion.java @@ -0,0 +1,62 @@ +package japicmp.maven; + +import japicmp.versioning.SemanticVersion; +import org.apache.maven.artifact.versioning.ArtifactVersion; + +public class SemanticArtifactVersion implements ArtifactVersion { + private SemanticVersion comparable; + + public SemanticArtifactVersion(String version) { + parseVersion(version); + } + + public SemanticArtifactVersion(ArtifactVersion otherVersion) { + this(otherVersion.toString()); + } + + public int compareTo(ArtifactVersion otherVersion) { + if(otherVersion instanceof SemanticArtifactVersion) { + return this.comparable.compareTo(((SemanticArtifactVersion) otherVersion).comparable); + } + else { + try { + // If comparing to a non-semantic version, try to transform it into semantic and compare. + SemanticArtifactVersion otherSemVer = new SemanticArtifactVersion(otherVersion.toString()); + return this.comparable.compareTo(otherSemVer.comparable); + } + catch(Exception e) { + // Else, other version not semantic. Consider it older by default. + return 1; + } + } + } + + public int getMajorVersion() { + return comparable.getMajor(); + } + + public int getMinorVersion() { + return comparable.getMinor(); + } + + public int getIncrementalVersion() { + return comparable.getPatch(); + } + + public int getBuildNumber() { + return 0; + } + + public String getQualifier() { + return comparable.getPreReleaseIdentifier(); + } + + public final void parseVersion(String version) { + comparable = new SemanticVersion(version); + } + + @Override + public String toString() { + return comparable.toString(); + } +} \ No newline at end of file diff --git a/japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticVersionRange.java b/japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticVersionRange.java new file mode 100644 index 000000000..6c38b31b3 --- /dev/null +++ b/japicmp-maven-plugin/src/main/java/japicmp/maven/SemanticVersionRange.java @@ -0,0 +1,42 @@ +package japicmp.maven; + +import java.util.List; +import org.apache.maven.artifact.versioning.ArtifactVersion; +import org.apache.maven.artifact.versioning.Restriction; +import org.apache.maven.artifact.versioning.VersionRange; + +public class SemanticVersionRange { + + /** + Return a semantic version range from an existing maven VersionRange, by converting . + + This is a bit of a dirty hack, necessary because : + - VersionRange hardcodes construction of DefaultArtifactVersion (not making it possible to use other implementations of ArtifactVersion) + - All the internals of VersionRange are private, so it cannot be extended easily. + + This is not reliable either : since it requires an existing maven range to have been created, and VersionRange.parseRestriction checks ordering of restriction bounds (using DefaultArtifactVersion order), some ranges could not be generated when the ordering differs. + + However, in the current case, it is only meant to be used on the versionRangeWithProjectVersion setting, which has only one bound : (,${project.version}). + Therefore this implementation is enough, and avoids rewriting all of VersionRange's parsing logic. + */ + public static VersionRange getFromExistingVersionRange(VersionRange existingRange) { + if(existingRange.getRecommendedVersion() != null && !(existingRange.getRecommendedVersion() instanceof SemanticArtifactVersion)) { + throw new UnsupportedOperationException("Cannot convert an already decided version range to semantic in current implementation."); + } + VersionRange newRange = existingRange.cloneOf(); + List restrictions = newRange.getRestrictions(); + for(int i = 0; i < restrictions.size(); i++) { + Restriction currentRestriction = restrictions.get(i); + + restrictions.set(i, new Restriction( + new SemanticArtifactVersion(currentRestriction.getLowerBound()), + currentRestriction.isLowerBoundInclusive(), + new SemanticArtifactVersion(currentRestriction.getUpperBound()), + currentRestriction.isUpperBoundInclusive() + )); + } + + return newRange; + } + +} \ No newline at end of file diff --git a/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java b/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java index 56d90b711..28f70140e 100644 --- a/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java +++ b/japicmp/src/main/java/japicmp/versioning/SemanticVersion.java @@ -95,6 +95,10 @@ public int getMinor() { public int getPatch() { return patch; } + + public String getPreReleaseIdentifier() { + return Joiner.on('.').join(preReleaseIdentifiers); + } @Override public boolean equals(Object o) {