Bring back MainArchitectureTest (#13462)

calixtus · koppor · web-flow · commit e29044c0a778 · 2025-07-04T17:14:57.000Z
* Bring back MainArchitectureTest

* Bring back MainArchitectureTest

Co-authored-by: Oliver Kopp &lt;kopp.dev@gmail.com&gt;

* JabLibArchitectureTests by inheritance

Co-authored-by: Oliver Kopp &lt;kopp.dev@gmail.com&gt;

* JabGuiArchitectureTests by inheritance
Fix architecture tests

Co-authored-by: Oliver Kopp &lt;kopp.dev@gmail.com&gt;

* Remove architecture tests, add todo

Co-authored-by: Oliver Kopp &lt;kopp.dev@gmail.com&gt;

* Fix issues by httpserver

Co-authored-by: Oliver Kopp &lt;kopp.dev@gmail.com&gt;

* Fix typo

Co-authored-by: Oliver Kopp &lt;kopp.dev@gmail.com&gt;

---------

Co-authored-by: Carl Christian Snethlage &lt;calixtus@users.noreply.github.com&gt;
Co-authored-by: Oliver Kopp &lt;kopp.dev@gmail.com&gt;
diff --git a/build-logic/src/main/kotlin/org.jabref.gradle.base.dependency-rules.gradle.kts b/build-logic/src/main/kotlin/org.jabref.gradle.base.dependency-rules.gradle.kts
@@ -276,13 +276,23 @@ extraJavaModuleInfo {
     module("com.github.javaparser:javaparser-symbol-solver-core", "com.github.javaparser.symbolsolver.core")
     module("net.sf.jopt-simple:jopt-simple", "net.sf.jopt.simple")
 
-    module("com.tngtech.archunit:archunit-junit5-api", "com.tngtech.archunit.junit5.api")
-    module("com.tngtech.archunit:archunit-junit5-engine", "com.tngtech.archunit.junit5.engine")
-    module("com.tngtech.archunit:archunit-junit5-engine-api", "com.tngtech.archunit.junit5.engineapi")
+    module("com.tngtech.archunit:archunit-junit5-api", "com.tngtech.archunit.junit5.api") {
+        exportAllPackages()
+        requireAllDefinedDependencies()
+    }
+    module("com.tngtech.archunit:archunit-junit5-engine", "com.tngtech.archunit.junit5.engine") {
+        exportAllPackages()
+        requireAllDefinedDependencies()
+    }
+    module("com.tngtech.archunit:archunit-junit5-engine-api", "com.tngtech.archunit.junit5.engineapi") {
+        exportAllPackages()
+        requireAllDefinedDependencies()
+    }
     module("com.tngtech.archunit:archunit", "com.tngtech.archunit") {
         exportAllPackages()
+        requireAllDefinedDependencies()
         requires("java.logging")
-        requires("org.slf4j")
+        uses("com.tngtech.archunit.lang.extension.ArchUnitExtension")
     }
 
     module("org.glassfish.hk2.external:aopalliance-repackaged", "org.aopalliance")
diff --git a/docs/code-howtos/faq.md b/docs/code-howtos/faq.md
@@ -61,13 +61,13 @@ Check the directory `jablib/src/main/resources/csl-locales`.
 If it is missing or empty, run `git submodule update`.
 If still not fixed, run `git reset --hard` **inside that directory**.
 
-### `org.jabref.architecture.MainArchitectureTest restrictStandardStreams` <span style="color:red">FAILED</span>
+### `org.jabref.support.CommonArchitectureTest restrictStandardStreams` <span style="color:red">FAILED</span>
 
 Check if you've used `System.out.println(...)` (the standard output stream) to log anything into the console.
 This is an architectural violation, as you should use the Logger instead for logging.
 More details on [how to log](https://devdocs.jabref.org/code-howtos/logging.html).
 
-### `org.jabref.architecture.MainArchitectureTest doNotUseLogicInModel` <span style="color:red">FAILED</span>
+### `org.jabref.support.CommonArchitectureTest doNotUseLogicInModel` <span style="color:red">FAILED</span>
 
 One common case when this test fails is when you put any class purely containing business logic inside the `model` package (i.e., inside the directory `org/jabref/model/`).
 To fix this, shift the class to a sub-package within the `logic` package (i.e., the directory`org/jabref/logic/`).
diff --git a/jabgui/build.gradle.kts b/jabgui/build.gradle.kts
@@ -121,6 +121,10 @@ dependencies {
 
     testImplementation("com.github.javaparser:javaparser-symbol-solver-core")
     testImplementation("org.ow2.asm:asm")
+
+    testImplementation("com.tngtech.archunit:archunit")
+    testImplementation("com.tngtech.archunit:archunit-junit5-api")
+    testRuntimeOnly("com.tngtech.archunit:archunit-junit5-engine")
 }
 
 application {
@@ -218,6 +222,9 @@ javaModuleTesting.whitebox(testing.suites["test"]) {
 
     requires.add("wiremock")
     requires.add("wiremock.slf4j.spi.shim")
+
+    requires.add("com.tngtech.archunit")
+    requires.add("com.tngtech.archunit.junit5.api")
 }
 
 tasks.test {
diff --git a/jabgui/src/main/java/org/jabref/gui/ClipBoardManager.java b/jabgui/src/main/java/org/jabref/gui/ClipBoardManager.java
@@ -28,7 +28,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-@AllowedToUseAwt("Requires ava.awt.datatransfer.Clipboard")
+@AllowedToUseAwt("Requires java.awt.datatransfer.Clipboard")
 public class ClipBoardManager {
 
     public static final DataFormat XML = new DataFormat("application/xml");
diff --git a/jabgui/src/test/java/org/jabref/gui/JabGuiArchitectureTests.java b/jabgui/src/test/java/org/jabref/gui/JabGuiArchitectureTests.java
@@ -0,0 +1,10 @@
+package org.jabref.gui;
+
+import org.jabref.support.CommonArchitectureTest;
+
+import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.junit.AnalyzeClasses;
+
+@AnalyzeClasses(packages = "org.jabref", importOptions = ImportOption.DoNotIncludeTests.class)
+public class JabGuiArchitectureTests extends CommonArchitectureTest {
+}
diff --git a/jablib/build.gradle.kts b/jablib/build.gradle.kts
@@ -207,8 +207,10 @@ dependencies {
     testImplementation("org.xmlunit:xmlunit-core")
     testImplementation("org.xmlunit:xmlunit-matchers")
     testImplementation("org.junit.jupiter:junit-jupiter-api")
-    testRuntimeOnly("com.tngtech.archunit:archunit-junit5-engine")
+
+    testImplementation("com.tngtech.archunit:archunit")
     testImplementation("com.tngtech.archunit:archunit-junit5-api")
+    testRuntimeOnly("com.tngtech.archunit:archunit-junit5-engine")
 
     testImplementation("org.hamcrest:hamcrest")
 
@@ -563,4 +565,7 @@ javaModuleTesting.whitebox(testing.suites["test"]) {
     requires.add("org.xmlunit.matchers")
     requires.add("wiremock")
     requires.add("wiremock.slf4j.spi.shim")
+
+    requires.add("com.tngtech.archunit")
+    requires.add("com.tngtech.archunit.junit5.api")
 }
diff --git a/jablib/src/main/java/org/jabref/logic/importer/fetcher/EuropePmcFetcher.java b/jablib/src/main/java/org/jabref/logic/importer/fetcher/EuropePmcFetcher.java
@@ -26,12 +26,15 @@
 import kong.unirest.core.json.JSONArray;
 import kong.unirest.core.json.JSONException;
 import kong.unirest.core.json.JSONObject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EuropePmcFetcher implements IdBasedParserFetcher {
+    private static final Logger LOGGER = LoggerFactory.getLogger(EuropePmcFetcher.class);
 
     @Override
     public URL getUrlForIdentifier(String identifier) throws URISyntaxException, MalformedURLException {
-        return URI.create("https://www.ebi.ac.uk/europepmc/webservices/rest/search?query=" + identifier + "&resultType=core&format=json").toURL();
+        return new URI("https://www.ebi.ac.uk/europepmc/webservices/rest/search?query=" + identifier + "&resultType=core&format=json").toURL();
     }
 
     @Override
@@ -49,7 +52,7 @@ private BibEntry jsonItemToBibEntry(JSONObject item) throws ParseException {
         try {
             JSONObject result = item.getJSONObject("resultList").getJSONArray("result").getJSONObject(0);
 
-            System.out.println(result.toString(2));
+            LOGGER.debug(result.toString(2));
 
             EntryType entryType = StandardEntryType.Article;
             if (result.has("pubTypeList")) {
diff --git a/jablib/src/main/java/org/jabref/logic/remote/RemotePreferences.java b/jablib/src/main/java/org/jabref/logic/remote/RemotePreferences.java
@@ -2,13 +2,15 @@
 
 import java.net.InetAddress;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.UnknownHostException;
 
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.IntegerProperty;
 import javafx.beans.property.SimpleBooleanProperty;
 import javafx.beans.property.SimpleIntegerProperty;
 
+import org.jspecify.annotations.NonNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -75,12 +77,17 @@ public static InetAddress getIpAddress() throws UnknownHostException {
         return InetAddress.getByName("localhost");
     }
 
-    public URI getHttpServerUri() {
+    public @NonNull URI getHttpServerUri() {
         try {
-            return URI.create("http://" + RemotePreferences.getIpAddress().getHostAddress() + ":23119");
-        } catch (UnknownHostException e) {
+            return new URI("http://" + RemotePreferences.getIpAddress().getHostAddress() + ":23119");
+        } catch (UnknownHostException | URISyntaxException e) {
             LOGGER.error("Could not create HTTP server URI. Falling back to default.", e);
-            return URI.create("http://localhost:23119");
+            try {
+                return new URI("http://localhost:23119");
+            } catch (URISyntaxException ex) {
+                LOGGER.error("Should never happen, raw string is already valid uri");
+                throw new RuntimeException(ex);
+            }
         }
     }
 }
diff --git a/jablib/src/main/java/org/jabref/model/strings/StringUtil.java b/jablib/src/main/java/org/jabref/model/strings/StringUtil.java
@@ -15,6 +15,7 @@
 import javafx.util.Pair;
 
 import org.jabref.architecture.AllowedToUseApacheCommonsLang3;
+import org.jabref.architecture.AllowedToUseLogic;
 import org.jabref.logic.bibtex.FieldWriter;
 import org.jabref.logic.os.OS;
 
@@ -23,6 +24,7 @@
 
 @SuppressWarnings("checkstyle:NoMultipleClosingBracesAtEndOfLine")
 @AllowedToUseApacheCommonsLang3("There is no equivalent in Google's Guava")
+@AllowedToUseLogic("OS.NewLine is most basic")
 public class StringUtil {
 
     // Non-letters which are used to denote accents in LaTeX-commands, e.g., in {\"{a}}
diff --git a/jablib/src/test/java/org/jabref/logic/JabLibArchitectureTests.java b/jablib/src/test/java/org/jabref/logic/JabLibArchitectureTests.java
@@ -0,0 +1,10 @@
+package org.jabref.logic;
+
+import org.jabref.support.CommonArchitectureTest;
+
+import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.junit.AnalyzeClasses;
+
+@AnalyzeClasses(packages = "org.jabref", importOptions = ImportOption.DoNotIncludeTests.class)
+public class JabLibArchitectureTests extends CommonArchitectureTest {
+}
diff --git a/jabsrv/src/main/java/org/jabref/http/server/Server.java b/jabsrv/src/main/java/org/jabref/http/server/Server.java
@@ -62,9 +62,9 @@ public HttpServer run(List<Path> files, URI uri) {
         // GUI uses HttpServerManager
         Runtime.getRuntime().addShutdownHook(new Thread(() -> {
             try {
-                System.out.println("Shutting down jabsrv...");
+                LOGGER.debug("Shutting down jabsrv...");
                 httpServer.shutdownNow();
-                System.out.println("Done, exit.");
+                LOGGER.debug("Done, exit.");
             } catch (Exception e) {
                 LOGGER.error("Could not shut down server", e);
             }
diff --git a/jabsrv/src/main/java/org/jabref/http/server/cayw/CAYWResource.java b/jabsrv/src/main/java/org/jabref/http/server/cayw/CAYWResource.java
@@ -17,6 +17,7 @@
 
 import javafx.application.Platform;
 
+import org.jabref.architecture.AllowedToUseAwt;
 import org.jabref.http.server.cayw.format.FormatterService;
 import org.jabref.http.server.cayw.gui.CAYWEntry;
 import org.jabref.http.server.cayw.gui.SearchDialog;
@@ -40,6 +41,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+@AllowedToUseAwt("Requires java.awt.datatransfer.Clipboard")
 @Path("better-bibtex/cayw")
 public class CAYWResource {
     private static final Logger LOGGER = LoggerFactory.getLogger(CAYWResource.class);
diff --git a/test-support/build.gradle.kts b/test-support/build.gradle.kts
@@ -25,4 +25,7 @@ dependencies {
     implementation("net.bytebuddy:byte-buddy")
 
     implementation("org.jspecify:jspecify")
+
+    implementation("com.tngtech.archunit:archunit")
+    implementation("com.tngtech.archunit:archunit-junit5-api")
 }
diff --git a/test-support/src/main/java/module-info.java b/test-support/src/main/java/module-info.java
@@ -2,6 +2,9 @@
     requires org.junit.jupiter.api;
     requires org.mockito;
     requires org.jabref.jablib;
+    requires com.tngtech.archunit;
+    requires com.tngtech.archunit.junit5.api;
+    requires javafx.graphics;
 
     exports org.jabref.support;
 }
diff --git a/test-support/src/main/java/org/jabref/support/CommonArchitectureTest.java b/test-support/src/main/java/org/jabref/support/CommonArchitectureTest.java
@@ -0,0 +1,142 @@
+package org.jabref.support;
+
+import java.net.URI;
+import java.nio.file.Paths;
+
+import org.jabref.architecture.AllowedToUseApacheCommonsLang3;
+import org.jabref.architecture.AllowedToUseAwt;
+import org.jabref.architecture.AllowedToUseClassGetResource;
+import org.jabref.architecture.AllowedToUseLogic;
+import org.jabref.architecture.AllowedToUseStandardStreams;
+import org.jabref.architecture.AllowedToUseSwing;
+
+import com.tngtech.archunit.core.domain.JavaClasses;
+import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.junit.AnalyzeClasses;
+import com.tngtech.archunit.junit.ArchTest;
+import com.tngtech.archunit.lang.syntax.ArchRuleDefinition;
+import com.tngtech.archunit.library.GeneralCodingRules;
+
+/**
+ * This class checks JabRef's shipped classes for architecture quality.
+ * Does not analyze test classes. Hint from <a href="https://stackoverflow.com/a/44681895/873282">StackOverflow</a>
+ */
+@AnalyzeClasses(packages = "org.jabref", importOptions = ImportOption.DoNotIncludeTests.class)
+public class CommonArchitectureTest {
+
+    private static final String PACKAGE_JAVAX_SWING = "javax.swing..";
+    private static final String PACKAGE_JAVA_AWT = "java.awt..";
+    private static final String PACKAGE_ORG_JABREF_GUI = "org.jabref.gui..";
+    private static final String PACKAGE_ORG_JABREF_LOGIC = "org.jabref.logic..";
+    private static final String PACKAGE_ORG_JABREF_MODEL = "org.jabref.model..";
+    private static final String PACKAGE_ORG_JABREF_CLI = "org.jabref.cli..";
+
+    @ArchTest
+    public void doNotUseApacheCommonsLang3(JavaClasses classes) {
+        ArchRuleDefinition.noClasses().that().areNotAnnotatedWith(AllowedToUseApacheCommonsLang3.class)
+                          .should().accessClassesThat().resideInAPackage("org.apache.commons.lang3")
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void doNotUseSwing(JavaClasses classes) {
+        // This checks for all Swing packages, but not the UndoManager
+        ArchRuleDefinition.noClasses().that().areNotAnnotatedWith(AllowedToUseSwing.class)
+                          .should().accessClassesThat()
+                          .resideInAnyPackage("javax.swing",
+                           "javax.swing.border..",
+                           "javax.swing.colorchooser..",
+                           "javax.swing.event..",
+                           "javax.swing.filechooser..",
+                           "javax.swing.plaf..",
+                           "javax.swing.table..",
+                           "javax.swing.text..",
+                           "javax.swing.tree..")
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void doNotUseAssertJ(JavaClasses classes) {
+        ArchRuleDefinition.noClasses().should().accessClassesThat().resideInAPackage("org.assertj..")
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void doNotUseJavaAWT(JavaClasses classes) {
+        ArchRuleDefinition.noClasses().that().areNotAnnotatedWith(AllowedToUseAwt.class)
+                          .should().accessClassesThat().resideInAPackage(PACKAGE_JAVA_AWT)
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void doNotUsePaths(JavaClasses classes) {
+        ArchRuleDefinition.noClasses().should()
+                          .accessClassesThat()
+                          .belongToAnyOf(Paths.class)
+                          .because("Path.of(...) should be used instead")
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void useStreamsOfResources(JavaClasses classes) {
+        // Reason: https://github.com/oracle/graal/issues/7682#issuecomment-1786704111
+        ArchRuleDefinition.noClasses().that().haveNameNotMatching(".*Test")
+                          .and().areNotAnnotatedWith(AllowedToUseClassGetResource.class)
+                          .and().areNotAssignableFrom("org.jabref.logic.importer.fileformat.ImporterTestEngine")
+                          .should()
+                          .callMethod(Class.class, "getResource", String.class)
+                          .because("getResourceAsStream(...) should be used instead")
+                          .check(classes);
+    }
+
+    // TODO: no org.jabref.gui package may reside in org.jabref.logic
+
+    @ArchTest
+    public void doNotUseLogicInModel(JavaClasses classes) {
+        ArchRuleDefinition.noClasses().that().resideInAPackage(PACKAGE_ORG_JABREF_MODEL)
+                          .and().areNotAnnotatedWith(AllowedToUseLogic.class)
+                          .should().dependOnClassesThat().resideInAPackage(PACKAGE_ORG_JABREF_LOGIC)
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void restrictUsagesInModel(JavaClasses classes) {
+        // Until we switch to Lucene, we need to access Globals.stateManager().getActiveDatabase() from the search classes,
+        // because the PDFSearch needs to access the index of the corresponding database
+        ArchRuleDefinition.noClasses().that().areNotAssignableFrom("org.jabref.model.search.rules.ContainBasedSearchRule")
+                          .and().areNotAssignableFrom("org.jabref.model.search.rules.RegexBasedSearchRule")
+                          .and().areNotAssignableFrom("org.jabref.model.search.rules.GrammarBasedSearchRule")
+                          .and().resideInAPackage(PACKAGE_ORG_JABREF_MODEL)
+                          .should().dependOnClassesThat().resideInAPackage(PACKAGE_JAVAX_SWING)
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void restrictUsagesInLogic(JavaClasses classes) {
+        ArchRuleDefinition.noClasses().that().resideInAPackage(PACKAGE_ORG_JABREF_LOGIC)
+                          .and().areNotAnnotatedWith(AllowedToUseSwing.class)
+                          .and().areNotAssignableFrom("org.jabref.logic.search.DatabaseSearcherWithBibFilesTest")
+                          .should().dependOnClassesThat().resideInAPackage(PACKAGE_JAVAX_SWING)
+                          .check(classes);
+    }
+
+    @ArchTest
+    public void restrictStandardStreams(JavaClasses classes) {
+        ArchRuleDefinition.noClasses().that().resideOutsideOfPackages(PACKAGE_ORG_JABREF_CLI)
+                          .and().resideOutsideOfPackages("org.jabref.gui.openoffice..") // Uses LibreOffice SDK
+                          .and().areNotAnnotatedWith(AllowedToUseStandardStreams.class)
+                          .should(GeneralCodingRules.ACCESS_STANDARD_STREAMS)
+                          .because("logging framework should be used instead or the class be marked explicitly as @AllowedToUseStandardStreams")
+                          .check(classes);
+    }
+
+    /// Use constructor new URI(...) instead
+    @ArchTest
+    public void shouldNotCallUriCreateMethod(JavaClasses classes) {
+       ArchRuleDefinition.noClasses()
+                         .that()
+                         .resideInAPackage("org.jabref..")
+                         .should().callMethod(URI.class, "create", String.class)
+                         .check(classes);
+    }
+}
diff --git a/versions/build.gradle.kts b/versions/build.gradle.kts
@@ -56,6 +56,7 @@ dependencies.constraints {
     api("com.squareup.okhttp3:okhttp:4.12.0")
     api("com.squareup.okio:okio-jvm:3.12.0")
     api("com.squareup.retrofit2:retrofit:3.0.0")
+    api("com.tngtech.archunit:archunit:1.4.1")
     api("com.tngtech.archunit:archunit-junit5-api:1.4.1")
     api("com.tngtech.archunit:archunit-junit5-engine:1.4.1")
     api("com.vladsch.flexmark:flexmark-html2md-converter:0.64.8")
