Skip to content

Add support for grouped rules #95

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

Open
wants to merge 1 commit 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
7 changes: 7 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@
<version>${tng.archunit.version}</version>
</dependency>

<!-- To be able to work with com.tngtech.archunit.junit.ArchTests -->
<dependency>
<groupId>com.tngtech.archunit</groupId>
<artifactId>archunit-junit5-api</artifactId>
<version>${tng.archunit.version}</version>
</dependency>

<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Deque;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
Expand All @@ -13,20 +16,20 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.societegenerale.commons.plugin.Log;
import com.societegenerale.commons.plugin.utils.ReflectionUtils;
import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.junit.ArchTests;
import com.tngtech.archunit.lang.ArchRule;

import static com.societegenerale.commons.plugin.utils.ReflectionUtils.getValue;
import static com.societegenerale.commons.plugin.utils.ReflectionUtils.invoke;
import static com.societegenerale.commons.plugin.utils.ReflectionUtils.loadClassWithContextClassLoader;
import static com.societegenerale.commons.plugin.utils.ReflectionUtils.newInstance;
import static java.lang.System.lineSeparator;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toSet;

class InvokableRules {
private final Class<?> rulesLocation;
private final Set<Field> archRuleFields;
private final Set<Method> archRuleMethods;

Expand All @@ -36,11 +39,17 @@ private InvokableRules(String rulesClassName, List<String> ruleChecks, Log log)

this.log=log;

rulesLocation = loadClassWithContextClassLoader(rulesClassName);
Class<?> definedRulesClass = loadClassWithContextClassLoader(rulesClassName);

Set<Field> allFieldsWhichAreArchRules = getAllFieldsWhichAreArchRules(rulesLocation.getDeclaredFields());
Set<Method> allMethodsWhichAreArchRules = getAllMethodsWhichAreArchRules(rulesLocation.getDeclaredMethods());
validateRuleChecks(Sets.union(allMethodsWhichAreArchRules, allFieldsWhichAreArchRules), ruleChecks);
Set<Class<?>> rulesClasses = getAllClassesWhichAreArchTests(definedRulesClass);
rulesClasses.add(definedRulesClass);
Set<Field> allFieldsWhichAreArchRules = new HashSet<>();
Set<Method> allMethodsWhichAreArchRules = new HashSet<>();
for (Class<?> rulesClass : rulesClasses) {
allFieldsWhichAreArchRules.addAll(getAllFieldsWhichAreArchRules(rulesClass.getDeclaredFields()));
allMethodsWhichAreArchRules.addAll(getAllMethodsWhichAreArchRules(rulesClass.getDeclaredMethods()));
}
validateRuleChecks(definedRulesClass, Sets.union(allMethodsWhichAreArchRules, allFieldsWhichAreArchRules), ruleChecks);

Predicate<String> isChosenCheck = ruleChecks.isEmpty() ? check -> true : ruleChecks::contains;

Expand All @@ -64,7 +73,7 @@ private void logBuiltInvokableRules(String rulesClassName) {

}

private void validateRuleChecks(Set<? extends Member> allFieldsAndMethods, Collection<String> ruleChecks) {
private void validateRuleChecks(Class<?> rulesLocation, Set<? extends Member> allFieldsAndMethods, Collection<String> ruleChecks) {
Set<String> allFieldAndMethodNames = allFieldsAndMethods.stream().map(Member::getName).collect(toSet());
Set<String> illegalChecks = Sets.difference(ImmutableSet.copyOf(ruleChecks), allFieldAndMethodNames);

Expand All @@ -91,9 +100,26 @@ private Set<Field> getAllFieldsWhichAreArchRules(Field[] fields) {
.collect(toSet());
}

InvocationResult invokeOn(JavaClasses importedClasses) {
private Set<Class<?>> getAllClassesWhichAreArchTests(Class<?> startClass) {
Set<Class<?>> allClassesWhichAreArchTests = new HashSet<>();
Deque<Class<?>> stack = new ArrayDeque<>();
stack.push(startClass);
while (!stack.isEmpty()) {
Class<?> currentClass = stack.pop();
stream(currentClass.getDeclaredFields())
.filter(f -> ArchTests.class.isAssignableFrom(f.getType()))
.map(f -> getValue(f, null))
.map(ArchTests.class::cast)
.map(ArchTests::getDefinitionLocation)
.forEach(childClass -> {
allClassesWhichAreArchTests.add(childClass);
stack.push(childClass);
});
}
return allClassesWhichAreArchTests;
}

Object instance = newInstance(rulesLocation);
InvocationResult invokeOn(JavaClasses importedClasses) {

if(log.isInfoEnabled()) {
log.info("applying rules on "+importedClasses.size()+" classe(s). To see the details, enable debug logs");
Expand All @@ -105,11 +131,11 @@ InvocationResult invokeOn(JavaClasses importedClasses) {

InvocationResult result = new InvocationResult();
for (Method method : archRuleMethods) {
checkForFailure(() -> invoke(method, instance, importedClasses))
checkForFailure(() -> invoke(method, null, importedClasses))
Copy link
Author

Choose a reason for hiding this comment

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

ArchUnit examples use static fields and methods so no need for an instance here.

If it turns out an instance is needed, then more work is needed.

.ifPresent(result::add);
}
for (Field field : archRuleFields) {
ArchRule rule = getValue(field, instance);
ArchRule rule = getValue(field, null);
checkForFailure(() -> rule.check(importedClasses))
.ifPresent(result::add);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.societegenerale.commons.plugin.rules.classesForTests;

import com.tngtech.archunit.junit.ArchTest;
import com.tngtech.archunit.junit.ArchTests;

public class DoubleIncludedCustomRule {

@ArchTest
static final ArchTests DOUBLE_INCLUDED = ArchTests.in(IncludedCustomRule.class);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.societegenerale.commons.plugin.rules.classesForTests;

import com.tngtech.archunit.junit.ArchTest;
import com.tngtech.archunit.junit.ArchTests;

public class IncludedCustomRule {

@ArchTest
static final ArchTests INCLUDED = ArchTests.in(DummyCustomRule.class);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package com.societegenerale.commons.plugin.service;

import com.societegenerale.commons.plugin.Log;
import com.societegenerale.commons.plugin.model.RootClassFolder;
import com.societegenerale.commons.plugin.rules.classesForTests.DoubleIncludedCustomRule;
import com.societegenerale.commons.plugin.rules.classesForTests.DummyCustomRule;
import com.societegenerale.commons.plugin.rules.classesForTests.IncludedCustomRule;
import com.societegenerale.commons.plugin.utils.ArchUtils;
import com.tngtech.archunit.core.domain.JavaClasses;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import java.util.Arrays;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

class InvokableRulesTest {

@BeforeAll
@SuppressWarnings("InstantiationOfUtilityClass")
static void instantiateArchUtils() {
new ArchUtils(mock(Log.class));
}

@Test
void shouldInvokeAllRulesDefinedAsFields() {
assertThat(invokeAndGetMessage(DummyCustomRule.class))
.contains("Rule 'classes should be annotated with @Test' was violated")
.contains("Rule 'classes should reside in a package 'myPackage'' was violated");
}

@Test
void shouldInvokeSpecificRuleDefinedAsField() {
assertThat(invokeAndGetMessage(DummyCustomRule.class, "annotatedWithTest"))
.contains("Rule 'classes should be annotated with @Test' was violated")
.doesNotContain("Rule 'classes should reside in a package 'myPackage'' was violated");
}

@Test
void shouldInvokeAllRulesIncludedViaField() {
assertThat(invokeAndGetMessage(IncludedCustomRule.class))
.contains("Rule 'classes should be annotated with @Test' was violated")
.contains("Rule 'classes should reside in a package 'myPackage'' was violated");
}

@Test
void shouldInvokeSpecificRuleIncludedViaField() {
assertThat(invokeAndGetMessage(IncludedCustomRule.class, "annotatedWithTest"))
.contains("Rule 'classes should be annotated with @Test' was violated")
.doesNotContain("Rule 'classes should reside in a package 'myPackage'' was violated");
}

@Test
void shouldInvokeAllRulesIncludedViaFieldThatItselfIncludes() {
assertThat(invokeAndGetMessage(DoubleIncludedCustomRule.class))
.contains("Rule 'classes should be annotated with @Test' was violated")
.contains("Rule 'classes should reside in a package 'myPackage'' was violated");
}

@Test
void shouldInvokeSpecificRuleIncludedViaFieldThatItselfIncludes() {
assertThat(invokeAndGetMessage(DoubleIncludedCustomRule.class,"annotatedWithTest"))
.contains("Rule 'classes should be annotated with @Test' was violated")
.doesNotContain("Rule 'classes should reside in a package 'myPackage'' was violated");
}


private static String invokeAndGetMessage(Class<?> rulesClass, String... checks) {
JavaClasses javaClasses = ArchUtils.importAllClassesInPackage(new RootClassFolder(""), "");
InvokableRules invokableRules = InvokableRules.of(rulesClass.getName(), Arrays.asList(checks), mock(Log.class));
InvokableRules.InvocationResult invocationResult = invokableRules.invokeOn(javaClasses);
return invocationResult.getMessage();
}
}