Skip to content

Commit

Permalink
raise error for empty violation store
Browse files Browse the repository at this point in the history
warn on empty rule violation file

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
  • Loading branch information
maxxkia committed Jan 19, 2025
1 parent 04445a0 commit 5e2af43
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -148,14 +148,14 @@ public boolean contains(ArchRule rule) {
@Override
public void save(ArchRule rule, List<String> 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)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand Down

0 comments on commit 5e2af43

Please sign in to comment.