raise error for empty violation store

warn on empty rule violation file delete empty rule violation file Signed-off-by: Masoud Kiaeeha <[email protected]> Resolves #1264
TNG · Jan 19, 2025 · 90c5338 · 90c5338
1 parent 73dfe66
commit 90c5338
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 2 deletions.
diff --git a/archunit/src/main/java/com/tngtech/archunit/library/freeze/StoreEmptyException.java b/archunit/src/main/java/com/tngtech/archunit/library/freeze/StoreEmptyException.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2014-2025 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.library.freeze;
+
+class StoreEmptyException extends RuntimeException {
+    StoreEmptyException(String message) {
+        super(message);
+    }
+
+}
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
@@ -73,11 +73,17 @@ public final class TextFileBasedViolationStore implements ViolationStore {
     private static final String ALLOW_STORE_CREATION_DEFAULT = "false";
     private static final String ALLOW_STORE_UPDATE_PROPERTY_NAME = "default.allowStoreUpdate";
     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 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 warnEmptyRuleViolation;
     private File storeFolder;
     private FileSyncedProperties storedRules;
 
@@ -103,6 +109,8 @@ public TextFileBasedViolationStore(RuleViolationFileNameStrategy ruleViolationFi
     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));
+        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);
@@ -140,15 +148,38 @@ 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 (!storeUpdateAllowed) {
             throw new StoreUpdateFailedException(String.format(
                     "Updating frozen violations is disabled (enable by configuration %s.%s=true)",
                     ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, ALLOW_STORE_UPDATE_PROPERTY_NAME));
         }
+        if (violations.isEmpty() && deleteEmptyRule) {
+            deleteRuleFile(rule);
+            return;
+        }
+
         String ruleFileName = ensureRuleFileName(rule);
         write(violations, new File(storeFolder, ruleFileName));
     }
 
+    private void deleteRuleFile(ArchRule rule) {
+        try {
+            String ruleFileName = storedRules.getProperty(rule.getDescription());
+            Files.delete(storeFolder.toPath().resolve(ruleFileName));
+        } catch (IOException e) {
+            throw new StoreUpdateFailedException(e);
+        }
+        storedRules.removeProperty(rule.getDescription());
+    }
+
     private void write(List<String> violations, File ruleDetails) {
         StringBuilder builder = new StringBuilder();
         for (String violation : violations) {
@@ -255,6 +286,11 @@ void setProperty(String propertyName, String value) {
             syncFileSystem();
         }
 
+        void removeProperty(String propertyName) {
+            loadedProperties.remove(ensureUnixLineBreaks(propertyName));
+            syncFileSystem();
+        }
+
         private void syncFileSystem() {
             try (FileOutputStream outputStream = new FileOutputStream(propertiesFile)) {
                 loadedProperties.store(outputStream, "");

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
@@ -45,6 +45,7 @@
 import static com.tngtech.archunit.testutil.Assertions.assertThatRule;
 import static java.nio.file.Files.delete;
 import static java.nio.file.Files.exists;
+import static java.nio.file.Files.readAllLines;
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -53,8 +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 WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.warnEmptyRuleViolation";
     private static final String LINE_MATCHER_PROPERTY_NAME = "freeze.lineMatcher";
 
     @Rule
@@ -152,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()))
@@ -482,6 +486,72 @@ 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();
+        ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");
+        RuleCreator someRule = rule("some description");
+        freeze(someRule.withViolations("remaining", "will be solved").create()).check(importClasses(getClass()));
+
+        // disallowing empty violation file should throw
+        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 " + WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)");
+    }
+
+    @Test
+    public void can_skip_default_ViolationStore_creation_for_empty_violations() throws IOException {
+        File storeFolder = useTemporaryDefaultStorePath();
+        ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, MyTextFileBasedViolationStore.class.getName());
+        ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");
+        ArchConfiguration.get().setProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true");
+
+        ArchRule rule = rule("some description").withoutViolations().create();
+        freeze(rule).check(importClasses(getClass()));
+
+        assertThat(storeFolder.list()).containsOnly("stored.rules");
+        String storeContents = String.join("\n", readAllLines(storeFolder.toPath().resolve("stored.rules")));
+        assertThat(storeContents).doesNotContain(rule.getDescription());
+    }
+
+    @Test
+    public void can_delete_default_ViolationStore_rule_file_for_empty_violations() throws IOException {
+        // given
+        File storeFolder = useTemporaryDefaultStorePath();
+        ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, MyTextFileBasedViolationStore.class.getName());
+        ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");
+        ArchConfiguration.get().setProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true");
+
+        freeze(rule("some description").withViolations("violation 1").create()).check(importClasses(getClass()));
+        assertThat(storeFolder.list()).containsOnly("stored.rules", "some description test");
+
+        // when
+        ArchRule rule = rule("some description").withoutViolations().create();
+        freeze(rule).check(importClasses(getClass()));
+
+        // then
+        assertThat(storeFolder.list()).containsOnly("stored.rules");
+        String storeContents = String.join("\n", readAllLines(storeFolder.toPath().resolve("stored.rules")));
+        assertThat(storeContents).doesNotContain(rule.getDescription());
+    }
+
     @Test
     public void allows_to_adjust_default_store_file_names_via_delegation() throws IOException {
         // GIVEN