From 5e2af43e31df907ddded7f009fc3bf66eef46edb Mon Sep 17 00:00:00 2001 From: Masoud Kiaeeha <6916434+maxxkia@users.noreply.github.com> Date: Sun, 19 Jan 2025 12:09:51 +0100 Subject: [PATCH] raise error for empty violation store warn on empty rule violation file Signed-off-by: Masoud Kiaeeha <6916434+maxxkia@users.noreply.github.com> Resolves #1264 --- .../freeze/TextFileBasedViolationStore.java | 16 ++++++------ .../library/freeze/FreezingArchRuleTest.java | 26 +++++++++++++++---- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java b/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java index 09192152b..2caf86fd1 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java @@ -75,15 +75,15 @@ public final class TextFileBasedViolationStore implements ViolationStore { private static final String ALLOW_STORE_UPDATE_DEFAULT = "true"; private static final String DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.deleteEmptyRuleViolation"; private static final String DELETE_EMPTY_RULE_VIOLATION_DEFAULT = "false"; - private static final String ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.allowStoreEmptyRuleViolation"; - private static final String ALLOW_STORE_EMPTY_RULE_VIOLATION_DEFAULT = "true"; + private static final String WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.warnEmptyRuleViolation"; + private static final String WARN_EMPTY_RULE_VIOLATION_DEFAULT = "false"; private final RuleViolationFileNameStrategy ruleViolationFileNameStrategy; private boolean storeCreationAllowed; private boolean storeUpdateAllowed; private boolean deleteEmptyRule; - private boolean storeEmptyRuleViolationAllowed; + private boolean warnEmptyRuleViolation; private File storeFolder; private FileSyncedProperties storedRules; @@ -110,7 +110,7 @@ public void initialize(Properties properties) { storeCreationAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, ALLOW_STORE_CREATION_DEFAULT)); storeUpdateAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_UPDATE_PROPERTY_NAME, ALLOW_STORE_UPDATE_DEFAULT)); deleteEmptyRule = Boolean.parseBoolean(properties.getProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, DELETE_EMPTY_RULE_VIOLATION_DEFAULT)); - storeEmptyRuleViolationAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, ALLOW_STORE_EMPTY_RULE_VIOLATION_DEFAULT)); + warnEmptyRuleViolation = Boolean.parseBoolean(properties.getProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, WARN_EMPTY_RULE_VIOLATION_DEFAULT)); String path = properties.getProperty(STORE_PATH_PROPERTY_NAME, STORE_PATH_DEFAULT); storeFolder = new File(path); ensureExistence(storeFolder); @@ -148,14 +148,14 @@ public boolean contains(ArchRule rule) { @Override public void save(ArchRule rule, List violations) { log.trace("Storing evaluated rule '{}' with {} violations: {}", rule.getDescription(), violations.size(), violations); + if (violations.isEmpty() && warnEmptyRuleViolation) { + throw new StoreEmptyException(String.format("Saving empty violations for freezing rule is disabled (enable by configuration %s.%s=true)", + ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME)); + } if (violations.isEmpty() && deleteEmptyRule && !contains(rule)) { // do nothing, new rule file should not be created return; } - if (violations.isEmpty() && !storeEmptyRuleViolationAllowed) { - throw new StoreEmptyException(String.format("Saving empty violations for freezing rule is disabled (enable by configuration %s.%s=true)", - ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME)); - } if (!storeUpdateAllowed) { throw new StoreUpdateFailedException(String.format( "Updating frozen violations is disabled (enable by configuration %s.%s=true)", diff --git a/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java b/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java index 7bb9e54aa..5a22364b7 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java @@ -54,10 +54,11 @@ 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 FREEZE_REFREEZE = "freeze.refreeze"; 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"; private static final String DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.deleteEmptyRuleViolation"; - private static final String ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.allowStoreEmptyRuleViolation"; + private static final String WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.warnEmptyRuleViolation"; private static final String LINE_MATCHER_PROPERTY_NAME = "freeze.lineMatcher"; @Rule @@ -155,13 +156,13 @@ public void allows_to_overwrite_frozen_violations_if_configured() { ArchRule anotherViolation = rule("some description").withViolations("first violation", "second violation").create(); ArchRule frozenWithNewViolation = freeze(anotherViolation).persistIn(violationStore); - ArchConfiguration.get().setProperty("freeze.refreeze", Boolean.TRUE.toString()); + ArchConfiguration.get().setProperty(FREEZE_REFREEZE, Boolean.TRUE.toString()); assertThatRule(frozenWithNewViolation) .checking(importClasses(getClass())) .hasNoViolation(); - ArchConfiguration.get().setProperty("freeze.refreeze", Boolean.FALSE.toString()); + ArchConfiguration.get().setProperty(FREEZE_REFREEZE, Boolean.FALSE.toString()); assertThatRule(frozenWithNewViolation) .checking(importClasses(getClass())) @@ -485,6 +486,21 @@ public void can_prevent_default_ViolationStore_from_updating_existing_rules() th expectStoreUpdateDisabledException(() -> frozenRule.check(importClasses(getClass()))); } + @Test + public void warns_when_default_ViolationStore_is_empty() throws IOException { + useTemporaryDefaultStorePath(); + ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true"); + ArchConfiguration.get().setProperty(FREEZE_REFREEZE, "true"); + FreezingArchRule frozenRule = freeze(rule("some description").withoutViolations().create()); + frozenRule.check(importClasses(getClass())); + + // disallowing empty violation file should throw + ArchConfiguration.get().setProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true"); + assertThatThrownBy(() -> frozenRule.check(importClasses(getClass()))) + .isInstanceOf(StoreEmptyException.class) + .hasMessageContaining("Saving empty violations for freezing rule is disabled (enable by configuration " + WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)"); + } + @Test public void warns_when_default_ViolationStore_gets_empty() throws IOException { useTemporaryDefaultStorePath(); @@ -493,11 +509,11 @@ public void warns_when_default_ViolationStore_gets_empty() throws IOException { freeze(someRule.withViolations("remaining", "will be solved").create()).check(importClasses(getClass())); // disallowing empty violation file should throw - ArchConfiguration.get().setProperty(ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "false"); + ArchConfiguration.get().setProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true"); FreezingArchRule frozenRule = freeze(someRule.withoutViolations().create()); assertThatThrownBy(() -> frozenRule.check(importClasses(getClass()))) .isInstanceOf(StoreEmptyException.class) - .hasMessageContaining("Saving empty violations for freezing rule is disabled (enable by configuration " + ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)"); + .hasMessageContaining("Saving empty violations for freezing rule is disabled (enable by configuration " + WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)"); } @Test