Skip to content

Commit ecedf1e

Browse files
authored
Add architecture test for logging infrastructure (#13534)
* Add logging framework architecture test * Fixed arch test * Added todo test * Update CommonArchitectureTest.java * Update CommonArchitectureTest.java * Added tinylog --------- Co-authored-by: Carl Christian Snethlage <calixtus@users.noreply.github.com>
1 parent 34af053 commit ecedf1e

File tree

4 files changed

+40
-12
lines changed

4 files changed

+40
-12
lines changed

jabgui/src/main/java/org/jabref/gui/fieldeditors/KeywordsEditorViewModel.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@
1818
import org.jabref.model.entry.KeywordList;
1919
import org.jabref.model.entry.field.Field;
2020

21-
import org.tinylog.Logger;
21+
import org.slf4j.Logger;
22+
import org.slf4j.LoggerFactory;
2223

2324
public class KeywordsEditorViewModel extends AbstractEditorViewModel {
2425

26+
private static final Logger LOGGER = LoggerFactory.getLogger(KeywordsEditorViewModel.class);
27+
2528
private final ListProperty<Keyword> keywordListProperty;
2629
private final Character keywordSeparator;
2730
private final SuggestionProvider<?> suggestionProvider;
@@ -62,7 +65,7 @@ public StringConverter<Keyword> getStringConverter() {
6265
@Override
6366
public String toString(Keyword keyword) {
6467
if (keyword == null) {
65-
Logger.debug("Keyword is null");
68+
LOGGER.debug("Keyword is null");
6669
return "";
6770
}
6871
return keyword.get();

jabgui/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@
2323
import org.jabref.logic.importer.plaincitation.PlainCitationParserChoice;
2424
import org.jabref.logic.l10n.Localization;
2525
import org.jabref.logic.preferences.FetcherApiKey;
26+
import org.jabref.model.strings.StringUtil;
2627

2728
import com.airhacks.afterburner.views.ViewLoader;
2829
import com.tobiasdiez.easybind.EasyBind;
29-
import org.apache.logging.log4j.util.Strings;
3030

3131
public class WebSearchTab extends AbstractPreferenceTabView<WebSearchTabViewModel> implements PreferencesTab {
3232

@@ -99,7 +99,7 @@ public void initialize() {
9999
citationsRelationStoreTTL
100100
.textProperty()
101101
.addListener((_, _, newValue) -> {
102-
if (Strings.isBlank(newValue)) {
102+
if (StringUtil.isBlank(newValue)) {
103103
return;
104104
}
105105
if (!newValue.matches("\\d*")) {

jablib/src/main/java/org/jabref/logic/os/OS.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
import java.nio.file.Path;
88
import java.util.Locale;
99
import java.util.Objects;
10-
import java.util.logging.Level;
11-
import java.util.logging.Logger;
1210

1311
import org.jabref.model.strings.StringUtil;
1412

@@ -17,6 +15,7 @@
1715
import com.github.javakeyring.PasswordAccessException;
1816
import mslinks.ShellLink;
1917
import mslinks.ShellLinkException;
18+
import org.slf4j.Logger;
2019
import org.slf4j.LoggerFactory;
2120

2221
/**
@@ -87,11 +86,10 @@ public static String detectProgramPath(String programName, String directoryName)
8786
try {
8887
ShellLink link = new ShellLink(texworksLinkPath);
8988
return link.resolveTarget();
90-
} catch (IOException
91-
| ShellLinkException e) {
89+
} catch (IOException | ShellLinkException e) {
9290
// Static logger instance cannot be used. See the class comment.
93-
Logger logger = Logger.getLogger(OS.class.getName());
94-
logger.log(Level.WARNING, "Had an error while reading .lnk file for TeXworks", e);
91+
Logger logger = LoggerFactory.getLogger(OS.class);
92+
logger.warn("Error while reading .lnk file for TeXworks", e);
9593
}
9694
}
9795
}

test-support/src/main/java/org/jabref/support/CommonArchitectureTest.java

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.net.URI;
44
import java.nio.file.Paths;
5+
import java.util.List;
56

67
import org.jabref.architecture.AllowedToUseApacheCommonsLang3;
78
import org.jabref.architecture.AllowedToUseAwt;
@@ -89,8 +90,6 @@ public void useStreamsOfResources(JavaClasses classes) {
8990
.check(classes);
9091
}
9192

92-
// TODO: no org.jabref.gui package may reside in org.jabref.logic
93-
9493
@ArchTest
9594
public void doNotUseLogicInModel(JavaClasses classes) {
9695
ArchRuleDefinition.noClasses().that().resideInAPackage(PACKAGE_ORG_JABREF_MODEL)
@@ -99,6 +98,13 @@ public void doNotUseLogicInModel(JavaClasses classes) {
9998
.check(classes);
10099
}
101100

101+
@ArchTest
102+
public void doNotUseGuiInLogic(JavaClasses classes) {
103+
ArchRuleDefinition.noClasses().that().resideInAPackage(PACKAGE_ORG_JABREF_LOGIC)
104+
.should().dependOnClassesThat().resideInAPackage(PACKAGE_ORG_JABREF_GUI)
105+
.check(classes);
106+
}
107+
102108
@ArchTest
103109
public void restrictUsagesInModel(JavaClasses classes) {
104110
// Until we switch to Lucene, we need to access Globals.stateManager().getActiveDatabase() from the search classes,
@@ -120,6 +126,27 @@ public void restrictUsagesInLogic(JavaClasses classes) {
120126
.check(classes);
121127
}
122128

129+
@ArchTest
130+
public void restrictToSlf4jLogger(JavaClasses classes) {
131+
List<String> restrictedLoggingPackages = List.of(
132+
"java.util.logging..", // Java's built-in logging
133+
"org.apache.log4j..", // Log4j 1.x
134+
"org.apache.logging.log4j..", // Log4j 2.x
135+
"org.slf4j.impl..", // Concrete SLF4J bindings
136+
"ch.qos.logback..", // Logback
137+
"org.apache.commons.logging..", // Apache Commons logging
138+
"org.tinylog..", // Tinylog
139+
"org.mariadb.jdbc.internal.logging.." // Mariadb logging
140+
);
141+
142+
ArchRuleDefinition.noClasses()
143+
.that().resideOutsideOfPackages("org.jabref.gui.errorconsole", "org.jabref.gui.logging", "org.jabref")
144+
.should().accessClassesThat()
145+
.resideInAnyPackage(restrictedLoggingPackages.toArray(String[]::new))
146+
.because("slf4j should be used for logging")
147+
.check(classes);
148+
}
149+
123150
@ArchTest
124151
public void restrictStandardStreams(JavaClasses classes) {
125152
ArchRuleDefinition.noClasses().that().resideOutsideOfPackages(PACKAGE_ORG_JABREF_CLI)

0 commit comments

Comments
 (0)