Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict use of standard streams #8816

Merged
merged 7 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.jabref.architecture;

/**
* Annotation to indicate that this class can use System.Out.* instead of using the logging framework
*/
public @interface AllowedToUseStandardStreams {

// The rationale
String value();
}
8 changes: 4 additions & 4 deletions src/main/java/org/jabref/gui/desktop/os/Linux.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ private void nativeOpenFile(String filePath) {
try {
File file = new File(filePath);
Desktop.getDesktop().open(file);
System.out.println("Open file in default application with Desktop integration");
LOGGER.debug("Open file in default application with Desktop integration");
} catch (IllegalArgumentException e) {
System.out.println("Fail back to xdg-open");
LOGGER.debug("Fail back to xdg-open");
try {
String[] cmd = {"xdg-open", filePath};
Runtime.getRuntime().exec(cmd);
} catch (Exception e2) {
System.out.println("Open operation not successful: " + e2);
LOGGER.warn("Open operation not successful: " + e2);
}
} catch (IOException e) {
System.out.println("Native open operation not successful: " + e);
LOGGER.warn("Native open operation not successful: " + e);
}
});
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/jabref/logic/util/io/XMLUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;

import org.jabref.architecture.AllowedToUseStandardStreams;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
Expand All @@ -25,6 +27,7 @@
/**
* Currently used for debugging only
*/
@AllowedToUseStandardStreams("Used for debugging only")
public class XMLUtil {
private static final Logger LOGGER = LoggerFactory.getLogger(XMLUtil.class);

Expand Down
14 changes: 13 additions & 1 deletion src/test/java/org/jabref/architecture/MainArchitectureTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.tngtech.archunit.junit.AnalyzeClasses;
import com.tngtech.archunit.junit.ArchIgnore;
import com.tngtech.archunit.junit.ArchTest;
import com.tngtech.archunit.library.GeneralCodingRules;

import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;
import static com.tngtech.archunit.library.Architectures.layeredArchitecture;
Expand All @@ -20,6 +21,7 @@ class MainArchitectureTests {
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 static void doNotUseApacheCommonsLang3(JavaClasses classes) {
Expand Down Expand Up @@ -92,7 +94,7 @@ public static void respectLayeredArchitecture(JavaClasses classes) {
.layer("Gui").definedBy(PACKAGE_ORG_JABREF_GUI)
.layer("Logic").definedBy(PACKAGE_ORG_JABREF_LOGIC)
.layer("Model").definedBy(PACKAGE_ORG_JABREF_MODEL)
.layer("Cli").definedBy("org.jabref.cli..")
.layer("Cli").definedBy(PACKAGE_ORG_JABREF_CLI)
.layer("Migrations").definedBy("org.jabref.migrations..") // TODO: Move to logic
.layer("Preferences").definedBy("org.jabref.preferences..")
.layer("Styletester").definedBy("org.jabref.styletester..")
Expand Down Expand Up @@ -135,4 +137,14 @@ public static void restrictUsagesInLogic(JavaClasses classes) {
.orShould().dependOnClassesThat().haveFullyQualifiedName(CLASS_ORG_JABREF_GLOBALS)
.check(classes);
}

@ArchTest
public static void restrictStandardStreams(JavaClasses classes) {
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.jabref.logic.bibtex.BibEntryAssert;
Expand All @@ -22,14 +21,18 @@
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Answers;
import org.mockito.Mockito;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;

public class ModsExportFormatTestFiles {

public class ModsExportFormatFilesTest {
private static final Logger LOGGER = LoggerFactory.getLogger(ModsExportFormatFilesTest.class);
private static Path resourceDir;

public Charset charset;

private BibDatabaseContext databaseContext;
private Path exportedFile;
private ModsExporter exporter;
Expand All @@ -39,12 +42,12 @@ public class ModsExportFormatTestFiles {

public static Stream<String> fileNames() throws Exception {
resourceDir = Path.of(MSBibExportFormatTestFiles.class.getResource("ModsExportFormatTestAllFields.bib").toURI()).getParent();
System.out.println(resourceDir);
LOGGER.debug(String.valueOf(resourceDir));

try (Stream<Path> stream = Files.list(resourceDir)) {
// stream.forEach(n -> System.out.println(n));

return stream.map(n -> n.getFileName().toString()).filter(n -> n.endsWith(".bib"))
.filter(n -> n.startsWith("Mods")).collect(Collectors.toList()).stream();
.filter(n -> n.startsWith("Mods")).toList().stream();
}
}

Expand All @@ -65,10 +68,10 @@ public void setUp(@TempDir Path testFolder) throws Exception {
@ParameterizedTest
@MethodSource("fileNames")
public final void testPerformExport(String filename) throws Exception {
importFile = Path.of(ModsExportFormatTestFiles.class.getResource(filename).toURI());
importFile = Path.of(ModsExportFormatFilesTest.class.getResource(filename).toURI());
String xmlFileName = filename.replace(".bib", ".xml");
List<BibEntry> entries = bibtexImporter.importDatabase(importFile).getDatabase().getEntries();
Path expectedFile = Path.of(ModsExportFormatTestFiles.class.getResource(xmlFileName).toURI());
Path expectedFile = Path.of(ModsExportFormatFilesTest.class.getResource(xmlFileName).toURI());

exporter.export(databaseContext, exportedFile, entries);

Expand All @@ -80,7 +83,7 @@ public final void testPerformExport(String filename) throws Exception {
@ParameterizedTest
@MethodSource("fileNames")
public final void testExportAsModsAndThenImportAsMods(String filename) throws Exception {
importFile = Path.of(ModsExportFormatTestFiles.class.getResource(filename).toURI());
importFile = Path.of(ModsExportFormatFilesTest.class.getResource(filename).toURI());
List<BibEntry> entries = bibtexImporter.importDatabase(importFile).getDatabase().getEntries();

exporter.export(databaseContext, exportedFile, entries);
Expand All @@ -90,9 +93,9 @@ public final void testExportAsModsAndThenImportAsMods(String filename) throws Ex
@ParameterizedTest
@MethodSource("fileNames")
public final void testImportAsModsAndExportAsMods(String filename) throws Exception {
importFile = Path.of(ModsExportFormatTestFiles.class.getResource(filename).toURI());
importFile = Path.of(ModsExportFormatFilesTest.class.getResource(filename).toURI());
String xmlFileName = filename.replace(".bib", ".xml");
Path xmlFile = Path.of(ModsExportFormatTestFiles.class.getResource(xmlFileName).toURI());
Path xmlFile = Path.of(ModsExportFormatFilesTest.class.getResource(xmlFileName).toURI());

List<BibEntry> entries = modsImporter.importDatabase(xmlFile).getDatabase().getEntries();

Expand Down
6 changes: 5 additions & 1 deletion src/test/java/org/jabref/logic/git/SlrGitHandlerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.junit.jupiter.api.Assertions.assertEquals;

class SlrGitHandlerTest {
private static final Logger LOGGER = LoggerFactory.getLogger(SlrGitHandlerTest.class);

@TempDir
Path repositoryPath;
private SlrGitHandler gitHandler;
Expand Down Expand Up @@ -44,7 +48,7 @@ void calculateDiffOnBranch() throws IOException, GitAPIException {
Files.writeString(Path.of(repositoryPath.toString(), "TestFolder", "Test1.txt"), "This is a new line of text 2\n" + Files.readString(Path.of(repositoryPath.toString(), "TestFolder", "Test1.txt")));
gitHandler.createCommitOnCurrentBranch("Commit 2 on branch1", false);

System.out.println(gitHandler.calculatePatchOfNewSearchResults("branch1"));
LOGGER.debug(gitHandler.calculatePatchOfNewSearchResults("branch1"));
assertEquals(expectedPatch, gitHandler.calculatePatchOfNewSearchResults("branch1"));
}

Expand Down
5 changes: 4 additions & 1 deletion src/test/java/org/jabref/logic/net/URLDownloadTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@

import kong.unirest.UnirestException;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class URLDownloadTest {
private static final Logger LOGGER = LoggerFactory.getLogger(URLDownloadTest.class);

@Test
public void testStringDownloadWithSetEncoding() throws IOException {
Expand All @@ -42,7 +45,7 @@ public void testFileDownload() throws IOException {
} finally {
// cleanup
if (!destination.delete()) {
System.err.println("Cannot delete downloaded file");
LOGGER.error("Cannot delete downloaded file");
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/test/java/org/jabref/logic/util/io/FileUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Answers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand All @@ -29,6 +31,8 @@
import static org.mockito.Mockito.mock;

class FileUtilTest {
private static final Logger LOGGER = LoggerFactory.getLogger(FileUtilTest.class);

private final Path nonExistingTestPath = Path.of("nonExistingTestPath");
private Path existingTestFile;
private Path otherExistingTestFile;
Expand Down Expand Up @@ -319,7 +323,7 @@ void testRenameFileSuccessful(@TempDir Path otherTemporaryFolder) {
// in the @BeforeEach method.
Path temp = Path.of(otherTemporaryFolder.resolve("123").toString());

System.out.println(temp);
LOGGER.debug(String.valueOf(temp));
FileUtil.renameFile(existingTestFile, temp);
assertFalse(Files.exists(existingTestFile));
}
Expand Down