Skip to content

Commit

Permalink
DI Part A (#11342)
Browse files Browse the repository at this point in the history
* Migrate BibEntryTypesManager

* Fix deprecation

* Migrate UndoManager

* Remove BibEntryTypesManager from Globals

* Migrate PreferencesService

* Migrate DialogService

* Migrate ThemeManager

* Migrate StateManager

* Migrate TaskExecutor

* Migrate KeyBindingRepository WIP

* Fix StringToList conversion

* Migrate KeyBindingRepository

* Fix some tests

* Checkstyle

* Rewrite

* Checkstyle

* Fix tests

* Migrate ProtectedTermsLoader

* Refactor Launcher and extract logging initialization

* Migrate JournalAbbreviationsRepository

* Move initialization to initialize

* Migrate ClipBoardManager

* Migrate RemoteListenerServerManager

* Migrate BuildInfo

* Fix merge errors

* Add some text on DI

* Migrate FileUpdateMonitor and DirectoryMonitor, Remove Globals

* Remove DefaultInjector

* Fix JavaDoc error

* Fix JavaDoc comment

* Remove null check for DirectoryMonitor

* Add javadoc to UiMessageHandler

* Split JabRefPreferencesTest

* Add missing field

---------

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
  • Loading branch information
calixtus and koppor committed Jun 17, 2024
1 parent 5c8eb42 commit e6b5470
Show file tree
Hide file tree
Showing 131 changed files with 862 additions and 749 deletions.
10 changes: 9 additions & 1 deletion docs/code-howtos/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,20 @@ See also [High Level Documentation](../getting-into-the-code/high-level-document

We really recommend reading the book [Java by Comparison](http://java.by-comparison.com).

Please read [https://github.com/cxxr/better-java](https://github.com/cxxr/better-java)
Please read <https://github.com/cxxr/better-java>.

* try not to abbreviate names of variables, classes or methods
* use lowerCamelCase instead of snake\_case
* name enums in singular, e.g. `Weekday` instead of `Weekdays` (except if they represent flags)

## Dependency injection

JabRef uses a [fork](https://github.com/JabRef/afterburner.fx) of the [afterburner.fx framework](https://github.com/AdamBien/afterburner.fx) by [Adam Bien](https://adam-bien.com/).

The main idea is to get instances by using `Injector.instantiateModelOrService(X.class)`, where `X` is the instance one needs.
The method `instantiateModelOrService` checks if there is already an instance of the given class. If yes, it returns it. If not, it creates a new one.
A singleton can be added by `com.airhacks.afterburner.injection.Injector#setModelOrService(X.class, y)`, where X is the class and y the instance you want to inject.

## Cleanup and Formatters

We try to build a cleanup mechanism based on formatters. The idea is that we can register these actions in arbitrary places, e.g., onSave, onImport, onExport, cleanup, etc. and apply them to different fields. The formatters themselves are independent of any logic and therefore easy to test.
Expand Down
8 changes: 5 additions & 3 deletions src/jmh/java/org/jabref/benchmarks/Benchmarks.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import java.util.Random;
import java.util.stream.Collectors;

import org.jabref.gui.Globals;
import org.jabref.logic.bibtex.FieldPreferences;
import org.jabref.logic.citationkeypattern.CitationKeyPatternPreferences;
import org.jabref.logic.exporter.BibWriter;
Expand All @@ -35,7 +34,9 @@
import org.jabref.model.metadata.MetaData;
import org.jabref.model.search.rules.SearchRules.SearchFlags;
import org.jabref.preferences.JabRefPreferences;
import org.jabref.preferences.PreferencesService;

import com.airhacks.afterburner.injection.Injector;
import org.openjdk.jmh.Main;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.Scope;
Expand All @@ -55,7 +56,7 @@ public class Benchmarks {

@Setup
public void init() throws Exception {
Globals.prefs = JabRefPreferences.getInstance();
Injector.setModelOrService(PreferencesService.class, JabRefPreferences.getInstance());

Random randomizer = new Random();
for (int i = 0; i < 1000; i++) {
Expand Down Expand Up @@ -92,7 +93,8 @@ private StringWriter getOutputWriter() throws IOException {

@Benchmark
public ParserResult parse() throws IOException {
BibtexParser parser = new BibtexParser(Globals.prefs.getImportFormatPreferences());
PreferencesService preferencesService = Injector.instantiateModelOrService(PreferencesService.class);
BibtexParser parser = new BibtexParser(preferencesService.getImportFormatPreferences());
return parser.parse(new StringReader(bibtexString));
}

Expand Down
2 changes: 0 additions & 2 deletions src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@

provides com.airhacks.afterburner.views.ResourceLocator
with org.jabref.gui.util.JabRefResourceLocator;
provides com.airhacks.afterburner.injection.PresenterFactory
with org.jabref.gui.DefaultInjector;

// Logging
requires org.slf4j;
Expand Down
83 changes: 42 additions & 41 deletions src/main/java/org/jabref/Launcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@

import org.jabref.cli.ArgumentProcessor;
import org.jabref.cli.JabRefCLI;
import org.jabref.gui.Globals;
import org.jabref.gui.JabRefExecutorService;
import org.jabref.gui.JabRefGUI;
import org.jabref.gui.util.DefaultDirectoryMonitor;
import org.jabref.gui.util.DefaultFileUpdateMonitor;
import org.jabref.logic.UiCommand;
import org.jabref.logic.journals.JournalAbbreviationLoader;
import org.jabref.logic.journals.JournalAbbreviationRepository;
import org.jabref.logic.net.ProxyAuthenticator;
import org.jabref.logic.net.ProxyPreferences;
import org.jabref.logic.net.ProxyRegisterer;
Expand All @@ -25,13 +28,16 @@
import org.jabref.logic.protectedterms.ProtectedTermsLoader;
import org.jabref.logic.remote.RemotePreferences;
import org.jabref.logic.remote.client.RemoteClient;
import org.jabref.logic.util.BuildInfo;
import org.jabref.logic.util.OS;
import org.jabref.migrations.PreferencesMigrations;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.util.DirectoryMonitor;
import org.jabref.model.util.FileUpdateMonitor;
import org.jabref.preferences.JabRefPreferences;
import org.jabref.preferences.PreferencesService;

import com.airhacks.afterburner.injection.Injector;
import org.apache.commons.cli.ParseException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -47,45 +53,42 @@
*/
public class Launcher {
private static Logger LOGGER;
private static boolean isDebugEnabled;

public static void main(String[] args) {
routeLoggingToSlf4J();
initLogging(args);

// We must configure logging as soon as possible, which is why we cannot wait for the usual
// argument parsing workflow to parse logging options .e.g. --debug
JabRefCLI jabRefCLI;
try {
jabRefCLI = new JabRefCLI(args);
isDebugEnabled = jabRefCLI.isDebugLogging();
} catch (ParseException e) {
isDebugEnabled = false;
}

addLogToDisk();
try {
BibEntryTypesManager entryTypesManager = new BibEntryTypesManager();
Globals.entryTypesManager = entryTypesManager;
Injector.setModelOrService(BuildInfo.class, new BuildInfo());

// Initialize preferences
final JabRefPreferences preferences = JabRefPreferences.getInstance();
Injector.setModelOrService(PreferencesService.class, preferences);

// Early exit in case another instance is already running
if (!handleMultipleAppInstances(args, preferences.getRemotePreferences())) {
return;
}

Globals.prefs = preferences;
BibEntryTypesManager entryTypesManager = preferences.getCustomEntryTypesRepository();
Injector.setModelOrService(BibEntryTypesManager.class, entryTypesManager);

PreferencesMigrations.runMigrations(preferences, entryTypesManager);

// Initialize rest of preferences
Injector.setModelOrService(JournalAbbreviationRepository.class, JournalAbbreviationLoader.loadRepository(preferences.getJournalAbbreviationPreferences()));
Injector.setModelOrService(ProtectedTermsLoader.class, new ProtectedTermsLoader(preferences.getProtectedTermsPreferences()));

configureProxy(preferences.getProxyPreferences());
configureSSL(preferences.getSSLPreferences());
initGlobals(preferences);

clearOldSearchIndices();

try {
FileUpdateMonitor fileUpdateMonitor = Globals.getFileUpdateMonitor();
DefaultFileUpdateMonitor fileUpdateMonitor = new DefaultFileUpdateMonitor();
Injector.setModelOrService(FileUpdateMonitor.class, fileUpdateMonitor);
JabRefExecutorService.INSTANCE.executeInterruptableTask(fileUpdateMonitor, "FileUpdateMonitor");

DirectoryMonitor directoryMonitor = new DefaultDirectoryMonitor();
Injector.setModelOrService(DirectoryMonitor.class, directoryMonitor);

// Process arguments
ArgumentProcessor argumentProcessor = new ArgumentProcessor(
Expand Down Expand Up @@ -114,25 +117,35 @@ public static void main(String[] args) {
}
}

private static void routeLoggingToSlf4J() {
SLF4JBridgeHandler.removeHandlersForRootLogger();
SLF4JBridgeHandler.install();
}

/**
* This needs to be called as early as possible. After the first log write, it
* is not possible to alter
* the log configuration programmatically anymore.
* is not possible to alter the log configuration programmatically anymore.
*/
private static void addLogToDisk() {
private static void initLogging(String[] args) {
// routeLoggingToSlf4J
SLF4JBridgeHandler.removeHandlersForRootLogger();
SLF4JBridgeHandler.install();

// We must configure logging as soon as possible, which is why we cannot wait for the usual
// argument parsing workflow to parse logging options .e.g. --debug
boolean isDebugEnabled;
try {
JabRefCLI jabRefCLI = new JabRefCLI(args);
isDebugEnabled = jabRefCLI.isDebugLogging();
} catch (ParseException e) {
isDebugEnabled = false;
}

// addLogToDisk
Path directory = OS.getNativeDesktop().getLogDirectory();
try {
Files.createDirectories(directory);
} catch (IOException e) {
initializeLogger();
LOGGER = LoggerFactory.getLogger(Launcher.class);
LOGGER.error("Could not create log directory {}", directory, e);
return;
}

// The "Shared File Writer" is explained at
// https://tinylog.org/v2/configuration/#shared-file-writer
Map<String, String> configuration = Map.of(
Expand All @@ -144,12 +157,8 @@ private static void addLogToDisk() {
"writerFile.charset", "UTF-8",
"writerFile.policies", "startup",
"writerFile.backups", "30");

configuration.forEach(Configuration::set);
initializeLogger();
}

private static void initializeLogger() {
LOGGER = LoggerFactory.getLogger(Launcher.class);
}

Expand Down Expand Up @@ -183,14 +192,6 @@ private static boolean handleMultipleAppInstances(String[] args, RemotePreferenc
return true;
}

private static void initGlobals(PreferencesService preferences) {
// Read list(s) of journal names and abbreviations
Globals.journalAbbreviationRepository = JournalAbbreviationLoader
.loadRepository(preferences.getJournalAbbreviationPreferences());
Globals.entryTypesManager = preferences.getCustomEntryTypesRepository();
Globals.protectedTermsLoader = new ProtectedTermsLoader(preferences.getProtectedTermsPreferences());
}

private static void configureProxy(ProxyPreferences proxyPreferences) {
ProxyRegisterer.register(proxyPreferences);
if (proxyPreferences.shouldUseProxy() && proxyPreferences.shouldUseAuthentication()) {
Expand Down
21 changes: 13 additions & 8 deletions src/main/java/org/jabref/cli/ArgumentProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import java.util.Set;
import java.util.prefs.BackingStoreException;

import org.jabref.gui.Globals;
import org.jabref.gui.externalfiles.AutoSetFileLinksUtil;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.logic.JabRefException;
Expand Down Expand Up @@ -59,6 +58,7 @@
import org.jabref.preferences.PreferencesService;
import org.jabref.preferences.SearchPreferences;

import com.airhacks.afterburner.injection.Injector;
import com.google.common.base.Throwables;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -250,9 +250,9 @@ public void processArguments() {
preferencesService.getXmpPreferences(),
preferencesService.getFilePreferences(),
preferencesService.getLibraryPreferences().getDefaultBibDatabaseMode(),
Globals.entryTypesManager,
Injector.instantiateModelOrService(BibEntryTypesManager.class),
preferencesService.getFieldPreferences(),
Globals.journalAbbreviationRepository,
Injector.instantiateModelOrService(JournalAbbreviationRepository.class),
cli.isWriteXMPtoPdf() || cli.isWriteMetadatatoPdf(),
cli.isEmbeddBibfileInPdf() || cli.isWriteMetadatatoPdf());
}
Expand Down Expand Up @@ -486,15 +486,20 @@ private boolean exportMatches(List<ParserResult> loaded) {
// export new database
ExporterFactory exporterFactory = ExporterFactory.create(
preferencesService,
Globals.entryTypesManager);
Injector.instantiateModelOrService(BibEntryTypesManager.class));
Optional<Exporter> exporter = exporterFactory.getExporterByName(formatName);
if (exporter.isEmpty()) {
System.err.println(Localization.lang("Unknown export format %0", formatName));
} else {
// We have an TemplateExporter instance:
try {
System.out.println(Localization.lang("Exporting %0", data[1]));
exporter.get().export(databaseContext, Path.of(data[1]), matches, Collections.emptyList(), Globals.journalAbbreviationRepository);
exporter.get().export(
databaseContext,
Path.of(data[1]),
matches,
Collections.emptyList(),
Injector.instantiateModelOrService(JournalAbbreviationRepository.class));
} catch (Exception ex) {
System.err.println(Localization.lang("Could not export file '%0' (reason: %1)", data[1], Throwables.getStackTraceAsString(ex)));
}
Expand Down Expand Up @@ -661,7 +666,7 @@ private void exportFile(List<ParserResult> loaded, String[] data) {
System.out.println(Localization.lang("Exporting %0", data[0]));
ExporterFactory exporterFactory = ExporterFactory.create(
preferencesService,
Globals.entryTypesManager);
Injector.instantiateModelOrService(BibEntryTypesManager.class));
Optional<Exporter> exporter = exporterFactory.getExporterByName(data[1]);
if (exporter.isEmpty()) {
System.err.println(Localization.lang("Unknown export format %0", data[1]));
Expand All @@ -673,7 +678,7 @@ private void exportFile(List<ParserResult> loaded, String[] data) {
Path.of(data[0]),
parserResult.getDatabaseContext().getDatabase().getEntries(),
fileDirForDatabase,
Globals.journalAbbreviationRepository);
Injector.instantiateModelOrService(JournalAbbreviationRepository.class));
} catch (Exception ex) {
System.err.println(Localization.lang("Could not export file '%0' (reason: %1)", data[0], Throwables.getStackTraceAsString(ex)));
}
Expand All @@ -684,7 +689,7 @@ private void exportFile(List<ParserResult> loaded, String[] data) {
private void importPreferences() {
try {
preferencesService.importPreferences(Path.of(cli.getPreferencesImport()));
Globals.entryTypesManager = preferencesService.getCustomEntryTypesRepository();
Injector.setModelOrService(BibEntryTypesManager.class, preferencesService.getCustomEntryTypesRepository());
} catch (JabRefException ex) {
LOGGER.error("Cannot import preferences", ex);
}
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/org/jabref/cli/JabRefCLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@

import javafx.util.Pair;

import org.jabref.gui.Globals;
import org.jabref.logic.exporter.ExporterFactory;
import org.jabref.logic.importer.ImportFormatReader;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.BuildInfo;
import org.jabref.logic.util.OS;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.strings.StringUtil;
import org.jabref.model.util.DummyFileUpdateMonitor;
import org.jabref.preferences.PreferencesService;

import com.airhacks.afterburner.injection.Injector;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.HelpFormatter;
Expand Down Expand Up @@ -326,7 +328,7 @@ public static void printUsage(PreferencesService preferencesService) {

ExporterFactory exporterFactory = ExporterFactory.create(
preferencesService,
Globals.entryTypesManager);
Injector.instantiateModelOrService(BibEntryTypesManager.class));
List<Pair<String, String>> exportFormats = exporterFactory
.getExporters().stream()
.map(format -> new Pair<>(format.getName(), format.getId()))
Expand All @@ -341,7 +343,8 @@ public static void printUsage(PreferencesService preferencesService) {
}

private String getVersionInfo() {
return "JabRef %s".formatted(Globals.BUILD_INFO.version);
BuildInfo buildInfo = Injector.instantiateModelOrService(BuildInfo.class);
return "JabRef %s".formatted(buildInfo.version);
}

public List<String> getLeftOver() {
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/org/jabref/gui/ClipBoardManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.jabref.model.entry.BibtexString;
import org.jabref.preferences.PreferencesService;

import com.airhacks.afterburner.injection.Injector;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -37,16 +38,13 @@ public class ClipBoardManager {
private static Clipboard clipboard;
private static java.awt.datatransfer.Clipboard primary;

private final PreferencesService preferencesService;

public ClipBoardManager(PreferencesService preferencesService) {
this(Clipboard.getSystemClipboard(), Toolkit.getDefaultToolkit().getSystemSelection(), preferencesService);
public ClipBoardManager() {
this(Clipboard.getSystemClipboard(), Toolkit.getDefaultToolkit().getSystemSelection());
}

public ClipBoardManager(Clipboard clipboard, java.awt.datatransfer.Clipboard primary, PreferencesService preferencesService) {
public ClipBoardManager(Clipboard clipboard, java.awt.datatransfer.Clipboard primary) {
ClipBoardManager.clipboard = clipboard;
ClipBoardManager.primary = primary;
this.preferencesService = preferencesService;
}

/**
Expand Down Expand Up @@ -169,6 +167,7 @@ public void setContent(List<BibEntry> entries, BibEntryTypesManager entryTypesMa
}

private String serializeEntries(List<BibEntry> entries, BibEntryTypesManager entryTypesManager) throws IOException {
PreferencesService preferencesService = Injector.instantiateModelOrService(PreferencesService.class);
// BibEntry is not Java serializable. Thus, we need to do the serialization manually
// At reading of the clipboard in JabRef, we parse the plain string in all cases, so we don't need to flag we put BibEntries here
// Furthermore, storing a string also enables other applications to work with the data
Expand Down
Loading

0 comments on commit e6b5470

Please sign in to comment.