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

Architecture fix by re-sorting the classes #6825

Merged
merged 18 commits into from
Sep 2, 2020
Merged
Show file tree
Hide file tree
Changes from 13 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
6 changes: 6 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ dependencies {
testImplementation 'org.mockito:mockito-core:3.5.7'
testImplementation 'org.xmlunit:xmlunit-core:2.7.0'
testImplementation 'org.xmlunit:xmlunit-matchers:2.7.0'
testRuntime 'com.tngtech.archunit:archunit-junit5-engine:0.14.1'
testImplementation 'com.tngtech.archunit:archunit-junit5-api:0.14.1'
testImplementation "org.testfx:testfx-core:4.0.17-alpha-SNAPSHOT"
testImplementation "org.testfx:testfx-junit5:4.0.17-alpha-SNAPSHOT"
Expand Down Expand Up @@ -442,6 +443,11 @@ test {
excludeTags 'DatabaseTest', 'FetcherTest', 'GUITest'
}

moduleOptions {
// TODO: Remove this as soon as archunit is modularized
runOnClasspath = true
}

testLogging {
// set options for log level LIFECYCLE
// for debugging tests: add "STANDARD_OUT", "STANDARD_ERROR"
Expand Down
40 changes: 40 additions & 0 deletions docs/adr/0015-allow-model-access-logic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Allow org.jabref.model to access org.jabref.logic

## Context and Problem Statement

- How to create a maintainable architecture?
- How to split model, logic, and UI

## Decision Drivers

- New comers should find the architecture "split" natural
- The architecture should be a help (and not a burden)

## Considered Options

- `org.jabref.model` uses `org.jabref.model` (and external libraries) only
- `org.jabref.model` may use `org.jabref.logic` in defined cases
- `org.jabref.model` and `org.jabref.logic` may access each other freely

## Decision Outcome

Chosen option: "`org.jabref.model` may use `org.jabref.logic` in defined cases", because comes out best \(see below\).

## Pros and Cons of the Options

### `org.jabref.model` uses `org.jabref.model` (and external libraries) only

- Good, because clear separation of model and logic
- Bad, because this leads to an [Anemic Domain Model](https://martinfowler.com/bliki/AnemicDomainModel.html)

### `org.jabref.model` may use `org.jabref.logic` in defined cases

- Good, because model and logic are still separated
- Neutral, because each exception has to be discussed and agreed
- Bad, because new comers have to be informed that there are certain (agreed) exceptions for model to access logic

### `org.jabref.model` and `org.jabref.logic` may access each other freely

- Bad, because may lead to spaghetti code
- Bad, because coupling between model and logic is increased
- Bad, because cohesion inside model is decreased
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
import org.jabref.logic.importer.OpenDatabase;
import org.jabref.logic.importer.ParserResult;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.shared.DatabaseNotSupportedException;
import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException;
import org.jabref.logic.shared.exception.NotASharedDatabaseException;
import org.jabref.model.database.shared.DatabaseNotSupportedException;
import org.jabref.preferences.JabRefPreferences;

import impl.org.controlsfx.skin.DecorationPane;
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/jabref/architecture/AllowedToUseLogic.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.jabref.architecture;

/**
* Annotation to indicate that this logic class can be used in the model package.
*/
public @interface AllowedToUseLogic {

// The rationale
String value();
}
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/cli/AuxCommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
import java.nio.file.Path;

import org.jabref.gui.auximport.AuxParserResultViewModel;
import org.jabref.logic.auxparser.AuxParser;
import org.jabref.logic.auxparser.AuxParserResult;
import org.jabref.logic.auxparser.DefaultAuxParser;
import org.jabref.model.auxparser.AuxParser;
import org.jabref.model.auxparser.AuxParserResult;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.strings.StringUtil;

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.pdf.FileAnnotationCache;
import org.jabref.logic.search.SearchQuery;
import org.jabref.logic.shared.DatabaseLocation;
import org.jabref.logic.util.UpdateField;
import org.jabref.model.FieldChange;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
import org.jabref.model.database.event.EntriesAddedEvent;
import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.database.shared.DatabaseLocation;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.event.EntriesEventSource;
import org.jabref.model.entry.event.EntryChangedEvent;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,13 @@
import org.jabref.logic.importer.ParserResult;
import org.jabref.logic.importer.WebFetchers;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.shared.DatabaseLocation;
import org.jabref.logic.undo.AddUndoableActionEvent;
import org.jabref.logic.undo.UndoChangeEvent;
import org.jabref.logic.undo.UndoRedoEvent;
import org.jabref.logic.util.OS;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.shared.DatabaseLocation;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.SpecialField;
import org.jabref.model.entry.field.StandardField;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.jabref.gui.auximport;

import org.jabref.logic.auxparser.AuxParserResult;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.auxparser.AuxParserResult;

public class AuxParserResultViewModel {

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/auximport/FromAuxDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.auxparser.AuxParser;
import org.jabref.logic.auxparser.AuxParserResult;
import org.jabref.logic.auxparser.DefaultAuxParser;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.StandardFileType;
import org.jabref.model.auxparser.AuxParser;
import org.jabref.model.auxparser.AuxParserResult;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.preferences.PreferencesService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import org.jabref.Globals;
import org.jabref.gui.BasePanel;
import org.jabref.gui.util.BaseDialog;
import org.jabref.logic.citationkeypattern.AbstractCitationKeyPattern;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.bibtexkeypattern.AbstractCitationKeyPattern;
import org.jabref.model.metadata.MetaData;

public class CitationKeyPatternDialog extends BaseDialog<Void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
import org.jabref.gui.actions.ActionFactory;
import org.jabref.gui.actions.StandardActions;
import org.jabref.gui.help.HelpAction;
import org.jabref.logic.citationkeypattern.AbstractCitationKeyPattern;
import org.jabref.logic.citationkeypattern.DatabaseCitationKeyPattern;
import org.jabref.logic.help.HelpFile;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.bibtexkeypattern.AbstractCitationKeyPattern;
import org.jabref.model.bibtexkeypattern.DatabaseCitationKeyPattern;
import org.jabref.model.database.BibDatabaseMode;
import org.jabref.model.entry.BibEntryType;
import org.jabref.model.entry.types.EntryType;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/cleanup/CleanupDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import org.jabref.logic.cleanup.CleanupPreset;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.metadata.FilePreferences;
import org.jabref.preferences.FilePreferences;

public class CleanupDialog extends BaseDialog<CleanupPreset> {

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@

import org.jabref.gui.commonfxcontrols.FieldFormatterCleanupsPanel;
import org.jabref.logic.cleanup.CleanupPreset;
import org.jabref.logic.cleanup.FieldFormatterCleanups;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.cleanup.FieldFormatterCleanups;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.metadata.FilePreferences;
import org.jabref.preferences.FilePreferences;

import com.airhacks.afterburner.views.ViewLoader;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.util.ValueTableCellFactory;
import org.jabref.logic.citationkeypattern.AbstractCitationKeyPattern;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.bibtexkeypattern.AbstractCitationKeyPattern;
import org.jabref.model.entry.BibEntryType;
import org.jabref.model.entry.types.EntryType;
import org.jabref.preferences.JabRefPreferences;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import javafx.beans.property.SimpleObjectProperty;
import javafx.collections.FXCollections;

import org.jabref.logic.citationkeypattern.AbstractCitationKeyPattern;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.bibtexkeypattern.AbstractCitationKeyPattern;
import org.jabref.model.entry.BibEntryType;
import org.jabref.model.entry.types.EntryType;
import org.jabref.preferences.JabRefPreferences;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
import org.jabref.gui.util.FieldsUtil;
import org.jabref.gui.util.ValueTableCellFactory;
import org.jabref.gui.util.ViewModelListCellFactory;
import org.jabref.logic.cleanup.FieldFormatterCleanup;
import org.jabref.logic.cleanup.Formatter;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.cleanup.FieldFormatterCleanup;
import org.jabref.model.cleanup.Formatter;
import org.jabref.model.entry.field.Field;

import com.airhacks.afterburner.views.ViewLoader;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
import org.jabref.Globals;
import org.jabref.gui.util.NoSelectionModel;
import org.jabref.logic.cleanup.Cleanups;
import org.jabref.model.cleanup.FieldFormatterCleanup;
import org.jabref.model.cleanup.Formatter;
import org.jabref.logic.cleanup.FieldFormatterCleanup;
import org.jabref.logic.cleanup.Formatter;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
import org.jabref.logic.exporter.SavePreferences;
import org.jabref.logic.l10n.Encodings;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.shared.DatabaseLocation;
import org.jabref.logic.shared.prefs.SharedDatabasePreferences;
import org.jabref.logic.util.StandardFileType;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.ChangePropagation;
import org.jabref.model.database.shared.DatabaseLocation;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.preferences.JabRefPreferences;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableFieldChange;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.logic.bibtex.FileFieldWriter;
import org.jabref.logic.util.io.AutoLinkPreferences;
import org.jabref.logic.util.io.FileFinder;
import org.jabref.logic.util.io.FileFinders;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.FileFieldWriter;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.metadata.FilePreferences;
import org.jabref.model.util.FileHelper;
import org.jabref.preferences.FilePreferences;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.metadata.FilePreferences;
import org.jabref.preferences.FilePreferences;

public class ExternalFilesEntryLinker {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import java.util.TreeSet;

import org.jabref.Globals;
import org.jabref.model.entry.FileFieldWriter;
import org.jabref.logic.bibtex.FileFieldWriter;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.strings.StringUtil;
import org.jabref.model.util.FileHelper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
import de.saxsys.mvvmfx.utils.validation.Validator;
import org.apache.commons.lang3.StringUtils;
import org.controlsfx.control.textfield.AutoCompletionBinding;

public class AbstractEditorViewModel extends AbstractViewModel {
Expand Down Expand Up @@ -66,10 +65,10 @@ public void bindToEntry(BibEntry entry) {
newValue -> {
if (newValue != null) {
// Controlsfx uses hardcoded \n for multiline fields, but JabRef stores them in OS Newlines format
String oldValue = entry.getField(field).map(value -> value.replace(OS.NEWLINE, "\n")).orElse(null);
String oldValue = entry.getField(field).map(value -> value.replace(OS.NEWLINE, "\n").trim()).orElse(null);
// Autosave and save action trigger the entry editor to reload the fields, so we have to
// check for changes here, otherwise the cursor position is annoyingly reset every few seconds
if (!(newValue.trim()).equals(StringUtils.trim(oldValue))) {
if (!(newValue.trim()).equals(oldValue)) {
entry.setField(field, newValue);
UndoManager undoManager = JabRefGUI.getMainFrame().getUndoManager();
undoManager.addEdit(new UndoableFieldChange(entry, field, oldValue, newValue));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.metadata.FilePreferences;
import org.jabref.model.strings.StringUtil;
import org.jabref.model.util.FileHelper;
import org.jabref.model.util.OptionalUtil;
import org.jabref.preferences.FilePreferences;

import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@
import org.jabref.gui.util.BindingsHelper;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.bibtex.FileFieldWriter;
import org.jabref.logic.importer.FulltextFetchers;
import org.jabref.logic.importer.util.FileFieldParser;
import org.jabref.logic.integrity.FieldCheckers;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.FileFieldParser;
import org.jabref.model.entry.FileFieldWriter;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.entry.field.Field;
import org.jabref.model.util.FileHelper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
import javafx.scene.control.Tooltip;

import org.jabref.Globals;
import org.jabref.logic.cleanup.Formatter;
import org.jabref.logic.formatter.Formatters;
import org.jabref.logic.formatter.casechanger.ProtectTermsFormatter;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.cleanup.Formatter;

class CaseChangeMenu extends Menu {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
import javafx.scene.control.Menu;
import javafx.scene.control.Tooltip;

import org.jabref.logic.cleanup.Formatter;
import org.jabref.logic.formatter.Formatters;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.cleanup.Formatter;

/**
* Menu to show up on right-click in a text field for converting text formats
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
import javafx.scene.control.TextInputControl;

import org.jabref.Globals;
import org.jabref.logic.cleanup.Formatter;
import org.jabref.logic.formatter.casechanger.ProtectTermsFormatter;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.protectedterms.ProtectedTermsList;
import org.jabref.logic.protectedterms.ProtectedTermsLoader;
import org.jabref.model.cleanup.Formatter;

class ProtectedTermsMenu extends Menu {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,12 @@
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.metadata.FilePreferences;
import org.jabref.model.util.FileUpdateMonitor;
import org.jabref.preferences.FilePreferences;
import org.jabref.preferences.PreferencesService;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class ImportEntriesViewModel extends AbstractViewModel {

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

private final StringProperty message;
private final TaskExecutor taskExecutor;
private final BibDatabaseContext databaseContext;
Expand Down
Loading