Skip to content

Commit

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

Signed-off-by: Masoud Kiaeeha <6916434+maxxkia@users.noreply.github.com>

Resolves TNG#1264
  • Loading branch information
@maxxkia
maxxkia committed Jan 19, 2025
1 parent 3401094 commit 04445a0
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 2 deletions.
28 changes: 28 additions & 0 deletions archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,16 @@ 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 ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.allowStoreEmptyRuleViolation";
private static final String ALLOW_STORE_EMPTY_RULE_VIOLATION_DEFAULT = "true";

private final RuleViolationFileNameStrategy ruleViolationFileNameStrategy;

private boolean storeCreationAllowed;
private boolean storeUpdateAllowed;
private boolean deleteEmptyRule;
private boolean storeEmptyRuleViolationAllowed;
private File storeFolder;
private FileSyncedProperties storedRules;
Expand All @@ -106,6 +109,7 @@ 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));
storeEmptyRuleViolationAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, ALLOW_STORE_EMPTY_RULE_VIOLATION_DEFAULT));
String path = properties.getProperty(STORE_PATH_PROPERTY_NAME, STORE_PATH_DEFAULT);
storeFolder = new File(path);
Expand Down Expand Up @@ -144,6 +148,10 @@ 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() && 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));
Expand All @@ -153,10 +161,25 @@ public void save(ArchRule rule, List<String> violations) {
"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) {
Expand Down Expand Up @@ -263,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, "");
Expand Down
49 changes: 47 additions & 2 deletions archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;

Expand Down Expand Up @@ -38,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;

Expand All @@ -48,6 +56,7 @@ public class FreezingArchRuleTest {
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";
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 LINE_MATCHER_PROPERTY_NAME = "freeze.lineMatcher";

Expand Down Expand Up @@ -480,17 +489,53 @@ public void can_prevent_default_ViolationStore_from_updating_existing_rules() th
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(ALLOW_STORE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "false");
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)");
}

@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
Expand Down

0 comments on commit 04445a0

Please sign in to comment.