Skip to content

Commit

Permalink
add method ArchRule.allowEmptyShould(..)
Browse files Browse the repository at this point in the history
This will allow to override the configuration property `archRule.failOnEmptyShould` on a per-rule basis. We overlooked the use case for certain rules that simply expect to have no match when we introduced `failOnEmptyShould`. An example would be rules like `classes().that(areUnwanted()).should().containNumberOfElements(equalTo(0))`. So making this only configurable on a global level renders it useless for a certain set of users. Also, we broke `optionalLayers` in `LayeredArchitecture` and `OnionArchitecture` with the default configuration, since individual access checks are modelled as rules. `LayeredArchitecture` and `OnionArchitecture` already have the concept of `optionalLayers` which is pretty much the same as expressed by `allowEmptyShould`. We now simply translate `allowEmptyShould` to the existing concept, otherwise the two concepts clash (e.g. declare an `optionalLayer` for which the allowed accesses are internally modelled as separate rule, and it still rejects empty classes if `failOnEmptyShould` is set to `true).`

Signed-off-by: Hannes Oberprantacher <h.oberprantacher@gmail.com>
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
(cherry picked from commit 4a3f53e)
  • Loading branch information
oberprah authored and codecholeric committed Feb 27, 2022
1 parent 2ebe9fb commit d7a928b
Show file tree
Hide file tree
Showing 14 changed files with 380 additions and 72 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2014-2022 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.lang;

import com.tngtech.archunit.ArchConfiguration;

enum AllowEmptyShould {
TRUE {
@Override
public boolean isAllowed() {
return true;
}
},
FALSE {
@Override
public boolean isAllowed() {
return false;
}
},
AS_CONFIGURED {
@Override
public boolean isAllowed() {
return ArchConfiguration.get().getPropertyOrDefault(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME, TRUE.toString())
.equalsIgnoreCase(FALSE.toString());
}
};

private static final String FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME = "archRule.failOnEmptyShould";

abstract boolean isAllowed();

static AllowEmptyShould fromBoolean(boolean allow) {
return allow ? TRUE : FALSE;
}
}
47 changes: 31 additions & 16 deletions archunit/src/main/java/com/tngtech/archunit/lang/ArchRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableSet;
import com.tngtech.archunit.ArchConfiguration;
import com.tngtech.archunit.Internal;
import com.tngtech.archunit.PublicAPI;
import com.tngtech.archunit.base.Optional;
Expand All @@ -41,7 +40,6 @@
import static com.google.common.io.Resources.readLines;
import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
import static com.tngtech.archunit.base.ClassLoaders.getCurrentClassLoader;
import static java.lang.Boolean.TRUE;
import static java.nio.charset.StandardCharsets.UTF_8;

/**
Expand All @@ -68,6 +66,18 @@ public interface ArchRule extends CanBeEvaluated, CanOverrideDescription<ArchRul
@PublicAPI(usage = ACCESS)
ArchRule because(String reason);

/**
* If set to {@code true} allows the should-clause of this rule to be checked against an empty set of elements.
* Otherwise, the rule will fail with a respective message. This is to prevent possible implementation errors,
* like filtering for a non-existing package in the that-clause causing an always-passing rule.<br>
* Note that this method will override the configuration property {@code archRule.failOnEmptyShould}.
*
* @param allowEmptyShould Whether the rule fails if the should-clause is evaluated with an empty set of elements
* @return A (new) {@link ArchRule} with adjusted {@code allowEmptyShould} behavior
*/
@PublicAPI(usage = ACCESS)
ArchRule allowEmptyShould(boolean allowEmptyShould);

@PublicAPI(usage = ACCESS)
final class Assertions {
private static final ArchUnitExtensions extensions = new ArchUnitExtensions();
Expand Down Expand Up @@ -166,7 +176,7 @@ public EvaluationResult getResult() {
@Internal
class Factory {
public static <T> ArchRule create(final ClassesTransformer<T> classesTransformer, final ArchCondition<T> condition, final Priority priority) {
return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.<String>empty());
return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.<String>empty(), AllowEmptyShould.AS_CONFIGURED);
}

public static ArchRule withBecause(ArchRule rule, String reason) {
Expand All @@ -184,18 +194,20 @@ private static class SimpleArchRule<T> implements ArchRule {
private final ClassesTransformer<T> classesTransformer;
private final ArchCondition<T> condition;
private final Optional<String> overriddenDescription;
private final AllowEmptyShould allowEmptyShould;

private SimpleArchRule(Priority priority, ClassesTransformer<T> classesTransformer, ArchCondition<T> condition,
Optional<String> overriddenDescription) {
Optional<String> overriddenDescription, AllowEmptyShould allowEmptyShould) {
this.priority = priority;
this.classesTransformer = classesTransformer;
this.condition = condition;
this.overriddenDescription = overriddenDescription;
this.allowEmptyShould = allowEmptyShould;
}

@Override
public ArchRule as(String newDescription) {
return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.of(newDescription));
return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.of(newDescription), allowEmptyShould);
}

@Override
Expand All @@ -208,6 +220,11 @@ public ArchRule because(String reason) {
return withBecause(this, reason);
}

@Override
public ArchRule allowEmptyShould(boolean allowEmptyShould) {
return new SimpleArchRule<>(priority, classesTransformer, condition, overriddenDescription, AllowEmptyShould.fromBoolean(allowEmptyShould));
}

@Override
public EvaluationResult evaluate(JavaClasses classes) {
Iterable<T> allObjects = classesTransformer.transform(classes);
Expand All @@ -223,20 +240,18 @@ public EvaluationResult evaluate(JavaClasses classes) {
}

private void verifyNoEmptyShouldIfEnabled(Iterable<T> allObjects) {
if (isEmpty(allObjects) && isFailOnEmptyShouldEnabled()) {
throw new AssertionError("Rule failed to check any classes. " +
"This means either that no classes have been passed to the rule at all, " +
"or that no classes passed to the rule matched the `that()` clause. " +
"To allow rules being evaluated without checking any classes you can set the ArchUnit property " +
FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME + " = " + false);
if (isEmpty(allObjects) && !allowEmptyShould.isAllowed()) {
throw new AssertionError(String.format(
"Rule '%s' failed to check any classes. "
+ "This means either that no classes have been passed to the rule at all, "
+ "or that no classes passed to the rule matched the `that()` clause. "
+ "To allow rules being evaluated without checking any classes you can either "
+ "use `%s.allowEmptyShould(true)` on a single rule or set the configuration property `%s = false` "
+ "to change the behavior globally.",
getDescription(), ArchRule.class.getSimpleName(), FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME));
}
}

private boolean isFailOnEmptyShouldEnabled() {
return ArchConfiguration.get().getPropertyOrDefault(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME, TRUE.toString())
.equals(TRUE.toString());
}

@Override
public String getDescription() {
return overriddenDescription.isPresent() ?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ public CompositeArchRule because(String reason) {
return new CompositeArchRule(priority, rules, createBecauseDescription(this, reason));
}

@Override
@PublicAPI(usage = ACCESS)
public ArchRule allowEmptyShould(boolean allowEmptyShould) {
ImmutableList.Builder<ArchRule> rulesWithOverriddenAllowEmptyShould = ImmutableList.builder();
for (ArchRule rule : rules) {
rulesWithOverriddenAllowEmptyShould.add(rule.allowEmptyShould(allowEmptyShould));
}
return new CompositeArchRule(priority, rulesWithOverriddenAllowEmptyShould.build(), description);
}

@Override
@PublicAPI(usage = ACCESS)
public EvaluationResult evaluate(JavaClasses classes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public ArchRule because(String reason) {
return ArchRule.Factory.withBecause(this, reason);
}

@Override
public ArchRule allowEmptyShould(boolean allowEmptyShould) {
return finishedRule.get().allowEmptyShould(allowEmptyShould);
}

@Override
public ArchRule as(String newDescription) {
return finishedRule.get().as(newDescription);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ private void checkEmptyLayers(JavaClasses classes, EvaluationResult result) {
private EvaluationResult evaluateLayersShouldNotBeEmpty(JavaClasses classes, LayerDefinition layerDefinition) {
return classes().that(layerDefinitions.containsPredicateFor(layerDefinition.name))
.should(notBeEmptyFor(layerDefinition))
// we need to set `allowEmptyShould(true)` to allow the layer not empty check to be evaluated. This will provide a nicer error message.
.allowEmptyShould(true)
.evaluate(classes);
}

Expand All @@ -231,6 +233,7 @@ private EvaluationResult evaluateDependenciesShouldBeSatisfied(JavaClasses class
: onlyHaveDependenciesWhere(targetMatchesIfDependencyIsRelevant(specification.layerName, specification.allowedLayers));
return classes().that(layerDefinitions.containsPredicateFor(specification.layerName))
.should(satisfyLayerDependenciesCondition)
.allowEmptyShould(true)
.evaluate(classes);
}

Expand All @@ -256,32 +259,6 @@ private DescribedPredicate<Dependency> ifDependencyIsRelevant(DescribedPredicate
originPackageMatches;
}

private static ArchCondition<JavaClass> notBeEmptyFor(final LayeredArchitecture.LayerDefinition layerDefinition) {
return new LayerShouldNotBeEmptyCondition(layerDefinition);
}

private static class LayerShouldNotBeEmptyCondition extends ArchCondition<JavaClass> {
private final LayeredArchitecture.LayerDefinition layerDefinition;
private boolean empty = true;

LayerShouldNotBeEmptyCondition(final LayeredArchitecture.LayerDefinition layerDefinition) {
super("not be empty");
this.layerDefinition = layerDefinition;
}

@Override
public void check(JavaClass item, ConditionEvents events) {
empty = false;
}

@Override
public void finish(ConditionEvents events) {
if (empty) {
events.add(violated(layerDefinition, String.format("Layer '%s' is empty", layerDefinition.name)));
}
}
}

@Override
@PublicAPI(usage = ACCESS)
public void check(JavaClasses classes) {
Expand All @@ -294,6 +271,15 @@ public ArchRule because(String reason) {
return ArchRule.Factory.withBecause(this, reason);
}

/**
* This method is equivalent to calling {@link #withOptionalLayers(boolean)}, which should be preferred in this context
* as the meaning is easier to understand.
*/
@Override
public ArchRule allowEmptyShould(boolean allowEmptyShould) {
return withOptionalLayers(allowEmptyShould);
}

@Override
@PublicAPI(usage = ACCESS)
public LayeredArchitecture as(String newDescription) {
Expand Down Expand Up @@ -352,6 +338,32 @@ private void checkLayerNamesExist(String... layerNames) {
}
}

private static ArchCondition<JavaClass> notBeEmptyFor(final LayeredArchitecture.LayerDefinition layerDefinition) {
return new LayerShouldNotBeEmptyCondition(layerDefinition);
}

private static class LayerShouldNotBeEmptyCondition extends ArchCondition<JavaClass> {
private final LayeredArchitecture.LayerDefinition layerDefinition;
private boolean empty = true;

LayerShouldNotBeEmptyCondition(final LayeredArchitecture.LayerDefinition layerDefinition) {
super("not be empty");
this.layerDefinition = layerDefinition;
}

@Override
public void check(JavaClass item, ConditionEvents events) {
empty = false;
}

@Override
public void finish(ConditionEvents events) {
if (empty) {
events.add(violated(layerDefinition, String.format("Layer '%s' is empty", layerDefinition.name)));
}
}
}

private static final class LayerDefinitions implements Iterable<LayerDefinition> {
private final Map<String, LayerDefinition> layerDefinitions = new LinkedHashMap<>();

Expand Down Expand Up @@ -570,6 +582,10 @@ public OnionArchitecture adapter(String name, String... packageIdentifiers) {
return this;
}

/**
* @param optionalLayers Whether the different parts of the Onion Architecture (domain models, domain services, ...) should be allowed to be empty.
* If set to {@code false} the {@link OnionArchitecture OnionArchitecture} will fail if any such layer does not contain any class.
*/
@PublicAPI(usage = ACCESS)
public OnionArchitecture withOptionalLayers(boolean optionalLayers) {
this.optionalLayers = optionalLayers;
Expand Down Expand Up @@ -637,6 +653,15 @@ public ArchRule because(String reason) {
return ArchRule.Factory.withBecause(this, reason);
}

/**
* This method is equivalent to calling {@link #withOptionalLayers(boolean)}, which should be preferred in this context
* as the meaning is easier to understand.
*/
@Override
public ArchRule allowEmptyShould(boolean allowEmptyShould) {
return withOptionalLayers(allowEmptyShould);
}

@Override
public OnionArchitecture as(String newDescription) {
return new OnionArchitecture(domainModelPackageIdentifiers, domainServicePackageIdentifiers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableList;
import com.tngtech.archunit.PublicAPI;
import com.tngtech.archunit.base.DescribedPredicate;
import com.tngtech.archunit.base.Optional;
import com.tngtech.archunit.core.domain.Dependency;
import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.domain.JavaClasses;
Expand All @@ -44,18 +45,32 @@ public final class SliceRule implements ArchRule {
private final List<Transformation> transformations;
private final DescribedPredicate<Dependency> ignoreDependency;
private final ConditionFactory conditionFactory;
private final Optional<Boolean> allowEmptyShould;

SliceRule(Slices.Transformer inputTransformer, Priority priority, ConditionFactory conditionFactory) {
this(inputTransformer, priority, Collections.<Transformation>emptyList(), DescribedPredicate.<Dependency>alwaysFalse(), conditionFactory);
this(
inputTransformer,
priority,
Collections.<Transformation>emptyList(),
DescribedPredicate.<Dependency>alwaysFalse(),
conditionFactory,
Optional.<Boolean>empty());
}

private SliceRule(Slices.Transformer inputTransformer, Priority priority, List<Transformation> transformations,
DescribedPredicate<Dependency> ignoreDependency, ConditionFactory conditionFactory) {
private SliceRule(
Slices.Transformer inputTransformer,
Priority priority,
List<Transformation> transformations,
DescribedPredicate<Dependency> ignoreDependency,
ConditionFactory conditionFactory,
Optional<Boolean> allowEmptyShould
) {
this.inputTransformer = inputTransformer;
this.priority = priority;
this.transformations = transformations;
this.ignoreDependency = ignoreDependency;
this.conditionFactory = conditionFactory;
this.allowEmptyShould = allowEmptyShould;
}

@Override
Expand All @@ -70,6 +85,12 @@ public SliceRule because(String reason) {
return copyWithTransformation(new Because(reason));
}

@Override
@PublicAPI(usage = ACCESS)
public ArchRule allowEmptyShould(boolean allowEmptyShould) {
return new SliceRule(inputTransformer, priority, transformations, ignoreDependency, conditionFactory, Optional.of(allowEmptyShould));
}

@Override
@PublicAPI(usage = ACCESS)
public EvaluationResult evaluate(JavaClasses classes) {
Expand Down Expand Up @@ -100,17 +121,22 @@ public SliceRule ignoreDependency(String origin, String target) {

@PublicAPI(usage = ACCESS)
public SliceRule ignoreDependency(DescribedPredicate<? super JavaClass> origin, DescribedPredicate<? super JavaClass> target) {
return new SliceRule(inputTransformer, priority, transformations, ignoreDependency.or(dependency(origin, target)), conditionFactory);
return new SliceRule(inputTransformer, priority, transformations, ignoreDependency.or(dependency(origin, target)), conditionFactory, allowEmptyShould);
}

private SliceRule copyWithTransformation(Transformation transformation) {
List<Transformation> newTransformations =
ImmutableList.<Transformation>builder().addAll(transformations).add(transformation).build();
return new SliceRule(inputTransformer, priority, newTransformations, ignoreDependency, conditionFactory);
return new SliceRule(inputTransformer, priority, newTransformations, ignoreDependency, conditionFactory, allowEmptyShould);
}

private ArchRule getArchRule() {
ArchRule rule = priority(priority).all(inputTransformer).should(conditionFactory.create(inputTransformer, not(ignoreDependency)));
ArchRule rule = priority(priority)
.all(inputTransformer)
.should(conditionFactory.create(inputTransformer, not(ignoreDependency)));
if (allowEmptyShould.isPresent()) {
rule = rule.allowEmptyShould(allowEmptyShould.get());
}
for (Transformation transformation : transformations) {
rule = transformation.apply(rule);
}
Expand Down
Loading

0 comments on commit d7a928b

Please sign in to comment.