Skip to content

Commit

Permalink
some review comments on FreezingArchRule PR#181 (documentation)
Browse files Browse the repository at this point in the history
Signed-off-by: Manfred Hanke <[email protected]>
  • Loading branch information
hankem authored and codecholeric committed Jul 4, 2019
1 parent e1e2c4b commit c8c98c6
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,30 @@

/**
* A decorator around an existing {@link ArchRule} that "freezes" the state of all violations on the first call instead of failing the test.
* This means in particular that the first run of a {@link FreezingArchRule} will always pass, consecutive calls will only fail if "unknown"
* violations are introduced (read below for further explanations when a violation is "unknown").
* This means in particular that the first run of a {@link FreezingArchRule} will always pass.
* Consecutive calls will only fail if "unknown" violations are introduced (read below for further explanations when a violation is "unknown").
* Once resolved, initially "known" violations will fail again if they were re-introduced.
* <br><br>
* You might consider using this class, if you introduce a new {@link ArchRule} to an existing project that causes too many violations to solve
* You might consider using this class when introducing a new {@link ArchRule} to an existing project that causes too many violations to solve
* at the current time. A typical example is a huge legacy project where a new rule might cause thousands of violations. Even if it is impossible
* to fix all those violations at the moment, it is typically a good idea to a) make sure no further violations are introduced and
* b) incrementally fix those violations over time one by one.
* <br><br>
* {@link FreezingArchRule} uses two concepts to support this use case. First a {@link ViolationStore} to store the result of the current
* evaluation of this rule (compare {@link #persistIn(ViolationStore)}). Second a {@link ViolationLineMatcher} to decide which violations are "known",
* i.e. have been present from the beginning (compare {@link #associateViolationLinesVia(ViolationLineMatcher)}).
* The reason to adjust the {@link ViolationLineMatcher} and not simply check for equality might be to make the comparison for known violations
* more resilient, e.g. by ignoring the current line number (assume a class has 500 lines, adding a line at the beginning should maybe not affect an
* unrelated known violation in line 490).
* <br><br>
* If you do not configure {@link ViolationStore} or {@link ViolationLineMatcher}, a default will be used (compare the javadoc of the
* respective class).
* {@link FreezingArchRule} uses two concepts to support this use case:
* <ul>
* <li>
* a {@link ViolationStore} to store the result of the current evaluation and retrieve the result of the previous evaluation of this rule.<br>
* It can be configured via {@link #persistIn(ViolationStore)}.<br>
* The default {@link ViolationStore} stores violations in plain text files
* within the {@code freeze.store.default.path} path of {@value com.tngtech.archunit.ArchConfiguration#ARCHUNIT_PROPERTIES_RESOURCE_NAME}
* (default: {@code archunit_store})
* </li>
* <li>
* a {@link ViolationLineMatcher} to decide which violations are "known", i.e. have already been present in the previous evaluation.<br>
* It can be configured via {@link #associateViolationLinesVia(ViolationLineMatcher)}.<br>
* The default {@link ViolationLineMatcher} compares violations ignoring the line number of their source code location.
* </li>
* </ul>
*/
@PublicAPI(usage = ACCESS)
public final class FreezingArchRule implements ArchRule {
Expand Down Expand Up @@ -149,9 +156,7 @@ public FreezingArchRule persistIn(ViolationStore store) {
}

/**
* Allows to reconfigure how this {@link FreezingArchRule} will decide if an occurring violation is known or not. For example a
* {@link ViolationLineMatcher} that filters out the line number of a violation will consider all violations known that have the same
* textual description regardless of the concrete lines in which those violations have occurred.
* Allows to reconfigure how this {@link FreezingArchRule} will decide if an occurring violation is known or not.
*
* @param matcher A {@link ViolationLineMatcher} that decides which lines of a violation description are known and which are unknown and should
* cause a failure of this rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,15 @@
import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE;

/**
* Allows to decide when two lines of two violations count as "equivalent". I.e. when {@link FreezingArchRule} determines if the description of an
* occurring violation matches the description of an already stored one, it will use the configured {@link ViolationLineMatcher} to do so
* by checking all the description lines of those two violations against each other.
* <br><br>
* A simple example could be to match any (xxx).java from lines and count all violations equivalent if they appear in the same class (as long as the
* violations comply to the default description pattern adding 'in (ClassName.java:y) to the end of the lines). This would then effectively count all
* violations in a class with any previous violation as known and not fail the check.
* Allows {@link FreezingArchRule} to decide when two lines of two violations count as "equivalent".
*/
@PublicAPI(usage = INHERITANCE)
public interface ViolationLineMatcher {

/**
* @param lineFromFirstViolation A line from the description of a violation of an {@link ArchRule}
* @param lineFromSecondViolation A line from the description of another violation of an {@link ArchRule}
* @return true, if and only if those two lines should be considered equivalent (e.g. because only the line number differs)
* @param lineFromFirstViolation A line from the description of a violation of an {@link ArchRule} being evaluated
* @param lineFromSecondViolation A line from the description of a stored violation of an {@link ArchRule}
* @return true, if and only if those two lines should be considered equivalent
*/
boolean matches(String lineFromFirstViolation, String lineFromSecondViolation);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,17 @@
import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE;

/**
* Allows to provide some sort of storage for existing violations. In particular on the first check of a {@link FreezingArchRule}, all existing
* violations will be persisted to the configured {@link ViolationStore}.
* Provides some sort of storage for violations to {@link FreezingArchRule}.
*/
@PublicAPI(usage = INHERITANCE)
public interface ViolationStore {

/**
* Provides custom initialization. The properties will be derived from
* {@value com.tngtech.archunit.ArchConfiguration#ARCHUNIT_PROPERTIES_RESOURCE_NAME} by considering the sub properties of {@code freeze.store}.
* I.e. if {@value com.tngtech.archunit.ArchConfiguration#ARCHUNIT_PROPERTIES_RESOURCE_NAME} contains
* Provides custom initialization with properties derived from
* {@value com.tngtech.archunit.ArchConfiguration#ARCHUNIT_PROPERTIES_RESOURCE_NAME}
* by considering the sub properties of {@code freeze.store}.
* <br><br>
* If {@value com.tngtech.archunit.ArchConfiguration#ARCHUNIT_PROPERTIES_RESOURCE_NAME} contains, e.g.,
*
* <pre><code>
* freeze.store.propOne=valueOne
Expand Down
33 changes: 16 additions & 17 deletions docs/userguide/008_The_Library_API.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,13 @@ https://github.com/TNG/ArchUnit-Examples/blob/master/example-plain/src/test/reso

=== Freezing Arch Rules

Often in grown projects when a new rule is introduced, there are hundreds or even thousands of violations,
When rules are introduced in grown projects, there are often hundreds or even thousands of violations,
way too many to fix immediately. The only way to tackle such extensive violations is to establish an
iterative approach which prevents the code base from further deterioration.
iterative approach, which prevents the code base from further deterioration.

`FreezingArchRule` can help in these scenarios by recording all existing violations to a `ViolationStore`
on the first run. Consecutive runs will then only report new violations and ignore known violations.
Furthermore if violations are fixed, `FreezingArchRule` will automatically reduce the known stored
violations to prevent any regression.
`FreezingArchRule` can help in these scenarios by recording all existing violations to a `ViolationStore`.
Consecutive runs will then only report new violations and ignore known violations.
If violations are fixed, `FreezingArchRule` will automatically reduce the known stored violations to prevent any regression.

==== Usage

Expand All @@ -213,10 +212,9 @@ and will not be reported.

==== Configuration

By default `FreezingArchRule` will use a simple text based `ViolationStore` to store the violations
occurred during the first run and update fixed violations in the future. This is sufficient to add
these files to any version control system to continuously track the progress. You can configure the
location of the violation store within `archunit.properties` (compare <<Advanced Configuration>>):
By default `FreezingArchRule` will use a simple `ViolationStore` based on plain text files.
This is sufficient to add these files to any version control system to continuously track the progress.
You can configure the location of the violation store within `archunit.properties` (compare <<Advanced Configuration>>):

[source,options="nowrap"]
.archunit.properties
Expand All @@ -232,7 +230,7 @@ is the `ViolationLineMatcher`, i.e. how `FreezingArchRule` will associate lines
with lines of actual violations. As mentioned, by default this is a line matcher that ignores the
line numbers of violations within the same class.

===== ViolationStore
===== Violation Store

As mentioned in <<Configuration>>, the default `ViolationStore` is a simple text based store.
It can be exchanged though, for example to store violations in a database.
Expand All @@ -251,7 +249,8 @@ Alternatively it can be configured via `archunit.properties` (compare <<Advanced
freeze.store=fully.qualified.name.of.MyCustomViolationStore
----

You can supply properties to initialize the store by using the namespace `freeze.store`. I.e. for properties
You can supply properties to initialize the store by using the namespace `freeze.store`.
For properties

[source,options="nowrap"]
----
Expand All @@ -261,17 +260,17 @@ freeze.store.propTwo=valueTwo

the method `ViolationStore.initialize(props)` will be called with the properties

[source,java,options="nowrap"]
[source,options="nowrap"]
----
propOne=valueOne
propTwo=valueTwo
----

===== ViolationLineMatcher
===== Violation Line Matcher

The `ViolationLineMatcher` will decide if a line of an occurred violation was actually be stored before.
As mentioned by default it will only ignore line numbers and count lines as equivalent when all other
details match. To exchange the default `ViolationLineMatcher`, you can again either do it programmatically:
The `ViolationLineMatcher` compares lines from occurred violations with lines from the store.
The default implementation ignores line numbers and counts lines as equivalent when all other details match.
A custom `ViolationLineMatcher` can again either be defined programmatically:

[source,java,options="nowrap"]
----
Expand Down

0 comments on commit c8c98c6

Please sign in to comment.