diff --git a/src/main/java/com/societegenerale/commons/plugin/maven/ArchUnitMojo.java b/src/main/java/com/societegenerale/commons/plugin/maven/ArchUnitMojo.java index e5d7275..e340bb0 100644 --- a/src/main/java/com/societegenerale/commons/plugin/maven/ArchUnitMojo.java +++ b/src/main/java/com/societegenerale/commons/plugin/maven/ArchUnitMojo.java @@ -1,13 +1,5 @@ package com.societegenerale.commons.plugin.maven; -import java.io.File; -import java.net.MalformedURLException; -import java.net.URL; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - import com.societegenerale.commons.plugin.Log; import com.societegenerale.commons.plugin.maven.model.MavenRules; import com.societegenerale.commons.plugin.model.Rules; @@ -23,6 +15,15 @@ import org.apache.maven.plugins.annotations.ResolutionScope; import org.apache.maven.project.MavenProject; +import java.io.File; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + import static java.net.URLClassLoader.newInstance; import static java.util.Collections.emptyList; @@ -80,55 +81,84 @@ public void execute() throws MojoFailureException { return; } - Rules coreRules=rules.toCoreRules(); - - if (!coreRules.isValid()) { + if (!rules.toCoreRules().isValid()) { throw new MojoFailureException("Arch unit Plugin should have at least one preconfigured/configurable rule"); } + String ruleFailureMessage; + + Rules coreRules; + List targetProjects; if ("pom".equals(mavenProject.getPackaging())) { - getLog().debug("module packaging is 'pom', so skipping execution"); - return; + coreRules = rules.toCoreRules(true); + targetProjects = mavenProject.getCollectedProjects(); + } + else { + coreRules = rules.toCoreRules(false); + targetProjects = List.of(mavenProject); } - if (!properties.isEmpty()) { - getLog().debug("configuring ArchUnit properties"); - final ArchConfiguration archConfiguration = ArchConfiguration.get(); - properties.forEach(archConfiguration::setProperty); + ruleFailureMessage = executeArchUnitRules(coreRules, targetProjects); + + if (!StringUtils.isEmpty(ruleFailureMessage)) { + if(!noFailOnError) { + throw new MojoFailureException(PREFIX_ARCH_VIOLATION_MESSAGE + ruleFailureMessage); + } + + getLog().info(PREFIX_ARCH_VIOLATION_MESSAGE + ruleFailureMessage); } + } - String ruleFailureMessage; + private String executeArchUnitRules(Rules coreRules, List projects) throws MojoFailureException { + if (!coreRules.isValid()) { + String debugMessage = "no rule apply for projects"; + if (!projects.isEmpty()) { + debugMessage += ": %s".formatted(projects.stream() + .map(MavenProject::getName) + .collect(Collectors.joining(";"))); + } + getLog().debug(debugMessage); + return null; + } + configureProperties(); try { - configureContextClassLoader(); + configureContextClassLoader(projects); final Log mavenLogAdapter = new MavenLogAdapter(getLog()); - ruleInvokerService = new RuleInvokerService(mavenLogAdapter, new MavenScopePathProvider(mavenProject), excludedPaths, projectBuildDir); + ruleInvokerService = new RuleInvokerService(mavenLogAdapter, + projects.stream().map(MavenScopePathProvider::new).collect(Collectors.toList()), + excludedPaths, + projectBuildDir); - ruleFailureMessage = ruleInvokerService.invokeRules(coreRules); + return ruleInvokerService.invokeRules(coreRules); } catch (final Exception e) { throw new MojoFailureException(e.getMessage(), e); } + } - if (!StringUtils.isEmpty(ruleFailureMessage)) { - if(!noFailOnError) { - throw new MojoFailureException(PREFIX_ARCH_VIOLATION_MESSAGE + ruleFailureMessage); - } - - getLog().warn(PREFIX_ARCH_VIOLATION_MESSAGE + ruleFailureMessage); + private void configureProperties() { + if (!properties.isEmpty()) { + getLog().debug("configuring ArchUnit properties"); + final ArchConfiguration archConfiguration = ArchConfiguration.get(); + properties.forEach(archConfiguration::setProperty); } } - private void configureContextClassLoader() throws DependencyResolutionRequiredException, MalformedURLException { + private void configureContextClassLoader(List projects) + throws DependencyResolutionRequiredException, MalformedURLException { List urls = new ArrayList<>(); - List elements = mavenProject.getTestClasspathElements(); - for (String element : elements) { - urls.add(new File(element).toURI().toURL()); + for (MavenProject project : projects) { + for (String element : project.getTestClasspathElements()) { + urls.add(new File(element).toURI().toURL()); + } } - ClassLoader contextClassLoader = newInstance(urls.toArray(new URL[0]), Thread.currentThread().getContextClassLoader()); + ClassLoader contextClassLoader = newInstance(urls.toArray(new URL[0]), + Thread.currentThread().getContextClassLoader()); Thread.currentThread().setContextClassLoader(contextClassLoader); } + void setProjectBuildDir(final String projectBuildDir) { this.projectBuildDir = projectBuildDir; } diff --git a/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenApplyOn.java b/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenApplyOn.java index 71cd5b3..de734cb 100644 --- a/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenApplyOn.java +++ b/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenApplyOn.java @@ -11,15 +11,19 @@ public class MavenApplyOn { @Parameter(property = "scope") private String scope; + @Parameter(property = "aggregator", defaultValue = "false") + private boolean aggregator; + //default constructor is required at runtime public MavenApplyOn() { } //convenience constructor when calling from unit tests - public MavenApplyOn(String packageName, String scope) { + public MavenApplyOn(String packageName, String scope, boolean aggregator) { this.packageName = packageName; this.scope = scope; + this.aggregator = aggregator; } public String getPackageName() { @@ -30,6 +34,10 @@ public String getScope() { return scope; } + public boolean getAggregator() { + return aggregator; + } + public void setPackageName(String packageName) { this.packageName = packageName; } @@ -38,7 +46,11 @@ public void setScope(String scope) { this.scope = scope; } + public void setAggregator(boolean aggregator) { + this.aggregator = aggregator; + } + public ApplyOn toCoreApplyOn() { - return new ApplyOn(packageName,scope); + return new ApplyOn(packageName,scope); } } diff --git a/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenRules.java b/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenRules.java index 7dd7abf..9c44ed5 100644 --- a/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenRules.java +++ b/src/main/java/com/societegenerale/commons/plugin/maven/model/MavenRules.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; import com.societegenerale.commons.plugin.model.Rules; import org.apache.maven.plugins.annotations.Parameter; @@ -29,8 +30,16 @@ public Rules toCoreRules(){ return new Rules(preConfiguredRules, configurableRules.stream() - .map(e -> e.toCoreConfigurableRule()) - .collect(toList())); + .map(e -> e.toCoreConfigurableRule()) + .collect(toList())); + } + + public Rules toCoreRules(boolean isApplyOnAggregator) { + return new Rules(isApplyOnAggregator ? List.of() : preConfiguredRules, + configurableRules.stream() + .filter(configurableRule -> Optional.ofNullable(configurableRule.getApplyOn()).map(MavenApplyOn::getAggregator).orElse(false) == isApplyOnAggregator) + .map(MavenConfigurableRule::toCoreConfigurableRule) + .toList()); } public List getPreConfiguredRules() { diff --git a/src/test/java/com/societegenerale/commons/plugin/maven/AbstractArchUnitMojoTest.java b/src/test/java/com/societegenerale/commons/plugin/maven/AbstractArchUnitMojoTest.java index b1c958a..f21b139 100644 --- a/src/test/java/com/societegenerale/commons/plugin/maven/AbstractArchUnitMojoTest.java +++ b/src/test/java/com/societegenerale/commons/plugin/maven/AbstractArchUnitMojoTest.java @@ -17,82 +17,90 @@ abstract class AbstractArchUnitMojoTest { protected static ExpectedRuleFailure.Creator expectRuleFailure(String ruleDescription) { - return new ExpectedRuleFailure.Creator(ruleDescription); + return new ExpectedRuleFailure.Creator(ruleDescription); } protected String getBasedir() { - String basedir = System.getProperty("basedir"); + String basedir = System.getProperty("basedir"); - if (basedir == null) { - basedir = new File("").getAbsolutePath(); - } + if (basedir == null) { + basedir = new File("").getAbsolutePath(); + } - return basedir; + return basedir; } protected void executeAndExpectViolations(ArchUnitMojo mojo, ExpectedRuleFailure... expectedRuleFailures) { - AbstractThrowableAssert throwableAssert = assertThatThrownBy(mojo::execute); - stream(expectedRuleFailures).forEach(expectedFailure -> { - throwableAssert.hasMessageContaining(String.format("Rule '%s' was violated", expectedFailure.ruleDescription)); - expectedFailure.details.forEach(throwableAssert::hasMessageContaining); - }); - throwableAssert.has(exactNumberOfViolatedRules(expectedRuleFailures.length)); + AbstractThrowableAssert throwableAssert = assertThatThrownBy(mojo::execute); + stream(expectedRuleFailures).forEach(expectedFailure -> { + throwableAssert.hasMessageContaining(String.format("Rule '%s' was violated", expectedFailure.ruleDescription)); + expectedFailure.details.forEach(throwableAssert::hasMessageContaining); + }); + throwableAssert.has(exactNumberOfViolatedRules(expectedRuleFailures.length)); } private Condition exactNumberOfViolatedRules(final int number) { - return new Condition("exactly " + number + " violated rules") { - @Override - public boolean matches(Throwable throwable) { - Matcher matcher = Pattern.compile("Rule '.*' was violated").matcher(throwable.getMessage()); - int numberOfOccurrences = 0; - while (matcher.find()) { - numberOfOccurrences++; - } - return numberOfOccurrences == number; - } - }; + return new Condition("exactly " + number + " violated rules") { + @Override + public boolean matches(Throwable throwable) { + Matcher matcher = Pattern.compile("Rule '.*' was violated").matcher(throwable.getMessage()); + int numberOfOccurrences = 0; + while (matcher.find()) { + numberOfOccurrences++; + } + return numberOfOccurrences == number; + } + }; } protected PlexusConfiguration buildApplyOnBlock(String packageName, String scope) { - PlexusConfiguration packageNameElement = new DefaultPlexusConfiguration("packageName", packageName); - PlexusConfiguration scopeElement = new DefaultPlexusConfiguration("scope", scope); - PlexusConfiguration applyOnElement = new DefaultPlexusConfiguration("applyOn"); - applyOnElement.addChild(packageNameElement); - applyOnElement.addChild(scopeElement); + return buildApplyOnBlock(packageName, scope, false); + } + + protected PlexusConfiguration buildApplyOnBlock(String packageName, String scope, Boolean aggregator) { + + PlexusConfiguration packageNameElement = new DefaultPlexusConfiguration("packageName", packageName); + PlexusConfiguration scopeElement = new DefaultPlexusConfiguration("scope", scope); + PlexusConfiguration aggregatorElement = new DefaultPlexusConfiguration("aggregator", aggregator.toString()); + PlexusConfiguration applyOnElement = new DefaultPlexusConfiguration("applyOn"); + applyOnElement.addChild(packageNameElement); + applyOnElement.addChild(scopeElement); + applyOnElement.addChild(aggregatorElement); - return applyOnElement; + return applyOnElement; } + protected PlexusConfiguration buildChecksBlock(String... checks) { - PlexusConfiguration checksElement = new DefaultPlexusConfiguration("checks"); - stream(checks).map(c -> new DefaultPlexusConfiguration("check", c)).forEach(checksElement::addChild); - return checksElement; + PlexusConfiguration checksElement = new DefaultPlexusConfiguration("checks"); + stream(checks).map(c -> new DefaultPlexusConfiguration("check", c)).forEach(checksElement::addChild); + return checksElement; } public static class ExpectedRuleFailure { - private final String ruleDescription; - private final Set details; - - private ExpectedRuleFailure(String ruleDescription, Set details) { - this.ruleDescription = ruleDescription; - this.details = details; - } - - public static class Creator { private final String ruleDescription; + private final Set details; - Creator(String ruleDescription) { - this.ruleDescription = ruleDescription; + private ExpectedRuleFailure(String ruleDescription, Set details) { + this.ruleDescription = ruleDescription; + this.details = details; } - ExpectedRuleFailure ofAnyKind() { - return withDetails(); - } + public static class Creator { + private final String ruleDescription; + + Creator(String ruleDescription) { + this.ruleDescription = ruleDescription; + } + + ExpectedRuleFailure ofAnyKind() { + return withDetails(); + } - ExpectedRuleFailure withDetails(String... detals) { - return new ExpectedRuleFailure(ruleDescription, ImmutableSet.copyOf(detals)); + ExpectedRuleFailure withDetails(String... detals) { + return new ExpectedRuleFailure(ruleDescription, ImmutableSet.copyOf(detals)); + } } - } } } diff --git a/src/test/java/com/societegenerale/commons/plugin/maven/ArchUnitMojoTest.java b/src/test/java/com/societegenerale/commons/plugin/maven/ArchUnitMojoTest.java index 22a77c9..f256e1f 100644 --- a/src/test/java/com/societegenerale/commons/plugin/maven/ArchUnitMojoTest.java +++ b/src/test/java/com/societegenerale/commons/plugin/maven/ArchUnitMojoTest.java @@ -1,8 +1,5 @@ package com.societegenerale.commons.plugin.maven; -import java.io.File; -import java.io.StringReader; - import com.societegenerale.aut.test.TestClassWithPowerMock; import com.societegenerale.commons.plugin.rules.MyCustomAndDummyRules; import com.societegenerale.commons.plugin.rules.NoPowerMockRuleTest; @@ -29,6 +26,10 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; +import java.io.File; +import java.io.StringReader; +import java.util.List; + import static com.tngtech.junit.dataprovider.DataProviders.testForEach; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; @@ -59,21 +60,21 @@ public class ArchUnitMojoTest extends AbstractArchUnitMojoTest // @formatter:off private static final String pomWithNoRule = - "" + - "" + - "" + - "" + - "arch-unit-maven-plugin" + - "" + - "" + + "" + + "" + + "" + + "" + + "arch-unit-maven-plugin" + + "" + + "" + "" + "" + - "" + - "" + - "" + - "" + - "" + - ""; // @formatter:on + "" + + "" + + "" + + "" + + "" + + ""; // @formatter:on @Before public void setUp() throws Exception { @@ -93,8 +94,8 @@ public void shouldFailWhenNoRuleConfigured() throws Exception { ArchUnitMojo mojo = (ArchUnitMojo) mojoRule.configureMojo(archUnitMojo, pluginConfiguration); assertThatExceptionOfType(MojoFailureException.class) - .isThrownBy(mojo::execute) - .withMessageContaining("Arch unit Plugin should have at least one preconfigured/configurable rule"); + .isThrownBy(mojo::execute) + .withMessageContaining("Arch unit Plugin should have at least one preconfigured/configurable rule"); } @Test @@ -107,8 +108,8 @@ public void shouldExecuteSinglePreconfiguredRule() throws Exception { ArchUnitMojo mojo = (ArchUnitMojo) mojoRule.configureMojo(archUnitMojo, pluginConfiguration); executeAndExpectViolations(mojo, - expectRuleFailure("classes should not use Powermock") - .withDetails("Favor Mockito and proper dependency injection - " + TestClassWithPowerMock.class.getName())); + expectRuleFailure("classes should not use Powermock") + .withDetails("Favor Mockito and proper dependency injection - " + TestClassWithPowerMock.class.getName())); } @Test @@ -127,12 +128,9 @@ public void shouldExecuteSinglePreconfiguredRuleWithNoFailOnError() throws Excep mojo.setLog(log); mojo.execute(); - verify(log, times(1)).warn( - "ArchUnit Maven plugin reported architecture failures listed below :Rule Violated - " + NoPowerMockRuleTest.class.getName() - + System.lineSeparator() + - "java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'classes should not use Powermock' was violated (1 times):" - + System.lineSeparator() + - "Favor Mockito and proper dependency injection - " + TestClassWithPowerMock.class.getName() + System.lineSeparator()); + verify(log, times(1)).info("ArchUnit Maven plugin reported architecture failures listed below :Rule Violated - " + NoPowerMockRuleTest.class.getName() + System.lineSeparator() + + "java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'classes should not use Powermock' was violated (1 times):" + System.lineSeparator() + + "Favor Mockito and proper dependency injection - " + TestClassWithPowerMock.class.getName() + System.lineSeparator()); } @Test @@ -149,8 +147,8 @@ public void shouldFailWronglyDefinedConfigurableRule() throws Exception { ArchUnitMojo mojo = (ArchUnitMojo) mojoRule.configureMojo(archUnitMojo, pluginConfiguration); assertThatExceptionOfType(MojoFailureException.class) - .isThrownBy(mojo::execute) - .withMessageContaining(String.format("The following configured checks are not present within %s: [%s]", ruleClass, missingCheck)); + .isThrownBy(mojo::execute) + .withMessageContaining(String.format("The following configured checks are not present within %s: [%s]", ruleClass, missingCheck)); } @DataProvider @@ -174,7 +172,7 @@ public void shouldExecuteSingleConfigurableRuleCheck(String checkName) throws Ex ArchUnitMojo mojo = (ArchUnitMojo) mojoRule.configureMojo(archUnitMojo, pluginConfiguration); executeAndExpectViolations(mojo, - expectRuleFailure("classes should be annotated with @Test").ofAnyKind()); + expectRuleFailure("classes should be annotated with @Test").ofAnyKind()); } @Test @@ -191,10 +189,10 @@ public void shouldExecuteAllConfigurableRuleChecksIfUnconfigured() throws Except ArchUnitMojo mojo = (ArchUnitMojo) mojoRule.configureMojo(archUnitMojo, pluginConfiguration); executeAndExpectViolations(mojo, - expectRuleFailure("classes should be annotated with @Test").ofAnyKind(), - expectRuleFailure("classes should be annotated with @Test").ofAnyKind(), - expectRuleFailure("classes should reside in a package 'myPackage'").ofAnyKind(), - expectRuleFailure("classes should reside in a package 'myPackage'").ofAnyKind() + expectRuleFailure("classes should be annotated with @Test").ofAnyKind(), + expectRuleFailure("classes should be annotated with @Test").ofAnyKind(), + expectRuleFailure("classes should reside in a package 'myPackage'").ofAnyKind(), + expectRuleFailure("classes should reside in a package 'myPackage'").ofAnyKind() ); } @@ -216,8 +214,33 @@ public void shouldExecuteBothConfigurableRule_and_PreConfiguredRule() throws Exc ArchUnitMojo mojo = (ArchUnitMojo) mojoRule.configureMojo(archUnitMojo, pluginConfiguration); executeAndExpectViolations(mojo, - expectRuleFailure("classes should be annotated with @Test").ofAnyKind(), - expectRuleFailure("classes should not use Powermock").ofAnyKind()); + expectRuleFailure("classes should be annotated with @Test").ofAnyKind(), + expectRuleFailure("classes should not use Powermock").ofAnyKind()); + } + + @Test + public void shouldExecuteConfigurableRuleOnAggregator() throws Exception { + PlexusConfiguration configurableRule = new DefaultPlexusConfiguration("configurableRule"); + + configurableRule.addChild("rule", MyCustomAndDummyRules.class.getName()); + configurableRule.addChild(buildChecksBlock("annotatedWithTest_asField")); + configurableRule.addChild(buildApplyOnBlock("com.societegenerale.aut.test.specificCase", "test", true)); + + PlexusConfiguration configurableRules = pluginConfiguration.getChild("rules").getChild("configurableRules"); + configurableRules.addChild(configurableRule); + + File testPom = new File(getBasedir(), "target/test-classes/unit/plugin-config.xml"); + ArchUnitMojo archUnitMojo = (ArchUnitMojo) mojoRule.lookupMojo("arch-test", testPom); + + MavenProject pomMavenProject = mock(MavenProject.class); + when(pomMavenProject.getPackaging()).thenReturn("pom"); + when(pomMavenProject.getCollectedProjects()).thenReturn(List.of(mavenProject)); + + mojoRule.setVariableValueToObject(archUnitMojo, "mavenProject", pomMavenProject); + mojoRule.configureMojo(archUnitMojo, pluginConfiguration); + + executeAndExpectViolations(archUnitMojo, + expectRuleFailure("classes should be annotated with @Test").ofAnyKind()); } @Test @@ -245,9 +268,9 @@ public void shouldNotExecuteConfigurableRule_and_PreConfiguredRule_IfSkipIsTrue( } @Test - public void shouldSkipIfPackagingIsPom() throws Exception { + public void shouldNotApplyPreConfiguredRuleIfPackagingIsPom() throws Exception { InterceptingLog interceptingLogger = new InterceptingLog( - mojoRule.getContainer().lookup(LoggerManager.class).getLoggerForComponent(Mojo.ROLE)); + mojoRule.getContainer().lookup(LoggerManager.class).getLoggerForComponent(Mojo.ROLE)); File testPom = new File(getBasedir(), "target/test-classes/unit/plugin-config.xml"); ArchUnitMojo archUnitMojo = (ArchUnitMojo) mojoRule.lookupMojo("arch-test", testPom); @@ -259,7 +282,7 @@ public void shouldSkipIfPackagingIsPom() throws Exception { assertThatCode(() -> archUnitMojo.execute()).doesNotThrowAnyException(); - assertThat(interceptingLogger.debugLogs).containsExactly("module packaging is 'pom', so skipping execution"); + assertThat(interceptingLogger.debugLogs).containsExactly("no rule apply for projects"); } @Test