Skip to content

Commit 90c5338

Browse files
maxxkiamaxxkia
committed
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
1 parent 73dfe66 commit 90c5338

File tree

3 files changed

+131
-2
lines changed

3 files changed

+131
-2
lines changed

‎archunit/src/main/java/com/tngtech/archunit/library/freeze/StoreEmptyException.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright 2014-2025 TNG Technology Consulting GmbH
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.tngtech.archunit.library.freeze;
17+
18+
class StoreEmptyException extends RuntimeException {
19+
StoreEmptyException(String message) {
20+
super(message);
21+
}
22+
23+
}

‎archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,17 @@ public final class TextFileBasedViolationStore implements ViolationStore {
7373
private static final String ALLOW_STORE_CREATION_DEFAULT = "false";
7474
private static final String ALLOW_STORE_UPDATE_PROPERTY_NAME = "default.allowStoreUpdate";
7575
private static final String ALLOW_STORE_UPDATE_DEFAULT = "true";
76+
private static final String DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.deleteEmptyRuleViolation";
77+
private static final String DELETE_EMPTY_RULE_VIOLATION_DEFAULT = "false";
78+
private static final String WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.warnEmptyRuleViolation";
79+
private static final String WARN_EMPTY_RULE_VIOLATION_DEFAULT = "false";
7680

7781
private final RuleViolationFileNameStrategy ruleViolationFileNameStrategy;
7882

7983
private boolean storeCreationAllowed;
8084
private boolean storeUpdateAllowed;
85+
private boolean deleteEmptyRule;
86+
private boolean warnEmptyRuleViolation;
8187
private File storeFolder;
8288
private FileSyncedProperties storedRules;
8389

@@ -103,6 +109,8 @@ public TextFileBasedViolationStore(RuleViolationFileNameStrategy ruleViolationFi
103109
public void initialize(Properties properties) {
104110
storeCreationAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, ALLOW_STORE_CREATION_DEFAULT));
105111
storeUpdateAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_UPDATE_PROPERTY_NAME, ALLOW_STORE_UPDATE_DEFAULT));
112+
deleteEmptyRule = Boolean.parseBoolean(properties.getProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, DELETE_EMPTY_RULE_VIOLATION_DEFAULT));
113+
warnEmptyRuleViolation = Boolean.parseBoolean(properties.getProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, WARN_EMPTY_RULE_VIOLATION_DEFAULT));
106114
String path = properties.getProperty(STORE_PATH_PROPERTY_NAME, STORE_PATH_DEFAULT);
107115
storeFolder = new File(path);
108116
ensureExistence(storeFolder);
@@ -140,15 +148,38 @@ public boolean contains(ArchRule rule) {
140148
@Override
141149
public void save(ArchRule rule, List<String> violations) {
142150
log.trace("Storing evaluated rule '{}' with {} violations: {}", rule.getDescription(), violations.size(), violations);
151+
if (violations.isEmpty() && warnEmptyRuleViolation) {
152+
throw new StoreEmptyException(String.format("Saving empty violations for freezing rule is disabled (enable by configuration %s.%s=true)",
153+
ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME));
154+
}
155+
if (violations.isEmpty() && deleteEmptyRule && !contains(rule)) {
156+
// do nothing, new rule file should not be created
157+
return;
158+
}
143159
if (!storeUpdateAllowed) {
144160
throw new StoreUpdateFailedException(String.format(
145161
"Updating frozen violations is disabled (enable by configuration %s.%s=true)",
146162
ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, ALLOW_STORE_UPDATE_PROPERTY_NAME));
147163
}
164+
if (violations.isEmpty() && deleteEmptyRule) {
165+
deleteRuleFile(rule);
166+
return;
167+
}
168+
148169
String ruleFileName = ensureRuleFileName(rule);
149170
write(violations, new File(storeFolder, ruleFileName));
150171
}
151172

173+
private void deleteRuleFile(ArchRule rule) {
174+
try {
175+
String ruleFileName = storedRules.getProperty(rule.getDescription());
176+
Files.delete(storeFolder.toPath().resolve(ruleFileName));
177+
} catch (IOException e) {
178+
throw new StoreUpdateFailedException(e);
179+
}
180+
storedRules.removeProperty(rule.getDescription());
181+
}
182+
152183
private void write(List<String> violations, File ruleDetails) {
153184
StringBuilder builder = new StringBuilder();
154185
for (String violation : violations) {
@@ -255,6 +286,11 @@ void setProperty(String propertyName, String value) {
255286
syncFileSystem();
256287
}
257288

289+
void removeProperty(String propertyName) {
290+
loadedProperties.remove(ensureUnixLineBreaks(propertyName));
291+
syncFileSystem();
292+
}
293+
258294
private void syncFileSystem() {
259295
try (FileOutputStream outputStream = new FileOutputStream(propertiesFile)) {
260296
loadedProperties.store(outputStream, "");

‎archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static com.tngtech.archunit.testutil.Assertions.assertThatRule;
4646
import static java.nio.file.Files.delete;
4747
import static java.nio.file.Files.exists;
48+
import static java.nio.file.Files.readAllLines;
4849
import static java.util.stream.Collectors.toList;
4950
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5051

@@ -53,8 +54,11 @@ public class FreezingArchRuleTest {
5354

5455
private static final String STORE_PROPERTY_NAME = "freeze.store";
5556
private static final String STORE_DEFAULT_PATH_PROPERTY_NAME = "freeze.store.default.path";
57+
private static final String FREEZE_REFREEZE = "freeze.refreeze";
5658
private static final String ALLOW_STORE_CREATION_PROPERTY_NAME = "freeze.store.default.allowStoreCreation";
5759
private static final String ALLOW_STORE_UPDATE_PROPERTY_NAME = "freeze.store.default.allowStoreUpdate";
60+
private static final String DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.deleteEmptyRuleViolation";
61+
private static final String WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.warnEmptyRuleViolation";
5862
private static final String LINE_MATCHER_PROPERTY_NAME = "freeze.lineMatcher";
5963

6064
@Rule
@@ -152,13 +156,13 @@ public void allows_to_overwrite_frozen_violations_if_configured() {
152156
ArchRule anotherViolation = rule("some description").withViolations("first violation", "second violation").create();
153157
ArchRule frozenWithNewViolation = freeze(anotherViolation).persistIn(violationStore);
154158

155-
ArchConfiguration.get().setProperty("freeze.refreeze", Boolean.TRUE.toString());
159+
ArchConfiguration.get().setProperty(FREEZE_REFREEZE, Boolean.TRUE.toString());
156160

157161
assertThatRule(frozenWithNewViolation)
158162
.checking(importClasses(getClass()))
159163
.hasNoViolation();
160164

161-
ArchConfiguration.get().setProperty("freeze.refreeze", Boolean.FALSE.toString());
165+
ArchConfiguration.get().setProperty(FREEZE_REFREEZE, Boolean.FALSE.toString());
162166

163167
assertThatRule(frozenWithNewViolation)
164168
.checking(importClasses(getClass()))
@@ -482,6 +486,72 @@ public void can_prevent_default_ViolationStore_from_updating_existing_rules() th
482486
expectStoreUpdateDisabledException(() -> frozenRule.check(importClasses(getClass())));
483487
}
484488

489+
@Test
490+
public void warns_when_default_ViolationStore_is_empty() throws IOException {
491+
useTemporaryDefaultStorePath();
492+
ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");
493+
ArchConfiguration.get().setProperty(FREEZE_REFREEZE, "true");
494+
FreezingArchRule frozenRule = freeze(rule("some description").withoutViolations().create());
495+
frozenRule.check(importClasses(getClass()));
496+
497+
// disallowing empty violation file should throw
498+
ArchConfiguration.get().setProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true");
499+
assertThatThrownBy(() -> frozenRule.check(importClasses(getClass())))
500+
.isInstanceOf(StoreEmptyException.class)
501+
.hasMessageContaining("Saving empty violations for freezing rule is disabled (enable by configuration " + WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)");
502+
}
503+
504+
@Test
505+
public void warns_when_default_ViolationStore_gets_empty() throws IOException {
506+
useTemporaryDefaultStorePath();
507+
ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");
508+
RuleCreator someRule = rule("some description");
509+
freeze(someRule.withViolations("remaining", "will be solved").create()).check(importClasses(getClass()));
510+
511+
// disallowing empty violation file should throw
512+
ArchConfiguration.get().setProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true");
513+
FreezingArchRule frozenRule = freeze(someRule.withoutViolations().create());
514+
assertThatThrownBy(() -> frozenRule.check(importClasses(getClass())))
515+
.isInstanceOf(StoreEmptyException.class)
516+
.hasMessageContaining("Saving empty violations for freezing rule is disabled (enable by configuration " + WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)");
517+
}
518+
519+
@Test
520+
public void can_skip_default_ViolationStore_creation_for_empty_violations() throws IOException {
521+
File storeFolder = useTemporaryDefaultStorePath();
522+
ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, MyTextFileBasedViolationStore.class.getName());
523+
ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");
524+
ArchConfiguration.get().setProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true");
525+
526+
ArchRule rule = rule("some description").withoutViolations().create();
527+
freeze(rule).check(importClasses(getClass()));
528+
529+
assertThat(storeFolder.list()).containsOnly("stored.rules");
530+
String storeContents = String.join("\n", readAllLines(storeFolder.toPath().resolve("stored.rules")));
531+
assertThat(storeContents).doesNotContain(rule.getDescription());
532+
}
533+
534+
@Test
535+
public void can_delete_default_ViolationStore_rule_file_for_empty_violations() throws IOException {
536+
// given
537+
File storeFolder = useTemporaryDefaultStorePath();
538+
ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, MyTextFileBasedViolationStore.class.getName());
539+
ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true");
540+
ArchConfiguration.get().setProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true");
541+
542+
freeze(rule("some description").withViolations("violation 1").create()).check(importClasses(getClass()));
543+
assertThat(storeFolder.list()).containsOnly("stored.rules", "some description test");
544+
545+
// when
546+
ArchRule rule = rule("some description").withoutViolations().create();
547+
freeze(rule).check(importClasses(getClass()));
548+
549+
// then
550+
assertThat(storeFolder.list()).containsOnly("stored.rules");
551+
String storeContents = String.join("\n", readAllLines(storeFolder.toPath().resolve("stored.rules")));
552+
assertThat(storeContents).doesNotContain(rule.getDescription());
553+
}
554+
485555
@Test
486556
public void allows_to_adjust_default_store_file_names_via_delegation() throws IOException {
487557
// GIVEN

0 commit comments

Comments
 (0)