Skip to content

Commit

Permalink
allow adjusting file names of TextFileBasedViolationStore
Browse files Browse the repository at this point in the history
To make the rule violation files easier to read for humans, we now allow to customize the way the rule violation file names are created, based on the rule description.

Signed-off-by: danhaywood <dan@haywood-associates.co.uk>
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
  • Loading branch information
danhaywood authored and codecholeric committed Feb 6, 2023
1 parent c58158c commit d48e0e1
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.io.Files.toByteArray;
import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE;
import static com.tngtech.archunit.library.freeze.FreezingArchRule.ensureUnixLineBreaks;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
Expand All @@ -44,14 +45,16 @@
* A text file based implementation of a {@link ViolationStore}.<br>
* This {@link ViolationStore} will store the violations of every single {@link FreezingArchRule} in a dedicated file.<br>
* It will keep an index of all stored rules as well as a mapping to the individual rule violation files in the same folder.<br>
* The layout within the configured store folder will look like:
* By default, the layout within the configured store folder will look like:
* <pre><code>
* storeFolder
* |-- stored.rules (the index file of all stored rules)
* |-- 6fc2fd04-b3ab-44e0-8f78-215c66f2174a (a rule violation file named randomly by UUID and referenced from stored.rules)
* |-- 2186b43a-c24c-417d-bd96-547e2dfdba1c (another rule violation file)
* |-- ... (more rule violation files for every rule that has been stored so far)
* </code></pre>
* To adjust the strategy how the individual rule violation files are named use the constructor
* {@link TextFileBasedViolationStore#TextFileBasedViolationStore(RuleViolationFileNameStrategy) TextFileBasedViolationStore(RuleViolationFileNameStrategy)}.<br>
* This {@link ViolationStore} can be configured through the following properties:
* <pre><code>
* default.path=... # string: the path of the folder where violation files will be stored
Expand All @@ -72,11 +75,31 @@ public final class TextFileBasedViolationStore implements ViolationStore {
private static final String ALLOW_STORE_UPDATE_PROPERTY_NAME = "default.allowStoreUpdate";
private static final String ALLOW_STORE_UPDATE_DEFAULT = "true";

private final RuleViolationFileNameStrategy ruleViolationFileNameStrategy;

private boolean storeCreationAllowed;
private boolean storeUpdateAllowed;
private File storeFolder;
private FileSyncedProperties storedRules;

/**
* Creates a standard {@link TextFileBasedViolationStore} that names rule violation files by random {@link UUID}s
*
* @see #TextFileBasedViolationStore(RuleViolationFileNameStrategy)
*/
public TextFileBasedViolationStore() {
this(__ -> UUID.randomUUID().toString());
}

/**
* Creates a {@link TextFileBasedViolationStore} with a custom strategy for rule violation file naming
*
* @param ruleViolationFileNameStrategy controls how the rule violation file name is derived from the rule description
*/
public TextFileBasedViolationStore(RuleViolationFileNameStrategy ruleViolationFileNameStrategy) {
this.ruleViolationFileNameStrategy = ruleViolationFileNameStrategy;
}

@Override
public void initialize(Properties properties) {
storeCreationAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, ALLOW_STORE_CREATION_DEFAULT));
Expand Down Expand Up @@ -149,23 +172,20 @@ private List<String> replaceCharacter(List<String> violations, String characterT
}

private String ensureRuleFileName(ArchRule rule) {
String ruleDescription = rule.getDescription();

String ruleFileName;
if (storedRules.containsKey(rule.getDescription())) {
ruleFileName = storedRules.getProperty(rule.getDescription());
log.trace("Rule '{}' is already stored in file {}", rule.getDescription(), ruleFileName);
if (storedRules.containsKey(ruleDescription)) {
ruleFileName = storedRules.getProperty(ruleDescription);
log.trace("Rule '{}' is already stored in file {}", ruleDescription, ruleFileName);
} else {
ruleFileName = createNewRuleId(rule).toString();
ruleFileName = ruleViolationFileNameStrategy.createRuleFileName(ruleDescription);
log.trace("Assigning new file {} to rule '{}'", ruleFileName, ruleDescription);
storedRules.setProperty(ruleDescription, ruleFileName);
}
return ruleFileName;
}

private UUID createNewRuleId(ArchRule rule) {
UUID ruleId = UUID.randomUUID();
log.trace("Assigning new ID {} to rule '{}'", ruleId, rule.getDescription());
storedRules.setProperty(rule.getDescription(), ruleId.toString());
return ruleId;
}

@Override
public List<String> getViolations(ArchRule rule) {
String ruleDetailsFileName = storedRules.getProperty(rule.getDescription());
Expand Down Expand Up @@ -244,4 +264,23 @@ private void syncFileSystem() {
}
}
}

/**
* Allows to adjust the rule violation file names of {@link TextFileBasedViolationStore}
*
* @see #TextFileBasedViolationStore(RuleViolationFileNameStrategy)
*/
@FunctionalInterface
@PublicAPI(usage = INHERITANCE)
public interface RuleViolationFileNameStrategy {
/**
* Returns the file name to store violations of an {@link ArchRule}, possibly based on the rule description.<br>
* The returned names <b>must</b> be sufficiently unique from any others;
* as long as the descriptions themselves are unique, this can be achieved by sanitizing the description into some sort of file name.
*
* @param ruleDescription The description of the {@link ArchRule} to store
* @return The file name the respective rule violation file will have (see {@link TextFileBasedViolationStore})
*/
String createRuleFileName(String ruleDescription);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,37 @@ public interface ViolationStore {
* @return The lines of violations currently stored for the passed {@link ArchRule}
*/
List<String> getViolations(ArchRule rule);

/**
* A simple delegate for a {@link ViolationStore} to allow adjusting the behavior of another
* {@link ViolationStore} by delegation (e.g. {@link TextFileBasedViolationStore})
*/
@PublicAPI(usage = INHERITANCE)
class Delegate implements ViolationStore {
private final ViolationStore delegate;

public Delegate(ViolationStore delegate) {
this.delegate = delegate;
}

@Override
public void initialize(Properties properties) {
delegate.initialize(properties);
}

@Override
public boolean contains(ArchRule rule) {
return delegate.contains(rule);
}

@Override
public void save(ArchRule rule, List<String> violations) {
delegate.save(rule, violations);
}

@Override
public List<String> getViolations(ArchRule rule) {
return delegate.getViolations(rule);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
@RunWith(DataProviderRunner.class)
public class FreezingArchRuleTest {

private static final String STORE_PROPERTY_NAME = "freeze.store";
private static final String STORE_DEFAULT_PATH_PROPERTY_NAME = "freeze.store.default.path";
private static final String ALLOW_STORE_CREATION_PROPERTY_NAME = "freeze.store.default.allowStoreCreation";
private static final String ALLOW_STORE_UPDATE_PROPERTY_NAME = "freeze.store.default.allowStoreUpdate";
Expand Down Expand Up @@ -311,7 +312,7 @@ public void works_with_multi_line_violations_with_different_line_separators(Stri

@Test
public void allows_to_customize_ViolationStore_by_configuration() {
ArchConfiguration.get().setProperty("freeze.store", TestViolationStore.class.getName());
ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, TestViolationStore.class.getName());
ArchConfiguration.get().setProperty("freeze.store.first.property", "first value");
ArchConfiguration.get().setProperty("freeze.store.second.property", "second value");
ArchConfiguration.get().setProperty("freeze.unrelated", "unrelated value");
Expand Down Expand Up @@ -481,6 +482,21 @@ public void can_prevent_default_ViolationStore_from_updating_existing_rules() th
expectStoreUpdateDisabledException(() -> frozenRule.check(importClasses(getClass())));
}

@Test
public void allows_to_adjust_default_store_file_names_via_delegation() throws IOException {
// GIVEN
File storeFolder = useTemporaryDefaultStorePath();
ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, MyTextFileBasedViolationStore.class.getName());
ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");

// WHEN
ArchRule rule = rule("some rule").withViolations("irrelevant").create();
freeze(rule).check(importClasses(getClass()));

// THEN
assertThat(storeFolder.list()).contains("some rule" + MyTextFileBasedViolationStore.FILE_NAME_SUFFIX);
}

@Test
public void violations_ignored_by_archunit_ignore_patterns_are_omitted_from_the_store() throws IOException {
Path ignoreFile = null;
Expand Down Expand Up @@ -516,9 +532,10 @@ private void createFrozen(TestViolationStore violationStore, ArchRule rule) {
.hasNoViolation();
}

private void useTemporaryDefaultStorePath() throws IOException {
private File useTemporaryDefaultStorePath() throws IOException {
File folder = temporaryFolder.newFolder();
ArchConfiguration.get().setProperty(STORE_DEFAULT_PATH_PROPERTY_NAME, folder.getAbsolutePath());
return folder;
}

private static RuleCreator rule(String description) {
Expand Down Expand Up @@ -629,6 +646,14 @@ private StoredRule(List<String> violations) {
}
}

static class MyTextFileBasedViolationStore extends ViolationStore.Delegate {
static final String FILE_NAME_SUFFIX = " test";

MyTextFileBasedViolationStore() {
super(new TextFileBasedViolationStore(description -> description + FILE_NAME_SUFFIX));
}
}

private static class ConsiderAllLinesWithTheSameStartLetterTheSame implements ViolationLineMatcher {
@Override
public boolean matches(String lineFromFirstViolation, String lineFromSecondViolation) {
Expand Down

0 comments on commit d48e0e1

Please sign in to comment.