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 05eba8224..aa6142c62 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 @@ -20,11 +20,16 @@ import java.io.FileOutputStream; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; import java.util.List; import java.util.Properties; +import java.util.Set; import java.util.UUID; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import com.google.common.base.Predicates; import com.google.common.base.Splitter; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.lang.ArchRule; @@ -110,6 +115,8 @@ public void initialize(Properties properties) { log.trace("Initializing {} at {}", TextFileBasedViolationStore.class.getSimpleName(), storedRulesFile.getAbsolutePath()); storedRules = new FileSyncedProperties(storedRulesFile); checkInitialization(storedRules.initializationSuccessful(), "Cannot create rule store at %s", storedRulesFile.getAbsolutePath()); + removeObsoleteRules(); + removeObsoleteRuleFiles(); } private File getStoredRulesFile() { @@ -132,6 +139,45 @@ private void checkInitialization(boolean initializationSuccessful, String messag } } + private void removeObsoleteRules() { + Set obsoleteStoredRules = storedRules.keySet().stream() + .filter(ruleDescription -> !new File(storeFolder, storedRules.getProperty(ruleDescription)).exists()) + .collect(Collectors.toSet()); + if (!obsoleteStoredRules.isEmpty() && !storeUpdateAllowed) { + throw new StoreUpdateFailedException(String.format( + "Failed to remove %d obsolete stored rule(s). Updating frozen violations is disabled (enable by configuration %s.%s=true)", + obsoleteStoredRules.size(), ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, ALLOW_STORE_UPDATE_PROPERTY_NAME)); + } + obsoleteStoredRules.forEach(storedRules::removeProperty); + } + + private void removeObsoleteRuleFiles() { + Set ruleFiles = storedRules.keySet().stream() + .map(storedRules::getProperty) + .collect(Collectors.toSet()); + + List danglingFiles = Arrays.stream(storeFolder.list()) + .filter(name -> !name.equals(STORED_RULES_FILE_NAME)) + .filter(Predicates.not(ruleFiles::contains)) + .collect(toList()); + + if (!danglingFiles.isEmpty() && !storeUpdateAllowed) { + throw new StoreUpdateFailedException(String.format( + "Failed to remove %d unreferenced rule files. Updating frozen store is disabled (enable by configuration %s.%s=true)", + danglingFiles.size(), ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, ALLOW_STORE_UPDATE_PROPERTY_NAME)); + + } + + for (String fileName : danglingFiles) { + Path path = new File(storeFolder, fileName).toPath(); + try { + Files.delete(path); + } catch (IOException e) { + throw new StoreInitializationFailedException("Cannot delete unreferenced rule file: " + fileName, e); + } + } + } + @Override public boolean contains(ArchRule rule) { return storedRules.containsKey(rule.getDescription()); @@ -255,6 +301,15 @@ void setProperty(String propertyName, String value) { syncFileSystem(); } + void removeProperty(String propertyName) { + loadedProperties.remove(ensureUnixLineBreaks(propertyName)); + syncFileSystem(); + } + + Set keySet() { + return loadedProperties.stringPropertyNames(); + } + private void syncFileSystem() { try (FileOutputStream outputStream = new FileOutputStream(propertiesFile)) { loadedProperties.store(outputStream, ""); diff --git a/archunit/src/test/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStoreTest.java b/archunit/src/test/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStoreTest.java index 9916799d5..44b9da9f1 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStoreTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStoreTest.java @@ -2,6 +2,7 @@ import java.io.File; import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.IOException; import java.util.LinkedList; import java.util.List; @@ -10,6 +11,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.io.Files; import com.tngtech.archunit.lang.ArchRule; +import org.assertj.core.api.ThrowableAssert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -38,6 +40,79 @@ public void setUp() throws Exception { "default.allowStoreCreation", String.valueOf(true))); } + @Test + public void throws_exception_when_there_are_obsolete_entries_in_storedRules_files() throws Exception { + // given + store.save(defaultRule(), ImmutableList.of("first violation", "second violation")); + Properties properties = readProperties(new File(configuredFolder, "stored.rules")); + File ruleViolationsFile = new File(configuredFolder, properties.getProperty(defaultRule().getDescription())); + assertThat(ruleViolationsFile.delete()).isTrue(); + + // when && then + ThrowableAssert.ThrowingCallable storeInitialization = () -> store.initialize(propertiesOf( + "default.path", configuredFolder.getAbsolutePath(), + "default.allowStoreUpdate", String.valueOf(false))); + assertThatThrownBy(storeInitialization) + .isInstanceOf(StoreUpdateFailedException.class) + .hasMessage("Failed to remove 1 obsolete stored rule(s). Updating frozen violations is disabled (enable by configuration freeze.store.default.allowStoreUpdate=true)"); + assertThat(store.contains(defaultRule())).isTrue(); + } + + @Test + public void deletes_obsolete_entries_from_storedRules_files() throws Exception { + // given + store.save(defaultRule(), ImmutableList.of("first violation", "second violation")); + Properties properties = readProperties(new File(configuredFolder, "stored.rules")); + File ruleViolationsFile = new File(configuredFolder, properties.getProperty(defaultRule().getDescription())); + assertThat(ruleViolationsFile.delete()).isTrue(); + + // when + store.initialize(propertiesOf("default.path", configuredFolder.getAbsolutePath())); + + // then + assertThat(store.contains(defaultRule())).isFalse(); + } + + @Test + public void throws_exception_when_there_are_unreferenced_in_store_directory() throws Exception { + // given + store.save(defaultRule(), ImmutableList.of("first violation", "second violation")); + File propertiesFile = new File(configuredFolder, "stored.rules"); + Properties properties = readProperties(propertiesFile); + File ruleViolationsFile = new File(configuredFolder, properties.getProperty(defaultRule().getDescription())); + assertThat(ruleViolationsFile).exists(); + properties.remove(defaultRule().getDescription()); + storeProperties(propertiesFile, properties); + + // when && then + ThrowableAssert.ThrowingCallable storeInitialization = () -> store.initialize(propertiesOf( + "default.path", configuredFolder.getAbsolutePath(), + "default.allowStoreUpdate", String.valueOf(false))); + assertThatThrownBy(storeInitialization) + .isInstanceOf(StoreUpdateFailedException.class) + .hasMessage("Failed to remove 1 unreferenced rule files. Updating frozen store is disabled (enable by configuration freeze.store.default.allowStoreUpdate=true)"); + assertThat(ruleViolationsFile).exists(); + } + + @Test + public void deletes_files_not_referenced_in_storedRules() throws Exception { + // given + store.save(defaultRule(), ImmutableList.of("first violation", "second violation")); + File propertiesFile = new File(configuredFolder, "stored.rules"); + Properties properties = readProperties(propertiesFile); + File ruleViolationsFile = new File(configuredFolder, properties.getProperty(defaultRule().getDescription())); + assertThat(ruleViolationsFile).exists(); + properties.remove(defaultRule().getDescription()); + storeProperties(propertiesFile, properties); + + // when + store.initialize(propertiesOf("default.path", configuredFolder.getAbsolutePath())); + + // then + assertThat(store.contains(defaultRule())).isFalse(); + assertThat(ruleViolationsFile).doesNotExist(); + } + @Test public void reports_unknown_rule_as_unstored() { assertThat(store.contains(defaultRule())).as("store contains random rule").isFalse(); @@ -126,6 +201,12 @@ private Properties readProperties(File file) throws IOException { return properties; } + private static void storeProperties(File propertiesFile, Properties properties) throws IOException { + try (FileOutputStream outputStream = new FileOutputStream(propertiesFile)) { + properties.store(outputStream, ""); + } + } + private Properties propertiesOf(String... keyValuePairs) { Properties result = new Properties(); LinkedList keyValues = new LinkedList<>(asList(keyValuePairs));