Skip to content

Commit 4952aae

Browse files
committed
raise error for empty violation store and delete empty rule file
These changes introduce two new features for FreezingRule default store * Raise error when a freezing rule has zero violations. This can be enabled by setting the property `default.warnEmptyRuleViolation=true`. For backward compatibility it is disabled by default. * Skip rule violation file creation or delete if it already exists, when there are zero violations. This can be enabled by setting the property `default.deleteEmptyRuleViolation=true`, it is disabled by default. Signed-off-by: Masoud Kiaeeha <[email protected]> Resolves #1264
1 parent d7395ed commit 4952aae

File tree

3 files changed

+139
-2
lines changed

3 files changed

+139
-2
lines changed
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: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.tngtech.archunit.library.freeze;
22

33
import java.io.File;
4+
import java.io.FileInputStream;
45
import java.io.IOException;
56
import java.nio.file.Path;
67
import java.util.ArrayList;
@@ -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+
Properties properties = readProperties(storeFolder.toPath().resolve("stored.rules").toFile());
531+
assertThat(properties).isEmpty();
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+
Properties properties = readProperties(storeFolder.toPath().resolve("stored.rules").toFile());
552+
assertThat(properties).isEmpty();
553+
}
554+
485555
@Test
486556
public void allows_to_adjust_default_store_file_names_via_delegation() throws IOException {
487557
// GIVEN
@@ -538,6 +608,14 @@ private File useTemporaryDefaultStorePath() throws IOException {
538608
return folder;
539609
}
540610

611+
private Properties readProperties(File file) throws IOException {
612+
Properties properties = new Properties();
613+
try (FileInputStream inputStream = new FileInputStream(file)) {
614+
properties.load(inputStream);
615+
}
616+
return properties;
617+
}
618+
541619
private static RuleCreator rule(String description) {
542620
return new RuleCreator(description);
543621
}

0 commit comments

Comments
 (0)