Skip to content

Commit

Permalink
8333303: Issues with DottedVersion class
Browse files Browse the repository at this point in the history 
Reviewed-by: almatvee
  • Loading branch information
Alexey Semenyuk committed May 31, 2024
1 parent e304a8a commit 1b7d59f
Show file tree
Hide file tree
Showing 6 changed files with 262 additions and 125 deletions.
226 changes: 140 additions & 86 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -22,33 +22,122 @@
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package jdk.jpackage.internal;

import java.math.BigInteger;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.List;
import java.util.Arrays;
import java.util.Objects;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Dotted numeric version string.
* E.g.: 1.0.37, 10, 0.5
* Dotted numeric version string. E.g.: 1.0.37, 10, 0.5
*/
final class DottedVersion implements Comparable<String> {
final class DottedVersion {

DottedVersion(String version) {
greedy = true;
components = parseVersionString(version, greedy);
value = version;
this(version, true);
}

private DottedVersion(String version, boolean greedy) {
this.greedy = greedy;
components = parseVersionString(version, greedy);
value = version;
this.value = version;
if (version.isEmpty()) {
if (greedy) {
throw new IllegalArgumentException(I18N.getString("error.version-string-empty"));
} else {
this.components = new BigInteger[0];
this.suffix = "";
}
} else {
var ds = new DigitsSupplier(version);
components = Stream.generate(ds::getNextDigits).takeWhile(Objects::nonNull).map(
digits -> {
if (digits.isEmpty()) {
if (!greedy) {
return null;
} else {
throw new IllegalArgumentException(MessageFormat.format(I18N.
getString("error.version-string-zero-length-component"),
version));
}
}

try {
return new BigInteger(digits);
} catch (NumberFormatException ex) {
if (!greedy) {
return null;
} else {
throw new IllegalArgumentException(MessageFormat.format(I18N.
getString("error.version-string-invalid-component"), version,
digits));
}
}
}).takeWhile(Objects::nonNull).toArray(BigInteger[]::new);
suffix = ds.getUnprocessedString();
if (!suffix.isEmpty() && greedy) {
throw new IllegalArgumentException(MessageFormat.format(I18N.getString(
"error.version-string-invalid-component"), version, suffix));
}
}
}

private static class DigitsSupplier {

DigitsSupplier(String input) {
this.input = input;
}

public String getNextDigits() {
if (stoped) {
return null;
}

var sb = new StringBuilder();
while (cursor != input.length()) {
var chr = input.charAt(cursor++);
if (Character.isDigit(chr)) {
sb.append(chr);
} else {
var curStopAtDot = (chr == '.');
if (!curStopAtDot) {
if (lastDotPos >= 0) {
cursor = lastDotPos;
} else {
cursor--;
}
stoped = true;
} else if (!sb.isEmpty()) {
lastDotPos = cursor - 1;
} else {
cursor = Math.max(lastDotPos, 0);
stoped = true;
}
return sb.toString();
}
}

if (sb.isEmpty()) {
if (lastDotPos >= 0) {
cursor = lastDotPos;
} else {
cursor--;
}
}

stoped = true;
return sb.toString();
}

public String getUnprocessedString() {
return input.substring(cursor);
}

private int cursor;
private int lastDotPos = -1;
private boolean stoped;
private final String input;
}

static DottedVersion greedy(String version) {
Expand All @@ -59,22 +148,22 @@ static DottedVersion lazy(String version) {
return new DottedVersion(version, false);
}

@Override
public int compareTo(String o) {
static int compareComponents(DottedVersion a, DottedVersion b) {
int result = 0;
BigInteger[] otherComponents = parseVersionString(o, greedy);
for (int i = 0; i < Math.max(components.length, otherComponents.length)
BigInteger[] aComponents = a.getComponents();
BigInteger[] bComponents = b.getComponents();
for (int i = 0; i < Math.max(aComponents.length, bComponents.length)
&& result == 0; ++i) {
final BigInteger x;
if (i < components.length) {
x = components[i];
if (i < aComponents.length) {
x = aComponents[i];
} else {
x = BigInteger.ZERO;
}

final BigInteger y;
if (i < otherComponents.length) {
y = otherComponents[i];
if (i < bComponents.length) {
y = bComponents[i];
} else {
y = BigInteger.ZERO;
}
Expand All @@ -84,85 +173,50 @@ public int compareTo(String o) {
return result;
}

private static BigInteger[] parseVersionString(String version, boolean greedy) {
Objects.requireNonNull(version);
if (version.isEmpty()) {
if (!greedy) {
return new BigInteger[] {BigInteger.ZERO};
}
throw new IllegalArgumentException(I18N.getString(
"error.version-string-empty"));
}

int lastNotZeroIdx = -1;
List<BigInteger> components = new ArrayList<>();
for (var component : version.split("\\.", -1)) {
if (component.isEmpty()) {
if (!greedy) {
break;
}

throw new IllegalArgumentException(MessageFormat.format(
I18N.getString(
"error.version-string-zero-length-component"),
version));
}

if (!DIGITS.matcher(component).matches()) {
// Catch "+N" and "-N" cases.
if (!greedy) {
break;
}

throw new IllegalArgumentException(MessageFormat.format(
I18N.getString(
"error.version-string-invalid-component"),
version, component));
}

final BigInteger num;
try {
num = new BigInteger(component);
} catch (NumberFormatException ex) {
if (!greedy) {
break;
}

throw new IllegalArgumentException(MessageFormat.format(
I18N.getString(
"error.version-string-invalid-component"),
version, component));
}
@Override
public int hashCode() {
int hash = 3;
hash = 29 * hash + Arrays.deepHashCode(this.components);
hash = 29 * hash + Objects.hashCode(this.suffix);
return hash;
}

if (num != BigInteger.ZERO) {
lastNotZeroIdx = components.size();
}
components.add(num);
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}

if (lastNotZeroIdx + 1 != components.size()) {
// Strip trailing zeros.
components = components.subList(0, lastNotZeroIdx + 1);
if (obj == null) {
return false;
}

if (components.isEmpty()) {
components.add(BigInteger.ZERO);
if (getClass() != obj.getClass()) {
return false;
}
return components.toArray(BigInteger[]::new);
final DottedVersion other = (DottedVersion) obj;
if (!Objects.equals(this.suffix, other.suffix)) {
return false;
}
return Arrays.deepEquals(this.components, other.components);
}

@Override
public String toString() {
return value;
}

public String getUnprocessedSuffix() {
return suffix;
}

String toComponentsString() {
return Stream.of(components).map(BigInteger::toString).collect(Collectors.joining("."));
}

BigInteger[] getComponents() {
return components;
}

private final BigInteger[] components;
private final String value;
private final boolean greedy;

private static final Pattern DIGITS = Pattern.compile("\\d+");
private final String suffix;
}
6 changes: 5 additions & 1 deletion src/jdk.jpackage/share/classes/jdk/jpackage/internal/ToolValidator.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -65,6 +65,10 @@ ToolValidator setMinimalVersion(Comparable<String> v) {
return this;
}

ToolValidator setMinimalVersion(DottedVersion v) {
return setMinimalVersion(t -> DottedVersion.compareComponents(v, DottedVersion.lazy(t)));
}

ToolValidator setVersionParser(Function<Stream<String>, String> v) {
versionParser = v;
return this;
Expand Down
6 changes: 3 additions & 3 deletions src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WixAppImageFragmentBuilder.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -785,7 +785,7 @@ private void addRegistryKeyPath(XMLStreamWriter xml, Path path,
xml.writeStartElement("RegistryKey");
xml.writeAttribute("Root", regRoot);
xml.writeAttribute("Key", registryKeyPath);
if (getWixVersion().compareTo("3.6") < 0) {
if (DottedVersion.compareComponents(getWixVersion(), DottedVersion.lazy("3.6")) < 0) {
xml.writeAttribute("Action", "createAndRemoveOnUninstall");
}
xml.writeStartElement("RegistryValue");
Expand All @@ -799,7 +799,7 @@ private void addRegistryKeyPath(XMLStreamWriter xml, Path path,

private String addDirectoryCleaner(XMLStreamWriter xml, Path path) throws
XMLStreamException, IOException {
if (getWixVersion().compareTo("3.6") < 0) {
if (DottedVersion.compareComponents(getWixVersion(), DottedVersion.lazy("3.6")) < 0) {
return null;
}

Expand Down
4 changes: 2 additions & 2 deletions src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WixFragmentBuilder.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -66,7 +66,7 @@ void initFromParams(Map<String, ? super Object> params) {
}

void logWixFeatures() {
if (wixVersion.compareTo("3.6") >= 0) {
if (DottedVersion.compareComponents(wixVersion, DottedVersion.lazy("3.6")) >= 0) {
Log.verbose(MessageFormat.format(I18N.getString(
"message.use-wix36-features"), wixVersion));
}
Expand Down
6 changes: 4 additions & 2 deletions ...jdk/tools/jpackage/junit/jdk.jpackage/jdk/jpackage/internal/CompareDottedVersionTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -68,6 +68,8 @@ public static List<Object[]> data() {

data.addAll(List.of(new Object[][] {
{ false, "", "1", -1 },
{ false, "", "0", 0 },
{ false, "0", "", 0 },
{ false, "1.2.4-R4", "1.2.4-R5", 0 },
{ false, "1.2.4.-R4", "1.2.4.R5", 0 },
{ false, "7+1", "7+4", 0 },
Expand All @@ -89,7 +91,7 @@ public void testIt() {
}

private int compare(String x, String y) {
int result = createTestee.apply(x).compareTo(y);
int result = DottedVersion.compareComponents(createTestee.apply(x), createTestee.apply(y));

if (result < 0) {
return -1;
Expand Down
Loading

1 comment on commit 1b7d59f

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.