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

Remove apache commons logging in favor of slf4j + log4j for JAVA 9 #3653

Merged
merged 8 commits into from
Jan 22, 2018
5 changes: 3 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ dependencies {
compile 'org.jsoup:jsoup:1.11.2'
compile 'com.mashape.unirest:unirest-java:1.4.9'

compile 'commons-logging:commons-logging:1.2'
// >1.8.0-beta is required for java 9 compatibility
compile 'org.slf4j:slf4j-api:1.8.0-beta0'
compile 'org.apache.logging.log4j:log4j-slf4j-impl:2.10.0'
compile 'org.apache.logging.log4j:log4j-jcl:2.10.0'
compile 'org.apache.logging.log4j:log4j-api:2.10.0'
compile 'org.apache.logging.log4j:log4j-core:2.10.0'
Expand Down Expand Up @@ -167,7 +169,6 @@ dependencies {
testCompile 'org.xmlunit:xmlunit-core:2.5.1'
testCompile 'org.xmlunit:xmlunit-matchers:2.5.1'
testCompile 'com.tngtech.archunit:archunit-junit:0.5.0'
testCompile 'org.slf4j:slf4j-jcl:1.7.25' // required by ArchUnit to enable logging over jcl
testCompile "org.testfx:testfx-core:4.0.+"
testCompile "org.testfx:testfx-junit:4.0.+"

Expand Down
43 changes: 43 additions & 0 deletions docs/adr/0002-use-slf4j-for-logging.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Use slf4j together with log4j2 for logging

* Up to version 4.1 JabRef uses apache-ommons-logging 1.2 for logging errors and messages. However, this is not compatible with java 9 and is superseded by log4j.
* SLF4J provides a facade for several logging frameworks, including log4j and supports already java 9
* Log4j is already defined as dependency and slf4j has already been required by a third party dependency

## Considered Alternatives

* [Log4j2](https://logging.apache.org/log4j/2.x/)
* [SLF4J with Log4j2 binding](https://logging.apache.org/log4j/2.x/maven-artifacts.html)
* [SLF4J with Logback binding](https://logback.qos.ch/)

## Decision Outcome
* We chose slf4j with log4j2 binding, because it only requires minimal changes to our logging infrastructure and it is claimed that
> Apache Log4j 2 is an upgrade to Log4j that provides significant improvements over its predecessor, Log4j 1.x, and provides many of the improvements available in Logback while fixing some inherent problems in Logback’s architecture.
* Furthermore, as slf4j is a facade for several loggers, the underlying implementation can easily be changed in the future


## Pros and Cons of the Alternatives

### Log4j2

* `+` Dependency already exists
* `+` Java 9 support since version 2.10
* `-` Direct dependency

### SLF4J with log4j2 binding

* `+` Supports other loggers as well
* `+` Java 9 support
* `+` Already defined
* `+` Migration tool available
* `-` Logger statements require a slight different syntax

### SLF4J with Logback binding

* `+` Migration tool available
* `+` Native implementation of slf4j
* `-` Java 9 support only available in alpha
* `-` Different syntax than log4j/commons logging



6 changes: 3 additions & 3 deletions src/main/java/org/jabref/FallbackExceptionHandler.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package org.jabref;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Catch and log any unhandled exceptions.
*/
public class FallbackExceptionHandler implements Thread.UncaughtExceptionHandler {

private static final Log LOGGER = LogFactory.getLog(FallbackExceptionHandler.class);
private static final Logger LOGGER = LoggerFactory.getLogger(FallbackExceptionHandler.class);

public static void installExceptionHandler() {
Thread.setDefaultUncaughtExceptionHandler(new FallbackExceptionHandler());
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/JabRefException.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package org.jabref;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class JabRefException extends Exception {

private static final Log LOGGER = LogFactory.getLog(JabRefException.class);
private static final Logger LOGGER = LoggerFactory.getLogger(JabRefException.class);
private String localizedMessage;

public JabRefException(String message) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/JabRefExecutorService.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;


/**
Expand All @@ -19,7 +19,7 @@
public class JabRefExecutorService implements Executor {

public static final JabRefExecutorService INSTANCE = new JabRefExecutorService();
private static final Log LOGGER = LogFactory.getLog(JabRefExecutorService.class);
private static final Logger LOGGER = LoggerFactory.getLogger(JabRefExecutorService.class);
private final ExecutorService executorService = Executors.newCachedThreadPool(r -> {
Thread thread = new Thread(r);
thread.setName("JabRef CachedThreadPool");
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/jabref/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@
import org.jabref.model.database.shared.DatabaseNotSupportedException;
import org.jabref.preferences.JabRefPreferences;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class JabRefGUI {
private static final Log LOGGER = LogFactory.getLog(JabRefGUI.class);

private static final Logger LOGGER = LoggerFactory.getLogger(JabRefGUI.class);
private static JabRefFrame mainFrame;

private final List<ParserResult> bibDatabases;
private final boolean isBlank;
private final List<ParserResult> failed = new ArrayList<>();
private final List<ParserResult> toOpenTab = new ArrayList<>();

private String focusedFile;
private final String focusedFile;

public JabRefGUI(List<ParserResult> argsDatabases, boolean isBlank) {
this.bibDatabases = argsDatabases;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/JabRefMain.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@
import org.jabref.model.entry.InternalBibtexFields;
import org.jabref.preferences.JabRefPreferences;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* JabRef MainClass
*/
public class JabRefMain extends Application {

private static final Log LOGGER = LogFactory.getLog(JabRefMain.class);
private static final Logger LOGGER = LoggerFactory.getLogger(JabRefMain.class);
private static String[] arguments;

public static void main(String[] args) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/cli/ArgumentProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@
import org.jabref.preferences.SearchPreferences;

import com.google.common.base.Throwables;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class ArgumentProcessor {

private static final Log LOGGER = LogFactory.getLog(ArgumentProcessor.class);
private static final Logger LOGGER = LoggerFactory.getLogger(ArgumentProcessor.class);
private final JabRefCLI cli;
private final List<ParserResult> parserResults;
private final Mode startupMode;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/cli/JabRefCLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
import org.apache.commons.cli.Option;
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class JabRefCLI {

private static final Log LOGGER = LogFactory.getLog(JabRefCLI.class);
private static final Logger LOGGER = LoggerFactory.getLogger(JabRefCLI.class);
private final CommandLine cl;
private List<String> leftOver;

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,12 @@
import com.google.common.eventbus.Subscribe;
import com.jgoodies.forms.builder.FormBuilder;
import com.jgoodies.forms.layout.FormLayout;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class BasePanel extends JPanel implements ClipboardOwner {

private static final Log LOGGER = LogFactory.getLog(BasePanel.class);
private static final Logger LOGGER = LoggerFactory.getLogger(BasePanel.class);

// Divider size for BaseFrame split pane. 0 means non-resizable.
private static final int SPLIT_PANE_DIVIDER_SIZE = 4;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/ClipBoardManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.identifier.DOI;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class ClipBoardManager implements ClipboardOwner {

private static final Log LOGGER = LogFactory.getLog(ClipBoardManager.class);
private static final Logger LOGGER = LoggerFactory.getLogger(ClipBoardManager.class);

private final Clipboard clipboard;

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/DefaultInjector.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@

import com.airhacks.afterburner.injection.Injector;
import com.airhacks.afterburner.injection.PresenterFactory;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class DefaultInjector implements PresenterFactory {

private static final Log LOGGER = LogFactory.getLog(DefaultInjector.class);
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultInjector.class);

/**
* This method takes care of creating dependencies.
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/EntryTypeDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,16 @@
import org.jabref.model.entry.IEEETranEntryTypes;

import com.jgoodies.forms.builder.ButtonBarBuilder;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Dialog that prompts the user to choose a type for an entry.
* Returns null if canceled.
*/
public class EntryTypeDialog extends JabRefDialog implements ActionListener {

private static final Log LOGGER = LogFactory.getLog(EntryTypeDialog.class);
private static final Logger LOGGER = LoggerFactory.getLogger(EntryTypeDialog.class);
private static final int COLUMN = 3;
private final JabRefFrame frame;
private final CancelAction cancelAction = new CancelAction();
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@
import org.jabref.preferences.JabRefPreferences;

import com.jgoodies.forms.builder.ButtonBarBuilder;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* GUI Dialog for the feature "Find unlinked files".
Expand All @@ -99,7 +99,7 @@ public class FindUnlinkedFilesDialog extends JabRefDialog {
public static final String ACTION_SHORT_DESCRIPTION = Localization
.lang("Searches for unlinked PDF files on the file system");

private static final Log LOGGER = LogFactory.getLog(FindUnlinkedFilesDialog.class);
private static final Logger LOGGER = LoggerFactory.getLogger(FindUnlinkedFilesDialog.class);
private static final String GLOBAL_PREFS_WORKING_DIRECTORY_KEY = "findUnlinkedFilesWD";

private static final String GLOBAL_PREFS_DIALOG_SIZE_KEY = "findUnlinkedFilesDialogSize";
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/GUIGlobals.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import org.jabref.model.entry.specialfields.SpecialField;
import org.jabref.preferences.JabRefPreferences;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Static variables for graphics files and keyboard shortcuts.
Expand All @@ -45,7 +45,7 @@ public class GUIGlobals {
public static final Color ENTRY_EDITOR_LABEL_COLOR = new Color(100, 100, 150); // Empty field, blue.

static final Color INACTIVE_TABBED_COLOR = Color.black; // inactive Database
private static final Log LOGGER = LogFactory.getLog(GUIGlobals.class);
private static final Logger LOGGER = LoggerFactory.getLogger(GUIGlobals.class);
private static final Map<String, JLabel> TABLE_ICONS = new HashMap<>(); // Contains table icon mappings. Set up
static final Color ACTIVE_TABBED_COLOR = ENTRY_EDITOR_LABEL_COLOR.darker(); // active Database (JTabbedPane)

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/IconTheme.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@

import de.jensd.fx.glyphs.materialdesignicons.MaterialDesignIcon;
import de.jensd.fx.glyphs.materialdesignicons.MaterialDesignIconView;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class IconTheme {

Expand All @@ -45,7 +45,7 @@ public class IconTheme {
public static final Color DEFAULT_DISABLED_COLOR = JabRefPreferences.getInstance().getColor(JabRefPreferences.ICON_DISABLED_COLOR);
public static Font FONT;
private static final String DEFAULT_ICON_PATH = "/images/external/red.png";
private static final Log LOGGER = LogFactory.getLog(IconTheme.class);
private static final Logger LOGGER = LoggerFactory.getLogger(IconTheme.class);
private static final Map<String, String> KEY_TO_ICON = readIconThemeFile(
IconTheme.class.getResource("/images/Icons.properties"), "/images/external/");

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/JEditorPaneWithHighlighting.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
import org.jabref.gui.fieldeditors.JTextAreaWithHighlighting;
import org.jabref.logic.search.SearchQueryHighlightListener;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class JEditorPaneWithHighlighting extends JEditorPane implements SearchQueryHighlightListener {

private static final Log LOGGER = LogFactory.getLog(JTextAreaWithHighlighting.class);
private static final Logger LOGGER = LoggerFactory.getLogger(JTextAreaWithHighlighting.class);

public void highlightPattern(Optional<Pattern> highlightPattern) {
Highlighter highlighter = getHighlighter();
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,15 @@
import org.jabref.preferences.SearchPreferences;

import com.google.common.eventbus.Subscribe;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import osx.macadapter.MacAdapter;

/**
* The main window of the application.
*/
public class JabRefFrame extends JFrame implements OutputPrinter {
private static final Log LOGGER = LogFactory.getLog(JabRefFrame.class);
private static final Logger LOGGER = LoggerFactory.getLogger(JabRefFrame.class);

// Frame titles.
private static final String FRAME_TITLE = "JabRef";
Expand Down Expand Up @@ -653,7 +653,7 @@ public void windowClosing(WindowEvent e) {
try {
new MacAdapter().registerMacEvents(this);
} catch (Exception e) {
LOGGER.fatal("Could not interface with Mac OS X methods.", e);
LOGGER.error("Could not interface with Mac OS X methods.", e);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/PreviewPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@
import org.jabref.preferences.PreviewPreferences;

import com.google.common.eventbus.Subscribe;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Displays an BibEntry using the given layout format.
*/
public class PreviewPanel extends ScrollPane implements SearchQueryHighlightListener, EntryContainer {

private static final Log LOGGER = LogFactory.getLog(PreviewPanel.class);
private static final Logger LOGGER = LoggerFactory.getLogger(PreviewPanel.class);

private final ClipBoardManager clipBoardManager;
private final DialogService dialogService;
Expand Down
Loading