This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push: new dec90acf24 [MNG-7866] Improvements to the logging API usage (technical debt) (#1220) dec90acf24 is described below commit dec90acf24f9188b946e89949b488e3f586f0be0 Author: sebastien-doyon <sebastie...@gmail.com> AuthorDate: Fri Sep 22 05:07:17 2023 -0400 [MNG-7866] Improvements to the logging API usage (technical debt) (#1220) --- .../project/artifact/MavenMetadataSource.java | 16 +- .../maven/toolchain/DefaultToolchainsBuilder.java | 2 +- maven-core/pom.xml | 2 +- .../main/java/org/apache/maven/DefaultMaven.java | 18 ++- .../maven/classrealm/DefaultClassRealmManager.java | 66 +++------ .../internal/aether/LoggingRepositoryListener.java | 39 +++-- .../org/apache/maven/internal/impl/DefaultLog.java | 32 +++- .../apache/maven/lifecycle/DefaultLifecycles.java | 13 +- .../internal/DefaultLifecyclePluginAnalyzer.java | 6 +- .../internal/LifecycleDependencyResolver.java | 2 +- .../multithreaded/MultiThreadedBuilder.java | 4 +- .../plugin/internal/DefaultMavenPluginManager.java | 16 +- .../DefaultPluginDependenciesResolver.java | 17 ++- .../maven/plugin/internal/MojoLogWrapper.java | 32 +++- .../internal/DefaultPluginPrefixResolver.java | 30 ++-- .../internal/DefaultPluginVersionResolver.java | 34 +++-- .../collector/MultiModuleCollectionStrategy.java | 5 +- .../apache/maven/toolchain/DefaultToolchain.java | 4 +- .../classrealm/DefaultClassRealmManagerTest.java | 164 +++++++++++++++++++++ .../maven/toolchain/DefaultToolchainTest.java | 4 +- 20 files changed, 352 insertions(+), 154 deletions(-) diff --git a/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java b/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java index 4e0877e9eb..4240cecba5 100644 --- a/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java +++ b/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java @@ -633,11 +633,19 @@ public class MavenMetadataSource implements ArtifactMetadataSource { if (artifact.getDependencyTrail() != null && artifact.getDependencyTrail().size() == 1) { - logger.warn("While downloading " + pomArtifact.getGroupId() + ":" - + pomArtifact.getArtifactId() + ":" + pomArtifact.getVersion() + message); + logger.warn( + "While downloading {}:{}:{}{}", + pomArtifact.getGroupId(), + pomArtifact.getArtifactId(), + pomArtifact.getVersion(), + message); } else { - logger.debug("While downloading " + pomArtifact.getGroupId() + ":" - + pomArtifact.getArtifactId() + ":" + pomArtifact.getVersion() + message); + logger.debug( + "While downloading {}:{}:{}{}", + pomArtifact.getGroupId(), + pomArtifact.getArtifactId(), + pomArtifact.getVersion(), + message); } } else { done = true; diff --git a/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java b/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java index c2c608b951..e78fd2ca8a 100644 --- a/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java +++ b/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java @@ -51,7 +51,7 @@ public class DefaultToolchainsBuilder implements ToolchainsBuilder { } } else if (userToolchainsFile != null) { - logger.debug("Toolchains configuration was not found at " + userToolchainsFile); + logger.debug("Toolchains configuration was not found at {}", userToolchainsFile); } return toolchains; diff --git a/maven-core/pom.xml b/maven-core/pom.xml index 87484b5a6b..6b20529582 100644 --- a/maven-core/pom.xml +++ b/maven-core/pom.xml @@ -151,7 +151,7 @@ under the License. </dependency> <dependency> <groupId>org.mockito</groupId> - <artifactId>mockito-core</artifactId> + <artifactId>mockito-inline</artifactId> <scope>test</scope> </dependency> <dependency> diff --git a/maven-core/src/main/java/org/apache/maven/DefaultMaven.java b/maven-core/src/main/java/org/apache/maven/DefaultMaven.java index 18f0dea4d3..f49d5409a4 100644 --- a/maven-core/src/main/java/org/apache/maven/DefaultMaven.java +++ b/maven-core/src/main/java/org/apache/maven/DefaultMaven.java @@ -410,7 +410,7 @@ public class DefaultMaven implements Maven { private void validateLocalRepository(MavenExecutionRequest request) throws IOException { File localRepoDir = request.getLocalRepositoryPath(); - logger.debug("Using local repository at " + localRepoDir); + logger.debug("Using local repository at {}", localRepoDir); localRepoDir.mkdirs(); @@ -426,7 +426,7 @@ public class DefaultMaven implements Maven { foundComponents.addAll(container.lookupList(role)); } catch (ComponentLookupException e) { // this is just silly, lookupList should return an empty list! - logger.warn("Failed to lookup " + role + ": " + e.getMessage()); + logger.warn("Failed to lookup {}: {}", role, e.getMessage()); } foundComponents.addAll(getProjectScopedExtensionComponents(projects, role)); @@ -455,7 +455,7 @@ public class DefaultMaven implements Maven { foundComponents.addAll(container.lookupList(role)); } catch (ComponentLookupException e) { // this is just silly, lookupList should return an empty list! - logger.warn("Failed to lookup " + role + ": " + e.getMessage()); + logger.warn("Failed to lookup {}: {}", role, e.getMessage()); } } } @@ -479,11 +479,13 @@ public class DefaultMaven implements Maven { Prerequisites prerequisites = mavenProject.getModel().getDelegate().getPrerequisites(); if (prerequisites != null && prerequisites.getMaven() != null) { - logger.warn("The project " + mavenProject.getId() + " uses prerequisites" - + " which is only intended for maven-plugin projects " - + "but not for non maven-plugin projects. " - + "For such purposes you should use the maven-enforcer-plugin. " - + "See https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html"); + logger.warn( + "The project {} uses prerequisites" + + " which is only intended for maven-plugin projects " + + "but not for non maven-plugin projects. " + + "For such purposes you should use the maven-enforcer-plugin. " + + "See https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html", + mavenProject.getId()); } } } diff --git a/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java b/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java index 06b412e2fa..512e7cae8f 100644 --- a/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java +++ b/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java @@ -26,7 +26,6 @@ import java.io.File; import java.net.MalformedURLException; import java.util.ArrayList; import java.util.Collections; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -116,9 +115,7 @@ public class DefaultClassRealmManager implements ClassRealmManager { try { ClassRealm classRealm = world.newRealm(realmId, null); - if (logger.isDebugEnabled()) { - logger.debug("Created new class realm " + realmId); - } + logger.debug("Created new class realm {}", realmId); return classRealm; } catch (DuplicateRealmException e) { @@ -151,17 +148,14 @@ public class DefaultClassRealmManager implements ClassRealmManager { List<String> parentImports, Map<String, ClassLoader> foreignImports, List<Artifact> artifacts) { - Set<String> artifactIds = new LinkedHashSet<>(); + List<ClassRealmConstituent> constituents = new ArrayList<>(artifacts == null ? 0 : artifacts.size()); - List<ClassRealmConstituent> constituents = new ArrayList<>(); - - if (artifacts != null) { + if (artifacts != null && !artifacts.isEmpty()) { for (Artifact artifact : artifacts) { - if (!isProvidedArtifact(artifact)) { - artifactIds.add(getId(artifact)); - if (artifact.getFile() != null) { - constituents.add(new ArtifactClassRealmConstituent(artifact)); - } + if (!isProvidedArtifact(artifact) && artifact.getFile() != null) { + constituents.add(new ArtifactClassRealmConstituent(artifact)); + } else if (logger.isDebugEnabled()) { + logger.debug(" Excluded: {}", getId(artifact)); } } } @@ -188,15 +182,7 @@ public class DefaultClassRealmManager implements ClassRealmManager { wireRealm(classRealm, parentImports, foreignImports); - Set<String> includedIds = populateRealm(classRealm, constituents); - - if (logger.isDebugEnabled()) { - artifactIds.removeAll(includedIds); - - for (String id : artifactIds) { - logger.debug(" Excluded: " + id); - } - } + populateRealm(classRealm, constituents); return classRealm; } @@ -299,21 +285,15 @@ public class DefaultClassRealmManager implements ClassRealmManager { } } - private Set<String> populateRealm(ClassRealm classRealm, List<ClassRealmConstituent> constituents) { - Set<String> includedIds = new LinkedHashSet<>(); - - if (logger.isDebugEnabled()) { - logger.debug("Populating class realm " + classRealm.getId()); - } + private void populateRealm(ClassRealm classRealm, List<ClassRealmConstituent> constituents) { + logger.debug("Populating class realm {}", classRealm.getId()); for (ClassRealmConstituent constituent : constituents) { File file = constituent.getFile(); - String id = getId(constituent); - includedIds.add(id); - if (logger.isDebugEnabled()) { - logger.debug(" Included: " + id); + String id = getId(constituent); + logger.debug(" Included: {}", id); } try { @@ -323,47 +303,37 @@ public class DefaultClassRealmManager implements ClassRealmManager { logger.error(e.getMessage(), e); } } - - return includedIds; } private void wireRealm(ClassRealm classRealm, List<String> parentImports, Map<String, ClassLoader> foreignImports) { if (foreignImports != null && !foreignImports.isEmpty()) { - if (logger.isDebugEnabled()) { - logger.debug("Importing foreign packages into class realm " + classRealm.getId()); - } + logger.debug("Importing foreign packages into class realm {}", classRealm.getId()); for (Map.Entry<String, ClassLoader> entry : foreignImports.entrySet()) { ClassLoader importedRealm = entry.getValue(); String imp = entry.getKey(); - if (logger.isDebugEnabled()) { - logger.debug(" Imported: " + imp + " < " + getId(importedRealm)); - } + logger.debug(" Imported: {} < {}", imp, getId(importedRealm)); classRealm.importFrom(importedRealm, imp); } } if (parentImports != null && !parentImports.isEmpty()) { - if (logger.isDebugEnabled()) { - logger.debug("Importing parent packages into class realm " + classRealm.getId()); - } + logger.debug("Importing parent packages into class realm {}", classRealm.getId()); for (String imp : parentImports) { - if (logger.isDebugEnabled()) { - logger.debug(" Imported: " + imp + " < " + getId(classRealm.getParentClassLoader())); - } + logger.debug(" Imported: {} < {}", imp, getId(classRealm.getParentClassLoader())); classRealm.importFromParent(imp); } } } - private String getId(ClassLoader classLoader) { + private static Object getId(ClassLoader classLoader) { if (classLoader instanceof ClassRealm) { return ((ClassRealm) classLoader).getId(); } - return String.valueOf(classLoader); + return classLoader; } } diff --git a/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java b/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java index d752da6a8d..fe9c074b33 100644 --- a/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java +++ b/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java @@ -37,12 +37,12 @@ class LoggingRepositoryListener extends AbstractRepositoryListener { @Override public void artifactInstalling(RepositoryEvent event) { - logger.info("Installing " + event.getArtifact().getFile() + " to " + event.getFile()); + logger.info("Installing {} to {}", event.getArtifact().getFile(), event.getFile()); } @Override public void metadataInstalling(RepositoryEvent event) { - logger.debug("Installing " + event.getMetadata() + " to " + event.getFile()); + logger.debug("Installing {} to {}", event.getMetadata(), event.getFile()); } @Override @@ -63,48 +63,45 @@ class LoggingRepositoryListener extends AbstractRepositoryListener { public void metadataInvalid(RepositoryEvent event) { Exception exception = event.getException(); - StringBuilder buffer = new StringBuilder(256); - buffer.append("The metadata "); + Object metadata; if (event.getMetadata().getFile() != null) { - buffer.append(event.getMetadata().getFile()); + metadata = event.getMetadata().getFile(); } else { - buffer.append(event.getMetadata()); + metadata = event.getMetadata(); } + String errorType = " is invalid"; if (exception instanceof FileNotFoundException) { - buffer.append(" is inaccessible"); - } else { - buffer.append(" is invalid"); + errorType = " is inaccessible"; } + String msg = ""; if (exception != null) { - buffer.append(": "); - buffer.append(exception.getMessage()); + msg = ": " + exception.getMessage(); } if (logger.isDebugEnabled()) { - logger.warn(buffer.toString(), exception); + logger.warn("The metadata {} {}{}", metadata, errorType, msg, exception); } else { - logger.warn(buffer.toString()); + logger.warn("The metadata {} {}{}", metadata, errorType, msg); } } @Override public void artifactDescriptorInvalid(RepositoryEvent event) { - StringBuilder buffer = new StringBuilder(256); - buffer.append("The POM for "); - buffer.append(event.getArtifact()); - buffer.append(" is invalid, transitive dependencies (if any) will not be available"); - + // The exception stack trace is not really interesting here + // but the message itself may be quite details and span multiple + // lines with errors in it, so only display it at debug level. + String msg = "The POM for {} is invalid, transitive dependencies (if any) will not be available: {}"; if (logger.isDebugEnabled()) { - logger.warn(buffer + ": " + event.getException().getMessage()); + logger.warn(msg, event.getArtifact(), event.getException().getMessage()); } else { - logger.warn(buffer + ", enable verbose output (-X) for more details"); + logger.warn(msg, event.getArtifact(), "enable verbose output (-X) for more details"); } } @Override public void artifactDescriptorMissing(RepositoryEvent event) { - logger.warn("The POM for " + event.getArtifact() + " is missing, no dependency information available"); + logger.warn("The POM for {} is missing, no dependency information available", event.getArtifact()); } } diff --git a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java index c1a2d8d2e3..fa26a10083 100644 --- a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java +++ b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java @@ -33,12 +33,16 @@ public class DefaultLog implements Log { } public void debug(CharSequence content) { - logger.debug(toString(content)); + if (isDebugEnabled()) { + logger.debug(toString(content)); + } } @Override public void debug(CharSequence content, Throwable error) { - logger.debug(toString(content), error); + if (isDebugEnabled()) { + logger.debug(toString(content), error); + } } @Override @@ -62,12 +66,16 @@ public class DefaultLog implements Log { @Override public void info(CharSequence content) { - logger.info(toString(content)); + if (isInfoEnabled()) { + logger.info(toString(content)); + } } @Override public void info(CharSequence content, Throwable error) { - logger.info(toString(content), error); + if (isInfoEnabled()) { + logger.info(toString(content), error); + } } @Override @@ -91,12 +99,16 @@ public class DefaultLog implements Log { @Override public void warn(CharSequence content) { - logger.warn(toString(content)); + if (isWarnEnabled()) { + logger.warn(toString(content)); + } } @Override public void warn(CharSequence content, Throwable error) { - logger.warn(toString(content), error); + if (isWarnEnabled()) { + logger.warn(toString(content), error); + } } @Override @@ -120,12 +132,16 @@ public class DefaultLog implements Log { @Override public void error(CharSequence content) { - logger.error(toString(content)); + if (isErrorEnabled()) { + logger.error(toString(content)); + } } @Override public void error(CharSequence content, Throwable error) { - logger.error(toString(content), error); + if (isErrorEnabled()) { + logger.error(toString(content), error); + } } @Override diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java b/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java index 2a6868baf5..76cb5a0e5a 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java @@ -94,18 +94,19 @@ public class DefaultLifecycles { Map<String, Lifecycle> phaseToLifecycleMap = new HashMap<>(); for (Lifecycle lifecycle : getLifeCycles()) { - if (logger.isDebugEnabled()) { - logger.debug("Lifecycle " + lifecycle); - } + logger.debug("Lifecycle {}", lifecycle); for (String phase : lifecycle.getPhases()) { // The first definition wins. if (!phaseToLifecycleMap.containsKey(phase)) { phaseToLifecycleMap.put(phase, lifecycle); - } else { + } else if (logger.isWarnEnabled()) { Lifecycle original = phaseToLifecycleMap.get(phase); - logger.warn("Duplicated lifecycle phase " + phase + ". Defined in " + original.getId() - + " but also in " + lifecycle.getId()); + logger.warn( + "Duplicated lifecycle phase {}. Defined in {} but also in {}", + phase, + original.getId(), + lifecycle.getId()); } } } diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java index b3c1dc29fb..652de2ca03 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java @@ -154,8 +154,10 @@ public class DefaultLifecyclePluginAnalyzer implements LifeCyclePluginAnalyzer { GoalSpec gs = parseGoalSpec(mojo.getGoal()); if (gs == null) { - logger.warn("Ignored invalid goal specification '" + mojo.getGoal() - + "' from lifecycle mapping for phase " + phase); + logger.warn( + "Ignored invalid goal specification '{}' from lifecycle mapping for phase {}", + mojo.getGoal(), + phase); continue; } diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java index 7c30835ebb..bd5fc4626b 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java @@ -250,7 +250,7 @@ public class LifecycleDependencyResolver { + " but seem to be part of the reactor:"); for (Dependency dependency : result.getUnresolvedDependencies()) { - logger.warn("o " + dependency); + logger.warn("o {}", dependency); } logger.warn("Try running the build up to the lifecycle phase \"package\""); diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java index 607a883b85..2a76afb50d 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java @@ -138,7 +138,7 @@ public class MultiThreadedBuilder implements Builder { // schedule independent projects for (MavenProject mavenProject : analyzer.getRootSchedulableBuilds()) { ProjectSegment projectSegment = projectBuildList.get(mavenProject); - logger.debug("Scheduling: " + projectSegment.getProject()); + logger.debug("Scheduling: {}", projectSegment.getProject()); Callable<ProjectSegment> cb = createBuildCallable( rootSession, projectSegment, reactorContext, taskSegment, muxer, duplicateArtifactIds); service.submit(cb); @@ -158,7 +158,7 @@ public class MultiThreadedBuilder implements Builder { analyzer.markAsFinished(projectBuild.getProject()); for (MavenProject mavenProject : newItemsThatCanBeBuilt) { ProjectSegment scheduledDependent = projectBuildList.get(mavenProject); - logger.debug("Scheduling: " + scheduledDependent); + logger.debug("Scheduling: {}", scheduledDependent); Callable<ProjectSegment> cb = createBuildCallable( rootSession, scheduledDependent, diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index b4628d3183..ef1322d2fa 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -638,8 +638,10 @@ public class DefaultMavenPluginManager implements MavenPluginManager { ValidatingConfigurationListener validator = new ValidatingConfigurationListener(mojo, mojoDescriptor, listener); - logger.debug("Configuring mojo execution '" + mojoDescriptor.getId() + ':' + executionId + "' with " - + configuratorId + " configurator -->"); + if (logger.isDebugEnabled()) { + logger.debug("Configuring mojo execution '" + mojoDescriptor.getId() + ':' + executionId + "' with " + + configuratorId + " configurator -->"); + } configurator.configureComponent(mojo, configuration, expressionEvaluator, pluginRealm, validator); @@ -749,10 +751,14 @@ public class DefaultMavenPluginManager implements MavenPluginManager { String goalExecId = mojoExecution.getGoal(); if (mojoExecution.getExecutionId() != null) { - goalExecId += " {execution: " + mojoExecution.getExecutionId() + "}"; + logger.debug( + "Error releasing mojo for {} {execution: {}}", + goalExecId, + mojoExecution.getExecutionId(), + e); + } else { + logger.debug("Error releasing mojo for {}", goalExecId, e); } - - logger.debug("Error releasing mojo for " + goalExecId, e); } } } diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java index d1f9b01111..2f7cb9af21 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java @@ -115,14 +115,15 @@ public class DefaultPluginDependenciesResolver implements PluginDependenciesReso pluginArtifact = result.getArtifact(); - if (logger.isWarnEnabled()) { - if (!result.getRelocations().isEmpty()) { - String message = pluginArtifact instanceof org.apache.maven.repository.internal.RelocatedArtifact - ? ((org.apache.maven.repository.internal.RelocatedArtifact) pluginArtifact).getMessage() - : null; - logger.warn("The artifact " + result.getRelocations().get(0) + " has been relocated to " - + pluginArtifact + (message != null ? ": " + message : "")); - } + if (logger.isWarnEnabled() && !result.getRelocations().isEmpty()) { + String message = pluginArtifact instanceof org.apache.maven.repository.internal.RelocatedArtifact + ? ": " + ((org.apache.maven.repository.internal.RelocatedArtifact) pluginArtifact).getMessage() + : ""; + logger.warn( + "The artifact {} has been relocated to {}{}", + result.getRelocations().get(0), + pluginArtifact, + message); } String requiredMavenVersion = (String) result.getProperties().get("prerequisites.maven"); diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java index 06d10193d2..2ffd05ba85 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java @@ -33,7 +33,9 @@ public class MojoLogWrapper implements Log { } public void debug(CharSequence content) { - logger.debug(toString(content)); + if (isDebugEnabled()) { + logger.debug(toString(content)); + } } private String toString(CharSequence content) { @@ -46,7 +48,9 @@ public class MojoLogWrapper implements Log { @Override public void debug(CharSequence content, Throwable error) { - logger.debug(toString(content), error); + if (isDebugEnabled()) { + logger.debug(toString(content), error); + } } @Override @@ -56,12 +60,16 @@ public class MojoLogWrapper implements Log { @Override public void info(CharSequence content) { - logger.info(toString(content)); + if (isInfoEnabled()) { + logger.info(toString(content)); + } } @Override public void info(CharSequence content, Throwable error) { - logger.info(toString(content), error); + if (isInfoEnabled()) { + logger.info(toString(content), error); + } } @Override @@ -71,12 +79,16 @@ public class MojoLogWrapper implements Log { @Override public void warn(CharSequence content) { - logger.warn(toString(content)); + if (isWarnEnabled()) { + logger.warn(toString(content)); + } } @Override public void warn(CharSequence content, Throwable error) { - logger.warn(toString(content), error); + if (isWarnEnabled()) { + logger.warn(toString(content), error); + } } @Override @@ -86,12 +98,16 @@ public class MojoLogWrapper implements Log { @Override public void error(CharSequence content) { - logger.error(toString(content)); + if (isErrorEnabled()) { + logger.error(toString(content)); + } } @Override public void error(CharSequence content, Throwable error) { - logger.error(toString(content), error); + if (isErrorEnabled()) { + logger.error(toString(content), error); + } } @Override diff --git a/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java b/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java index d99ff05aef..8bcea49685 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java @@ -78,7 +78,7 @@ public class DefaultPluginPrefixResolver implements PluginPrefixResolver { } public PluginPrefixResult resolve(PluginPrefixRequest request) throws NoPluginFoundForPrefixException { - logger.debug("Resolving plugin prefix " + request.getPrefix() + " from " + request.getPluginGroups()); + logger.debug("Resolving plugin prefix {} from {}", request.getPrefix(), request.getPluginGroups()); PluginPrefixResult result = resolveFromProject(request); @@ -91,16 +91,21 @@ public class DefaultPluginPrefixResolver implements PluginPrefixResolver { request.getPluginGroups(), request.getRepositorySession().getLocalRepository(), request.getRepositories()); - } else if (logger.isDebugEnabled()) { - logger.debug("Resolved plugin prefix " + request.getPrefix() + " to " + result.getGroupId() + ":" - + result.getArtifactId() + " from repository " - + (result.getRepository() != null - ? result.getRepository().getId() - : "null")); + } else { + logger.debug( + "Resolved plugin prefix {} to {}:{} from repository {}", + request.getPrefix(), + result.getGroupId(), + result.getArtifactId(), + (result.getRepository() != null ? result.getRepository().getId() : "null")); } - } else if (logger.isDebugEnabled()) { - logger.debug("Resolved plugin prefix " + request.getPrefix() + " to " + result.getGroupId() + ":" - + result.getArtifactId() + " from POM " + request.getPom()); + } else { + logger.debug( + "Resolved plugin prefix {} to {}:{} from POM {}", + request.getPrefix(), + result.getGroupId(), + result.getArtifactId(), + request.getPom()); } return result; @@ -133,10 +138,9 @@ public class DefaultPluginPrefixResolver implements PluginPrefixResolver { } } catch (Exception e) { if (logger.isDebugEnabled()) { - logger.warn( - "Failed to retrieve plugin descriptor for " + plugin.getId() + ": " + e.getMessage(), e); + logger.warn("Failed to retrieve plugin descriptor for {}: {}", plugin.getId(), e.getMessage(), e); } else { - logger.warn("Failed to retrieve plugin descriptor for " + plugin.getId() + ": " + e.getMessage()); + logger.warn("Failed to retrieve plugin descriptor for {}: {}", plugin.getId(), e.getMessage()); } } } diff --git a/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java b/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java index b8c24cb86d..904606a2a0 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java @@ -105,19 +105,29 @@ public class DefaultPluginVersionResolver implements PluginVersionResolver { if (result == null) { result = resolveFromRepository(request); - if (logger.isDebugEnabled()) { - logger.debug("Resolved plugin version for " + request.getGroupId() + ":" + request.getArtifactId() - + " to " + result.getVersion() + " from repository " + result.getRepository()); - } + logger.debug( + "Resolved plugin version for {}:{} to {} from repository {}", + request.getGroupId(), + request.getArtifactId(), + result.getVersion(), + result.getRepository()); cache.putIfAbsent(key, result); - } else if (logger.isDebugEnabled()) { - logger.debug("Reusing cached resolved plugin version for " + request.getGroupId() + ":" - + request.getArtifactId() + " to " + result.getVersion() + " from POM " + request.getPom()); + } else { + logger.debug( + "Reusing cached resolved plugin version for {}:{} to {} from POM {}", + request.getGroupId(), + request.getArtifactId(), + result.getVersion(), + request.getPom()); } - } else if (logger.isDebugEnabled()) { - logger.debug("Resolved plugin version for " + request.getGroupId() + ":" + request.getArtifactId() + " to " - + result.getVersion() + " from POM " + request.getPom()); + } else { + logger.debug( + "Reusing cached resolved plugin version for {}:{} to {} from POM {}", + request.getGroupId(), + request.getArtifactId(), + result.getVersion(), + request.getPom()); } return result; @@ -242,7 +252,7 @@ public class DefaultPluginVersionResolver implements PluginVersionResolver { pluginDescriptor = pluginManager.getPluginDescriptor( plugin, request.getRepositories(), request.getRepositorySession()); } catch (PluginResolutionException e) { - logger.debug("Ignoring unresolvable plugin version " + version, e); + logger.debug("Ignoring unresolvable plugin version {}", version, e); return false; } catch (Exception e) { // ignore for now and delay failure to higher level processing @@ -252,7 +262,7 @@ public class DefaultPluginVersionResolver implements PluginVersionResolver { try { pluginManager.checkPrerequisites(pluginDescriptor); } catch (Exception e) { - logger.warn("Ignoring incompatible plugin version " + version, e); + logger.warn("Ignoring incompatible plugin version {}", version, e); return false; } diff --git a/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java b/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java index 9f81694d40..1216f081b2 100644 --- a/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java +++ b/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java @@ -107,8 +107,9 @@ public class MultiModuleCollectionStrategy implements ProjectCollectionStrategy "Maven detected that the requested POM file is part of a multi-module project, " + "but could not find a pom.xml file in the multi-module root directory '{}'.", multiModuleProjectDirectory); - LOGGER.info("The reactor is limited to all projects under: " - + request.getPom().getParent()); + LOGGER.info( + "The reactor is limited to all projects under: {}", + request.getPom().getParent()); return request.getPom(); } diff --git a/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java b/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java index b12380f963..96789c43c5 100644 --- a/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java +++ b/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java @@ -88,11 +88,11 @@ implements Toolchain, ToolchainPrivate { RequirementMatcher matcher = provides.get(key); if (matcher == null) { - getLog().debug("Toolchain " + this + " is missing required property: " + key); + getLog().debug("Toolchain {} is missing required property: {}", this, key); return false; } if (!matcher.matches(requirement.getValue())) { - getLog().debug("Toolchain " + this + " doesn't match required property: " + key); + getLog().debug("Toolchain {} doesn't match required property: {}", this, key); return false; } } diff --git a/maven-core/src/test/java/org/apache/maven/classrealm/DefaultClassRealmManagerTest.java b/maven-core/src/test/java/org/apache/maven/classrealm/DefaultClassRealmManagerTest.java new file mode 100644 index 0000000000..f1908adb42 --- /dev/null +++ b/maven-core/src/test/java/org/apache/maven/classrealm/DefaultClassRealmManagerTest.java @@ -0,0 +1,164 @@ +/* + * 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. + */ +package org.apache.maven.classrealm; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; + +import org.apache.maven.extension.internal.CoreExports; +import org.apache.maven.model.Model; +import org.codehaus.plexus.DefaultPlexusContainer; +import org.codehaus.plexus.PlexusContainer; +import org.codehaus.plexus.PlexusContainerException; +import org.codehaus.plexus.classworlds.realm.ClassRealm; +import org.eclipse.aether.artifact.Artifact; +import org.junit.jupiter.api.Test; +import org.mockito.InOrder; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.endsWith; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.calls; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.when; + +/** + * @author Sebastien Doyon + */ +class DefaultClassRealmManagerTest { + + private DefaultClassRealmManager newDefaultClassRealmManager(PlexusContainer container) { + HashSet<String> exportedPackages = new HashSet<String>(); + exportedPackages.add("group1:artifact1"); + + return new DefaultClassRealmManager( + container, + new ArrayList<ClassRealmManagerDelegate>(), + new CoreExports(new ClassRealm(null, "test", null), new HashSet<String>(), exportedPackages)); + } + + private List<Artifact> newTestArtifactList() { + List<Artifact> artifacts = new ArrayList<Artifact>(); + + Artifact artifact = mock(Artifact.class); + when(artifact.getFile()).thenReturn(new File(new File("local/repository"), "some/path")); + when(artifact.getGroupId()).thenReturn("group1"); + when(artifact.getArtifactId()).thenReturn("artifact1"); + when(artifact.getExtension()).thenReturn("ext"); + when(artifact.getClassifier()).thenReturn("classifier1"); + when(artifact.getVersion()).thenReturn("1"); + artifacts.add(artifact); + + Artifact artifact2 = mock(Artifact.class); + when(artifact2.getFile()).thenReturn(null); + when(artifact2.getGroupId()).thenReturn("group1"); + when(artifact2.getArtifactId()).thenReturn("artifact2"); + when(artifact2.getExtension()).thenReturn("ext"); + when(artifact2.getClassifier()).thenReturn("classifier1"); + when(artifact2.getVersion()).thenReturn("1"); + artifacts.add(artifact2); + + return artifacts; + } + + private Model newTestModel() { + Model model = new Model(); + model.setGroupId("modelGroup1"); + model.setArtifactId("modelArtifact1"); + model.setVersion("modelVersion1"); + + return model; + } + + @Test + void testDebugEnabled() throws PlexusContainerException { + Logger logger = mock(Logger.class); + when(logger.isDebugEnabled()).thenReturn(true); + + DefaultClassRealmManager classRealmManager; + ClassRealm classRealm; + + InOrder verifier = inOrder(logger); + + PlexusContainer container = new DefaultPlexusContainer(); + + try (MockedStatic<LoggerFactory> mockedLoggerFactory = Mockito.mockStatic(LoggerFactory.class)) { + mockedLoggerFactory + .when(() -> LoggerFactory.getLogger(DefaultClassRealmManager.class)) + .thenReturn(logger); + + classRealmManager = newDefaultClassRealmManager(container); + classRealm = classRealmManager.createProjectRealm(newTestModel(), newTestArtifactList()); + } + + assertEquals(classRealmManager.getMavenApiRealm(), classRealm.getParentClassLoader()); + assertEquals("project>modelGroup1:modelArtifact1:modelVersion1", classRealm.getId()); + assertEquals(1, classRealm.getURLs().length); + assertThat(classRealm.getURLs()[0].getPath(), endsWith("local/repository/some/path")); + + verifier.verify(logger, calls(1)).debug("Importing foreign packages into class realm {}", "maven.api"); + verifier.verify(logger, calls(1)).debug(" Imported: {} < {}", "group1:artifact1", "test"); + verifier.verify(logger, calls(1)).debug(" Excluded: {}", "group1:artifact2:ext:classifier1:null"); + verifier.verify(logger, calls(1)) + .debug("Populating class realm {}", "project>modelGroup1:modelArtifact1:modelVersion1"); + verifier.verify(logger, calls(1)).debug(" Included: {}", "group1:artifact1:ext:classifier1:null"); + } + + @Test + void testDebugDisabled() throws PlexusContainerException { + Logger logger = mock(Logger.class); + when(logger.isDebugEnabled()).thenReturn(false); + + DefaultClassRealmManager classRealmManager; + ClassRealm classRealm; + + InOrder verifier = inOrder(logger); + + PlexusContainer container = new DefaultPlexusContainer(); + + try (MockedStatic<LoggerFactory> mockedLoggerFactory = Mockito.mockStatic(LoggerFactory.class)) { + mockedLoggerFactory + .when(() -> LoggerFactory.getLogger(DefaultClassRealmManager.class)) + .thenReturn(logger); + + classRealmManager = newDefaultClassRealmManager(container); + classRealm = classRealmManager.createProjectRealm(newTestModel(), newTestArtifactList()); + } + + assertEquals(classRealmManager.getMavenApiRealm(), classRealm.getParentClassLoader()); + assertEquals("project>modelGroup1:modelArtifact1:modelVersion1", classRealm.getId()); + assertEquals(1, classRealm.getURLs().length); + assertThat(classRealm.getURLs()[0].getPath(), endsWith("local/repository/some/path")); + + verifier.verify(logger, calls(1)).debug("Importing foreign packages into class realm {}", "maven.api"); + verifier.verify(logger, calls(1)).debug(" Imported: {} < {}", "group1:artifact1", "test"); + verifier.verify(logger, calls(1)) + .debug("Populating class realm {}", "project>modelGroup1:modelArtifact1:modelVersion1"); + verifier.verify(logger, never()).debug(" Included: {}", "group1:artifact1:ext:classifier1:null"); + verifier.verify(logger, never()).debug(" Excluded: {}", "group1:artifact2:ext:classifier1:null"); + } +} diff --git a/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java b/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java index faffb3c75d..d6efd846df 100644 --- a/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java +++ b/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java @@ -92,7 +92,7 @@ class DefaultToolchainTest { DefaultToolchain toolchain = newDefaultToolchain(model); assertFalse(toolchain.matchesRequirements(Collections.singletonMap("name", "John Doe"))); - verify(logger).debug("Toolchain type:TYPE{} is missing required property: name"); + verify(logger).debug("Toolchain {} is missing required property: {}", toolchain, "name"); } @Test @@ -103,7 +103,7 @@ class DefaultToolchainTest { toolchain.addProvideToken("name", RequirementMatcherFactory.createExactMatcher("Jane Doe")); assertFalse(toolchain.matchesRequirements(Collections.singletonMap("name", "John Doe"))); - verify(logger).debug("Toolchain type:TYPE{name = Jane Doe} doesn't match required property: name"); + verify(logger).debug("Toolchain {} doesn't match required property: {}", toolchain, "name"); } @Test