Skip to content

Commit

Permalink
add automatic conversions to violation handling (#1251)
Browse files Browse the repository at this point in the history
The extensive freedom of how to create a `ConditionEvent` causes
problems for any client that needs more structured results (e.g. tools).
`ViolationHandler` provides a convenient API to obtain only those sort
of violations that can be handled by a client (e.g. tackling
dependencies).
However, the type of the `correspondingObject` attached to the
`ConditionEvent` can take many forms.
From classes to methods, method calls, dependencies or aggregated
dependencies (like module dependencies) it's hard to determine the
possibilities and handle them all.
Furthermore, the type of some objects (e.g. `ComponentDependency` isn't
even public,
so there is hardly any clean way to handle these objects.

To mitigate this a little we now support transparent conversions between
compatible objects.
E.g. a component dependency can also be considered a set of class
dependencies.
`ViolationHandler` now allows to be used including these conversions
that are implemented by standard ArchUnit objects where reasonable.
This allows e.g. to obtain all module dependencies as `Set<Dependency>`
for every violation.
  • Loading branch information
codecholeric authored Mar 26, 2024
2 parents fc18fa9 + 771df06 commit b7d9dd5
Show file tree
Hide file tree
Showing 36 changed files with 731 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import static com.tngtech.archunit.core.domain.JavaMember.Predicates.declaredIn;
import static com.tngtech.archunit.core.domain.JavaModifier.FINAL;
import static com.tngtech.archunit.core.domain.JavaModifier.PUBLIC;
import static com.tngtech.archunit.core.domain.JavaModifier.SYNTHETIC;
import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith;
import static com.tngtech.archunit.core.domain.properties.HasModifiers.Predicates.modifier;
import static com.tngtech.archunit.core.domain.properties.HasName.Utils.namesOf;
Expand Down Expand Up @@ -95,6 +96,7 @@ public class PublicAPIRules {
public static final ArchRule all_public_API_members_are_accessible =
members()
.that().areAnnotatedWith(PublicAPI.class)
.and().doNotHaveModifier(SYNTHETIC)
.should(bePubliclyAccessible());

@ArchTest
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,24 @@ static void addTo(ExpectedTestFailures expectedTestFailures) {
.inLine(18))
.by(callFromMethod(AdministrationCLI.class, "handle", String[].class, AdministrationPort.class)
.toMethod(ProductRepository.class, "getTotalCount")
.inLine(19).asDependency())
.inLine(19))
.by(callFromMethod(ShoppingController.class, "addToShoppingCart", UUID.class, UUID.class, int.class)
.toConstructor(ProductId.class, UUID.class)
.inLine(20).asDependency())
.inLine(20))
.by(callFromMethod(ShoppingController.class, "addToShoppingCart", UUID.class, UUID.class, int.class)
.toConstructor(ShoppingCartId.class, UUID.class)
.inLine(20).asDependency())
.inLine(20))
.by(method(ShoppingService.class, "addToShoppingCart").withParameter(ProductId.class))
.by(method(ShoppingService.class, "addToShoppingCart").withParameter(ShoppingCartId.class))
.by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class)
.toMethod(ShoppingCartRepository.class, "read", ShoppingCartId.class)
.inLine(21).asDependency())
.inLine(21))
.by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class)
.toMethod(ProductRepository.class, "read", ProductId.class)
.inLine(22).asDependency())
.inLine(22))
.by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class)
.toMethod(ShoppingCartRepository.class, "save", ShoppingCart.class)
.inLine(25).asDependency())
.inLine(25))
.by(constructor(OrderItem.class).withParameter(OrderQuantity.class))
.by(field(OrderItem.class, "quantity").ofType(OrderQuantity.class));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ private String detailLinePattern(String string) {
return ".*" + quote(string) + ".*";
}

@Override
public void addTo(HandlingAssertion handlingAssertion) {
details.values().forEach(relation -> relation.addTo(handlingAssertion));
}

@Override
public String getDescription() {
return String.format("Message contains cycle description '%s' and details '%s'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ public ExpectedDependency asDependency() {
@Override
public void addTo(HandlingAssertion assertion) {
assertion.byFieldAccess(this);
if (!getOrigin().getDeclaringClass().equals(getTarget().getDeclaringClass())) {
assertion.byDependency(asDependency());
}
}
}

Expand All @@ -192,6 +195,9 @@ public void addTo(HandlingAssertion assertion) {
} else {
assertion.byMethodCall(this);
}
if (!getOrigin().getDeclaringClass().equals(getTarget().getDeclaringClass())) {
assertion.byDependency(asDependency());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
import java.util.Set;

import com.google.common.base.Joiner;
import com.tngtech.archunit.core.domain.Dependency;
import com.tngtech.archunit.testutils.ExpectedRelation.LineAssociation;

import static com.tngtech.archunit.testutils.MessageAssertionChain.matchesLine;
import static java.util.regex.Pattern.quote;

public class ExpectedModuleDependency implements MessageAssertionChain.Link {
Expand All @@ -22,8 +21,13 @@ public static ModuleDependencyCreator fromModule(String moduleName) {
return new ModuleDependencyCreator(moduleName);
}

public static UncontainedCreator uncontainedFrom(Class<?> origin) {
return new UncontainedCreator(origin);
public static MessageAssertionChain.Link uncontained(ExpectedRelation call) {
return new ExpectedUncontainedModuleDependency(call);
}

@Override
public void addTo(HandlingAssertion handlingAssertion) {
details.forEach(it -> it.addTo(handlingAssertion));
}

@Override
Expand Down Expand Up @@ -58,51 +62,38 @@ public ExpectedModuleDependency toModule(String moduleName) {
}
}

public static class UncontainedCreator {
private final Class<?> origin;

private UncontainedCreator(Class<?> origin) {
this.origin = origin;
}
private static class ExpectedUncontainedModuleDependency implements MessageAssertionChain.Link {
private final ExpectedRelation delegate;

public ExpectedUncontainedModuleDependency to(Class<?> target) {
return new ExpectedUncontainedModuleDependency(origin, target);
}
}

private static class ExpectedUncontainedModuleDependency implements MessageAssertionChain.Link, ExpectedRelation {
private final String dependencyPattern;
private final MessageAssertionChain.Link delegate;

private ExpectedUncontainedModuleDependency(Class<?> origin, Class<?> target) {
dependencyPattern = String.format("Dependency not contained in any module: .*%s.*%s.*",
quote(origin.getName()), quote(target.getName()));
this.delegate = matchesLine(dependencyPattern);
private ExpectedUncontainedModuleDependency(ExpectedRelation delegate) {
this.delegate = delegate;
}

@Override
public void addTo(HandlingAssertion assertion) {
assertion.byDependency(this);
}

@Override
public void associateLines(LineAssociation association) {
association.associateIfPatternMatches(dependencyPattern);
}

@Override
public boolean correspondsTo(Object object) {
return object instanceof Dependency;
delegate.addTo(assertion);
}

@Override
public Result filterMatching(List<String> lines) {
return delegate.filterMatching(lines);
Result.Builder builder = new Result.Builder();
delegate.associateLines(new LineAssociation() {
@Override
public void associateIfPatternMatches(String pattern) {
builder.matchesLine("Dependency not contained in any module:" + pattern);
}

@Override
public void associateIfStringIsContained(String string) {
associateIfPatternMatches(".*" + quote(string) + ".*");
}
});
return builder.build(lines);
}

@Override
public String getDescription() {
return delegate.getDescription();
return "Uncontained module dependency: " + delegate;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.tngtech.archunit.core.domain.JavaAccess;
import com.tngtech.archunit.lang.ConditionEvent;

interface ExpectedRelation {
public interface ExpectedRelation {

void addTo(HandlingAssertion assertion);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,7 @@ void by(ExpectedDependency inheritance) {

void by(MessageAssertionChain.Link assertion) {
expectedViolation.by(assertion);
if (assertion instanceof ExpectedRelation) {
((ExpectedRelation) assertion).addTo(handlingAssertion);
}
assertion.addTo(handlingAssertion);
}

ExpectedViolationToAssign copy() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Iterator;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.BiPredicate;

import com.google.common.base.Joiner;
import com.google.common.collect.Sets;
Expand All @@ -16,14 +17,13 @@
import com.tngtech.archunit.core.domain.JavaMethodCall;
import com.tngtech.archunit.lang.EvaluationResult;

import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.collect.Sets.union;
import static java.lang.System.lineSeparator;
import static java.util.Collections.emptySet;
import static java.util.Collections.singleton;
import static java.util.stream.Collectors.toSet;

class HandlingAssertion {
public class HandlingAssertion {
private final Set<ExpectedRelation> expectedFieldAccesses;
private final Set<ExpectedRelation> expectedMethodCalls;
private final Set<ExpectedRelation> expectedConstructorCalls;
Expand Down Expand Up @@ -57,8 +57,8 @@ void byMethodCall(ExpectedRelation call) {
expectedMethodCalls.add(call);
}

void byDependency(ExpectedRelation inheritance) {
expectedDependencies.add(inheritance);
void byDependency(ExpectedRelation dependency) {
expectedDependencies.add(dependency);
}

static HandlingAssertion ofRule() {
Expand Down Expand Up @@ -131,15 +131,13 @@ private Set<String> evaluateDependencies(EvaluationResult result) {
}

private Set<String> removeExpectedAccesses(Collection<?> violatingObjects, Set<? extends ExpectedRelation> left) {
Object violatingObject = getOnlyElement(violatingObjects);
for (Iterator<? extends ExpectedRelation> actualMethodCalls = left.iterator(); actualMethodCalls.hasNext(); ) {
ExpectedRelation next = actualMethodCalls.next();
if (next.correspondsTo(violatingObject)) {
actualMethodCalls.remove();
return emptySet();
}
removeMatchingElements(violatingObjects, left, (object, relation) -> relation.correspondsTo(object));

if (!violatingObjects.isEmpty()) {
return singleton("Unhandled violations: " + violatingObjects);
}
return singleton("Unexpected violation handling: " + violatingObject);

return emptySet();
}

private Set<String> errorMessagesFrom(Set<?> set) {
Expand All @@ -150,6 +148,19 @@ HandlingAssertion copy() {
return new HandlingAssertion(expectedFieldAccesses, expectedMethodCalls, expectedConstructorCalls, expectedDependencies);
}

private <T, U> void removeMatchingElements(Collection<T> actual, Collection<U> expected, BiPredicate<T, U> matches) {
for (Iterator<? extends T> actualIterator = actual.iterator(); actualIterator.hasNext(); ) {
T actualElement = actualIterator.next();
for (Iterator<? extends U> expectedIterator = expected.iterator(); expectedIterator.hasNext(); ) {
if (matches.test(actualElement, expectedIterator.next())) {
actualIterator.remove();
expectedIterator.remove();
break;
}
}
}
}

static class Result {
private final Set<String> errorMessages = new TreeSet<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ public interface Link {

String getDescription();

default void addTo(HandlingAssertion handlingAssertion) {
}

@Internal
class Result {
private final boolean matches;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public Result filterMatching(List<String> lines) {
return new Result(true, remainingLines);
}

@Override
public void addTo(HandlingAssertion handlingAssertion) {
expectedAccesses.forEach(it -> it.addTo(handlingAssertion));
}

@Override
public String getDescription() {
return Joiner.on(System.lineSeparator()).join(ImmutableList.builder()
Expand Down
46 changes: 46 additions & 0 deletions archunit/src/main/java/com/tngtech/archunit/core/Convertible.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright 2014-2024 TNG Technology Consulting GmbH
*
* Licensed 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 com.tngtech.archunit.core;

import java.util.Set;

import com.tngtech.archunit.PublicAPI;
import com.tngtech.archunit.core.domain.Dependency;
import com.tngtech.archunit.core.domain.JavaAccess;

import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE;

/**
* Can be implemented to express that this object might also be considered as object(s) of a different type.
* E.g. {@link JavaAccess} and {@link Dependency} (compare {@link #convertTo(Class)}).
*/
@PublicAPI(usage = INHERITANCE)
public interface Convertible {
/**
* Converts this type to a set of other types.
* For example a {@link JavaAccess} can also be
* considered a {@link Dependency}, so <code>javaAccess.convertTo(Dependency.class)</code>
* will yield a set with a single {@link Dependency} representing this access.
* Or a component dependency grouping many class dependencies could be considered a set of exactly
* these class dependencies.
* The result will be an empty set if no conversion is possible
* (e.g. calling <code>javaAccess.convertTo(Integer.class)</code>.
*
* @param type The type to convert to
* @return A set of converted elements, empty if no conversion is possible
*/
<T> Set<T> convertTo(Class<T> type);
}
Loading

0 comments on commit b7d9dd5

Please sign in to comment.