From 6431099fe7d5d90da678a78051f12894da82c68d Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Fri, 20 Dec 2019 13:46:59 +0100 Subject: [PATCH] Improve things arround change detection (#5770) * Dismiss changes hides change message * Show added entries in change dialog * Improve message when current value is null * Remove unused reference file * Apply post-load actions before comparing with currently load database --- src/main/java/org/jabref/gui/BasePanel.java | 8 ---- .../gui/collab/ChangeDisplayDialog.java | 5 ++- .../org/jabref/gui/collab/ChangeScanner.java | 6 +-- .../gui/collab/DatabaseChangeMonitor.java | 18 -------- .../jabref/gui/collab/DatabaseChangePane.java | 9 ++-- .../gui/collab/EntryAddChangeViewModel.java | 18 +++++--- .../gui/collab/EntryChangeViewModel.java | 6 ++- .../gui/exporter/SaveDatabaseAction.java | 2 - .../actions/AppendDatabaseAction.java | 2 +- .../org/jabref/gui/preview/PreviewViewer.java | 2 +- .../logic/importer/ImportFormatReader.java | 2 +- .../jabref/logic/importer/OpenDatabase.java | 29 ++++++------ src/main/resources/l10n/JabRef_en.properties | 2 +- .../logic/importer/OpenDatabaseTest.java | 45 +++++++++---------- 14 files changed, 64 insertions(+), 90 deletions(-) diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java index c0c9731f703..ff63746c6cc 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/src/main/java/org/jabref/gui/BasePanel.java @@ -991,10 +991,6 @@ public BibDatabaseContext getBibDatabaseContext() { return this.bibDatabaseContext; } - public void markExternalChangesAsResolved() { - changeMonitor.ifPresent(DatabaseChangeMonitor::markExternalChangesAsResolved); - } - public SidePaneManager getSidePaneManager() { return sidePaneManager; } @@ -1059,10 +1055,6 @@ public void resetChangeMonitorAndChangePane() { this.getChildren().setAll(changePane); } - public void updateTimeStamp() { - changeMonitor.ifPresent(DatabaseChangeMonitor::markAsSaved); - } - public void copy() { mainTable.copy(); } diff --git a/src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java b/src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java index 1a522953e24..30398bfa585 100644 --- a/src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java +++ b/src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java @@ -42,13 +42,14 @@ public ChangeDisplayDialog(BibDatabaseContext database, List { - if (button == ButtonType.CANCEL) { + if (button == dismissChanges) { return false; } else { // Perform all accepted changes diff --git a/src/main/java/org/jabref/gui/collab/ChangeScanner.java b/src/main/java/org/jabref/gui/collab/ChangeScanner.java index 65316188db5..424018d9080 100644 --- a/src/main/java/org/jabref/gui/collab/ChangeScanner.java +++ b/src/main/java/org/jabref/gui/collab/ChangeScanner.java @@ -10,8 +10,8 @@ import org.jabref.logic.bibtex.comparator.BibEntryDiff; import org.jabref.logic.bibtex.comparator.BibStringDiff; import org.jabref.logic.importer.ImportFormatPreferences; +import org.jabref.logic.importer.OpenDatabase; import org.jabref.logic.importer.ParserResult; -import org.jabref.logic.importer.fileformat.BibtexImporter; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.util.DummyFileUpdateMonitor; @@ -36,9 +36,9 @@ public List scanForChanges() { List changes = new ArrayList<>(); // Parse the modified file + // Important: apply all post-load actions ImportFormatPreferences importFormatPreferences = Globals.prefs.getImportFormatPreferences(); - ParserResult result = new BibtexImporter(importFormatPreferences, new DummyFileUpdateMonitor()) - .importDatabase(database.getDatabasePath().get(), importFormatPreferences.getEncoding()); + ParserResult result = OpenDatabase.loadDatabase(database.getDatabasePath().get(), importFormatPreferences, new DummyFileUpdateMonitor()); BibDatabaseContext databaseOnDisk = result.getDatabaseContext(); // Start looking at changes. diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java index 0db0316af73..f43a5b0cc54 100644 --- a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java +++ b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java @@ -1,14 +1,11 @@ package org.jabref.gui.collab; import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.TaskExecutor; -import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.util.FileUpdateListener; import org.jabref.model.util.FileUpdateMonitor; @@ -22,7 +19,6 @@ public class DatabaseChangeMonitor implements FileUpdateListener { private final BibDatabaseContext database; private final FileUpdateMonitor fileMonitor; private final List listeners; - private Path referenceFile; private TaskExecutor taskExecutor; public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor fileMonitor, TaskExecutor taskExecutor) { @@ -34,9 +30,6 @@ public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor file this.database.getDatabasePath().ifPresent(path -> { try { fileMonitor.addListenerForFile(path, this); - referenceFile = Files.createTempFile("jabref", ".bib"); - referenceFile.toFile().deleteOnExit(); - setAsReference(path); } catch (IOException e) { LOGGER.error("Error while trying to monitor " + path, e); } @@ -64,15 +57,4 @@ public void unregister() { database.getDatabasePath().ifPresent(file -> fileMonitor.removeListener(file, this)); } - public void markExternalChangesAsResolved() { - markAsSaved(); - } - - public void markAsSaved() { - database.getDatabasePath().ifPresent(this::setAsReference); - } - - private void setAsReference(Path file) { - FileUtil.copyFile(file, referenceFile, true); - } } diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangePane.java b/src/main/java/org/jabref/gui/collab/DatabaseChangePane.java index 17582a41ba9..a879f81489a 100644 --- a/src/main/java/org/jabref/gui/collab/DatabaseChangePane.java +++ b/src/main/java/org/jabref/gui/collab/DatabaseChangePane.java @@ -30,16 +30,13 @@ public DatabaseChangePane(Node parent, BibDatabaseContext database, DatabaseChan private void onDatabaseChanged(List changes) { this.getActions().setAll( new Action(Localization.lang("Dismiss changes"), event -> { - monitor.markExternalChangesAsResolved(); this.hide(); }), new Action(Localization.lang("Review changes"), event -> { ChangeDisplayDialog changeDialog = new ChangeDisplayDialog(database, changes); - boolean changesHandled = changeDialog.showAndWait().orElse(false); - if (changesHandled) { - monitor.markExternalChangesAsResolved(); - this.hide(); - } + changeDialog.showAndWait(); + + this.hide(); })); this.show(); } diff --git a/src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java b/src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java index 45832985b5b..cb41f3d4d0a 100644 --- a/src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java +++ b/src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java @@ -13,23 +13,27 @@ class EntryAddChangeViewModel extends DatabaseChangeViewModel { - private final BibEntry diskEntry; + private final BibEntry entry; - public EntryAddChangeViewModel(BibEntry diskEntry) { - super(Localization.lang("Added entry")); - this.diskEntry = diskEntry; + public EntryAddChangeViewModel(BibEntry entry) { + super(); + this.name = entry.getCiteKeyOptional() + .map(key -> Localization.lang("Added entry") + ": '" + key + '\'') + .orElse(Localization.lang("Added entry")); + this.entry = entry; } @Override public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) { - database.getDatabase().insertEntry(diskEntry); - undoEdit.addEdit(new UndoableInsertEntry(database.getDatabase(), diskEntry)); + database.getDatabase().insertEntry(entry); + undoEdit.addEdit(new UndoableInsertEntry(database.getDatabase(), entry)); } @Override public Node description() { PreviewViewer previewViewer = new PreviewViewer(new BibDatabaseContext(), JabRefGUI.getMainFrame().getDialogService(), Globals.stateManager); - previewViewer.setEntry(diskEntry); + previewViewer.setLayout(Globals.prefs.getPreviewPreferences().getCurrentPreviewStyle()); + previewViewer.setEntry(entry); return previewViewer; } } diff --git a/src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java b/src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java index 65631c0a26c..e1126a6a918 100644 --- a/src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java +++ b/src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java @@ -116,7 +116,11 @@ public Node description() { container.getChildren().add(new Label(Localization.lang("Value cleared externally"))); } - container.getChildren().add(new Label(Localization.lang("Current value") + ": " + value)); + if (StringUtil.isNotBlank(value)) { + container.getChildren().add(new Label(Localization.lang("Current value") + ": " + value)); + } else { + container.getChildren().add(new Label(Localization.lang("Current value") + ": " + Localization.lang("empty"))); + } return container; } diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index f3b55ea48ec..f131795a608 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -135,14 +135,12 @@ private boolean doSave() { SavePreferences.DatabaseSaveType.ALL); if (success) { - panel.updateTimeStamp(); panel.getUndoManager().markUnchanged(); // (Only) after a successful save the following // statement marks that the base is unchanged // since last save: panel.setNonUndoableChange(false); panel.setBaseChanged(false); - panel.markExternalChangesAsResolved(); // Reset title of tab frame.setTabTitle(panel, panel.getTabTitle(), diff --git a/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java index 5e9d0fa323d..3e066cdece8 100644 --- a/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java @@ -179,7 +179,7 @@ private String openIt(Path file, boolean importEntries, boolean importStrings, b boolean importSelectorWords) throws IOException, KeyCollisionException { Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent().toString()); // Should this be done _after_ we know it was successfully opened? - ParserResult parserResult = OpenDatabase.loadDatabase(file.toFile(), + ParserResult parserResult = OpenDatabase.loadDatabase(file, Globals.prefs.getImportFormatPreferences(), Globals.getFileUpdateMonitor()); AppendDatabaseAction.mergeFromBibtex(panel, parserResult, importEntries, importStrings, importGroups, importSelectorWords); diff --git a/src/main/java/org/jabref/gui/preview/PreviewViewer.java b/src/main/java/org/jabref/gui/preview/PreviewViewer.java index 937a16b987a..7f154c6e78d 100644 --- a/src/main/java/org/jabref/gui/preview/PreviewViewer.java +++ b/src/main/java/org/jabref/gui/preview/PreviewViewer.java @@ -173,7 +173,7 @@ public void setEntry(BibEntry newEntry) { } private void update() { - if (!entry.isPresent() || layout == null) { + if (entry.isEmpty() || layout == null) { // Nothing to do return; } diff --git a/src/main/java/org/jabref/logic/importer/ImportFormatReader.java b/src/main/java/org/jabref/logic/importer/ImportFormatReader.java index 3451c839e1e..aa7e0207e7d 100644 --- a/src/main/java/org/jabref/logic/importer/ImportFormatReader.java +++ b/src/main/java/org/jabref/logic/importer/ImportFormatReader.java @@ -170,7 +170,7 @@ public UnknownFormatImport importUnknownFormat(Path filePath, FileUpdateMonitor // First, see if it is a BibTeX file: try { - ParserResult parserResult = OpenDatabase.loadDatabase(filePath.toFile(), importFormatPreferences, fileMonitor); + ParserResult parserResult = OpenDatabase.loadDatabase(filePath, importFormatPreferences, fileMonitor); if (parserResult.getDatabase().hasEntries() || !parserResult.getDatabase().hasNoStrings()) { parserResult.setFile(filePath.toFile()); return new UnknownFormatImport(ImportFormatReader.BIBTEX_FORMAT, parserResult); diff --git a/src/main/java/org/jabref/logic/importer/OpenDatabase.java b/src/main/java/org/jabref/logic/importer/OpenDatabase.java index 331245da777..d05900c92a5 100644 --- a/src/main/java/org/jabref/logic/importer/OpenDatabase.java +++ b/src/main/java/org/jabref/logic/importer/OpenDatabase.java @@ -1,7 +1,9 @@ package org.jabref.logic.importer; -import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Arrays; import java.util.List; @@ -29,31 +31,26 @@ private OpenDatabase() { * * @param name Name of the BIB-file to open * @return ParserResult which never is null + * @deprecated use {@link #loadDatabase(Path, ImportFormatPreferences, FileUpdateMonitor)} instead */ + @Deprecated public static ParserResult loadDatabase(String name, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) { - File file = new File(name); - LOGGER.info("Opening: " + name); + LOGGER.debug("Opening: " + name); + Path file = Paths.get(name); - if (!file.exists()) { + if (!Files.exists(file)) { ParserResult pr = ParserResult.fromErrorMessage(Localization.lang("File not found")); - pr.setFile(file); + pr.setFile(file.toFile()); LOGGER.error(Localization.lang("Error") + ": " + Localization.lang("File not found")); return pr; } try { - ParserResult pr = OpenDatabase.loadDatabase(file, importFormatPreferences, fileMonitor); - pr.setFile(file); - if (pr.hasWarnings()) { - for (String aWarn : pr.warnings()) { - LOGGER.warn(aWarn); - } - } - return pr; + return OpenDatabase.loadDatabase(file, importFormatPreferences, fileMonitor); } catch (IOException ex) { ParserResult pr = ParserResult.fromError(ex); - pr.setFile(file); + pr.setFile(file.toFile()); LOGGER.error("Problem opening .bib-file", ex); return pr; } @@ -62,9 +59,9 @@ public static ParserResult loadDatabase(String name, ImportFormatPreferences imp /** * Opens a new database. */ - public static ParserResult loadDatabase(File fileToOpen, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) + public static ParserResult loadDatabase(Path fileToOpen, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) throws IOException { - ParserResult result = new BibtexImporter(importFormatPreferences, fileMonitor).importDatabase(fileToOpen.toPath(), + ParserResult result = new BibtexImporter(importFormatPreferences, fileMonitor).importDatabase(fileToOpen, importFormatPreferences.getEncoding()); if (importFormatPreferences.isKeywordSyncEnabled()) { diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 57f342b16ec..f970b98dade 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -2094,4 +2094,4 @@ Reset\ All=Reset All Column\ type\ %0\ is\ unknown.=Column type %0 is unknown. Linked\ identifiers=Linked identifiers Special\ field\ type\ %0\ is\ unknown.\ Using\ normal\ column\ type.=Special field type %0 is unknown. Using normal column type. - +empty=empty diff --git a/src/test/java/org/jabref/logic/importer/OpenDatabaseTest.java b/src/test/java/org/jabref/logic/importer/OpenDatabaseTest.java index eb2f326a13c..ddc83596db2 100644 --- a/src/test/java/org/jabref/logic/importer/OpenDatabaseTest.java +++ b/src/test/java/org/jabref/logic/importer/OpenDatabaseTest.java @@ -1,10 +1,10 @@ package org.jabref.logic.importer; -import java.io.File; import java.io.IOException; import java.net.URISyntaxException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collection; import java.util.Optional; @@ -23,61 +23,60 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class OpenDatabaseTest { +class OpenDatabaseTest { private final Charset defaultEncoding = StandardCharsets.UTF_8; private ImportFormatPreferences importFormatPreferences; - private final File bibNoHeader; - private final File bibWrongHeader; - private final File bibHeader; - private final File bibHeaderAndSignature; - private final File bibEncodingWithoutNewline; + private final Path bibNoHeader; + private final Path bibWrongHeader; + private final Path bibHeader; + private final Path bibHeaderAndSignature; + private final Path bibEncodingWithoutNewline; private final FileUpdateMonitor fileMonitor = new DummyFileUpdateMonitor(); - public OpenDatabaseTest() throws URISyntaxException { - bibNoHeader = Paths.get(OpenDatabaseTest.class.getResource("headerless.bib").toURI()).toFile(); - bibWrongHeader = Paths.get(OpenDatabaseTest.class.getResource("wrong-header.bib").toURI()).toFile(); - bibHeader = Paths.get(OpenDatabaseTest.class.getResource("encoding-header.bib").toURI()).toFile(); - bibHeaderAndSignature = Paths.get(OpenDatabaseTest.class.getResource("jabref-header.bib").toURI()) - .toFile(); + OpenDatabaseTest() throws URISyntaxException { + bibNoHeader = Paths.get(OpenDatabaseTest.class.getResource("headerless.bib").toURI()); + bibWrongHeader = Paths.get(OpenDatabaseTest.class.getResource("wrong-header.bib").toURI()); + bibHeader = Paths.get(OpenDatabaseTest.class.getResource("encoding-header.bib").toURI()); + bibHeaderAndSignature = Paths.get(OpenDatabaseTest.class.getResource("jabref-header.bib").toURI()); bibEncodingWithoutNewline = Paths - .get(OpenDatabaseTest.class.getResource("encodingWithoutNewline.bib").toURI()).toFile(); + .get(OpenDatabaseTest.class.getResource("encodingWithoutNewline.bib").toURI()); } @BeforeEach - public void setUp() { + void setUp() { importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS); when(importFormatPreferences.getEncoding()).thenReturn(StandardCharsets.UTF_8); } @Test - public void useFallbackEncodingIfNoHeader() throws IOException { + void useFallbackEncodingIfNoHeader() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibNoHeader, importFormatPreferences, fileMonitor); assertEquals(defaultEncoding, result.getMetaData().getEncoding().get()); } @Test - public void useFallbackEncodingIfUnknownHeader() throws IOException { + void useFallbackEncodingIfUnknownHeader() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibWrongHeader, importFormatPreferences, fileMonitor); assertEquals(defaultEncoding, result.getMetaData().getEncoding().get()); } @Test - public void useSpecifiedEncoding() throws IOException { + void useSpecifiedEncoding() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibHeader, importFormatPreferences.withEncoding(StandardCharsets.US_ASCII), fileMonitor); assertEquals(defaultEncoding, result.getMetaData().getEncoding().get()); } @Test - public void useSpecifiedEncodingWithSignature() throws IOException { + void useSpecifiedEncodingWithSignature() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibHeaderAndSignature, importFormatPreferences.withEncoding(StandardCharsets.US_ASCII), fileMonitor); assertEquals(defaultEncoding, result.getMetaData().getEncoding().get()); } @Test - public void entriesAreParsedNoHeader() throws IOException { + void entriesAreParsedNoHeader() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibNoHeader, importFormatPreferences, fileMonitor); BibDatabase db = result.getDatabase(); @@ -87,7 +86,7 @@ public void entriesAreParsedNoHeader() throws IOException { } @Test - public void entriesAreParsedHeader() throws IOException { + void entriesAreParsedHeader() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibHeader, importFormatPreferences, fileMonitor); BibDatabase db = result.getDatabase(); @@ -97,7 +96,7 @@ public void entriesAreParsedHeader() throws IOException { } @Test - public void entriesAreParsedHeaderAndSignature() throws IOException { + void entriesAreParsedHeaderAndSignature() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibHeaderAndSignature, importFormatPreferences, fileMonitor); BibDatabase db = result.getDatabase(); @@ -110,7 +109,7 @@ public void entriesAreParsedHeaderAndSignature() throws IOException { * Test for #669 */ @Test - public void correctlyParseEncodingWithoutNewline() throws IOException { + void correctlyParseEncodingWithoutNewline() throws IOException { ParserResult result = OpenDatabase.loadDatabase(bibEncodingWithoutNewline, importFormatPreferences, fileMonitor); assertEquals(StandardCharsets.US_ASCII, result.getMetaData().getEncoding().get());