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-8510] Rename cling.invoker.Utils class and reduce public API surface #2041

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.ServiceLoader;
import java.util.function.UnaryOperator;
import java.util.stream.Collectors;

import org.apache.maven.api.Constants;
import org.apache.maven.api.annotations.Nonnull;
import org.apache.maven.api.annotations.Nullable;
import org.apache.maven.api.cli.InvokerRequest;
import org.apache.maven.api.cli.Options;
Expand All @@ -45,21 +47,33 @@
import org.apache.maven.api.cli.ParserRequest;
import org.apache.maven.api.cli.extensions.CoreExtension;
import org.apache.maven.api.services.Interpolator;
import org.apache.maven.api.services.model.RootLocator;
import org.apache.maven.cling.internal.extension.io.CoreExtensionsStaxReader;
import org.apache.maven.cling.props.MavenPropertiesLoader;
import org.apache.maven.cling.utils.CLIReportingUtils;
import org.apache.maven.properties.internal.EnvironmentUtils;
import org.apache.maven.properties.internal.SystemProperties;

import static java.util.Objects.requireNonNull;
import static org.apache.maven.cling.invoker.Utils.getCanonicalPath;
import static org.apache.maven.cling.invoker.Utils.or;
import static org.apache.maven.cling.invoker.Utils.prefix;
import static org.apache.maven.cling.invoker.Utils.stripLeadingAndTrailingQuotes;
import static org.apache.maven.cling.invoker.Utils.toMap;
import static org.apache.maven.cling.invoker.InvokerUtils.getCanonicalPath;
import static org.apache.maven.cling.invoker.InvokerUtils.or;
import static org.apache.maven.cling.invoker.InvokerUtils.prefix;
import static org.apache.maven.cling.invoker.InvokerUtils.toMap;

public abstract class BaseParser implements Parser {

@Nullable
private static Path findRoot(Path topDirectory) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not at ease with splitting the findRoot and findMandatoryRoot in different classes. Those are related and very close.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They don't share code and each is used in a single different place. Both are implementation details that should be hidden from public view. Neither is directly tested. This all suggests they should be private methods where they're used.

// TODO is this OK? Tracing through the code it looks like topDirectory is nullable
requireNonNull(topDirectory, "topDirectory");
Path rootDirectory =
ServiceLoader.load(RootLocator.class).iterator().next().findRoot(topDirectory);
if (rootDirectory != null) {
return getCanonicalPath(rootDirectory);
}
return null;
}

@SuppressWarnings("VisibilityModifier")
public static class LocalContext {
public final ParserRequest parserRequest;
Expand Down Expand Up @@ -185,6 +199,19 @@ protected void mayOverrideDirectorySystemProperty(LocalContext context, String j
}
}

@Nonnull
private static String stripLeadingAndTrailingQuotes(String str) {
requireNonNull(str, "str");
final int length = str.length();
if (length > 1
&& str.startsWith("\"")
&& str.endsWith("\"")
&& str.substring(1, length - 1).indexOf('"') == -1) {
str = str.substring(1, length - 1);
}
return str;
}

protected Path getTopDirectory(LocalContext context) throws ParserException {
// We need to locate the top level project which may be pointed at using
// the -f/--file option.
Expand Down Expand Up @@ -217,7 +244,7 @@ protected Path getTopDirectory(LocalContext context) throws ParserException {

@Nullable
protected Path getRootDirectory(LocalContext context) throws ParserException {
return Utils.findRoot(context.topDirectory);
return findRoot(context.topDirectory);
}

protected Map<String, String> populateSystemProperties(LocalContext context) throws ParserException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import org.apache.maven.jline.MessageUtils;

import static java.util.Objects.requireNonNull;
import static org.apache.maven.cling.invoker.Utils.toMap;
import static org.apache.maven.cling.invoker.InvokerUtils.toMap;

public abstract class CommonsCliOptions implements Options {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,42 +20,29 @@

import java.io.IOException;
import java.nio.file.Path;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.ServiceLoader;
import java.util.function.UnaryOperator;

import org.apache.maven.api.annotations.Nonnull;
import org.apache.maven.api.annotations.Nullable;
import org.apache.maven.api.services.Interpolator;
import org.apache.maven.api.services.model.RootLocator;
import org.apache.maven.cling.logging.Slf4jConfiguration;
import org.apache.maven.execution.MavenExecutionRequest;
import org.apache.maven.internal.impl.model.DefaultInterpolator;
import org.codehaus.plexus.interpolation.AbstractValueSource;
import org.codehaus.plexus.interpolation.BasicInterpolator;
import org.codehaus.plexus.interpolation.StringSearchInterpolator;
import org.codehaus.plexus.logging.Logger;

import static java.util.Objects.requireNonNull;

/**
* Various utilities, mostly to bridge "old" and "new" stuff, like Properties vs Maps, File vs Paths, etc.
* Various internal utilities used in org.apache.maven.cling.invoker and its subpackages.
* Not documented, tested, or intended for external uses.
*/
public final class Utils {
private Utils() {}

@Nonnull
public static String stripLeadingAndTrailingQuotes(String str) {
requireNonNull(str, "str");
final int length = str.length();
if (length > 1
&& str.startsWith("\"")
&& str.endsWith("\"")
&& str.substring(1, length - 1).indexOf('"') == -1) {
str = str.substring(1, length - 1);
}
return str;
}
public final class InvokerUtils {
private InvokerUtils() {}

@Nonnull
public static Path getCanonicalPath(Path path) {
Expand All @@ -68,7 +55,7 @@ public static Path getCanonicalPath(Path path) {
}

@Nonnull
public static Map<String, String> toMap(Properties properties) {
static Map<String, String> toMap(Properties properties) {
requireNonNull(properties, "properties");
HashMap<String, String> map = new HashMap<>();
for (String key : properties.stringPropertyNames()) {
Expand All @@ -78,13 +65,21 @@ public static Map<String, String> toMap(Properties properties) {
}

@Nonnull
public static Properties toProperties(Map<String, String> properties) {
requireNonNull(properties, "properties");
Properties map = new Properties();
for (String key : properties.keySet()) {
map.put(key, properties.get(key));
}
return map;
public static BasicInterpolator createInterpolator(Collection<Map<String, String>> properties) {
StringSearchInterpolator interpolator = new StringSearchInterpolator();
interpolator.addValueSource(new AbstractValueSource(false) {
@Override
public Object getValue(String expression) {
for (Map<String, String> props : properties) {
String val = props.get(expression);
if (val != null) {
return val;
}
}
return null;
}
});
return interpolator;
}

@Nonnull
Expand Down Expand Up @@ -117,40 +112,13 @@ public static UnaryOperator<String> or(UnaryOperator<String>... callbacks) {
};
}

public static int toMavenExecutionRequestLoggingLevel(Slf4jConfiguration.Level level) {
requireNonNull(level, "level");
return switch (level) {
case DEBUG -> MavenExecutionRequest.LOGGING_LEVEL_DEBUG;
case INFO -> MavenExecutionRequest.LOGGING_LEVEL_INFO;
case ERROR -> MavenExecutionRequest.LOGGING_LEVEL_ERROR;
};
}

public static int toPlexusLoggingLevel(Slf4jConfiguration.Level level) {
@Nonnull
static int toPlexusLoggingLevel(Slf4jConfiguration.Level level) {
requireNonNull(level, "level");
return switch (level) {
case DEBUG -> Logger.LEVEL_DEBUG;
case INFO -> Logger.LEVEL_INFO;
case ERROR -> Logger.LEVEL_ERROR;
};
}

@Nullable
public static Path findRoot(Path topDirectory) {
requireNonNull(topDirectory, "topDirectory");
Path rootDirectory =
ServiceLoader.load(RootLocator.class).iterator().next().findRoot(topDirectory);
if (rootDirectory != null) {
return getCanonicalPath(rootDirectory);
}
return null;
}

@Nonnull
public static Path findMandatoryRoot(Path topDirectory) {
requireNonNull(topDirectory, "topDirectory");
return getCanonicalPath(Optional.ofNullable(
ServiceLoader.load(RootLocator.class).iterator().next().findMandatoryRoot(topDirectory))
.orElseThrow());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

import org.apache.maven.api.Constants;
import org.apache.maven.api.ProtoSession;
import org.apache.maven.api.annotations.Nonnull;
import org.apache.maven.api.annotations.Nullable;
import org.apache.maven.api.cli.Invoker;
import org.apache.maven.api.cli.InvokerException;
Expand Down Expand Up @@ -87,8 +88,6 @@
import org.slf4j.spi.LocationAwareLogger;

import static java.util.Objects.requireNonNull;
import static org.apache.maven.cling.invoker.Utils.toMavenExecutionRequestLoggingLevel;
import static org.apache.maven.cling.invoker.Utils.toProperties;

/**
* Lookup invoker implementation, that boots up DI container.
Expand Down Expand Up @@ -462,6 +461,15 @@ protected void init(C context) throws Exception {
context.eventSpyDispatcher.init(() -> data);
}

private static Properties toProperties(Map<String, String> properties) {
requireNonNull(properties, "properties");
Properties map = new Properties();
for (String key : properties.keySet()) {
map.put(key, properties.get(key));
}
return map;
}

protected void postCommands(C context) throws Exception {
InvokerRequest invokerRequest = context.invokerRequest;
Logger logger = context.logger;
Expand Down Expand Up @@ -674,6 +682,16 @@ protected Path localRepositoryPath(C context) {
.resolve("repository");
}

@Nonnull
private static int toMavenExecutionRequestLoggingLevel(Slf4jConfiguration.Level level) {
requireNonNull(level, "level");
return switch (level) {
case DEBUG -> MavenExecutionRequest.LOGGING_LEVEL_DEBUG;
case INFO -> MavenExecutionRequest.LOGGING_LEVEL_INFO;
case ERROR -> MavenExecutionRequest.LOGGING_LEVEL_ERROR;
};
}

protected void populateRequest(C context, Lookup lookup, MavenExecutionRequest request) throws Exception {
populateRequestFromSettings(request, context.effectiveSettings);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.codehaus.plexus.DefaultPlexusContainer;

import static java.util.Objects.requireNonNull;
import static org.apache.maven.cling.invoker.Utils.toPlexusLoggingLevel;
import static org.apache.maven.cling.invoker.InvokerUtils.toPlexusLoggingLevel;

/**
* Container capsule backed by Plexus Container.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
import org.codehaus.plexus.logging.LoggerManager;
import org.slf4j.ILoggerFactory;

import static org.apache.maven.cling.invoker.Utils.toPlexusLoggingLevel;
import static org.apache.maven.cling.invoker.InvokerUtils.toPlexusLoggingLevel;

/**
* Container capsule backed by Plexus Container.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import org.apache.maven.api.services.InterpolatorException;
import org.apache.maven.cling.invoker.CommonsCliOptions;

import static org.apache.maven.cling.invoker.Utils.createInterpolator;
import static org.apache.maven.cling.invoker.InvokerUtils.createInterpolator;

public class CommonsCliMavenOptions extends CommonsCliOptions implements MavenOptions {
public static CommonsCliMavenOptions parse(String source, String[] args) throws ParseException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.ServiceLoader;
import java.util.function.Consumer;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -33,6 +35,7 @@
import org.apache.maven.Maven;
import org.apache.maven.api.Constants;
import org.apache.maven.api.MonotonicClock;
import org.apache.maven.api.annotations.Nonnull;
import org.apache.maven.api.annotations.Nullable;
import org.apache.maven.api.cli.InvokerException;
import org.apache.maven.api.cli.InvokerRequest;
Expand All @@ -45,10 +48,11 @@
import org.apache.maven.api.services.ToolchainsBuilderRequest;
import org.apache.maven.api.services.ToolchainsBuilderResult;
import org.apache.maven.api.services.model.ModelProcessor;
import org.apache.maven.api.services.model.RootLocator;
import org.apache.maven.cling.event.ExecutionEventLogger;
import org.apache.maven.cling.invoker.InvokerUtils;
import org.apache.maven.cling.invoker.LookupContext;
import org.apache.maven.cling.invoker.LookupInvoker;
import org.apache.maven.cling.invoker.Utils;
import org.apache.maven.cling.transfer.ConsoleMavenTransferListener;
import org.apache.maven.cling.transfer.QuietMavenTransferListener;
import org.apache.maven.cling.transfer.SimplexTransferListener;
Expand Down Expand Up @@ -85,6 +89,13 @@ public MavenInvoker(Lookup protoLookup, @Nullable Consumer<LookupContext> contex
super(protoLookup, contextConsumer);
}

@Nonnull
private static Path findMandatoryRoot(@Nonnull Path topDirectory) {
return InvokerUtils.getCanonicalPath(Optional.ofNullable(
ServiceLoader.load(RootLocator.class).iterator().next().findMandatoryRoot(topDirectory))
.orElseThrow());
}

@Override
protected MavenContext createContext(InvokerRequest invokerRequest) throws InvokerException {
return new MavenContext(invokerRequest);
Expand Down Expand Up @@ -252,7 +263,7 @@ protected void populateRequest(MavenContext context, Lookup lookup, MavenExecuti

// project present, but we could not determine rootDirectory: extra work needed
if (context.invokerRequest.rootDirectory().isEmpty()) {
Path rootDirectory = Utils.findMandatoryRoot(context.invokerRequest.topDirectory());
Path rootDirectory = findMandatoryRoot(context.invokerRequest.topDirectory());
request.setMultiModuleProjectDirectory(rootDirectory.toFile());
request.setRootDirectory(rootDirectory);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import org.apache.maven.api.services.InterpolatorException;
import org.apache.maven.cling.invoker.CommonsCliOptions;

import static org.apache.maven.cling.invoker.Utils.createInterpolator;
import static org.apache.maven.cling.invoker.InvokerUtils.createInterpolator;

/**
* Implementation of {@link EncryptOptions} (base + mvnenc).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.apache.maven.api.services.InterpolatorException;
import org.apache.maven.cling.invoker.CommonsCliOptions;

import static org.apache.maven.cling.invoker.Utils.createInterpolator;
import static org.apache.maven.cling.invoker.InvokerUtils.createInterpolator;

/**
* Implementation of {@link ShellOptions} (base + shell).
Expand Down
Loading