Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNG-8146] Drop commons-lang #1564

Merged
merged 3 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions maven-artifact/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ under the License.
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.Map;
import java.util.regex.Matcher;

import org.apache.commons.lang3.Validate;
import org.apache.maven.artifact.versioning.VersionRange;

/**
Expand Down Expand Up @@ -91,7 +90,9 @@ public static String key(String groupId, String artifactId, String version) {
private static void notBlank(String str, String message) {
int c = str != null && str.length() > 0 ? str.charAt(0) : 0;
if ((c < '0' || c > '9') && (c < 'a' || c > 'z')) {
Validate.notBlank(str, message);
if (str == null || str.trim().isEmpty()) {
throw new IllegalArgumentException(message);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

import java.util.StringTokenizer;

import static org.apache.commons.lang3.math.NumberUtils.isDigits;

/**
* Default implementation of artifact versioning.
*
Expand Down Expand Up @@ -176,6 +174,18 @@ private static Integer getNextIntegerToken(StringTokenizer tok) {
return tryParseInt(s);
}

private static boolean isDigits(String str) {
if (str == null || str.trim().isEmpty()) {
return false;
}
for (int i = 0; i < str.length(); i++) {
if (!Character.isDigit(str.charAt(i))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to allow digits from other alphabets beyond the usual 0-9. I don't think we want that, do we?

Copy link
Member Author

Choose a reason for hiding this comment

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

return false;
}
}
return true;
}

private static Integer tryParseInt(String s) {
// for performance, check digits instead of relying later on catching NumberFormatException
if (!isDigits(s)) {
Expand Down
4 changes: 0 additions & 4 deletions maven-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,6 @@ under the License.
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-component-annotations</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package org.apache.maven.configuration;

import org.apache.commons.lang3.Validate;
import org.apache.maven.model.Build;
import org.apache.maven.model.Model;
import org.apache.maven.model.Plugin;
Expand Down Expand Up @@ -104,8 +103,12 @@ public DefaultBeanConfigurationRequest setConfiguration(
}

private Plugin findPlugin(Model model, String groupId, String artifactId) {
Validate.notBlank(groupId, "groupId can neither be null, empty nor blank");
Validate.notBlank(artifactId, "artifactId can neither be null, empty nor blank");
if (groupId == null || groupId.trim().isEmpty()) {
throw new IllegalArgumentException("groupId can neither be null, empty nor blank");
}
if (artifactId == null || artifactId.trim().isEmpty()) {
throw new IllegalArgumentException("artifactId can neither be null, empty nor blank");
}

if (model != null) {
Build build = model.getBuild();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
import java.io.InputStream;
import java.util.Properties;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Validate;
import org.apache.maven.rtinfo.RuntimeInformation;
import org.codehaus.plexus.component.annotations.Component;
import org.codehaus.plexus.component.annotations.Requirement;
Expand Down Expand Up @@ -80,10 +78,11 @@ public String getMavenVersion() {
}

public boolean isMavenVersion(String versionRange) {
if (versionRange == null || versionRange.trim().isEmpty()) {
throw new IllegalArgumentException("versionRange can neither be null, empty nor blank");
}
VersionScheme versionScheme = new GenericVersionScheme();

Validate.notBlank(versionRange, "versionRange can neither be null, empty nor blank");

VersionConstraint constraint;
try {
constraint = versionScheme.parseVersionConstraint(versionRange);
Expand All @@ -94,7 +93,9 @@ public boolean isMavenVersion(String versionRange) {
Version current;
try {
String mavenVersion = getMavenVersion();
Validate.validState(StringUtils.isNotEmpty(mavenVersion), "Could not determine current Maven version");
if (mavenVersion == null || mavenVersion.trim().isEmpty()) {
throw new IllegalStateException("Could not determine current Maven version");
}

current = versionScheme.parseVersion(mavenVersion);
} catch (InvalidVersionSpecificationException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ public void testIsMavenVersion() throws Exception {
try {
rtInfo.isMavenVersion(null);
fail("Bad version range wasn't rejected");
} catch (NullPointerException e) {
} catch (IllegalArgumentException e) {
// distinction between IAEx and NPE makes no sense: argument is wrong
// moreover, the distinction does not give any benefit either
assertTrue(true);
}
}
Expand Down
4 changes: 0 additions & 4 deletions maven-embedder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,6 @@ under the License.
<artifactId>commons-io</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
import java.util.Locale;
import java.util.Properties;

import org.apache.commons.lang3.StringUtils;
import org.codehaus.plexus.util.Os;
import org.codehaus.plexus.util.StringUtils;
import org.slf4j.Logger;

import static org.apache.maven.shared.utils.logging.MessageUtils.buffer;
Expand Down
32 changes: 14 additions & 18 deletions maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import org.apache.commons.cli.Option;
import org.apache.commons.cli.ParseException;
import org.apache.commons.cli.UnrecognizedOptionException;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.maven.BuildAbort;
import org.apache.maven.InternalErrorException;
import org.apache.maven.Maven;
Expand Down Expand Up @@ -1442,27 +1441,24 @@ int calculateDegreeOfConcurrency(String threadConfiguration) {
if (threadConfiguration.endsWith("C")) {
threadConfiguration = threadConfiguration.substring(0, threadConfiguration.length() - 1);

if (!NumberUtils.isParsable(threadConfiguration)) {
throw new IllegalArgumentException("Invalid threads core multiplier value: '" + threadConfiguration
+ "C'. Supported are int and float values ending with C.");
}

float coreMultiplier = Float.parseFloat(threadConfiguration);
try {
float coreMultiplier = Float.parseFloat(threadConfiguration);

if (coreMultiplier <= 0.0f) {
throw new IllegalArgumentException("Invalid threads core multiplier value: '" + threadConfiguration
+ "C'. Value must be positive.");
}
if (coreMultiplier <= 0.0f) {
throw new IllegalArgumentException("Invalid threads core multiplier value: '" + threadConfiguration
+ "C'. Value must be positive.");
}

int procs = Runtime.getRuntime().availableProcessors();
int threads = (int) (coreMultiplier * procs);
return threads == 0 ? 1 : threads;
} else {
if (!NumberUtils.isParsable(threadConfiguration)) {
int procs = Runtime.getRuntime().availableProcessors();
int threads = (int) (coreMultiplier * procs);
return threads == 0 ? 1 : threads;
} catch (NumberFormatException e) {
throw new IllegalArgumentException(
"Invalid threads value: '" + threadConfiguration + "'. Supported are int values.");
"Invalid threads core multiplier value: '" + threadConfiguration
+ "C'. Supported are int and float values ending with C.",
e);
}

} else {
try {
int threads = Integer.parseInt(threadConfiguration);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.text.DecimalFormatSymbols;
import java.util.Locale;

import org.apache.commons.lang3.Validate;
import org.apache.maven.shared.utils.logging.MessageUtils;
import org.eclipse.aether.transfer.AbstractTransferListener;
import org.eclipse.aether.transfer.TransferCancelledException;
Expand Down Expand Up @@ -105,7 +104,9 @@ public String symbol() {
public abstract String symbol();

public static ScaleUnit getScaleUnit(long size) {
Validate.isTrue(size >= 0L, "file size cannot be negative: %s", size);
if (size < 0L) {
throw new IllegalArgumentException("file size cannot be negative: " + size);
}

if (size >= GIGABYTE.bytes()) {
return GIGABYTE;
Expand Down Expand Up @@ -137,7 +138,9 @@ public String format(long size, ScaleUnit unit) {

@SuppressWarnings("checkstyle:magicnumber")
public String format(long size, ScaleUnit unit, boolean omitSymbol) {
Validate.isTrue(size >= 0L, "file size cannot be negative: %s", size);
if (size < 0L) {
throw new IllegalArgumentException("file size cannot be negative: " + size);
}

if (unit == null) {
unit = ScaleUnit.getScaleUnit(size);
Expand All @@ -162,12 +165,13 @@ public String format(long size, ScaleUnit unit, boolean omitSymbol) {
}

public String formatProgress(long progressedSize, long size) {
Validate.isTrue(progressedSize >= 0L, "progressed file size cannot be negative: %s", progressedSize);
Validate.isTrue(
size >= 0L && progressedSize <= size || size < 0L,
"progressed file size cannot be greater than size: %s > %s",
progressedSize,
size);
if (progressedSize < 0L) {
throw new IllegalArgumentException("progressed file size cannot be negative: " + progressedSize);
}
if (size >= 0L && progressedSize > size) {
throw new IllegalArgumentException(
"progressed file size cannot be greater than size: " + progressedSize + " > " + size);
}

if (size >= 0L && progressedSize != size) {
ScaleUnit unit = ScaleUnit.getScaleUnit(size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.Locale;
import java.util.Map;

import org.apache.commons.lang3.StringUtils;
import org.eclipse.aether.transfer.TransferCancelledException;
import org.eclipse.aether.transfer.TransferEvent;
import org.eclipse.aether.transfer.TransferResource;
Expand All @@ -37,10 +36,10 @@
*/
public class ConsoleMavenTransferListener extends AbstractMavenTransferListener {

private Map<TransferResource, Long> transfers =
private final Map<TransferResource, Long> transfers =
Collections.synchronizedMap(new LinkedHashMap<TransferResource, Long>());

private boolean printResourceNames;
private final boolean printResourceNames;
private int lastLength;

public ConsoleMavenTransferListener(PrintStream out, boolean printResourceNames) {
Expand Down Expand Up @@ -97,7 +96,7 @@ private String getStatus(String resourceName, long complete, long total) {
StringBuilder status = new StringBuilder();

if (printResourceNames) {
status.append(StringUtils.substringAfterLast(resourceName, "/"));
status.append(resourceName(resourceName));
status.append(" (");
}

Expand All @@ -110,6 +109,17 @@ private String getStatus(String resourceName, long complete, long total) {
return status.toString();
}

private String resourceName(String resourceName) {
if (resourceName == null || resourceName.trim().isEmpty()) {
return "";
}
final int pos = resourceName.lastIndexOf("/");
if (pos == -1 || pos == resourceName.length() - 1) {
return "";
}
return resourceName.substring(pos + 1);
}

private void pad(StringBuilder buffer, int spaces) {
String block = " ";
while (spaces > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ public void testCalculateDegreeOfConcurrency() {
int cpus = Runtime.getRuntime().availableProcessors();
assertEquals((int) (cpus * 2.2), cli.calculateDegreeOfConcurrency("2.2C"));
assertEquals(1, cli.calculateDegreeOfConcurrency("0.0001C"));
assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("2.C"));
// Note: this assertion below makes no sense, first Float.parseFloat("2.") DOES parse the string in 2.0.
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, just delete this then

// Second, the limitation to reject string "2." was actually commons-lang3 NumberUtils.isParsable limitation
// We should not "skew" our input validation by some utility, while using Java classes to perform actual parsing
// assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("2.C"));
assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("-2.2C"));
assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("0C"));
}
Expand Down
16 changes: 0 additions & 16 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ under the License.
<classWorldsVersion>2.8.0</classWorldsVersion>
<commonsCliVersion>1.8.0</commonsCliVersion>
<commonsIoVersion>2.16.1</commonsIoVersion>
<commonsLangVersion>3.14.0</commonsLangVersion>
<junitVersion>4.13.2</junitVersion>
<hamcrestVersion>2.2</hamcrestVersion>
<mockitoVersion>4.11.0</mockitoVersion>
Expand Down Expand Up @@ -442,16 +441,6 @@ under the License.
<groupId>commons-cli</groupId>
<artifactId>commons-cli</artifactId>
<version>${commonsCliVersion}</version>
<exclusions>
<exclusion>
<groupId>commons-lang</groupId>
<artifactId>commons-lang</artifactId>
</exclusion>
<exclusion>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove the commons-logging exclusion? This is unrelated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because commons-cli does not depend on any of these, this seems legacy that was never removed? Commons cli deps are only these:

  <dependencies>
    <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter-api</artifactId>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter-engine</artifactId>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter-params</artifactId>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>commons-io</groupId>
      <artifactId>commons-io</artifactId>
      <version>2.16.1</version>
      <scope>test</scope>
    </dependency>
  </dependencies>

Copy link
Member

Choose a reason for hiding this comment

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

Commons Logging has been removed as a dep?

</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>commons-io</groupId>
Expand All @@ -463,11 +452,6 @@ under the License.
<artifactId>commons-jxpath</artifactId>
<version>${jxpathVersion}</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>${commonsLangVersion}</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-sec-dispatcher</artifactId>
Expand Down