From bdf106154dafffbc99804b1e714995fb33c6e075 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 1 Sep 2018 11:40:59 +0200 Subject: [PATCH 1/9] Make FileHistory use Paths instead of strings --- .../org/jabref/gui/menus/FileHistoryMenu.java | 30 ++++---- .../org/jabref/logic/util/io/FileHistory.java | 32 ++++----- .../jabref/logic/util/io/FileHistoryTest.java | 69 +++++++++---------- 3 files changed, 58 insertions(+), 73 deletions(-) diff --git a/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java b/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java index 83cd9613807..a5b8d48454a 100644 --- a/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java +++ b/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java @@ -2,7 +2,6 @@ import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import javafx.scene.control.Menu; import javafx.scene.control.MenuItem; @@ -38,26 +37,24 @@ public FileHistoryMenu(JabRefPreferences preferences, JabRefFrame frame) { /** * Adds the filename to the top of the menu. If it already is in * the menu, it is merely moved to the top. - * - * @param filename a String value */ - public void newFile(String filename) { - history.newFile(filename); + public void newFile(Path file) { + history.newFile(file); setItems(); setDisable(false); } private void setItems() { getItems().clear(); - for (int count = 0; count < history.size(); count++) { - addItem(history.getFileName(count), count + 1); + for (int index = 0; index < history.size(); index++) { + addItem(history.getFileAt(index), index + 1); } } - private void addItem(String fileName, int num) { + private void addItem(Path file, int num) { String number = Integer.toString(num); - MenuItem item = new MenuItem(number + ". " + fileName); - item.setOnAction(event -> openFile(fileName)); + MenuItem item = new MenuItem(number + ". " + file); + item.setOnAction(event -> openFile(file)); getItems().add(item); } @@ -65,19 +62,16 @@ public void storeHistory() { preferences.storeFileHistory(history); } - public void openFile(String fileName) { - final Path fileToOpen = Paths.get(fileName); - - // the existence check has to be done here (and not in open.openIt) as we have to call "removeItem" if the file does not exist - if (!Files.exists(fileToOpen)) { + public void openFile(Path file) { + if (!Files.exists(file)) { dialogService.showErrorDialogAndWait( Localization.lang("File not found"), - Localization.lang("File not found") + ": " + fileName); - history.removeItem(fileName); + Localization.lang("File not found") + ": " + file); + history.removeItem(file); setItems(); return; } - JabRefExecutorService.INSTANCE.execute(() -> frame.getOpenDatabaseAction().openFile(fileToOpen, true)); + JabRefExecutorService.INSTANCE.execute(() -> frame.getOpenDatabaseAction().openFile(file, true)); } diff --git a/src/main/java/org/jabref/logic/util/io/FileHistory.java b/src/main/java/org/jabref/logic/util/io/FileHistory.java index 387dee3347c..0cb63cd4ca4 100644 --- a/src/main/java/org/jabref/logic/util/io/FileHistory.java +++ b/src/main/java/org/jabref/logic/util/io/FileHistory.java @@ -1,5 +1,7 @@ package org.jabref.logic.util.io; +import java.nio.file.Path; +import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -8,11 +10,10 @@ public class FileHistory { private static final int HISTORY_SIZE = 8; - private final LinkedList history; + private final LinkedList history; - - public FileHistory(List fileList) { - history = new LinkedList<>(Objects.requireNonNull(fileList)); + public FileHistory(List files) { + history = new LinkedList<>(Objects.requireNonNull(files)); } public int size() { @@ -24,28 +25,25 @@ public boolean isEmpty() { } /** - * Adds the filename to the top of the list. If it already is in the list, it is merely moved to the top. - * - * @param filename a String value + * Adds the file to the top of the list. If it already is in the list, it is merely moved to the top. */ - - public void newFile(String filename) { - removeItem(filename); - history.addFirst(filename); + public void newFile(Path file) { + removeItem(file); + history.addFirst(file); while (size() > HISTORY_SIZE) { history.removeLast(); } } - public String getFileName(int i) { - return history.get(i); + public Path getFileAt(int index) { + return history.get(index); } - public void removeItem(String filename) { - history.remove(filename); + public void removeItem(Path file) { + history.remove(file); } - public List getHistory() { - return history; + public List getHistory() { + return Collections.unmodifiableList(history); } } diff --git a/src/test/java/org/jabref/logic/util/io/FileHistoryTest.java b/src/test/java/org/jabref/logic/util/io/FileHistoryTest.java index 715e461dee9..af895b2bcfc 100644 --- a/src/test/java/org/jabref/logic/util/io/FileHistoryTest.java +++ b/src/test/java/org/jabref/logic/util/io/FileHistoryTest.java @@ -1,52 +1,45 @@ package org.jabref.logic.util.io; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; -public class FileHistoryTest { +class FileHistoryTest { + private FileHistory history; + + @BeforeEach + void setUp() { + history = new FileHistory(new ArrayList<>()); + } + + @Test + void newItemsAreAddedInRightOrder() { + history.newFile(Paths.get("aa")); + history.newFile(Paths.get("bb")); + assertEquals(Arrays.asList(Paths.get("bb"), Paths.get("aa")), history.getHistory()); + } @Test - public void testFileHistory() { - FileHistory fh = new FileHistory(new ArrayList<>()); - - fh.newFile("aa"); - assertEquals("aa", fh.getFileName(0)); - fh.newFile("bb"); - assertEquals("bb", fh.getFileName(0)); - - fh.newFile("aa"); - assertEquals("aa", fh.getFileName(0)); - - fh.newFile("cc"); - assertEquals("cc", fh.getFileName(0)); - assertEquals("aa", fh.getFileName(1)); - assertEquals("bb", fh.getFileName(2)); - - fh.newFile("dd"); - assertEquals("dd", fh.getFileName(0)); - assertEquals("cc", fh.getFileName(1)); - assertEquals("aa", fh.getFileName(2)); - fh.newFile("ee"); - fh.newFile("ff"); - fh.newFile("gg"); - fh.newFile("hh"); - assertEquals("bb", fh.getFileName(7)); - assertEquals(8, fh.size()); - fh.newFile("ii"); - assertEquals("aa", fh.getFileName(7)); - fh.removeItem("ff"); - assertEquals(7, fh.size()); - fh.removeItem("ee"); - fh.removeItem("dd"); - fh.removeItem("cc"); - fh.removeItem("cc"); - fh.removeItem("aa"); - - assertEquals(Arrays.asList("ii", "hh", "gg"), fh.getHistory()); + void itemsAlreadyInListIsMovedToTop() { + history.newFile(Paths.get("aa")); + history.newFile(Paths.get("bb")); + history.newFile(Paths.get("aa")); + assertEquals(Arrays.asList(Paths.get("aa"), Paths.get("bb")), history.getHistory()); } + @Test + void removeItemsLeavesOtherItemsInRightOrder() { + history.newFile(Paths.get("aa")); + history.newFile(Paths.get("bb")); + history.newFile(Paths.get("cc")); + + history.removeItem(Paths.get("bb")); + + assertEquals(Arrays.asList(Paths.get("cc"), Paths.get("aa")), history.getHistory()); + } } From a9c31b8ab83f73aa81d949aa0142a39b9ed7500e Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 1 Sep 2018 12:15:47 +0200 Subject: [PATCH 2/9] Almost completely rewrite save infrastructure --- .../org/jabref/benchmarks/Benchmarks.java | 19 +- .../org/jabref/cli/ArgumentProcessor.java | 70 +-- src/main/java/org/jabref/gui/BasePanel.java | 153 +----- src/main/java/org/jabref/gui/JabRefFrame.java | 3 +- .../org/jabref/gui/collab/ChangeScanner.java | 12 +- .../jabref/gui/dialogs/AutosaveUIManager.java | 2 +- .../jabref/gui/exporter/SaveAllAction.java | 27 +- .../gui/exporter/SaveDatabaseAction.java | 478 ++++++------------ .../importer/actions/OpenDatabaseAction.java | 2 +- .../shared/ConnectToSharedDatabaseDialog.java | 2 +- .../autosaveandbackup/BackupManager.java | 11 +- .../exporter/AtomicFileOutputStream.java | 197 ++++++++ .../logic/exporter/AtomicFileWriter.java | 49 ++ .../logic/exporter/BibDatabaseWriter.java | 137 ++--- .../logic/exporter/BibtexDatabaseWriter.java | 173 +++---- .../logic/exporter/FileSaveSession.java | 129 ----- .../jabref/logic/exporter/MSBibExporter.java | 7 +- .../logic/exporter/SavePreferences.java | 24 +- .../jabref/logic/exporter/SaveSession.java | 68 --- .../logic/exporter/StringSaveSession.java | 54 -- .../logic/exporter/TemplateExporter.java | 18 +- .../logic/exporter/VerifyingWriter.java | 52 -- .../org/jabref/logic/util/io/FileUtil.java | 3 +- .../model/database/BibDatabaseContext.java | 4 + .../jabref/preferences/JabRefPreferences.java | 46 +- .../logic/bibtex/BibEntryWriterTest.java | 20 + .../exporter/BibtexDatabaseWriterTest.java | 214 ++++---- .../model/entry/FileFieldBibEntryTest.java | 61 --- 28 files changed, 764 insertions(+), 1271 deletions(-) create mode 100644 src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java create mode 100644 src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java delete mode 100644 src/main/java/org/jabref/logic/exporter/FileSaveSession.java delete mode 100644 src/main/java/org/jabref/logic/exporter/SaveSession.java delete mode 100644 src/main/java/org/jabref/logic/exporter/StringSaveSession.java delete mode 100644 src/main/java/org/jabref/logic/exporter/VerifyingWriter.java delete mode 100644 src/test/java/org/jabref/model/entry/FileFieldBibEntryTest.java diff --git a/src/jmh/java/org/jabref/benchmarks/Benchmarks.java b/src/jmh/java/org/jabref/benchmarks/Benchmarks.java index c261a64589c..efcb7f937c2 100644 --- a/src/jmh/java/org/jabref/benchmarks/Benchmarks.java +++ b/src/jmh/java/org/jabref/benchmarks/Benchmarks.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.io.StringReader; +import java.io.StringWriter; import java.util.List; import java.util.Random; import java.util.stream.Collectors; @@ -9,9 +10,7 @@ import org.jabref.Globals; import org.jabref.logic.exporter.BibtexDatabaseWriter; import org.jabref.logic.exporter.SavePreferences; -import org.jabref.logic.exporter.StringSaveSession; import org.jabref.logic.formatter.bibtexfields.HtmlToLatexFormatter; -import org.jabref.logic.importer.ParseException; import org.jabref.logic.importer.ParserResult; import org.jabref.logic.importer.fileformat.BibtexParser; import org.jabref.logic.layout.format.HTMLChars; @@ -61,11 +60,12 @@ public void init() throws Exception { entry.setField("rnd", "2" + randomizer.nextInt()); database.insertEntry(entry); } - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>(StringSaveSession::new); - StringSaveSession saveSession = databaseWriter.savePartOfDatabase( + StringWriter outputWriter = new StringWriter(); + BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(outputWriter); + databaseWriter.savePartOfDatabase( new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries(), new SavePreferences()); - bibtexString = saveSession.getStringValue(); + bibtexString = outputWriter.toString(); latexConversionString = "{A} \\textbf{bold} approach {\\it to} ${{\\Sigma}}{\\Delta}$ modulator \\textsuperscript{2} \\$"; @@ -80,11 +80,12 @@ public ParserResult parse() throws IOException { @Benchmark public String write() throws Exception { - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>(StringSaveSession::new); - StringSaveSession saveSession = databaseWriter.savePartOfDatabase( + StringWriter outputWriter = new StringWriter(); + BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(outputWriter); + databaseWriter.savePartOfDatabase( new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries(), new SavePreferences()); - return saveSession.getStringValue(); + return outputWriter.toString(); } @Benchmark @@ -125,7 +126,7 @@ public String htmlToLatexConversion() { } @Benchmark - public boolean keywordGroupContains() throws ParseException { + public boolean keywordGroupContains() { KeywordGroup group = new WordKeywordGroup("testGroup", GroupHierarchyType.INDEPENDENT, "keyword", "testkeyword", false, ',', false); return group.containsAll(database.getEntries()); } diff --git a/src/main/java/org/jabref/cli/ArgumentProcessor.java b/src/main/java/org/jabref/cli/ArgumentProcessor.java index 120fd25c775..60def445da7 100644 --- a/src/main/java/org/jabref/cli/ArgumentProcessor.java +++ b/src/main/java/org/jabref/cli/ArgumentProcessor.java @@ -16,14 +16,12 @@ import org.jabref.JabRefException; import org.jabref.gui.externalfiles.AutoSetLinks; import org.jabref.logic.bibtexkeypattern.BibtexKeyGenerator; +import org.jabref.logic.exporter.AtomicFileWriter; import org.jabref.logic.exporter.BibDatabaseWriter; import org.jabref.logic.exporter.BibtexDatabaseWriter; import org.jabref.logic.exporter.Exporter; import org.jabref.logic.exporter.ExporterFactory; -import org.jabref.logic.exporter.FileSaveSession; -import org.jabref.logic.exporter.SaveException; import org.jabref.logic.exporter.SavePreferences; -import org.jabref.logic.exporter.SaveSession; import org.jabref.logic.exporter.TemplateExporter; import org.jabref.logic.importer.FetcherException; import org.jabref.logic.importer.ImportException; @@ -374,27 +372,7 @@ private boolean generateAux(List loaded, String[] data) { // write an output, if something could be resolved if ((newBase != null) && newBase.hasEntries()) { String subName = StringUtil.getCorrectFileName(data[1], "bib"); - - try { - System.out.println(Localization.lang("Saving") + ": " + subName); - SavePreferences prefs = Globals.prefs.loadForSaveFromPreferences(); - BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>(FileSaveSession::new); - Defaults defaults = new Defaults(Globals.prefs.getDefaultBibDatabaseMode()); - SaveSession session = databaseWriter.saveDatabase(new BibDatabaseContext(newBase, defaults), prefs); - - // Show just a warning message if encoding did not work for all characters: - if (!session.getWriter().couldEncodeAll()) { - System.err.println(Localization.lang("Warning") + ": " - + Localization.lang( - "The chosen encoding '%0' could not encode the following characters:", - session.getEncoding().displayName()) - + " " + session.getWriter().getProblemCharacters()); - } - session.commit(subName); - } catch (SaveException ex) { - System.err.println(Localization.lang("Could not save file.") + "\n" + ex.getLocalizedMessage()); - } - + saveDatabase(newBase, subName); notSavedMsg = true; } @@ -407,6 +385,28 @@ private boolean generateAux(List loaded, String[] data) { } } + private void saveDatabase(BibDatabase newBase, String subName) { + try { + System.out.println(Localization.lang("Saving") + ": " + subName); + SavePreferences prefs = Globals.prefs.loadForSaveFromPreferences(); + AtomicFileWriter fileWriter = new AtomicFileWriter(Paths.get(subName), prefs.getEncoding()); + BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter); + Defaults defaults = new Defaults(Globals.prefs.getDefaultBibDatabaseMode()); + databaseWriter.saveDatabase(new BibDatabaseContext(newBase, defaults), prefs); + + // Show just a warning message if encoding did not work for all characters: + if (fileWriter.hasEncodingProblems()) { + System.err.println(Localization.lang("Warning") + ": " + + Localization.lang( + "The chosen encoding '%0' could not encode the following characters:", + prefs.getEncoding().displayName()) + + " " + fileWriter.getEncodingProblems()); + } + } catch (IOException ex) { + System.err.println(Localization.lang("Could not save file.") + "\n" + ex.getLocalizedMessage()); + } + } + private void exportFile(List loaded, String[] data) { if (data.length == 1) { // This signals that the latest import should be stored in BibTeX @@ -414,27 +414,7 @@ private void exportFile(List loaded, String[] data) { if (!loaded.isEmpty()) { ParserResult pr = loaded.get(loaded.size() - 1); if (!pr.isInvalid()) { - try { - System.out.println(Localization.lang("Saving") + ": " + data[0]); - SavePreferences prefs = Globals.prefs.loadForSaveFromPreferences(); - Defaults defaults = new Defaults(Globals.prefs.getDefaultBibDatabaseMode()); - BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>( - FileSaveSession::new); - SaveSession session = databaseWriter.saveDatabase( - new BibDatabaseContext(pr.getDatabase(), pr.getMetaData(), defaults), prefs); - - // Show just a warning message if encoding did not work for all characters: - if (!session.getWriter().couldEncodeAll()) { - System.err.println(Localization.lang("Warning") + ": " - + Localization.lang( - "The chosen encoding '%0' could not encode the following characters:", - session.getEncoding().displayName()) - + " " + session.getWriter().getProblemCharacters()); - } - session.commit(data[0]); - } catch (SaveException ex) { - System.err.println(Localization.lang("Could not save file.") + "\n" + ex.getLocalizedMessage()); - } + saveDatabase(pr.getDatabase(), data[0]); } } else { System.err.println(Localization.lang("The output option depends on a valid import option.")); diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java index 229ee4ad24b..c19d76d2ddc 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/src/main/java/org/jabref/gui/BasePanel.java @@ -4,8 +4,6 @@ import java.io.IOException; import java.io.StringReader; import java.lang.reflect.InvocationTargetException; -import java.nio.charset.Charset; -import java.nio.charset.UnsupportedCharsetException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; @@ -18,8 +16,6 @@ import java.util.Set; import java.util.stream.Collectors; -import javax.swing.JOptionPane; -import javax.swing.JTextArea; import javax.swing.SwingUtilities; import javax.swing.undo.CannotRedoException; import javax.swing.undo.CannotUndoException; @@ -74,27 +70,17 @@ import org.jabref.gui.undo.UndoableChangeType; import org.jabref.gui.undo.UndoableFieldChange; import org.jabref.gui.undo.UndoableInsertEntry; -import org.jabref.gui.undo.UndoableKeyChange; import org.jabref.gui.undo.UndoableRemoveEntry; import org.jabref.gui.util.DefaultTaskExecutor; -import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.gui.worker.CitationStyleToClipboardWorker; import org.jabref.gui.worker.SendAsEMailAction; -import org.jabref.logic.bibtexkeypattern.BibtexKeyGenerator; import org.jabref.logic.citationstyle.CitationStyleCache; import org.jabref.logic.citationstyle.CitationStyleOutputFormat; -import org.jabref.logic.exporter.BibtexDatabaseWriter; -import org.jabref.logic.exporter.FileSaveSession; -import org.jabref.logic.exporter.SaveException; -import org.jabref.logic.exporter.SavePreferences; -import org.jabref.logic.exporter.SaveSession; -import org.jabref.logic.l10n.Encodings; import org.jabref.logic.l10n.Localization; import org.jabref.logic.layout.Layout; import org.jabref.logic.layout.LayoutHelper; import org.jabref.logic.pdf.FileAnnotationCache; import org.jabref.logic.search.SearchQuery; -import org.jabref.logic.util.StandardFileType; import org.jabref.logic.util.UpdateField; import org.jabref.logic.util.io.FileFinder; import org.jabref.logic.util.io.FileFinders; @@ -118,13 +104,10 @@ import org.jabref.model.entry.event.EntryEventSource; import org.jabref.model.entry.specialfields.SpecialField; import org.jabref.model.entry.specialfields.SpecialFieldValue; -import org.jabref.model.strings.StringUtil; import org.jabref.preferences.JabRefPreferences; import org.jabref.preferences.PreviewPreferences; import com.google.common.eventbus.Subscribe; -import com.jgoodies.forms.builder.FormBuilder; -import com.jgoodies.forms.layout.FormLayout; import org.fxmisc.easybind.EasyBind; import org.fxmisc.easybind.Subscription; import org.slf4j.Logger; @@ -300,11 +283,11 @@ private void setupActions() { actions.put(Actions.EDIT, this::showAndEdit); // The action for saving a database. - actions.put(Actions.SAVE, saveAction); + actions.put(Actions.SAVE, saveAction::save); actions.put(Actions.SAVE_AS, saveAction::saveAs); - actions.put(Actions.SAVE_SELECTED_AS_PLAIN, new SaveSelectedAction(SavePreferences.DatabaseSaveType.PLAIN_BIBTEX)); + actions.put(Actions.SAVE_SELECTED_AS_PLAIN, saveAction::saveSelectedAsPlain); // The action for copying selected entries. actions.put(Actions.COPY, mainTable::copy); @@ -672,87 +655,9 @@ public void runCommand(final Actions command) { } } - /** - * FIXME: high code duplication with {@link SaveDatabaseAction#saveDatabase(File, boolean, Charset)} - */ - private boolean saveDatabase(File file, boolean selectedOnly, Charset encoding, - SavePreferences.DatabaseSaveType saveType) - throws SaveException { - SaveSession session; - final String SAVE_DATABASE = Localization.lang("Save library"); - try { - SavePreferences prefs = Globals.prefs.loadForSaveFromPreferences() - .withEncoding(encoding) - .withSaveType(saveType); - - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>( - FileSaveSession::new); - if (selectedOnly) { - session = databaseWriter.savePartOfDatabase(bibDatabaseContext, mainTable.getSelectedEntries(), prefs); - } else { - session = databaseWriter.saveDatabase(bibDatabaseContext, prefs); - } - - registerUndoableChanges(session); - } - // FIXME: not sure if this is really thrown anywhere - catch (UnsupportedCharsetException ex) { - frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file.") - + Localization.lang("Character encoding '%0' is not supported.", encoding.displayName())); - throw new SaveException("rt"); - } catch (SaveException ex) { - if (ex.specificEntry()) { - // Error occurred during processing of the entry. Highlight it: - clearAndSelect(ex.getEntry()); - showAndEdit(ex.getEntry()); - } else { - LOGGER.warn("Could not save", ex); - } - - dialogService.showErrorDialogAndWait(SAVE_DATABASE, Localization.lang("Could not save file."), ex); - throw new SaveException("rt"); - } - - boolean commit = true; - if (!session.getWriter().couldEncodeAll()) { - FormBuilder builder = FormBuilder.create() - .layout(new FormLayout("left:pref, 4dlu, fill:pref", "pref, 4dlu, pref")); - JTextArea ta = new JTextArea(session.getWriter().getProblemCharacters()); - ta.setEditable(false); - builder.add(Localization.lang("The chosen encoding '%0' could not encode the following characters:", session.getEncoding().displayName())).xy(1, 1); - builder.add(ta).xy(3, 1); - builder.add(Localization.lang("What do you want to do?")).xy(1, 3); - String tryDiff = Localization.lang("Try different encoding"); - int answer = JOptionPane.showOptionDialog(null, builder.getPanel(), SAVE_DATABASE, JOptionPane.YES_NO_CANCEL_OPTION, JOptionPane.WARNING_MESSAGE, null, new String[] {Localization.lang("Save"), tryDiff, Localization.lang("Cancel")}, tryDiff); - - if (answer == JOptionPane.NO_OPTION) { - - // The user wants to use another encoding. - Object choice = JOptionPane.showInputDialog(null, Localization.lang("Select encoding"), SAVE_DATABASE, JOptionPane.QUESTION_MESSAGE, null, Encodings.ENCODINGS_DISPLAYNAMES, encoding); - if (choice == null) { - commit = false; - } else { - Charset newEncoding = Charset.forName((String) choice); - return saveDatabase(file, selectedOnly, newEncoding, saveType); - } - } else if (answer == JOptionPane.CANCEL_OPTION) { - commit = false; - } - } - - if (commit) { - session.commit(file.toPath()); - this.bibDatabaseContext.getMetaData().setEncoding(encoding); // Make sure to remember which encoding we used. - } else { - session.cancel(); - } - - return commit; - } - - public void registerUndoableChanges(SaveSession session) { + public void registerUndoableChanges(List changes) { NamedCompound ce = new NamedCompound(Localization.lang("Save actions")); - for (FieldChange change : session.getFieldChanges()) { + for (FieldChange change : changes) { ce.addEdit(new UndoableFieldChange(change)); } ce.end(); @@ -1267,30 +1172,6 @@ public boolean showDeleteConfirmationDialog(int numberOfEntries) { } } - /** - * If the relevant option is set, autogenerate keys for all entries that are lacking keys. - */ - public void autoGenerateKeysBeforeSaving() { - if (Globals.prefs.getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING)) { - NamedCompound ce = new NamedCompound(Localization.lang("Autogenerate BibTeX keys")); - - BibtexKeyGenerator keyGenerator = new BibtexKeyGenerator(bibDatabaseContext, Globals.prefs.getBibtexKeyPatternPreferences()); - for (BibEntry bes : bibDatabaseContext.getDatabase().getEntries()) { - Optional oldKey = bes.getCiteKeyOptional(); - if (StringUtil.isBlank(oldKey)) { - Optional change = keyGenerator.generateAndSetKey(bes); - change.ifPresent(fieldChange -> ce.addEdit(new UndoableKeyChange(fieldChange))); - } - } - - // Store undo information, if any: - if (ce.hasEdits()) { - ce.end(); - getUndoManager().addEdit(ce); - } - } - } - /** * Depending on whether a preview or an entry editor is showing, save the current divider location in the correct preference setting. */ @@ -1645,30 +1526,4 @@ public void action() { preview.print(); } } - - private class SaveSelectedAction implements BaseAction { - - private final SavePreferences.DatabaseSaveType saveType; - - public SaveSelectedAction(SavePreferences.DatabaseSaveType saveType) { - this.saveType = saveType; - } - - @Override - public void action() throws SaveException { - FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder() - .withDefaultExtension(StandardFileType.BIBTEX_DB) - .addExtensionFilter(String.format("%1s %2s", "BibTex", Localization.lang("Library")), StandardFileType.BIBTEX_DB) - .withInitialDirectory(Globals.prefs.get(JabRefPreferences.WORKING_DIRECTORY)) - .build(); - - Optional chosenFile = dialogService.showFileSaveDialog(fileDialogConfiguration); - if (chosenFile.isPresent()) { - Path path = chosenFile.get(); - saveDatabase(path.toFile(), true, Globals.prefs.getDefaultEncoding(), saveType); - frame.getFileHistory().newFile(path.toString()); - frame.output(Localization.lang("Saved selected to '%0'.", path.toString())); - } - } - } } diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 179bcb9f0f5..2506c6a11d3 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -1343,8 +1343,7 @@ private boolean confirmClose(BasePanel panel) { // The user wants to save. try { SaveDatabaseAction saveAction = new SaveDatabaseAction(panel); - saveAction.runCommand(); - if (saveAction.isCanceled() || !saveAction.isSuccess()) { + if (!saveAction.save()) { // The action was either canceled or unsuccessful. output(Localization.lang("Unable to save library")); return false; diff --git a/src/main/java/org/jabref/gui/collab/ChangeScanner.java b/src/main/java/org/jabref/gui/collab/ChangeScanner.java index ba99990d46d..8e6616c24a4 100644 --- a/src/main/java/org/jabref/gui/collab/ChangeScanner.java +++ b/src/main/java/org/jabref/gui/collab/ChangeScanner.java @@ -1,5 +1,6 @@ package org.jabref.gui.collab; +import java.io.IOException; import java.nio.file.Path; import java.util.Comparator; import java.util.List; @@ -16,12 +17,10 @@ import org.jabref.logic.bibtex.comparator.BibDatabaseDiff; import org.jabref.logic.bibtex.comparator.BibEntryDiff; import org.jabref.logic.bibtex.comparator.BibStringDiff; +import org.jabref.logic.exporter.AtomicFileWriter; import org.jabref.logic.exporter.BibDatabaseWriter; import org.jabref.logic.exporter.BibtexDatabaseWriter; -import org.jabref.logic.exporter.FileSaveSession; -import org.jabref.logic.exporter.SaveException; import org.jabref.logic.exporter.SavePreferences; -import org.jabref.logic.exporter.SaveSession; import org.jabref.logic.importer.ImportFormatPreferences; import org.jabref.logic.importer.OpenDatabase; import org.jabref.logic.importer.ParserResult; @@ -105,10 +104,9 @@ private void storeTempDatabase() { .getEncoding() .orElse(Globals.prefs.getDefaultEncoding())); - BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>(FileSaveSession::new); - SaveSession ss = databaseWriter.saveDatabase(databaseInTemp, prefs); - ss.commit(tempFile); - } catch (SaveException ex) { + BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter(new AtomicFileWriter(tempFile, prefs.getEncoding())); + databaseWriter.saveDatabase(databaseInTemp, prefs); + } catch (IOException ex) { LOGGER.warn("Problem updating tmp file after accepting external changes", ex); } }); diff --git a/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java b/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java index 312258be7b3..1fb08fbb184 100644 --- a/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java +++ b/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java @@ -25,7 +25,7 @@ public AutosaveUIManager(BasePanel panel) { @Subscribe public void listen(@SuppressWarnings("unused") AutosaveEvent event) { try { - new SaveDatabaseAction(panel).runCommand(); + new SaveDatabaseAction(panel).save(); } catch (Throwable e) { LOGGER.error("Problem occured while saving.", e); } diff --git a/src/main/java/org/jabref/gui/exporter/SaveAllAction.java b/src/main/java/org/jabref/gui/exporter/SaveAllAction.java index 8b075676ebb..7a58e2895b4 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveAllAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveAllAction.java @@ -6,37 +6,26 @@ import org.jabref.gui.actions.SimpleCommand; import org.jabref.logic.l10n.Localization; -public class SaveAllAction extends SimpleCommand implements Runnable { +public class SaveAllAction extends SimpleCommand { private final JabRefFrame frame; - private int databases; - - /** Creates a new instance of SaveAllAction */ public SaveAllAction(JabRefFrame frame) { this.frame = frame; } @Override public void execute() { - databases = frame.getBasePanelCount(); frame.output(Localization.lang("Saving all libraries...")); - run(); - frame.output(Localization.lang("Save all finished.")); - } + for (BasePanel panel : frame.getBasePanelList()) { + if (!panel.getBibDatabaseContext().getDatabasePath().isPresent()) { + frame.showBasePanel(panel); - @Override - public void run() { - for (int i = 0; i < databases; i++) { - if (i < frame.getBasePanelCount()) { - BasePanel panel = frame.getBasePanelAt(i); - if (!panel.getBibDatabaseContext().getDatabaseFile().isPresent()) { - frame.showBasePanelAt(i); - } - panel.runCommand(Actions.SAVE); - // TODO: can we find out whether the save was actually done or not? + // TODO: Ask for path } + panel.runCommand(Actions.SAVE); + // TODO: can we find out whether the save was actually done or not? } + frame.output(Localization.lang("Save all finished.")); } - } diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index 9187ecee7b8..22845cb1bc1 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -1,140 +1,157 @@ package org.jabref.gui.exporter; -import java.io.File; +import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.UnsupportedCharsetException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; -import javafx.scene.control.Alert.AlertType; -import javafx.scene.control.ButtonBar.ButtonData; +import javafx.scene.control.ButtonBar; import javafx.scene.control.ButtonType; import javafx.scene.control.DialogPane; -import javafx.scene.control.TextArea; import javafx.scene.layout.VBox; import javafx.scene.text.Text; import org.jabref.Globals; -import org.jabref.JabRefExecutorService; import org.jabref.gui.BasePanel; import org.jabref.gui.DialogService; import org.jabref.gui.JabRefFrame; -import org.jabref.gui.SidePaneType; -import org.jabref.gui.actions.BaseAction; -import org.jabref.gui.collab.ChangeScanner; import org.jabref.gui.dialogs.AutosaveUIManager; import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.logic.autosaveandbackup.AutosaveManager; import org.jabref.logic.autosaveandbackup.BackupManager; +import org.jabref.logic.exporter.AtomicFileWriter; import org.jabref.logic.exporter.BibtexDatabaseWriter; -import org.jabref.logic.exporter.FileSaveSession; import org.jabref.logic.exporter.SaveException; import org.jabref.logic.exporter.SavePreferences; -import org.jabref.logic.exporter.SaveSession; import org.jabref.logic.l10n.Encodings; import org.jabref.logic.l10n.Localization; import org.jabref.logic.shared.prefs.SharedDatabasePreferences; import org.jabref.logic.util.StandardFileType; -import org.jabref.logic.util.io.FileBasedLock; +import org.jabref.logic.util.io.FileUtil; 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.BibEntry; import org.jabref.preferences.JabRefPreferences; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Action for the "Save" and "Save as" operations called from BasePanel. This class is also used for - * save operations when closing a database or quitting the applications. + * Action for the "Save" and "Save as" operations called from BasePanel. This class is also used for save operations + * when closing a database or quitting the applications. * - * The save operation is loaded off of the GUI thread using {@link BackgroundTask}. - * Callers can query whether the operation was canceled, or whether it was successful. + * The save operation is loaded off of the GUI thread using {@link BackgroundTask}. Callers can query whether the + * operation was canceled, or whether it was successful. */ -public class SaveDatabaseAction implements BaseAction { - +public class SaveDatabaseAction { private static final Logger LOGGER = LoggerFactory.getLogger(SaveDatabaseAction.class); private final BasePanel panel; private final JabRefFrame frame; - private boolean success; - private boolean canceled; - private boolean fileLockedError; - - private Optional filePath; + private final DialogService dialogService; public SaveDatabaseAction(BasePanel panel) { this.panel = panel; this.frame = panel.frame(); - this.filePath = Optional.empty(); + this.dialogService = frame.getDialogService(); } - /** - * @param panel BasePanel which contains the database to be saved - * @param filePath Path to the file the database should be saved to - */ - public SaveDatabaseAction(BasePanel panel, Path filePath) { - this(panel); - this.filePath = Optional.ofNullable(filePath); - } + private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, SavePreferences.DatabaseSaveType saveType) throws SaveException { + try { + SavePreferences preferences = Globals.prefs.loadForSaveFromPreferences() + .withEncoding(encoding) + .withSaveType(saveType); + if (preferences.makeBackup()) { + try { + makeBackup(file); + } catch (IOException exception) { + LOGGER.error("Problems saving during backup creation", exception); + boolean saveWithoutBackup = frame.getDialogService().showConfirmationDialogAndWait( + Localization.lang("Unable to create backup"), + Localization.lang("Save failed during backup creation") + ". " + Localization.lang("Save without backup?"), + Localization.lang("Save without backup")); + + if (!saveWithoutBackup) { + return false; + } + } + } - public void init() throws Exception { - success = false; - canceled = false; - fileLockedError = false; - if (panel.getBibDatabaseContext().getDatabaseFile().isPresent()) { - // Check for external modifications: if true, save not performed so do not tell the user a save is underway but return instead. - if (checkExternalModification()) { - return; + AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding()); + BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter); + + if (selectedOnly) { + databaseWriter.savePartOfDatabase(panel.getBibDatabaseContext(), panel.getSelectedEntries(), preferences); + } else { + databaseWriter.saveDatabase(panel.getBibDatabaseContext(), preferences); } - panel.frame().output(Localization.lang("Saving library") + "..."); - panel.setSaving(true); - } else if (filePath.isPresent()) { - // save as directly if the target file location is known - saveAs(filePath.get().toFile()); - } else { - saveAs(); - } - } + panel.registerUndoableChanges(databaseWriter.getSaveActionsFieldChanges()); - private void doSave() { - if (canceled || !panel.getBibDatabaseContext().getDatabaseFile().isPresent()) { - return; + if (fileWriter.hasEncodingProblems()) { + saveWithDifferentEncoding(file, selectedOnly, preferences.getEncoding(), fileWriter.getEncodingProblems(), saveType); + } + } catch (UnsupportedCharsetException ex) { + throw new SaveException(Localization.lang("Character encoding '%0' is not supported.", encoding.displayName()), ex); + } catch (IOException ex) { + throw new SaveException(ex); } - try { - // If set in preferences, generate missing BibTeX keys - panel.autoGenerateKeysBeforeSaving(); - - if (FileBasedLock.waitForFileLock(panel.getBibDatabaseContext().getDatabaseFile().get().toPath())) { - // Check for external modifications to alleviate multiuser concurrency issue when near - // simultaneous saves occur to a shared database file: if true, do not perform the save - // rather return instead. - if (checkExternalModification()) { - return; - } + return true; + } - // Save the database - success = saveDatabase(panel.getBibDatabaseContext().getDatabaseFile().get(), false, - panel.getBibDatabaseContext() - .getMetaData() - .getEncoding() - .orElse(Globals.prefs.getDefaultEncoding())); + private void makeBackup(Path file) throws IOException { + Files.copy(file, FileUtil.addExtension(file, ".bak")); + } - panel.updateTimeStamp(); - } else { - success = false; - fileLockedError = true; + private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset encoding, Set encodingProblems, SavePreferences.DatabaseSaveType saveType) throws SaveException { + DialogPane pane = new DialogPane(); + VBox vbox = new VBox(); + vbox.getChildren().addAll( + new Text(Localization.lang("The chosen encoding '%0' could not encode the following characters:", encoding.displayName())), + new Text(encodingProblems.stream().map(Object::toString).collect(Collectors.joining("."))), + new Text(Localization.lang("What do you want to do?")) + ); + pane.setContent(vbox); + + ButtonType tryDifferentEncoding = new ButtonType(Localization.lang("Try different encoding"), ButtonBar.ButtonData.OTHER); + ButtonType ignore = new ButtonType(Localization.lang("Ignore"), ButtonBar.ButtonData.APPLY); + boolean saveWithDifferentEncoding = frame.getDialogService() + .showCustomDialogAndWait(Localization.lang("Save library"), pane, ignore, tryDifferentEncoding) + .filter(buttonType -> buttonType.equals(tryDifferentEncoding)) + .isPresent(); + if (saveWithDifferentEncoding) { + Optional newEncoding = frame.getDialogService().showChoiceDialogAndWait(Localization.lang("Save library"), Localization.lang("Select new encoding"), Localization.lang("Save library"), encoding, Encodings.getCharsets()); + if (newEncoding.isPresent()) { + saveDatabase(file, selectedOnly, newEncoding.get(), saveType); + + // Make sure to remember which encoding we used. + panel.getBibDatabaseContext().getMetaData().setEncoding(newEncoding.get(), ChangePropagation.DO_NOT_POST_EVENT); } + } + } + + private boolean doSave() { + try { + // Save the database + boolean success = saveDatabase(panel.getBibDatabaseContext().getDatabasePath().get(), false, + panel.getBibDatabaseContext() + .getMetaData() + .getEncoding() + .orElse(Globals.prefs.getDefaultEncoding()), + SavePreferences.DatabaseSaveType.ALL); // release panel from save status panel.setSaving(false); if (success) { + panel.updateTimeStamp(); panel.getUndoManager().markUnchanged(); // (Only) after a successful save the following // statement marks that the base is unchanged @@ -142,209 +159,83 @@ private void doSave() { panel.setNonUndoableChange(false); panel.setBaseChanged(false); panel.markExternalChangesAsResolved(); - } - } catch (SaveException ex) { - if (ex == SaveException.FILE_LOCKED) { - success = false; - fileLockedError = true; - return; - } - LOGGER.error("Problem saving file", ex); - } - - if (success) { - DefaultTaskExecutor.runInJavaFXThread(() -> { - // Reset title of tab - frame.setTabTitle(panel, panel.getTabTitle(), - panel.getBibDatabaseContext().getDatabaseFile().get().getAbsolutePath()); - frame.output(Localization.lang("Saved library") + " '" - + panel.getBibDatabaseContext().getDatabaseFile().get().getPath() + "'."); - frame.setWindowTitle(); - frame.updateAllTabTitles(); - }); - } else if (!canceled) { - if (fileLockedError) { - // TODO: user should have the option to override the lock file. - frame.output(Localization.lang("Could not save, file locked by another JabRef instance.")); - } else { - frame.output(Localization.lang("Save failed")); - } - } - } - - private boolean saveDatabase(File file, boolean selectedOnly, Charset encoding) throws SaveException { - SaveSession session; - try { - SavePreferences prefs = Globals.prefs.loadForSaveFromPreferences().withEncoding(encoding); - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>(FileSaveSession::new); - - if (selectedOnly) { - session = databaseWriter.savePartOfDatabase(panel.getBibDatabaseContext(), panel.getSelectedEntries(), - prefs); - } else { - session = databaseWriter.saveDatabase(panel.getBibDatabaseContext(), prefs); + DefaultTaskExecutor.runInJavaFXThread(() -> { + // Reset title of tab + frame.setTabTitle(panel, panel.getTabTitle(), + panel.getBibDatabaseContext().getDatabaseFile().get().getAbsolutePath()); + frame.output(Localization.lang("Saved library") + " '" + + panel.getBibDatabaseContext().getDatabaseFile().get().getPath() + "'."); + frame.setWindowTitle(); + frame.updateAllTabTitles(); + }); } - - panel.registerUndoableChanges(session); - - } catch (UnsupportedCharsetException ex) { - frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file.") - + Localization.lang("Character encoding '%0' is not supported.", encoding.displayName())); - throw new SaveException(ex.getMessage(), ex); + return success; } catch (SaveException ex) { - if (ex == SaveException.FILE_LOCKED) { - throw ex; - } - if (ex.specificEntry()) { - BibEntry entry = ex.getEntry(); - // Error occured during processing of an entry. Highlight it! - panel.clearAndSelect(entry); - } else { - LOGGER.error("A problem occured when trying to save the file", ex); - } - + LOGGER.error("A problem occurred when trying to save the file", ex); frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); - throw ex; - } - - // handle encoding problems - boolean success = true; - if (!session.getWriter().couldEncodeAll()) { - - DialogPane pane = new DialogPane(); - TextArea area = new TextArea(session.getWriter().getProblemCharacters()); - VBox vbox = new VBox(); - vbox.getChildren().addAll( - new Text(Localization.lang("The chosen encoding '%0' could not encode the following characters:", - session.getEncoding().displayName())), - area, - new Text(Localization.lang("What do you want to do?")) - - ); - pane.setContent(vbox); - - ButtonType tryDiff = new ButtonType(Localization.lang("Try different encoding"), ButtonData.OTHER); - ButtonType save = new ButtonType(Localization.lang("Save"), ButtonData.APPLY); - - Optional clickedBtn = frame.getDialogService().showCustomDialogAndWait(Localization.lang("Save library"), pane, save, tryDiff, ButtonType.CANCEL); - - if (clickedBtn.isPresent() && clickedBtn.get().equals(tryDiff)) { - Optional selectedCharSet = frame.getDialogService().showChoiceDialogAndWait(Localization.lang("Save library"), Localization.lang("Select encoding"), Localization.lang("Save library"), encoding, Encodings.getCharsets()); - - if (selectedCharSet.isPresent()) { - - Charset newEncoding = selectedCharSet.get(); - return saveDatabase(file, selectedOnly, newEncoding); - } else { - success = false; - } - } - } - - // backup file? - try { - if (success) { - session.commit(file.toPath()); - // Make sure to remember which encoding we used. - panel.getBibDatabaseContext().getMetaData().setEncoding(encoding, ChangePropagation.DO_NOT_POST_EVENT); - } else { - session.cancel(); - } - } catch (SaveException e) { - LOGGER.debug("Problems saving during backup creationg", e); - boolean saveWithoutBackupClicked = frame.getDialogService().showConfirmationDialogAndWait(Localization.lang("Unable to create backup"), - Localization.lang("Save failed during backup creation") + ". " + Localization.lang("Save without backup?"), - Localization.lang("Save without backup"), Localization.lang("Cancel")); - - if (saveWithoutBackupClicked) { - session.setUseBackup(false); - - session.commit(file.toPath()); - panel.getBibDatabaseContext().getMetaData().setEncoding(encoding, ChangePropagation.DO_NOT_POST_EVENT); - } else { - success = false; - } + return false; } - - return success; } - /** - * Run the "Save" operation. This method offloads the actual save operation to a background thread. - */ - public void runCommand() throws Exception { - action(); + public boolean save() { + if (panel.getBibDatabaseContext().getDatabasePath().isPresent()) { + panel.frame().output(Localization.lang("Saving library") + "..."); + panel.setSaving(true); + return doSave(); + } + return false; } - public void save() throws Exception { - runCommand(); + public void saveAs() { + getSavePath().ifPresent(this::saveAs); } - public void saveAs() throws Exception { - // configure file dialog - + private Optional getSavePath() { FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder() .addExtensionFilter(StandardFileType.BIBTEX_DB) .withDefaultExtension(StandardFileType.BIBTEX_DB) .withInitialDirectory(Globals.prefs.get(JabRefPreferences.WORKING_DIRECTORY)) .build(); - DialogService dialogService = frame.getDialogService(); - Optional path = dialogService.showFileSaveDialog(fileDialogConfiguration); - if (path.isPresent()) { - saveAs(path.get().toFile()); - } else { - canceled = true; - return; - } + Optional selectedPath = dialogService.showFileSaveDialog(fileDialogConfiguration); + selectedPath.ifPresent(path -> Globals.prefs.setWorkingDir(path.getParent())); + return selectedPath; } - /** - * Run the "Save as" operation. This method offloads the actual save operation to a background thread. - */ - public void saveAs(File file) throws Exception { + public void saveAs(Path file) { BibDatabaseContext context = panel.getBibDatabaseContext(); - if (context.getLocation() == DatabaseLocation.SHARED) { - // Save all properties dependent on the ID. This makes it possible to restore them. - new SharedDatabasePreferences(context.getDatabase().generateSharedDatabaseID()) - .putAllDBMSConnectionProperties(context.getDBMSSynchronizer().getConnectionProperties()); - - } - - context.setDatabaseFile(file); - if (file.getParent() != null) { - Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent()); - } - runCommand(); - // If the operation failed, revert the file field and return: - if (!success) { - return; - } - + // Close AutosaveManager and BackupManager for original library Optional databasePath = context.getDatabasePath(); if (databasePath.isPresent()) { final Path oldFile = databasePath.get(); context.setDatabaseFile(oldFile.toFile()); - //closing AutosaveManager and BackupManager for original library AutosaveManager.shutdown(context); BackupManager.shutdown(context); - } else { - LOGGER.info("Old file not found, just creating a new file"); + } + + // Set new location + if (context.getLocation() == DatabaseLocation.SHARED) { + // Save all properties dependent on the ID. This makes it possible to restore them. + new SharedDatabasePreferences(context.getDatabase().generateSharedDatabaseID()) + .putAllDBMSConnectionProperties(context.getDBMSSynchronizer().getConnectionProperties()); } context.setDatabaseFile(file); - panel.resetChangeMonitor(); + // Save + save(); + + // Reinstall AutosaveManager and BackupManager + panel.resetChangeMonitor(); if (readyForAutosave(context)) { AutosaveManager autosaver = AutosaveManager.start(context); autosaver.registerListener(new AutosaveUIManager(panel)); } - if (readyForBackup(context)) { BackupManager.start(context); } - context.getDatabaseFile().ifPresent(presentFile -> frame.getFileHistory().newFile(presentFile.getPath())); + context.getDatabasePath().ifPresent(presentFile -> frame.getFileHistory().newFile(presentFile)); } private boolean readyForAutosave(BibDatabaseContext context) { @@ -352,108 +243,23 @@ private boolean readyForAutosave(BibDatabaseContext context) { ((context.getLocation() == DatabaseLocation.LOCAL) && Globals.prefs.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE))) && - context.getDatabaseFile().isPresent(); + context.getDatabasePath().isPresent(); } private boolean readyForBackup(BibDatabaseContext context) { - return (context.getLocation() == DatabaseLocation.LOCAL) && context.getDatabaseFile().isPresent(); - } - - /** - * Query whether the last operation was successful. - * - * @return true if the last Save/SaveAs operation completed successfully, false otherwise. - */ - public boolean isSuccess() { - return success; + return (context.getLocation() == DatabaseLocation.LOCAL) && context.getDatabasePath().isPresent(); } - /** - * Query whether the last operation was canceled. - * - * @return true if the last Save/SaveAs operation was canceled from the file dialog or from another - * query dialog, false otherwise. - */ - public boolean isCanceled() { - return canceled; - } - - /** - * Check whether or not the external database has been modified. If so need to alert the user to accept external updates prior to - * saving the database. This is necessary to avoid overwriting other users work when using a multiuser database file. - * - * @return true if the external database file has been modified and the user must choose to accept the changes and false if no modifications - * were found or there is no requested protection for the database file. - */ - private boolean checkExternalModification() { - // Check for external modifications: - if (panel.isUpdatedExternally()) { - - ButtonType save = new ButtonType(Localization.lang("Save")); - ButtonType reviewChanges = new ButtonType(Localization.lang("Review changes")); - - Optional buttonPressed = DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().showCustomButtonDialogAndWait(AlertType.CONFIRMATION, Localization.lang("File updated externally"), - Localization.lang("File has been updated externally. " + "What do you want to do?"), - reviewChanges, - save, - ButtonType.CANCEL)); - - if (buttonPressed.isPresent()) { - if (buttonPressed.get() == ButtonType.CANCEL) { - canceled = true; - return true; - } - if (buttonPressed.get().equals(reviewChanges)) { - canceled = true; - - JabRefExecutorService.INSTANCE.execute(() -> { - - if (!FileBasedLock - .waitForFileLock(panel.getBibDatabaseContext().getDatabasePath().get())) { - // TODO: GUI handling of the situation when the externally modified file keeps being locked. - LOGGER.error("File locked, this will be trouble."); - } - - ChangeScanner scanner = new ChangeScanner(panel.frame(), panel, - panel.getBibDatabaseContext().getDatabasePath().orElse(null), panel.getTempFile()); - JabRefExecutorService.INSTANCE.executeInterruptableTaskAndWait(scanner); - if (scanner.changesFound()) { - scanner.displayResult(resolved -> { - if (resolved) { - panel.markExternalChangesAsResolved(); - DefaultTaskExecutor.runInJavaFXThread(() -> panel.getSidePaneManager().hide(SidePaneType.FILE_UPDATE_NOTIFICATION)); - } else { - canceled = true; - } - }); - } - }); - - return true; - } else { // User indicated to store anyway. - if (panel.getBibDatabaseContext().getMetaData().isProtected()) { - - frame.getDialogService().showErrorDialogAndWait(Localization.lang("Protected library"), - Localization.lang("Library is protected. Cannot save until external changes have been reviewed.")); - - canceled = true; - } else { - panel.markExternalChangesAsResolved(); - panel.getSidePaneManager().hide(SidePaneType.FILE_UPDATE_NOTIFICATION); - } - } + public void saveSelectedAsPlain() { + getSavePath().ifPresent(path -> { + try { + saveDatabase(path, true, Globals.prefs.getDefaultEncoding(), SavePreferences.DatabaseSaveType.PLAIN_BIBTEX); + frame.getFileHistory().newFile(path); + frame.output(Localization.lang("Saved selected to '%0'.", path.toString())); + } catch (SaveException ex) { + LOGGER.error("A problem occurred when trying to save the file", ex); + frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); } - } - - // Return false as either no external database file modifications have been found or overwrite is requested any way - return false; - } - - @Override - public void action() throws Exception { - init(); - BackgroundTask - .wrap(this::doSave) - .executeWith(Globals.TASK_EXECUTOR); + }); } } diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index ef98593fc5a..49c6e1cb06c 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -175,7 +175,7 @@ public void openFiles(List filesToOpen, boolean raisePanel) { } }); for (Path theFile : theFiles) { - frame.getFileHistory().newFile(theFile.toString()); + frame.getFileHistory().newFile(theFile); } } // If no files are remaining to open, this could mean that a file was diff --git a/src/main/java/org/jabref/gui/shared/ConnectToSharedDatabaseDialog.java b/src/main/java/org/jabref/gui/shared/ConnectToSharedDatabaseDialog.java index 9dd594d2418..4097ef93b94 100644 --- a/src/main/java/org/jabref/gui/shared/ConnectToSharedDatabaseDialog.java +++ b/src/main/java/org/jabref/gui/shared/ConnectToSharedDatabaseDialog.java @@ -146,7 +146,7 @@ public void openSharedDatabase() { dispose(); if (!fileLocationField.getText().isEmpty()) { try { - new SaveDatabaseAction(panel, Paths.get(fileLocationField.getText())).runCommand(); + new SaveDatabaseAction(panel).saveAs(Paths.get(fileLocationField.getText())); } catch (Throwable e) { LOGGER.error("Error while saving the database", e); } diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 4fdb9d05f18..ebe05ee616e 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -16,9 +16,8 @@ import java.util.concurrent.TimeUnit; import org.jabref.logic.bibtex.InvalidFieldValueException; +import org.jabref.logic.exporter.AtomicFileWriter; import org.jabref.logic.exporter.BibtexDatabaseWriter; -import org.jabref.logic.exporter.FileSaveSession; -import org.jabref.logic.exporter.SaveException; import org.jabref.logic.exporter.SavePreferences; import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; @@ -121,14 +120,14 @@ private void performBackup(Path backupPath) { Charset charset = bibDatabaseContext.getMetaData().getEncoding().orElse(preferences.getDefaultEncoding()); SavePreferences savePreferences = preferences.loadForSaveFromPreferences().withEncoding (charset).withMakeBackup(false); - new BibtexDatabaseWriter<>(FileSaveSession::new).saveDatabase(bibDatabaseContext, savePreferences).commit - (backupPath); - } catch (SaveException e) { + new BibtexDatabaseWriter(new AtomicFileWriter(backupPath, savePreferences.getEncoding())) + .saveDatabase(bibDatabaseContext, savePreferences); + } catch (IOException e) { logIfCritical(e); } } - private void logIfCritical(SaveException e) { + private void logIfCritical(IOException e) { Throwable innermostCause = e; while (innermostCause.getCause() != null) { innermostCause = innermostCause.getCause(); diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java new file mode 100644 index 00000000000..2f0f221c81c --- /dev/null +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -0,0 +1,197 @@ +package org.jabref.logic.exporter; + +import java.io.FileOutputStream; +import java.io.FilterOutputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.PosixFilePermission; +import java.util.EnumSet; +import java.util.Set; + +import org.jabref.logic.util.io.FileUtil; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A file output stream that is similar to the standard {@link FileOutputStream}, except that all writes are first + * redirected to a temporary file. When the stream is closed, the temporary file (atomically) replaces the target file. + * + * In detail, the strategy is to: + *
    + *
  1. Write to a temporary file (with .tmp suffix) in the same directory as the destination file.
    2. + *
  2. Create a backup of the original file (if it exists) in the same directory.
    3. + *
  3. Move the temporary file to the correct place, overwriting any file that already exists at that location.
    4. + *
  4. Delete the backup file.
    5. + *
+ * If all goes well, no temporary or backup files will remain on disk after closing the stream. + * + * Errors are handled as follows: + *
    + *
  1. If anything goes wrong while writing to the temporary file, the temporary file will be deleted (leaving the + * original file untouched).
    2. + *
  2. If anything goes wrong while copying the temporary file to the target file, the backup of the original file is + * kept.
    3. + *
+ * + * Implementation inspired by code from Marty + * Lamb and Apache. + */ +public class AtomicFileOutputStream extends FilterOutputStream { + + private static final Logger LOGGER = LoggerFactory.getLogger(AtomicFileOutputStream.class); + + private static final String TEMPORARY_EXTENSION = ".tmp"; + private static final String BACKUP_EXTENSION = ".sav"; + + /** + * The file we want to create/replace. + */ + private final Path targetFile; + /** + * The file to which writes are redirected to. + */ + private final Path temporaryFile; + /** + * A backup of the target file (if it exists), created when the stream is closed + */ + private final Path backupFile; + + /** + * Creates a new output stream to write to or replace the file at the specified path. + * + * @param path the path of the file to write to or replace + */ + public AtomicFileOutputStream(Path path) throws IOException { + super(Files.newOutputStream(getPathOfTemporaryFile(path))); + + targetFile = path; + temporaryFile = getPathOfTemporaryFile(path); + backupFile = getPathOfBackupFile(path); + } + + private static Path getPathOfTemporaryFile(Path targetFile) { + return FileUtil.addExtension(targetFile, TEMPORARY_EXTENSION); + } + + private static Path getPathOfBackupFile(Path targetFile) { + return FileUtil.addExtension(targetFile, BACKUP_EXTENSION); + } + + /** + * Returns the path of the backup copy of the original file (may not exist) + */ + public Path getBackup() { + return backupFile; + } + + /** + * Override for performance reasons. + */ + @Override + public void write(byte b[], int off, int len) throws IOException { + try { + out.write(b, off, len); + } catch (IOException exception) { + cleanup(); + throw exception; + } + } + + /** + * Closes the write process to the temporary file but does not commit to the target file. + */ + public void abort() throws IOException { + try { + super.close(); + Files.deleteIfExists(temporaryFile); + Files.deleteIfExists(backupFile); + } catch (IOException exception) { + LOGGER.debug("Unable to abort writing to file " + temporaryFile, exception); + } + } + + private void cleanup() { + try { + Files.deleteIfExists(temporaryFile); + } catch (IOException exception) { + LOGGER.debug("Unable to delete file " + temporaryFile, exception); + } + } + + // perform the final operations to move the temporary file to its final destination + @Override + public void close() throws IOException { + try { + try { + // Make sure we have written everything to the temporary file + flush(); + ((FileOutputStream) out).getFD().sync(); + } catch (IOException exception) { + // Try to close nonetheless + super.close(); + throw exception; + } + super.close(); + + // We successfully wrote everything to the temporary file, lets copy it to the correct place + // First, make backup of original file and try to save file permissions to restore them later (by default: 664) + Set oldFilePermissions = EnumSet.of(PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.GROUP_READ, + PosixFilePermission.GROUP_WRITE, + PosixFilePermission.OTHERS_READ); + if (Files.exists(targetFile)) { + Files.copy(targetFile, backupFile, StandardCopyOption.REPLACE_EXISTING); + if (FileUtil.IS_POSIX_COMPILANT) { + try { + oldFilePermissions = Files.getPosixFilePermissions(targetFile); + } catch (IOException exception) { + LOGGER.warn("Error getting file permissions for file {}.", targetFile, exception); + } + } + } + + // Move temporary file (replace original if it exists) + Files.move(temporaryFile, targetFile, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + + // Restore file permissions + if (FileUtil.IS_POSIX_COMPILANT) { + try { + Files.setPosixFilePermissions(targetFile, oldFilePermissions); + } catch (IOException exception) { + LOGGER.warn("Error writing file permissions to file {}.", targetFile, exception); + } + } + + // Remove backup file + Files.deleteIfExists(backupFile); + } finally { + // Remove temporary file (but not the backup!) + cleanup(); + } + } + + @Override + public void flush() throws IOException { + try { + super.flush(); + } catch (IOException exception) { + cleanup(); + throw exception; + } + } + + @Override + public void write(int b) throws IOException { + try { + super.write(b); + } catch (IOException exception) { + cleanup(); + throw exception; + } + } +} + diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java b/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java new file mode 100644 index 00000000000..febbd4e7d45 --- /dev/null +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java @@ -0,0 +1,49 @@ +package org.jabref.logic.exporter; + +import java.io.IOException; +import java.io.OutputStreamWriter; +import java.nio.charset.Charset; +import java.nio.charset.CharsetEncoder; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Set; +import java.util.TreeSet; + +/** + * Writer that similar to the built-in {@link java.io.FileWriter} but uses the {@link AtomicFileOutputStream} as the + * underlying output stream. In this way, we make sure that the errors during the write process do not destroy the + * contents of the target file. + * Moreover, this writer checks if the chosen encoding supports all text that is written. Characters whose encoding + * was problematic can be retrieved by {@link #getEncodingProblems()}. + */ +public class AtomicFileWriter extends OutputStreamWriter { + + private final CharsetEncoder encoder; + private final Set problemCharacters = new TreeSet<>(); + + public AtomicFileWriter(Path file, Charset encoding) throws IOException { + super(new AtomicFileOutputStream(file), encoding); + encoder = encoding.newEncoder(); + } + + @Override + public void write(String str) throws IOException { + super.write(str); + if (!encoder.canEncode(str)) { + for (int i = 0; i < str.length(); i++) { + char character = str.charAt(i); + if (!encoder.canEncode(character)) { + problemCharacters.add(character); + } + } + } + } + + public boolean hasEncodingProblems() { + return !problemCharacters.isEmpty(); + } + + public Set getEncodingProblems() { + return Collections.unmodifiableSet(problemCharacters); + } +} diff --git a/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java b/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java index b43e8663924..364908f9161 100644 --- a/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java +++ b/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java @@ -1,9 +1,9 @@ package org.jabref.logic.exporter; import java.io.IOException; +import java.io.Writer; import java.nio.charset.Charset; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -17,12 +17,14 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.jabref.Globals; import org.jabref.logic.bibtex.LatexFieldFormatterPreferences; import org.jabref.logic.bibtex.comparator.BibtexStringComparator; import org.jabref.logic.bibtex.comparator.CrossRefEntryComparator; import org.jabref.logic.bibtex.comparator.FieldComparator; import org.jabref.logic.bibtex.comparator.FieldComparatorStack; import org.jabref.logic.bibtex.comparator.IdComparator; +import org.jabref.logic.bibtexkeypattern.BibtexKeyGenerator; import org.jabref.model.EntryTypes; import org.jabref.model.FieldChange; import org.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern; @@ -36,20 +38,16 @@ import org.jabref.model.entry.EntryType; import org.jabref.model.metadata.MetaData; import org.jabref.model.metadata.SaveOrderConfig; +import org.jabref.model.strings.StringUtil; -public abstract class BibDatabaseWriter { +public abstract class BibDatabaseWriter { private static final Pattern REFERENCE_PATTERN = Pattern.compile("(#[A-Za-z]+#)"); // Used to detect string references in strings - private final SaveSessionFactory saveSessionFactory; + private final Writer writer; + private final List saveActionsFieldChanges = new ArrayList<>(); - private E session; - - public BibDatabaseWriter(SaveSessionFactory saveSessionFactory) { - this.saveSessionFactory = saveSessionFactory; - } - - public interface SaveSessionFactory { - E createSaveSession(Charset encoding, Boolean makeBackup) throws SaveException; + public BibDatabaseWriter(Writer writer) { + this.writer = Objects.requireNonNull(writer); } private static List applySaveActions(List toChange, MetaData metaData) { @@ -67,11 +65,10 @@ private static List applySaveActions(List toChange, MetaD } public static List applySaveActions(BibEntry entry, MetaData metaData) { - return applySaveActions(Arrays.asList(entry), metaData); + return applySaveActions(Collections.singletonList(entry), metaData); } private static List> getSaveComparators(SavePreferences preferences, MetaData metaData) { - List> comparators = new ArrayList<>(); Optional saveOrder = getSaveOrder(preferences, metaData); @@ -79,7 +76,7 @@ private static List> getSaveComparators(SavePreferences pre // ones. This is a necessary requirement for BibTeX to be able to resolve referenced entries correctly. comparators.add(new CrossRefEntryComparator()); - if (! saveOrder.isPresent()) { + if (!saveOrder.isPresent()) { // entries will be sorted based on their internal IDs comparators.add(new IdComparator()); } else { @@ -94,33 +91,27 @@ private static List> getSaveComparators(SavePreferences pre return comparators; } - /* - * We have begun to use getSortedEntries() for both database save operations - * and non-database save operations. In a non-database save operation - * (such as the exportDatabase call), we do not wish to use the - * global preference of saving in standard order. - */ + /** + * We have begun to use getSortedEntries() for both database save operations and non-database save operations. In a + * non-database save operation (such as the exportDatabase call), we do not wish to use the global preference of + * saving in standard order. + */ public static List getSortedEntries(BibDatabaseContext bibDatabaseContext, List entriesToSort, - SavePreferences preferences) { + SavePreferences preferences) { Objects.requireNonNull(bibDatabaseContext); Objects.requireNonNull(entriesToSort); //if no meta data are present, simply return in original order if (bibDatabaseContext.getMetaData() == null) { - List result = new LinkedList<>(); - result.addAll(entriesToSort); - return result; + return new LinkedList<>(entriesToSort); } List> comparators = BibDatabaseWriter.getSaveComparators(preferences, bibDatabaseContext.getMetaData()); FieldComparatorStack comparatorStack = new FieldComparatorStack<>(comparators); - List sorted = new ArrayList<>(); - sorted.addAll(entriesToSort); - - Collections.sort(sorted, comparatorStack); - + List sorted = new ArrayList<>(entriesToSort); + sorted.sort(comparatorStack); return sorted; } @@ -142,21 +133,22 @@ private static Optional getSaveOrder(SavePreferences preference return Optional.ofNullable(preferences.getSaveOrder()); } + public List getSaveActionsFieldChanges() { + return Collections.unmodifiableList(saveActionsFieldChanges); + } + /** * Saves the complete database. */ - public E saveDatabase(BibDatabaseContext bibDatabaseContext, SavePreferences preferences) - throws SaveException { - return savePartOfDatabase(bibDatabaseContext, bibDatabaseContext.getDatabase().getEntries(), preferences); + public void saveDatabase(BibDatabaseContext bibDatabaseContext, SavePreferences preferences) throws IOException { + savePartOfDatabase(bibDatabaseContext, bibDatabaseContext.getDatabase().getEntries(), preferences); } /** * Saves the database, including only the specified entries. */ - public E savePartOfDatabase(BibDatabaseContext bibDatabaseContext, - List entries, SavePreferences preferences) throws SaveException { - - session = saveSessionFactory.createSaveSession(preferences.getEncodingOrDefault(), preferences.getMakeBackup()); + public void savePartOfDatabase(BibDatabaseContext bibDatabaseContext, + List entries, SavePreferences preferences) throws IOException { Optional sharedDatabaseIDOptional = bibDatabaseContext.getDatabase().getSharedDatabaseID(); @@ -182,7 +174,11 @@ public E savePartOfDatabase(BibDatabaseContext bibDatabaseContext, // Write database entries. List sortedEntries = getSortedEntries(bibDatabaseContext, entries, preferences); List saveActionChanges = applySaveActions(sortedEntries, bibDatabaseContext.getMetaData()); - session.addFieldChanges(saveActionChanges); + saveActionsFieldChanges.addAll(saveActionChanges); + if (preferences.generateBibtexKeysBeforeSaving()) { + List keyChanges = generateBibtexKeys(bibDatabaseContext, sortedEntries); + saveActionsFieldChanges.addAll(keyChanges); + } for (BibEntry entry : sortedEntries) { // Check if we must write the type definition for this @@ -210,25 +206,20 @@ public E savePartOfDatabase(BibDatabaseContext bibDatabaseContext, //finally write whatever remains of the file, but at least a concluding newline writeEpilogue(bibDatabaseContext.getDatabase().getEpilog()); - try { - session.getWriter().close(); - } catch (IOException e) { - throw new SaveException(e); - } - return session; + writer.close(); } - protected abstract void writePrelogue(BibDatabaseContext bibDatabaseContext, Charset encoding) throws SaveException; + protected abstract void writePrelogue(BibDatabaseContext bibDatabaseContext, Charset encoding) throws IOException; protected abstract void writeEntry(BibEntry entry, BibDatabaseMode mode, Boolean isReformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws SaveException; + LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException; - protected abstract void writeEpilogue(String epilogue) throws SaveException; + protected abstract void writeEpilogue(String epilogue) throws IOException; /** * Writes all data to the specified writer, using each object's toString() method. */ - protected void writeMetaData(MetaData metaData, GlobalBibtexKeyPattern globalCiteKeyPattern) throws SaveException { + protected void writeMetaData(MetaData metaData, GlobalBibtexKeyPattern globalCiteKeyPattern) throws IOException { Objects.requireNonNull(metaData); Map serializedMetaData = MetaDataSerializer.getSerializedStringMap(metaData, @@ -239,23 +230,25 @@ protected void writeMetaData(MetaData metaData, GlobalBibtexKeyPattern globalCit } } - protected abstract void writeMetaDataItem(Map.Entry metaItem) throws SaveException; + protected abstract void writeMetaDataItem(Map.Entry metaItem) throws IOException; - protected abstract void writePreamble(String preamble) throws SaveException; + protected abstract void writePreamble(String preamble) throws IOException; - protected abstract void writeDatabaseID(String sharedDatabaseID) throws SaveException; + protected abstract void writeDatabaseID(String sharedDatabaseID) throws IOException; /** - * Write all strings in alphabetical order, modified to produce a safe (for - * BibTeX) order of the strings if they reference each other. + * Write all strings in alphabetical order, modified to produce a safe (for BibTeX) order of the strings if they + * reference each other. * * @param database The database whose strings we should write. */ private void writeStrings(BibDatabase database, Boolean reformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws SaveException { - List strings = database.getStringKeySet().stream().map(database::getString).collect( - Collectors.toList()); - strings.sort(new BibtexStringComparator(true)); + LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException { + List strings = database.getStringKeySet() + .stream() + .map(database::getString) + .sorted(new BibtexStringComparator(true)) + .collect(Collectors.toList()); // First, make a Map of all entries: Map remaining = new HashMap<>(); int maxKeyLength = 0; @@ -277,8 +270,8 @@ private void writeStrings(BibDatabase database, Boolean reformatFile, } protected void writeString(BibtexString bibtexString, boolean isFirstString, Map remaining, int maxKeyLength, - Boolean reformatFile, LatexFieldFormatterPreferences latexFieldFormatterPreferences) - throws SaveException { + Boolean reformatFile, LatexFieldFormatterPreferences latexFieldFormatterPreferences) + throws IOException { // First remove this from the "remaining" list so it can't cause problem with circular refs: remaining.remove(bibtexString.getName()); @@ -305,10 +298,10 @@ protected void writeString(BibtexString bibtexString, boolean isFirstString, Map } protected abstract void writeString(BibtexString bibtexString, boolean isFirstString, int maxKeyLength, - Boolean reformatFile, LatexFieldFormatterPreferences latexFieldFormatterPreferences) - throws SaveException; + Boolean reformatFile, LatexFieldFormatterPreferences latexFieldFormatterPreferences) + throws IOException; - protected void writeEntryTypeDefinitions(Map types) throws SaveException { + protected void writeEntryTypeDefinitions(Map types) throws IOException { for (EntryType type : types.values()) { if (type instanceof CustomEntryType) { writeEntryTypeDefinition((CustomEntryType) type); @@ -316,9 +309,25 @@ protected void writeEntryTypeDefinitions(Map types) throws Sa } } - protected abstract void writeEntryTypeDefinition(CustomEntryType customType) throws SaveException; + protected abstract void writeEntryTypeDefinition(CustomEntryType customType) throws IOException; - protected SaveSession getActiveSession() { - return session; + protected Writer getWriter() { + return writer; + } + + /** + * Generate keys for all entries that are lacking keys. + */ + protected List generateBibtexKeys(BibDatabaseContext databaseContext, List entries) { + List changes = new ArrayList<>(); + BibtexKeyGenerator keyGenerator = new BibtexKeyGenerator(databaseContext, Globals.prefs.getBibtexKeyPatternPreferences()); + for (BibEntry bes : entries) { + Optional oldKey = bes.getCiteKeyOptional(); + if (StringUtil.isBlank(oldKey)) { + Optional change = keyGenerator.generateAndSetKey(bes); + change.ifPresent(changes::add); + } + } + return changes; } } diff --git a/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java b/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java index 32ea57162a2..1af38c29e8d 100644 --- a/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java +++ b/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java @@ -18,159 +18,120 @@ import org.jabref.model.metadata.MetaData; import org.jabref.model.strings.StringUtil; -public class BibtexDatabaseWriter extends BibDatabaseWriter { +public class BibtexDatabaseWriter extends BibDatabaseWriter { public static final String DATABASE_ID_PREFIX = "DBID:"; private static final String STRING_PREFIX = "@String"; private static final String COMMENT_PREFIX = "@Comment"; private static final String PREAMBLE_PREFIX = "@Preamble"; - public BibtexDatabaseWriter(SaveSessionFactory saveSessionFactory) { - super(saveSessionFactory); + public BibtexDatabaseWriter(Writer writer) { + super(writer); } @Override - protected void writeEpilogue(String epilogue) throws SaveException { + protected void writeEpilogue(String epilogue) throws IOException { if (!StringUtil.isNullOrEmpty(epilogue)) { - try { - getWriter().write(OS.NEWLINE); - getWriter().write(epilogue); - getWriter().write(OS.NEWLINE); - } catch (IOException e) { - throw new SaveException(e); - } + getWriter().write(OS.NEWLINE); + getWriter().write(epilogue); + getWriter().write(OS.NEWLINE); } } @Override - protected void writeMetaDataItem(Map.Entry metaItem) throws SaveException { - StringBuilder stringBuilder = new StringBuilder(); - stringBuilder.append(OS.NEWLINE); - stringBuilder.append(COMMENT_PREFIX + "{").append(MetaData.META_FLAG).append(metaItem.getKey()).append(":"); - stringBuilder.append(metaItem.getValue()); - stringBuilder.append("}"); - stringBuilder.append(OS.NEWLINE); - - try { - getWriter().write(stringBuilder.toString()); - } catch (IOException e) { - throw new SaveException(e); - } + protected void writeMetaDataItem(Map.Entry metaItem) throws IOException { + getWriter().write(OS.NEWLINE); + getWriter().write(COMMENT_PREFIX + "{"); + getWriter().write(MetaData.META_FLAG); + getWriter().write(metaItem.getKey()); + getWriter().write(":"); + getWriter().write(metaItem.getValue()); + getWriter().write("}"); + getWriter().write(OS.NEWLINE); } @Override - protected void writePreamble(String preamble) throws SaveException { + protected void writePreamble(String preamble) throws IOException { if (!StringUtil.isNullOrEmpty(preamble)) { - try { - getWriter().write(OS.NEWLINE); - getWriter().write(PREAMBLE_PREFIX + "{"); - getWriter().write(preamble); - getWriter().write('}' + OS.NEWLINE); - } catch (IOException e) { - throw new SaveException(e); - } + getWriter().write(OS.NEWLINE); + getWriter().write(PREAMBLE_PREFIX + "{"); + getWriter().write(preamble); + getWriter().write('}' + OS.NEWLINE); } } @Override protected void writeString(BibtexString bibtexString, boolean isFirstString, int maxKeyLength, Boolean reformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws SaveException { - try { - // If the string has not been modified, write it back as it was - if (!reformatFile && !bibtexString.hasChanged()) { - getWriter().write(bibtexString.getParsedSerialization()); - return; - } + LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException { + // If the string has not been modified, write it back as it was + if (!reformatFile && !bibtexString.hasChanged()) { + getWriter().write(bibtexString.getParsedSerialization()); + return; + } - // Write user comments - String userComments = bibtexString.getUserComments(); - if (!userComments.isEmpty()) { - getWriter().write(userComments + OS.NEWLINE); - } + // Write user comments + String userComments = bibtexString.getUserComments(); + if (!userComments.isEmpty()) { + getWriter().write(userComments + OS.NEWLINE); + } - if (isFirstString) { - getWriter().write(OS.NEWLINE); - } + if (isFirstString) { + getWriter().write(OS.NEWLINE); + } - getWriter().write(STRING_PREFIX + "{" + bibtexString.getName() + StringUtil - .repeatSpaces(maxKeyLength - bibtexString.getName().length()) + " = "); - if (bibtexString.getContent().isEmpty()) { - getWriter().write("{}"); - } else { - try { - String formatted = new LatexFieldFormatter(latexFieldFormatterPreferences) - .format(bibtexString.getContent(), - LatexFieldFormatter.BIBTEX_STRING); - getWriter().write(formatted); - } catch (InvalidFieldValueException ex) { - throw new SaveException(ex); - } + getWriter().write(STRING_PREFIX + "{" + bibtexString.getName() + StringUtil + .repeatSpaces(maxKeyLength - bibtexString.getName().length()) + " = "); + if (bibtexString.getContent().isEmpty()) { + getWriter().write("{}"); + } else { + try { + String formatted = new LatexFieldFormatter(latexFieldFormatterPreferences) + .format(bibtexString.getContent(), + LatexFieldFormatter.BIBTEX_STRING); + getWriter().write(formatted); + } catch (InvalidFieldValueException ex) { + throw new IOException(ex); } - - getWriter().write("}" + OS.NEWLINE); - } catch (IOException e) { - throw new SaveException(e); } + + getWriter().write("}" + OS.NEWLINE); } @Override - protected void writeEntryTypeDefinition(CustomEntryType customType) throws SaveException { - try { - getWriter().write(OS.NEWLINE); - getWriter().write(COMMENT_PREFIX + "{"); - getWriter().write(customType.getAsString()); - getWriter().write("}"); - getWriter().write(OS.NEWLINE); - } catch (IOException e) { - throw new SaveException(e); - } + protected void writeEntryTypeDefinition(CustomEntryType customType) throws IOException { + getWriter().write(OS.NEWLINE); + getWriter().write(COMMENT_PREFIX + "{"); + getWriter().write(customType.getAsString()); + getWriter().write("}"); + getWriter().write(OS.NEWLINE); } @Override - protected void writePrelogue(BibDatabaseContext bibDatabaseContext, Charset encoding) throws SaveException { + protected void writePrelogue(BibDatabaseContext bibDatabaseContext, Charset encoding) throws IOException { if (encoding == null) { return; } // Writes the file encoding information. - try { - getWriter().write("% "); - getWriter().write(SavePreferences.ENCODING_PREFIX + encoding); - getWriter().write(OS.NEWLINE); - - } catch (IOException e) { - throw new SaveException(e); - } + getWriter().write("% "); + getWriter().write(SavePreferences.ENCODING_PREFIX + encoding); + getWriter().write(OS.NEWLINE); } @Override - protected void writeDatabaseID(String sharedDatabaseID) throws SaveException { - try { - StringBuilder stringBuilder = new StringBuilder() - .append("% ") - .append(DATABASE_ID_PREFIX) - .append(" ") - .append(sharedDatabaseID) - .append(OS.NEWLINE); - getWriter().write(stringBuilder.toString()); - } catch (IOException e) { - throw new SaveException(e); - } + protected void writeDatabaseID(String sharedDatabaseID) throws IOException { + getWriter().write("% " + + DATABASE_ID_PREFIX + + " " + + sharedDatabaseID + + OS.NEWLINE); } @Override protected void writeEntry(BibEntry entry, BibDatabaseMode mode, Boolean isReformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws SaveException { + LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException { BibEntryWriter bibtexEntryWriter = new BibEntryWriter( new LatexFieldFormatter(latexFieldFormatterPreferences), true); - try { - bibtexEntryWriter.write(entry, getWriter(), mode, isReformatFile); - } catch (IOException e) { - throw new SaveException(e, entry); - } - } - - private Writer getWriter() { - return getActiveSession().getWriter(); + bibtexEntryWriter.write(entry, getWriter(), mode, isReformatFile); } } diff --git a/src/main/java/org/jabref/logic/exporter/FileSaveSession.java b/src/main/java/org/jabref/logic/exporter/FileSaveSession.java deleted file mode 100644 index 697c319c69b..00000000000 --- a/src/main/java/org/jabref/logic/exporter/FileSaveSession.java +++ /dev/null @@ -1,129 +0,0 @@ -package org.jabref.logic.exporter; - -import java.io.IOException; -import java.nio.charset.Charset; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.attribute.PosixFilePermission; -import java.util.EnumSet; -import java.util.Set; - -import org.jabref.logic.util.io.FileBasedLock; -import org.jabref.logic.util.io.FileUtil; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Class used to handle safe storage to disk. - *

- * Usage: create a SaveSession giving the file to save to, the encoding, and whether to make a backup. The SaveSession - * will provide a Writer to store to, which actually goes to a temporary file. The Writer keeps track of whether all - * characters could be saved, and if not, which characters were not encodable. - *

- * After saving is finished, the client should close the Writer. If the save should be put into effect, call commit(), - * otherwise call cancel(). When canceling, the temporary file is simply deleted and the target file remains unchanged. - * When committing, the temporary file is copied to the target file after making a backup if requested and if the target - * file already existed, and finally the temporary file is deleted. - *

- * If committing fails, the temporary file will not be deleted. - */ -public class FileSaveSession extends SaveSession { - private static final Logger LOGGER = LoggerFactory.getLogger(FileSaveSession.class); - - // Filenames. - private static final String BACKUP_EXTENSION = ".bak"; - private static final String TEMP_PREFIX = "jabref"; - private static final String TEMP_SUFFIX = "save.bib"; - private final Path temporaryFile; - - public FileSaveSession(Charset encoding, boolean backup) throws SaveException { - this(encoding, backup, createTemporaryFile()); - } - - public FileSaveSession(Charset encoding, boolean backup, Path temporaryFile) throws SaveException { - super(encoding, backup, getWriterForFile(encoding, temporaryFile)); - this.temporaryFile = temporaryFile; - } - - private static VerifyingWriter getWriterForFile(Charset encoding, Path file) throws SaveException { - try { - return new VerifyingWriter(Files.newOutputStream(file), encoding); - } catch (IOException e) { - throw new SaveException(e); - } - } - - private static Path createTemporaryFile() throws SaveException { - try { - return Files.createTempFile(FileSaveSession.TEMP_PREFIX, FileSaveSession.TEMP_SUFFIX); - } catch (IOException e) { - throw new SaveException(e); - } - } - - @Override - public void commit(Path file) throws SaveException { - if (file == null) { - return; - } - if (backup && Files.exists(file)) { - Path backupFile = FileUtil.addExtension(file, BACKUP_EXTENSION); - FileUtil.copyFile(file, backupFile, true); - } - try { - // Always use a lock file - try { - if (FileBasedLock.createLockFile(file)) { - // Oops, the lock file already existed. Try to wait it out: - if (!FileBasedLock.waitForFileLock(file)) { - throw SaveException.FILE_LOCKED; - } - } - } catch (IOException ex) { - LOGGER.error("Error when creating lock file.", ex); - } - - // Try to save file permissions to restore them later (by default: 664) - Set oldFilePermissions = EnumSet.of(PosixFilePermission.OWNER_READ, - PosixFilePermission.OWNER_WRITE, - PosixFilePermission.GROUP_READ, - PosixFilePermission.GROUP_WRITE, - PosixFilePermission.OTHERS_READ); - if (FileUtil.IS_POSIX_COMPILANT && Files.exists(file)) { - try { - oldFilePermissions = Files.getPosixFilePermissions(file); - } catch (IOException exception) { - LOGGER.warn("Error getting file permissions.", exception); - } - } - - FileUtil.copyFile(temporaryFile, file, true); - - // Restore file permissions - if (FileUtil.IS_POSIX_COMPILANT) { - try { - Files.setPosixFilePermissions(file, oldFilePermissions); - } catch (IOException exception) { - throw new SaveException(exception); - } - } - } finally { - FileBasedLock.deleteLockFile(file); - } - try { - Files.deleteIfExists(temporaryFile); - } catch (IOException e) { - LOGGER.warn("Cannot delete temporary file", e); - } - } - - @Override - public void cancel() { - try { - Files.deleteIfExists(temporaryFile); - } catch (IOException e) { - LOGGER.warn("Cannot delete temporary file", e); - } - } -} diff --git a/src/main/java/org/jabref/logic/exporter/MSBibExporter.java b/src/main/java/org/jabref/logic/exporter/MSBibExporter.java index 0196abb051f..9a4ff1a2088 100644 --- a/src/main/java/org/jabref/logic/exporter/MSBibExporter.java +++ b/src/main/java/org/jabref/logic/exporter/MSBibExporter.java @@ -38,11 +38,11 @@ public void export(final BibDatabaseContext databaseContext, final Path file, if (entries.isEmpty()) { return; } - // forcing to use UTF8 output format for some problems with xml export in other encodings - SaveSession session = new FileSaveSession(StandardCharsets.UTF_8, false); + MSBibDatabase msBibDatabase = new MSBibDatabase(databaseContext.getDatabase(), entries); - try (VerifyingWriter ps = session.getWriter()) { + // forcing to use UTF8 output format for some problems with xml export in other encodings + try (AtomicFileWriter ps = new AtomicFileWriter(file, StandardCharsets.UTF_8)) { try { DOMSource source = new DOMSource(msBibDatabase.getDomForExport()); StreamResult result = new StreamResult(ps); @@ -52,7 +52,6 @@ public void export(final BibDatabaseContext databaseContext, final Path file, } catch (TransformerException | IllegalArgumentException | TransformerFactoryConfigurationError e) { throw new SaveException(e); } - session.finalize(file); } catch (IOException ex) { throw new SaveException(ex); } diff --git a/src/main/java/org/jabref/logic/exporter/SavePreferences.java b/src/main/java/org/jabref/logic/exporter/SavePreferences.java index c9d68b0c7b6..69e3ccafeb1 100644 --- a/src/main/java/org/jabref/logic/exporter/SavePreferences.java +++ b/src/main/java/org/jabref/logic/exporter/SavePreferences.java @@ -21,15 +21,16 @@ public class SavePreferences { private final boolean takeMetadataSaveOrderInAccount; private final LatexFieldFormatterPreferences latexFieldFormatterPreferences; private final GlobalBibtexKeyPattern globalCiteKeyPattern; + private final Boolean generateBibtexKeysBeforeSaving; public SavePreferences() { this(true, null, null, false, DatabaseSaveType.ALL, true, false, new LatexFieldFormatterPreferences(), - new GlobalBibtexKeyPattern(Collections.emptyList())); + new GlobalBibtexKeyPattern(Collections.emptyList()), false); } public SavePreferences(Boolean saveInOriginalOrder, SaveOrderConfig saveOrder, Charset encoding, Boolean makeBackup, - DatabaseSaveType saveType, Boolean takeMetadataSaveOrderInAccount, Boolean reformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences, GlobalBibtexKeyPattern globalCiteKeyPattern) { + DatabaseSaveType saveType, Boolean takeMetadataSaveOrderInAccount, Boolean reformatFile, + LatexFieldFormatterPreferences latexFieldFormatterPreferences, GlobalBibtexKeyPattern globalCiteKeyPattern, Boolean generateBibtexKeysBeforeSaving) { this.saveInOriginalOrder = saveInOriginalOrder; this.saveOrder = saveOrder; this.encoding = encoding; @@ -39,6 +40,7 @@ public SavePreferences(Boolean saveInOriginalOrder, SaveOrderConfig saveOrder, C this.reformatFile = reformatFile; this.latexFieldFormatterPreferences = latexFieldFormatterPreferences; this.globalCiteKeyPattern = globalCiteKeyPattern; + this.generateBibtexKeysBeforeSaving = generateBibtexKeysBeforeSaving; } public Boolean getTakeMetadataSaveOrderInAccount() { @@ -56,17 +58,17 @@ public boolean isSaveInOriginalOrder() { public SavePreferences withSaveInOriginalOrder(Boolean newSaveInOriginalOrder) { return new SavePreferences(newSaveInOriginalOrder, this.saveOrder, this.encoding, this.makeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - globalCiteKeyPattern); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); } - public boolean getMakeBackup() { + public boolean makeBackup() { return makeBackup; } public SavePreferences withMakeBackup(Boolean newMakeBackup) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, this.encoding, newMakeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - globalCiteKeyPattern); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); } public Charset getEncoding() { @@ -76,7 +78,7 @@ public Charset getEncoding() { public SavePreferences withEncoding(Charset newEncoding) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, newEncoding, this.makeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - globalCiteKeyPattern); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); } public DatabaseSaveType getSaveType() { @@ -86,7 +88,7 @@ public DatabaseSaveType getSaveType() { public SavePreferences withSaveType(DatabaseSaveType newSaveType) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, this.encoding, this.makeBackup, newSaveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - globalCiteKeyPattern); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); } public Boolean isReformatFile() { @@ -96,7 +98,7 @@ public Boolean isReformatFile() { public SavePreferences withReformatFile(boolean newReformatFile) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, this.encoding, this.makeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, newReformatFile, this.latexFieldFormatterPreferences, - globalCiteKeyPattern); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); } public Charset getEncodingOrDefault() { @@ -111,6 +113,10 @@ public GlobalBibtexKeyPattern getGlobalCiteKeyPattern() { return globalCiteKeyPattern; } + public Boolean generateBibtexKeysBeforeSaving() { + return generateBibtexKeysBeforeSaving; + } + public enum DatabaseSaveType { ALL, PLAIN_BIBTEX diff --git a/src/main/java/org/jabref/logic/exporter/SaveSession.java b/src/main/java/org/jabref/logic/exporter/SaveSession.java deleted file mode 100644 index 225e2fab267..00000000000 --- a/src/main/java/org/jabref/logic/exporter/SaveSession.java +++ /dev/null @@ -1,68 +0,0 @@ -package org.jabref.logic.exporter; - -import java.io.IOException; -import java.nio.charset.Charset; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; - -import org.jabref.model.FieldChange; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public abstract class SaveSession { - - private static final Logger LOGGER = LoggerFactory.getLogger(SaveSession.class); - - protected boolean backup; - protected final Charset encoding; - protected final VerifyingWriter writer; - private final List undoableFieldChanges = new ArrayList<>(); - - protected SaveSession(Charset encoding, boolean backup, VerifyingWriter writer) { - this.encoding = Objects.requireNonNull(encoding); - this.backup = backup; - this.writer = Objects.requireNonNull(writer); - } - - public VerifyingWriter getWriter() { - return writer; - } - - public Charset getEncoding() { - return encoding; - } - - public void setUseBackup(boolean useBackup) { - this.backup = useBackup; - } - - public abstract void commit(Path file) throws SaveException; - - public void commit(String path) throws SaveException { - commit(Paths.get(path)); - } - - public abstract void cancel(); - - public List getFieldChanges() { - return undoableFieldChanges; - } - - public void addFieldChanges(List newUndoableFieldChanges) { - this.undoableFieldChanges.addAll(newUndoableFieldChanges); - } - - public void finalize(Path file) throws SaveException, IOException { - getWriter().flush(); - getWriter().close(); - - if (!getWriter().couldEncodeAll()) { - LOGGER.warn("Could not encode..."); - } - commit(file); - } -} diff --git a/src/main/java/org/jabref/logic/exporter/StringSaveSession.java b/src/main/java/org/jabref/logic/exporter/StringSaveSession.java deleted file mode 100644 index 7948e98f94f..00000000000 --- a/src/main/java/org/jabref/logic/exporter/StringSaveSession.java +++ /dev/null @@ -1,54 +0,0 @@ -package org.jabref.logic.exporter; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.nio.charset.Charset; -import java.nio.file.Files; -import java.nio.file.Path; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class StringSaveSession extends SaveSession { - - private static final Logger LOGGER = LoggerFactory.getLogger(StringSaveSession.class); - - private final ByteArrayOutputStream outputStream; - - public StringSaveSession(Charset encoding, boolean backup) { - this(encoding, backup, new ByteArrayOutputStream()); - } - - private StringSaveSession(Charset encoding, boolean backup, ByteArrayOutputStream outputStream) { - super(encoding, backup, new VerifyingWriter(outputStream, encoding)); - this.outputStream = outputStream; - } - - public String getStringValue() { - try { - return outputStream.toString(encoding.name()); - } catch (UnsupportedEncodingException e) { - LOGGER.warn("Encoding problem", e); - return ""; - } - } - - @Override - public void commit(Path file) throws SaveException { - try { - Files.write(file, outputStream.toByteArray()); - } catch (IOException e) { - throw new SaveException(e); - } - } - - @Override - public void cancel() { - try { - outputStream.close(); - } catch (IOException e) { - LOGGER.warn("Error while cancel", e); - } - } -} diff --git a/src/main/java/org/jabref/logic/exporter/TemplateExporter.java b/src/main/java/org/jabref/logic/exporter/TemplateExporter.java index cf7d465602f..de3a7ab022c 100644 --- a/src/main/java/org/jabref/logic/exporter/TemplateExporter.java +++ b/src/main/java/org/jabref/logic/exporter/TemplateExporter.java @@ -180,22 +180,8 @@ public void export(final BibDatabaseContext databaseContext, final Path file, if (entries.isEmpty()) { // Do not export if no entries to export -- avoids exports with only template text return; } - SaveSession saveSession = null; - if (this.encoding != null) { - try { - saveSession = new FileSaveSession(this.encoding, false); - } catch (SaveException ex) { - // Perhaps the overriding encoding doesn't work? - // We will fall back on the default encoding. - LOGGER.warn("Cannot get save session.", ex); - } - } - if (saveSession == null) { - saveSession = new FileSaveSession(encoding, false); - } - - try (VerifyingWriter ps = saveSession.getWriter()) { + try (AtomicFileWriter ps = new AtomicFileWriter(file, encoding)) { Layout beginLayout = null; // Check if this export filter has bundled name formatters: @@ -305,9 +291,7 @@ public void export(final BibDatabaseContext databaseContext, final Path file, sb.append(String.join(", ", missingFormatters)); LOGGER.warn("Formatters not found", sb); } - saveSession.finalize(file); } - } /** diff --git a/src/main/java/org/jabref/logic/exporter/VerifyingWriter.java b/src/main/java/org/jabref/logic/exporter/VerifyingWriter.java deleted file mode 100644 index a55bf9a6740..00000000000 --- a/src/main/java/org/jabref/logic/exporter/VerifyingWriter.java +++ /dev/null @@ -1,52 +0,0 @@ -package org.jabref.logic.exporter; - -import java.io.IOException; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.nio.charset.Charset; -import java.nio.charset.CharsetEncoder; -import java.util.Set; -import java.util.TreeSet; - -/** - * Writer that extends OutputStreamWriter, but also checks if the chosen - * encoding supports all text that is written. Currently only a boolean value is - * stored to remember whether everything has gone well or not. - */ -public class VerifyingWriter extends OutputStreamWriter { - - private final CharsetEncoder encoder; - private boolean couldEncodeAll = true; - private final Set problemCharacters = new TreeSet<>(); - - - public VerifyingWriter(OutputStream out, Charset encoding) { - super(out, encoding); - encoder = encoding.newEncoder(); - } - - @Override - public void write(String str) throws IOException { - super.write(str); - if (!encoder.canEncode(str)) { - for (int i = 0; i < str.length(); i++) { - if (!encoder.canEncode(str.charAt(i))) { - problemCharacters.add(str.charAt(i)); - } - } - couldEncodeAll = false; - } - } - - public boolean couldEncodeAll() { - return couldEncodeAll; - } - - public String getProblemCharacters() { - StringBuilder chars = new StringBuilder(); - for (Character ch : problemCharacters) { - chars.append(ch.charValue()); - } - return chars.toString(); - } -} diff --git a/src/main/java/org/jabref/logic/util/io/FileUtil.java b/src/main/java/org/jabref/logic/util/io/FileUtil.java index 8a759739528..89ec1af49d2 100644 --- a/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -96,8 +96,7 @@ public static String getValidFileName(String fileName) { * @return the with the modified file name */ public static Path addExtension(Path path, String extension) { - Path fileName = path.getFileName(); - return path.resolveSibling(fileName + extension); + return path.resolveSibling(path.getFileName() + extension); } /** diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 76c6bfb6c9f..0b9c8cc1a21 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -117,6 +117,10 @@ public void setDatabaseFile(File file) { this.file = Optional.ofNullable(file).map(File::toPath); } + public void setDatabaseFile(Path file) { + this.file = Optional.ofNullable(file); + } + public Optional getDatabasePath() { return file; } diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 1be4e2a6052..9ba3f30909d 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1382,12 +1382,12 @@ public void setDefaultEncoding(Charset encoding) { } public FileHistory getFileHistory() { - return new FileHistory(getStringList(RECENT_DATABASES)); + return new FileHistory(getStringList(RECENT_DATABASES).stream().map(Paths::get).collect(Collectors.toList())); } public void storeFileHistory(FileHistory history) { if (!history.isEmpty()) { - putStringList(RECENT_DATABASES, history.getHistory()); + putStringList(RECENT_DATABASES, history.getHistory().stream().map(Path::toAbsolutePath).map(Path::toString).collect(Collectors.toList())); } } @@ -1437,29 +1437,31 @@ public SavePreferences loadForExportFromPreferences() { saveOrder = this.loadTableSaveOrder(); } } - Charset encoding = this.getDefaultEncoding(); - Boolean makeBackup = this.getBoolean(JabRefPreferences.BACKUP); - SavePreferences.DatabaseSaveType saveType = SavePreferences.DatabaseSaveType.ALL; - Boolean takeMetadataSaveOrderInAccount = false; - Boolean reformatFile = this.getBoolean(JabRefPreferences.REFORMAT_FILE_ON_SAVE_AND_EXPORT); - LatexFieldFormatterPreferences latexFieldFormatterPreferences = this.getLatexFieldFormatterPreferences(); - GlobalBibtexKeyPattern globalCiteKeyPattern = this.getKeyPattern(); - return new SavePreferences(saveInOriginalOrder, saveOrder, encoding, makeBackup, saveType, - takeMetadataSaveOrderInAccount, reformatFile, latexFieldFormatterPreferences, globalCiteKeyPattern); + return new SavePreferences( + saveInOriginalOrder, + saveOrder, + this.getDefaultEncoding(), + this.getBoolean(JabRefPreferences.BACKUP), + SavePreferences.DatabaseSaveType.ALL, + false, + this.getBoolean(JabRefPreferences.REFORMAT_FILE_ON_SAVE_AND_EXPORT), + this.getLatexFieldFormatterPreferences(), + this.getKeyPattern(), + getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING)); } public SavePreferences loadForSaveFromPreferences() { - Boolean saveInOriginalOrder = false; - SaveOrderConfig saveOrder = null; - Charset encoding = this.getDefaultEncoding(); - Boolean makeBackup = this.getBoolean(JabRefPreferences.BACKUP); - SavePreferences.DatabaseSaveType saveType = SavePreferences.DatabaseSaveType.ALL; - Boolean takeMetadataSaveOrderInAccount = true; - Boolean reformatFile = this.getBoolean(JabRefPreferences.REFORMAT_FILE_ON_SAVE_AND_EXPORT); - LatexFieldFormatterPreferences latexFieldFormatterPreferences = this.getLatexFieldFormatterPreferences(); - GlobalBibtexKeyPattern globalCiteKeyPattern = this.getKeyPattern(); - return new SavePreferences(saveInOriginalOrder, saveOrder, encoding, makeBackup, saveType, - takeMetadataSaveOrderInAccount, reformatFile, latexFieldFormatterPreferences, globalCiteKeyPattern); + return new SavePreferences( + false, + null, + this.getDefaultEncoding(), + this.getBoolean(JabRefPreferences.BACKUP), + SavePreferences.DatabaseSaveType.ALL, + true, + this.getBoolean(JabRefPreferences.REFORMAT_FILE_ON_SAVE_AND_EXPORT), + this.getLatexFieldFormatterPreferences(), + this.getKeyPattern(), + getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING)); } public ExporterFactory getExporterFactory(JournalAbbreviationLoader abbreviationLoader) { diff --git a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java index 71aeae47d96..29b492f2c61 100644 --- a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java +++ b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java @@ -12,6 +12,7 @@ import org.jabref.logic.util.OS; import org.jabref.model.database.BibDatabaseMode; import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.LinkedFile; import org.jabref.model.util.DummyFileUpdateMonitor; import org.jabref.model.util.FileUpdateMonitor; @@ -84,6 +85,25 @@ public void writeOtherTypeTest() throws Exception { assertEquals(expected, actual); } + @Test + void writeEntryWithFile() throws Exception { + BibEntry entry = new BibEntry(); + LinkedFile file = new LinkedFile("test", "/home/uers/test.pdf", "PDF"); + entry.addFile(file); + + StringWriter stringWriter = new StringWriter(); + writer.write(entry, stringWriter, BibDatabaseMode.BIBTEX); + + assertEquals(OS.NEWLINE + + "@Article{," + + OS.NEWLINE + + " file = {test:/home/uers/test.pdf:PDF}," + + OS.NEWLINE + + "}" + OS.NEWLINE + + OS.NEWLINE + + "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, stringWriter.toString()); + } + @Test public void writeReallyunknownTypeTest() throws Exception { String expected = OS.NEWLINE + "@Reallyunknowntype{test," + OS.NEWLINE + diff --git a/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java b/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java index 00a96b5d7fa..a49da4ec252 100644 --- a/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java +++ b/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java @@ -1,7 +1,7 @@ package org.jabref.logic.exporter; -import java.io.IOException; import java.io.StringReader; +import java.io.StringWriter; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Path; @@ -46,9 +46,10 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; -public class BibtexDatabaseWriterTest { +class BibtexDatabaseWriterTest { - private BibtexDatabaseWriter databaseWriter; + private StringWriter stringWriter; + private BibtexDatabaseWriter databaseWriter; private BibDatabase database; private MetaData metaData; private BibDatabaseContext bibtexContext; @@ -56,9 +57,9 @@ public class BibtexDatabaseWriterTest { private final FileUpdateMonitor fileMonitor = new DummyFileUpdateMonitor(); @BeforeEach - public void setUp() { - // Write to a string instead of to a file - databaseWriter = new BibtexDatabaseWriter<>(StringSaveSession::new); + void setUp() { + stringWriter = new StringWriter(); + databaseWriter = new BibtexDatabaseWriter(stringWriter); database = new BibDatabase(); metaData = new MetaData(); @@ -67,151 +68,151 @@ public void setUp() { } @Test - public void writeWithNullContextThrowsException() throws Exception { + void writeWithNullContextThrowsException() throws Exception { assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(null, Collections.emptyList(), new SavePreferences())); } @Test - public void writeWithNullEntriesThrowsException() throws Exception { + void writeWithNullEntriesThrowsException() throws Exception { assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(bibtexContext, null, new SavePreferences())); } @Test - public void writeWithNullPreferencesThrowsException() throws Exception { + void writeWithNullPreferencesThrowsException() throws Exception { assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), null)); } @Test - public void writeEncoding() throws Exception { + void writeEncoding() throws Exception { SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); - assertEquals("% Encoding: US-ASCII" + OS.NEWLINE, session.getStringValue()); + assertEquals("% Encoding: US-ASCII" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writePreamble() throws Exception { + void writePreamble() throws Exception { database.setPreamble("Test preamble"); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); - assertEquals(OS.NEWLINE + "@Preamble{Test preamble}" + OS.NEWLINE, session.getStringValue()); + assertEquals(OS.NEWLINE + "@Preamble{Test preamble}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writePreambleAndEncoding() throws Exception { + void writePreambleAndEncoding() throws Exception { SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); database.setPreamble("Test preamble"); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + - "@Preamble{Test preamble}" + OS.NEWLINE, session.getStringValue()); + "@Preamble{Test preamble}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeEntry() throws Exception { + void writeEntry() throws Exception { BibEntry entry = new BibEntry(); entry.setType(BibtexEntryTypes.ARTICLE); database.insertEntry(entry); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); assertEquals(OS.NEWLINE + "@Article{," + OS.NEWLINE + "}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: databaseType:bibtex;}" - + OS.NEWLINE, session.getStringValue()); + + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeEncodingAndEntry() throws Exception { + void writeEncodingAndEntry() throws Exception { SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); BibEntry entry = new BibEntry(); entry.setType(BibtexEntryTypes.ARTICLE); database.insertEntry(entry); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), preferences); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + "@Article{," + OS.NEWLINE + "}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: databaseType:bibtex;}" - + OS.NEWLINE, session.getStringValue()); + + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeEpilogue() throws Exception { + void writeEpilogue() throws Exception { database.setEpilog("Test epilog"); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); - assertEquals(OS.NEWLINE + "Test epilog" + OS.NEWLINE, session.getStringValue()); + assertEquals(OS.NEWLINE + "Test epilog" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeEpilogueAndEncoding() throws Exception { + void writeEpilogueAndEncoding() throws Exception { SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); database.setEpilog("Test epilog"); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + - "Test epilog" + OS.NEWLINE, session.getStringValue()); + "Test epilog" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeMetadata() throws Exception { + void writeMetadata() throws Exception { DatabaseBibtexKeyPattern bibtexKeyPattern = new DatabaseBibtexKeyPattern(mock(GlobalBibtexKeyPattern.class)); bibtexKeyPattern.setDefaultValue("test"); metaData.setCiteKeyPattern(bibtexKeyPattern); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: keypatterndefault:test;}" + OS.NEWLINE, - session.getStringValue()); + stringWriter.toString()); } @Test - public void writeMetadataAndEncoding() throws Exception { + void writeMetadataAndEncoding() throws Exception { SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); DatabaseBibtexKeyPattern bibtexKeyPattern = new DatabaseBibtexKeyPattern(mock(GlobalBibtexKeyPattern.class)); bibtexKeyPattern.setDefaultValue("test"); metaData.setCiteKeyPattern(bibtexKeyPattern); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + - "@Comment{jabref-meta: keypatterndefault:test;}" + OS.NEWLINE, session.getStringValue()); + "@Comment{jabref-meta: keypatterndefault:test;}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeGroups() throws Exception { + void writeGroups() throws Exception { GroupTreeNode groupRoot = GroupTreeNode.fromGroup(new AllEntriesGroup("")); groupRoot.addSubgroup(new ExplicitGroup("test", GroupHierarchyType.INCLUDING, ',')); metaData.setGroups(groupRoot); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); // @formatter:off assertEquals(OS.NEWLINE + "@Comment{jabref-meta: grouping:" + OS.NEWLINE + "0 AllEntriesGroup:;" + OS.NEWLINE + "1 StaticGroup:test\\;2\\;1\\;\\;\\;\\;;" + OS.NEWLINE - + "}" + OS.NEWLINE, session.getStringValue()); + + "}" + OS.NEWLINE, stringWriter.toString()); // @formatter:on } @Test - public void writeGroupsAndEncoding() throws Exception { + void writeGroupsAndEncoding() throws Exception { SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); GroupTreeNode groupRoot = GroupTreeNode.fromGroup(new AllEntriesGroup("")); groupRoot.addChild(GroupTreeNode.fromGroup(new ExplicitGroup("test", GroupHierarchyType.INCLUDING, ','))); metaData.setGroups(groupRoot); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); // @formatter:off assertEquals( @@ -220,39 +221,39 @@ public void writeGroupsAndEncoding() throws Exception { + "@Comment{jabref-meta: grouping:" + OS.NEWLINE + "0 AllEntriesGroup:;" + OS.NEWLINE + "1 StaticGroup:test\\;2\\;1\\;\\;\\;\\;;" + OS.NEWLINE - + "}" + OS.NEWLINE, session.getStringValue()); + + "}" + OS.NEWLINE, stringWriter.toString()); // @formatter:on } @Test - public void writeString() throws Exception { + void writeString() throws Exception { database.addString(new BibtexString("name", "content")); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); - assertEquals(OS.NEWLINE + "@String{name = {content}}" + OS.NEWLINE, session.getStringValue()); + assertEquals(OS.NEWLINE + "@String{name = {content}}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeStringAndEncoding() throws Exception { + void writeStringAndEncoding() throws Exception { SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); database.addString(new BibtexString("name", "content")); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + - "@String{name = {content}}" + OS.NEWLINE, session.getStringValue()); + "@String{name = {content}}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeEntryWithCustomizedTypeAlsoWritesTypeDeclaration() throws Exception { + void writeEntryWithCustomizedTypeAlsoWritesTypeDeclaration() throws Exception { try { EntryTypes.addOrModifyCustomEntryType(new CustomEntryType("customizedType", "required", "optional"), BibDatabaseMode.BIBTEX); BibEntry entry = new BibEntry(); entry.setType("customizedType"); database.insertEntry(entry); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); assertEquals( OS.NEWLINE + @@ -260,14 +261,14 @@ public void writeEntryWithCustomizedTypeAlsoWritesTypeDeclaration() throws Excep + "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-entrytype: Customizedtype: req[required] opt[optional]}" + OS.NEWLINE, - session.getStringValue()); + stringWriter.toString()); } finally { EntryTypes.removeAllCustomEntryTypes(); } } @Test - public void roundtrip() throws Exception { + void roundtrip() throws Exception { Path testBibtexFile = Paths.get("src/test/resources/testbib/complex.bib"); Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); @@ -276,14 +277,14 @@ public void roundtrip() throws Exception { BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - StringSaveSession session = databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { - assertEquals(scanner.useDelimiter("\\A").next(), session.getStringValue()); + assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } } @Test - public void roundtripWithUserComment() throws Exception { + void roundtripWithUserComment() throws Exception { Path testBibtexFile = Paths.get("src/test/resources/testbib/bibWithUserComments.bib"); Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); @@ -292,14 +293,14 @@ public void roundtripWithUserComment() throws Exception { BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - StringSaveSession session = databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { - assertEquals(scanner.useDelimiter("\\A").next(), session.getStringValue()); + assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } } @Test - public void roundtripWithUserCommentAndEntryChange() throws Exception { + void roundtripWithUserCommentAndEntryChange() throws Exception { Path testBibtexFile = Paths.get("src/test/resources/testbib/bibWithUserComments.bib"); Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); @@ -311,15 +312,15 @@ public void roundtripWithUserCommentAndEntryChange() throws Exception { BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - StringSaveSession session = databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); - try (Scanner scanner = new Scanner(Paths.get("src/test/resources/testbib/bibWithUserCommentAndEntryChange.bib"),encoding.name())) { - assertEquals(scanner.useDelimiter("\\A").next(), session.getStringValue()); + try (Scanner scanner = new Scanner(Paths.get("src/test/resources/testbib/bibWithUserCommentAndEntryChange.bib"), encoding.name())) { + assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } } @Test - public void roundtripWithUserCommentBeforeStringAndChange() throws Exception { + void roundtripWithUserCommentBeforeStringAndChange() throws Exception { Path testBibtexFile = Paths.get("src/test/resources/testbib/complex.bib"); Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); @@ -333,15 +334,15 @@ public void roundtripWithUserCommentBeforeStringAndChange() throws Exception { BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - StringSaveSession session = databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { - assertEquals(scanner.useDelimiter("\\A").next(), session.getStringValue()); + assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } } @Test - public void roundtripWithUnknownMetaData() throws Exception { + void roundtripWithUnknownMetaData() throws Exception { Path testBibtexFile = Paths.get("src/test/resources/testbib/unknownMetaData.bib"); Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); @@ -350,14 +351,14 @@ public void roundtripWithUnknownMetaData() throws Exception { BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - StringSaveSession session = databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { - assertEquals(scanner.useDelimiter("\\A").next(), session.getStringValue()); + assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } } @Test - public void writeSavedSerializationOfEntryIfUnchanged() throws Exception { + void writeSavedSerializationOfEntryIfUnchanged() throws Exception { BibEntry entry = new BibEntry(); entry.setType(BibtexEntryTypes.ARTICLE); entry.setField("author", "Mr. author"); @@ -365,14 +366,14 @@ public void writeSavedSerializationOfEntryIfUnchanged() throws Exception { entry.setChanged(false); database.insertEntry(entry); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); assertEquals("presaved serialization" + OS.NEWLINE + "@Comment{jabref-meta: databaseType:bibtex;}" - + OS.NEWLINE, session.getStringValue()); + + OS.NEWLINE, stringWriter.toString()); } @Test - public void reformatEntryIfAskedToDoSo() throws Exception { + void reformatEntryIfAskedToDoSo() throws Exception { BibEntry entry = new BibEntry(); entry.setType(BibtexEntryTypes.ARTICLE); entry.setField("author", "Mr. author"); @@ -381,114 +382,114 @@ public void reformatEntryIfAskedToDoSo() throws Exception { database.insertEntry(entry); SavePreferences preferences = new SavePreferences().withReformatFile(true); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), preferences); assertEquals(OS.NEWLINE + "@Article{," + OS.NEWLINE + " author = {Mr. author}," + OS.NEWLINE + "}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, - session.getStringValue()); + stringWriter.toString()); } @Test - public void writeSavedSerializationOfStringIfUnchanged() throws Exception { + void writeSavedSerializationOfStringIfUnchanged() throws Exception { BibtexString string = new BibtexString("name", "content"); string.setParsedSerialization("serialization"); database.addString(string); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); - assertEquals("serialization", session.getStringValue()); + assertEquals("serialization", stringWriter.toString()); } @Test - public void reformatStringIfAskedToDoSo() throws Exception { + void reformatStringIfAskedToDoSo() throws Exception { BibtexString string = new BibtexString("name", "content"); string.setParsedSerialization("wrong serialization"); database.addString(string); SavePreferences preferences = new SavePreferences().withReformatFile(true); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); - assertEquals(OS.NEWLINE + "@String{name = {content}}" + OS.NEWLINE, session.getStringValue()); + assertEquals(OS.NEWLINE + "@String{name = {content}}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeSaveActions() throws Exception { + void writeSaveActions() throws Exception { FieldFormatterCleanups saveActions = new FieldFormatterCleanups(true, Collections.singletonList(new FieldFormatterCleanup("title", new LowerCaseFormatter()))); metaData.setSaveActions(saveActions); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: saveActions:enabled;" + OS.NEWLINE - + "title[lower_case]" + OS.NEWLINE + ";}" + OS.NEWLINE, session.getStringValue()); + + "title[lower_case]" + OS.NEWLINE + ";}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeSaveOrderConfig() throws Exception { + void writeSaveOrderConfig() throws Exception { SaveOrderConfig saveOrderConfig = new SaveOrderConfig(false, new SaveOrderConfig.SortCriterion("author", false), new SaveOrderConfig.SortCriterion("year", true), new SaveOrderConfig.SortCriterion("abstract", false)); metaData.setSaveOrderConfig(saveOrderConfig); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: saveOrderConfig:specified;author;false;year;true;abstract;false;}" - + OS.NEWLINE, session.getStringValue()); + + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeCustomKeyPattern() throws Exception { + void writeCustomKeyPattern() throws Exception { AbstractBibtexKeyPattern pattern = new DatabaseBibtexKeyPattern(mock(GlobalBibtexKeyPattern.class)); pattern.setDefaultValue("test"); pattern.addBibtexKeyPattern("article", "articleTest"); metaData.setCiteKeyPattern(pattern); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: keypattern_article:articleTest;}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: keypatterndefault:test;}" + OS.NEWLINE, - session.getStringValue()); + stringWriter.toString()); } @Test - public void writeBiblatexMode() throws Exception { + void writeBiblatexMode() throws Exception { metaData.setMode(BibDatabaseMode.BIBLATEX); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: databaseType:biblatex;}" + OS.NEWLINE, - session.getStringValue()); + stringWriter.toString()); } @Test - public void writeProtectedFlag() throws Exception { + void writeProtectedFlag() throws Exception { metaData.markAsProtected(); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: protectedFlag:true;}" + OS.NEWLINE, - session.getStringValue()); + stringWriter.toString()); } @Test - public void writeFileDirectories() throws Exception { + void writeFileDirectories() throws Exception { metaData.setDefaultFileDirectory("\\Literature\\"); metaData.setUserFileDirectory("defaultOwner-user", "D:\\Documents"); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: fileDirectory:\\\\Literature\\\\;}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: fileDirectory-defaultOwner-user:D:\\\\Documents;}" - + OS.NEWLINE, session.getStringValue()); + + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeEntriesSorted() throws Exception { + void writeEntriesSorted() throws Exception { SaveOrderConfig saveOrderConfig = new SaveOrderConfig(false, new SaveOrderConfig.SortCriterion("author", false), new SaveOrderConfig.SortCriterion("year", true), new SaveOrderConfig.SortCriterion("abstract", false)); @@ -513,7 +514,7 @@ public void writeEntriesSorted() throws Exception { database.insertEntry(thirdEntry); database.insertEntry(firstEntry); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries(), new SavePreferences()); assertEquals( OS.NEWLINE + @@ -533,11 +534,11 @@ public void writeEntriesSorted() throws Exception { + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: saveOrderConfig:specified;author;false;year;true;abstract;false;}" + OS.NEWLINE - , session.getStringValue()); + , stringWriter.toString()); } @Test - public void writeEntriesInOriginalOrderWhenNoSaveOrderConfigIsSetInMetadata() throws Exception { + void writeEntriesInOriginalOrderWhenNoSaveOrderConfigIsSetInMetadata() throws Exception { BibEntry firstEntry = new BibEntry(); firstEntry.setType(BibtexEntryTypes.ARTICLE); firstEntry.setField("author", "A"); @@ -558,7 +559,7 @@ public void writeEntriesInOriginalOrderWhenNoSaveOrderConfigIsSetInMetadata() th database.insertEntry(thirdEntry); SavePreferences preferences = new SavePreferences().withSaveInOriginalOrder(false); - StringSaveSession session = databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries(), preferences); assertEquals( OS.NEWLINE + @@ -577,11 +578,11 @@ public void writeEntriesInOriginalOrderWhenNoSaveOrderConfigIsSetInMetadata() th + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE - , session.getStringValue()); + , stringWriter.toString()); } @Test - public void roundtripWithContentSelectorsAndUmlauts() throws IOException, SaveException { + void roundtripWithContentSelectorsAndUmlauts() throws Exception { String fileContent = "% Encoding: UTF-8" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: selector_journal:Test {\\\\\"U}mlaut;}" + OS.NEWLINE; Charset encoding = StandardCharsets.UTF_8; @@ -591,9 +592,8 @@ public void roundtripWithContentSelectorsAndUmlauts() throws IOException, SaveEx BibDatabaseContext context = new BibDatabaseContext(firstParse.getDatabase(), firstParse.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - StringSaveSession session = databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries(), preferences); - assertEquals(fileContent, session.getStringValue()); + assertEquals(fileContent, stringWriter.toString()); } - } diff --git a/src/test/java/org/jabref/model/entry/FileFieldBibEntryTest.java b/src/test/java/org/jabref/model/entry/FileFieldBibEntryTest.java deleted file mode 100644 index c6505dc437d..00000000000 --- a/src/test/java/org/jabref/model/entry/FileFieldBibEntryTest.java +++ /dev/null @@ -1,61 +0,0 @@ -package org.jabref.model.entry; - -import org.jabref.logic.exporter.BibtexDatabaseWriter; -import org.jabref.logic.exporter.SaveException; -import org.jabref.logic.exporter.SavePreferences; -import org.jabref.logic.exporter.StringSaveSession; -import org.jabref.logic.util.OS; -import org.jabref.model.Defaults; -import org.jabref.model.database.BibDatabase; -import org.jabref.model.database.BibDatabaseContext; -import org.jabref.model.metadata.MetaData; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -public class FileFieldBibEntryTest { - - private BibEntry emptyEntry; - - @BeforeEach - public void setUp() { - emptyEntry = new BibEntry(); - emptyEntry.setType("article"); - emptyEntry.setChanged(false); - } - - @Test - public void testFileFieldSerialization() { - LinkedFile file = new LinkedFile("test", "/home/uers/test.pdf", "PDF"); - emptyEntry.addFile(file); - - assertEquals("@article{,\n" + - " file = {test:/home/uers/test.pdf:PDF}\n" + - "}", emptyEntry.toString()); - } - - @Test - public void testFileFieldSerializationDatabase() throws SaveException { - BibDatabase database = new BibDatabase(); - - LinkedFile file = new LinkedFile("test", "/home/uers/test.pdf", "PDF"); - emptyEntry.addFile(file); - database.insertEntries(emptyEntry); - - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter<>(StringSaveSession::new); - StringSaveSession saveSession = databaseWriter.savePartOfDatabase( - new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries(), - new SavePreferences()); - - assertEquals(OS.NEWLINE + - "@Article{," - + OS.NEWLINE - + " file = {test:/home/uers/test.pdf:PDF}," - + OS.NEWLINE - + "}" + OS.NEWLINE - + OS.NEWLINE - + "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, saveSession.getStringValue()); - } -} From dbc9f45a43c2ae48fd5967847cb590940ec78295 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 1 Sep 2018 12:19:31 +0200 Subject: [PATCH 3/9] Fix build --- .../jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java b/src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java index d03300bba12..18d0c881baf 100644 --- a/src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java +++ b/src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java @@ -161,7 +161,7 @@ private void openSharedDatabase(DBMSConnectionProperties connectionProperties) { if (!folder.getValue().isEmpty()) { try { - new SaveDatabaseAction(panel, Paths.get(folder.getValue())).runCommand(); + new SaveDatabaseAction(panel).saveAs(Paths.get(folder.getValue())); } catch (Throwable e) { LOGGER.error("Error while saving the database", e); } From 161a7a0151adabfba2ef9d6a182bb2ca25292024 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 1 Sep 2018 23:55:10 +0200 Subject: [PATCH 4/9] Fix tests --- .../org/jabref/benchmarks/Benchmarks.java | 13 +- .../org/jabref/cli/ArgumentProcessor.java | 4 +- .../jabref/gui/SaveOrderConfigDisplay.java | 35 ++-- .../org/jabref/gui/collab/ChangeScanner.java | 4 +- .../DatabasePropertiesDialog.java | 2 +- .../gui/exporter/SaveDatabaseAction.java | 6 +- .../autosaveandbackup/BackupManager.java | 4 +- .../exporter/AtomicFileOutputStream.java | 4 +- .../logic/exporter/BibDatabaseWriter.java | 73 +++----- .../logic/exporter/BibtexDatabaseWriter.java | 81 ++++----- .../logic/exporter/SavePreferences.java | 28 +-- .../model/metadata/SaveOrderConfig.java | 167 ++++++++---------- .../jabref/preferences/JabRefPreferences.java | 46 +++-- .../java/org/jabref/gui/AbstractUITest.java | 9 +- .../java/org/jabref/gui/EntryTableTest.java | 2 +- src/test/java/org/jabref/gui/UndoTest.java | 2 +- .../logic/bibtex/BibEntryWriterTest.java | 3 +- .../exporter/BibtexDatabaseWriterTest.java | 136 +++++++------- .../importer/fileformat/BibtexParserTest.java | 9 +- .../logic/shared/DBMSProcessorTest.java | 19 +- 20 files changed, 304 insertions(+), 343 deletions(-) diff --git a/src/jmh/java/org/jabref/benchmarks/Benchmarks.java b/src/jmh/java/org/jabref/benchmarks/Benchmarks.java index efcb7f937c2..c17b4c6e76b 100644 --- a/src/jmh/java/org/jabref/benchmarks/Benchmarks.java +++ b/src/jmh/java/org/jabref/benchmarks/Benchmarks.java @@ -36,6 +36,8 @@ import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.runner.RunnerException; +import static org.mockito.Mockito.mock; + @State(Scope.Thread) public class Benchmarks { @@ -61,10 +63,9 @@ public void init() throws Exception { database.insertEntry(entry); } StringWriter outputWriter = new StringWriter(); - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(outputWriter); + BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(outputWriter, mock(SavePreferences.class)); databaseWriter.savePartOfDatabase( - new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries(), - new SavePreferences()); + new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries()); bibtexString = outputWriter.toString(); latexConversionString = "{A} \\textbf{bold} approach {\\it to} ${{\\Sigma}}{\\Delta}$ modulator \\textsuperscript{2} \\$"; @@ -81,10 +82,8 @@ public ParserResult parse() throws IOException { @Benchmark public String write() throws Exception { StringWriter outputWriter = new StringWriter(); - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(outputWriter); - databaseWriter.savePartOfDatabase( - new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries(), - new SavePreferences()); + BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(outputWriter, mock(SavePreferences.class)); + databaseWriter.savePartOfDatabase(new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries()); return outputWriter.toString(); } diff --git a/src/main/java/org/jabref/cli/ArgumentProcessor.java b/src/main/java/org/jabref/cli/ArgumentProcessor.java index 60def445da7..74669554436 100644 --- a/src/main/java/org/jabref/cli/ArgumentProcessor.java +++ b/src/main/java/org/jabref/cli/ArgumentProcessor.java @@ -390,9 +390,9 @@ private void saveDatabase(BibDatabase newBase, String subName) { System.out.println(Localization.lang("Saving") + ": " + subName); SavePreferences prefs = Globals.prefs.loadForSaveFromPreferences(); AtomicFileWriter fileWriter = new AtomicFileWriter(Paths.get(subName), prefs.getEncoding()); - BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter); + BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter, prefs); Defaults defaults = new Defaults(Globals.prefs.getDefaultBibDatabaseMode()); - databaseWriter.saveDatabase(new BibDatabaseContext(newBase, defaults), prefs); + databaseWriter.saveDatabase(new BibDatabaseContext(newBase, defaults)); // Show just a warning message if encoding did not work for all characters: if (fileWriter.hasEncodingProblems()) { diff --git a/src/main/java/org/jabref/gui/SaveOrderConfigDisplay.java b/src/main/java/org/jabref/gui/SaveOrderConfigDisplay.java index c55dd92fd35..72bd104f5c8 100644 --- a/src/main/java/org/jabref/gui/SaveOrderConfigDisplay.java +++ b/src/main/java/org/jabref/gui/SaveOrderConfigDisplay.java @@ -83,30 +83,29 @@ public void setEnabled(boolean enabled) { saveTerDesc.setDisable(!enabled); } - public void setSaveOrderConfig(SaveOrderConfig saveOrderConfig) { - Objects.requireNonNull(saveOrderConfig); - - savePriSort.setValue(saveOrderConfig.sortCriteria[0].field); - savePriDesc.setSelected(saveOrderConfig.sortCriteria[0].descending); - saveSecSort.setValue(saveOrderConfig.sortCriteria[1].field); - saveSecDesc.setSelected(saveOrderConfig.sortCriteria[1].descending); - saveTerSort.setValue(saveOrderConfig.sortCriteria[2].field); - saveTerDesc.setSelected(saveOrderConfig.sortCriteria[2].descending); - - } - public SaveOrderConfig getSaveOrderConfig() { SaveOrderConfig saveOrderConfig = new SaveOrderConfig(); - saveOrderConfig.sortCriteria[0].field = getSelectedItemAsLowerCaseTrim(savePriSort); - saveOrderConfig.sortCriteria[0].descending = savePriDesc.isSelected(); - saveOrderConfig.sortCriteria[1].field = getSelectedItemAsLowerCaseTrim(saveSecSort); - saveOrderConfig.sortCriteria[1].descending = saveSecDesc.isSelected(); - saveOrderConfig.sortCriteria[2].field = getSelectedItemAsLowerCaseTrim(saveTerSort); - saveOrderConfig.sortCriteria[2].descending = saveTerDesc.isSelected(); + saveOrderConfig.getSortCriteria().get(0).field = getSelectedItemAsLowerCaseTrim(savePriSort); + saveOrderConfig.getSortCriteria().get(0).descending = savePriDesc.isSelected(); + saveOrderConfig.getSortCriteria().get(1).field = getSelectedItemAsLowerCaseTrim(saveSecSort); + saveOrderConfig.getSortCriteria().get(1).descending = saveSecDesc.isSelected(); + saveOrderConfig.getSortCriteria().get(2).field = getSelectedItemAsLowerCaseTrim(saveTerSort); + saveOrderConfig.getSortCriteria().get(2).descending = saveTerDesc.isSelected(); return saveOrderConfig; } + public void setSaveOrderConfig(SaveOrderConfig saveOrderConfig) { + Objects.requireNonNull(saveOrderConfig); + + savePriSort.setValue(saveOrderConfig.getSortCriteria().get(0).field); + savePriDesc.setSelected(saveOrderConfig.getSortCriteria().get(0).descending); + saveSecSort.setValue(saveOrderConfig.getSortCriteria().get(1).field); + saveSecDesc.setSelected(saveOrderConfig.getSortCriteria().get(1).descending); + saveTerSort.setValue(saveOrderConfig.getSortCriteria().get(2).field); + saveTerDesc.setSelected(saveOrderConfig.getSortCriteria().get(2).descending); + } + private String getSelectedItemAsLowerCaseTrim(ComboBox sortBox) { return sortBox.getValue().toLowerCase(Locale.ROOT).trim(); } diff --git a/src/main/java/org/jabref/gui/collab/ChangeScanner.java b/src/main/java/org/jabref/gui/collab/ChangeScanner.java index 8e6616c24a4..7fb1b74e388 100644 --- a/src/main/java/org/jabref/gui/collab/ChangeScanner.java +++ b/src/main/java/org/jabref/gui/collab/ChangeScanner.java @@ -104,8 +104,8 @@ private void storeTempDatabase() { .getEncoding() .orElse(Globals.prefs.getDefaultEncoding())); - BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter(new AtomicFileWriter(tempFile, prefs.getEncoding())); - databaseWriter.saveDatabase(databaseInTemp, prefs); + BibDatabaseWriter databaseWriter = new BibtexDatabaseWriter(new AtomicFileWriter(tempFile, prefs.getEncoding()), prefs); + databaseWriter.saveDatabase(databaseInTemp); } catch (IOException ex) { LOGGER.warn("Problem updating tmp file after accepting external changes", ex); } diff --git a/src/main/java/org/jabref/gui/dbproperties/DatabasePropertiesDialog.java b/src/main/java/org/jabref/gui/dbproperties/DatabasePropertiesDialog.java index 3199b30b423..1a908f2814b 100644 --- a/src/main/java/org/jabref/gui/dbproperties/DatabasePropertiesDialog.java +++ b/src/main/java/org/jabref/gui/dbproperties/DatabasePropertiesDialog.java @@ -238,7 +238,7 @@ private void setValues() { } else { SaveOrderConfig saveOrderConfig = storedSaveOrderConfig.get(); oldSaveOrderConfig = saveOrderConfig; - if (saveOrderConfig.saveInOriginalOrder) { + if (saveOrderConfig.saveInOriginalOrder()) { saveInOriginalOrder.setSelected(true); selected = false; } else { diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index 22845cb1bc1..7ea864a8c25 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -84,12 +84,12 @@ private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, } AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding()); - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter); + BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter, preferences); if (selectedOnly) { - databaseWriter.savePartOfDatabase(panel.getBibDatabaseContext(), panel.getSelectedEntries(), preferences); + databaseWriter.savePartOfDatabase(panel.getBibDatabaseContext(), panel.getSelectedEntries()); } else { - databaseWriter.saveDatabase(panel.getBibDatabaseContext(), preferences); + databaseWriter.saveDatabase(panel.getBibDatabaseContext()); } panel.registerUndoableChanges(databaseWriter.getSaveActionsFieldChanges()); diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index ebe05ee616e..4e9ead5d3e5 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -120,8 +120,8 @@ private void performBackup(Path backupPath) { Charset charset = bibDatabaseContext.getMetaData().getEncoding().orElse(preferences.getDefaultEncoding()); SavePreferences savePreferences = preferences.loadForSaveFromPreferences().withEncoding (charset).withMakeBackup(false); - new BibtexDatabaseWriter(new AtomicFileWriter(backupPath, savePreferences.getEncoding())) - .saveDatabase(bibDatabaseContext, savePreferences); + new BibtexDatabaseWriter(new AtomicFileWriter(backupPath, savePreferences.getEncoding()), savePreferences) + .saveDatabase(bibDatabaseContext); } catch (IOException e) { logIfCritical(e); } diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java index 2f0f221c81c..6d2c6db6ff8 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -128,7 +128,9 @@ public void close() throws IOException { try { // Make sure we have written everything to the temporary file flush(); - ((FileOutputStream) out).getFD().sync(); + if (out instanceof FileOutputStream) { + ((FileOutputStream) out).getFD().sync(); + } } catch (IOException exception) { // Try to close nonetheless super.close(); diff --git a/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java b/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java index 364908f9161..28ad14efc19 100644 --- a/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java +++ b/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java @@ -17,8 +17,6 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.jabref.Globals; -import org.jabref.logic.bibtex.LatexFieldFormatterPreferences; import org.jabref.logic.bibtex.comparator.BibtexStringComparator; import org.jabref.logic.bibtex.comparator.CrossRefEntryComparator; import org.jabref.logic.bibtex.comparator.FieldComparator; @@ -43,11 +41,13 @@ public abstract class BibDatabaseWriter { private static final Pattern REFERENCE_PATTERN = Pattern.compile("(#[A-Za-z]+#)"); // Used to detect string references in strings - private final Writer writer; - private final List saveActionsFieldChanges = new ArrayList<>(); + protected final Writer writer; + protected final SavePreferences preferences; + protected final List saveActionsFieldChanges = new ArrayList<>(); - public BibDatabaseWriter(Writer writer) { + public BibDatabaseWriter(Writer writer, SavePreferences preferences) { this.writer = Objects.requireNonNull(writer); + this.preferences = preferences; } private static List applySaveActions(List toChange, MetaData metaData) { @@ -68,9 +68,9 @@ public static List applySaveActions(BibEntry entry, MetaData metaDa return applySaveActions(Collections.singletonList(entry), metaData); } - private static List> getSaveComparators(SavePreferences preferences, MetaData metaData) { + private static List> getSaveComparators(MetaData metaData, SavePreferences preferences) { List> comparators = new ArrayList<>(); - Optional saveOrder = getSaveOrder(preferences, metaData); + Optional saveOrder = getSaveOrder(metaData, preferences); // Take care, using CrossRefEntry-Comparator, that referred entries occur after referring // ones. This is a necessary requirement for BibTeX to be able to resolve referenced entries correctly. @@ -81,10 +81,11 @@ private static List> getSaveComparators(SavePreferences pre comparators.add(new IdComparator()); } else { // use configured sorting strategy - comparators.add(new FieldComparator(saveOrder.get().sortCriteria[0])); - comparators.add(new FieldComparator(saveOrder.get().sortCriteria[1])); - comparators.add(new FieldComparator(saveOrder.get().sortCriteria[2])); - + List fieldComparators = saveOrder.get() + .getSortCriteria().stream() + .map(FieldComparator::new) + .collect(Collectors.toList()); + comparators.addAll(fieldComparators); comparators.add(new FieldComparator(BibEntry.KEY_FIELD)); } @@ -96,8 +97,7 @@ private static List> getSaveComparators(SavePreferences pre * non-database save operation (such as the exportDatabase call), we do not wish to use the global preference of * saving in standard order. */ - public static List getSortedEntries(BibDatabaseContext bibDatabaseContext, List entriesToSort, - SavePreferences preferences) { + public static List getSortedEntries(BibDatabaseContext bibDatabaseContext, List entriesToSort, SavePreferences preferences) { Objects.requireNonNull(bibDatabaseContext); Objects.requireNonNull(entriesToSort); @@ -106,8 +106,7 @@ public static List getSortedEntries(BibDatabaseContext bibDatabaseCont return new LinkedList<>(entriesToSort); } - List> comparators = BibDatabaseWriter.getSaveComparators(preferences, - bibDatabaseContext.getMetaData()); + List> comparators = getSaveComparators(bibDatabaseContext.getMetaData(), preferences); FieldComparatorStack comparatorStack = new FieldComparatorStack<>(comparators); List sorted = new ArrayList<>(entriesToSort); @@ -115,7 +114,7 @@ public static List getSortedEntries(BibDatabaseContext bibDatabaseCont return sorted; } - private static Optional getSaveOrder(SavePreferences preferences, MetaData metaData) { + private static Optional getSaveOrder(MetaData metaData, SavePreferences preferences) { /* three options: * 1. original order * 2. order specified in metaData @@ -126,7 +125,7 @@ private static Optional getSaveOrder(SavePreferences preference return Optional.empty(); } - if (preferences.getTakeMetadataSaveOrderInAccount()) { + if (preferences.takeMetadataSaveOrderInAccount()) { return metaData.getSaveOrderConfig(); } @@ -140,16 +139,14 @@ public List getSaveActionsFieldChanges() { /** * Saves the complete database. */ - public void saveDatabase(BibDatabaseContext bibDatabaseContext, SavePreferences preferences) throws IOException { - savePartOfDatabase(bibDatabaseContext, bibDatabaseContext.getDatabase().getEntries(), preferences); + public void saveDatabase(BibDatabaseContext bibDatabaseContext) throws IOException { + savePartOfDatabase(bibDatabaseContext, bibDatabaseContext.getDatabase().getEntries()); } /** * Saves the database, including only the specified entries. */ - public void savePartOfDatabase(BibDatabaseContext bibDatabaseContext, - List entries, SavePreferences preferences) throws IOException { - + public void savePartOfDatabase(BibDatabaseContext bibDatabaseContext, List entries) throws IOException { Optional sharedDatabaseIDOptional = bibDatabaseContext.getDatabase().getSharedDatabaseID(); if (sharedDatabaseIDOptional.isPresent()) { @@ -168,8 +165,7 @@ public void savePartOfDatabase(BibDatabaseContext bibDatabaseContext, writePreamble(bibDatabaseContext.getDatabase().getPreamble().orElse("")); // Write strings if there are any. - writeStrings(bibDatabaseContext.getDatabase(), preferences.isReformatFile(), - preferences.getLatexFieldFormatterPreferences()); + writeStrings(bibDatabaseContext.getDatabase()); // Write database entries. List sortedEntries = getSortedEntries(bibDatabaseContext, entries, preferences); @@ -191,8 +187,7 @@ public void savePartOfDatabase(BibDatabaseContext bibDatabaseContext, entryType -> typesToWrite.put(entryType.getName(), entryType)); } - writeEntry(entry, bibDatabaseContext.getMode(), preferences.isReformatFile(), - preferences.getLatexFieldFormatterPreferences()); + writeEntry(entry, bibDatabaseContext.getMode()); } if (preferences.getSaveType() != SavePreferences.DatabaseSaveType.PLAIN_BIBTEX) { @@ -211,8 +206,7 @@ public void savePartOfDatabase(BibDatabaseContext bibDatabaseContext, protected abstract void writePrelogue(BibDatabaseContext bibDatabaseContext, Charset encoding) throws IOException; - protected abstract void writeEntry(BibEntry entry, BibDatabaseMode mode, Boolean isReformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException; + protected abstract void writeEntry(BibEntry entry, BibDatabaseMode mode) throws IOException; protected abstract void writeEpilogue(String epilogue) throws IOException; @@ -242,8 +236,7 @@ protected void writeMetaData(MetaData metaData, GlobalBibtexKeyPattern globalCit * * @param database The database whose strings we should write. */ - private void writeStrings(BibDatabase database, Boolean reformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException { + private void writeStrings(BibDatabase database) throws IOException { List strings = database.getStringKeySet() .stream() .map(database::getString) @@ -261,16 +254,14 @@ private void writeStrings(BibDatabase database, Boolean reformatFile, boolean isFirstStringInType = true; for (BibtexString bs : strings) { if (remaining.containsKey(bs.getName()) && (bs.getType() == t)) { - writeString(bs, isFirstStringInType, remaining, maxKeyLength, reformatFile, - latexFieldFormatterPreferences); + writeString(bs, isFirstStringInType, remaining, maxKeyLength); isFirstStringInType = false; } } } } - protected void writeString(BibtexString bibtexString, boolean isFirstString, Map remaining, int maxKeyLength, - Boolean reformatFile, LatexFieldFormatterPreferences latexFieldFormatterPreferences) + protected void writeString(BibtexString bibtexString, boolean isFirstString, Map remaining, int maxKeyLength) throws IOException { // First remove this from the "remaining" list so it can't cause problem with circular refs: remaining.remove(bibtexString.getName()); @@ -289,16 +280,14 @@ protected void writeString(BibtexString bibtexString, boolean isFirstString, Map // If the label we found exists as a key in the "remaining" Map, we go on and write it now: if (remaining.containsKey(label)) { BibtexString referred = remaining.get(label); - writeString(referred, isFirstString, remaining, maxKeyLength, reformatFile, - latexFieldFormatterPreferences); + writeString(referred, isFirstString, remaining, maxKeyLength); } } - writeString(bibtexString, isFirstString, maxKeyLength, reformatFile, latexFieldFormatterPreferences); + writeString(bibtexString, isFirstString, maxKeyLength); } - protected abstract void writeString(BibtexString bibtexString, boolean isFirstString, int maxKeyLength, - Boolean reformatFile, LatexFieldFormatterPreferences latexFieldFormatterPreferences) + protected abstract void writeString(BibtexString bibtexString, boolean isFirstString, int maxKeyLength) throws IOException; protected void writeEntryTypeDefinitions(Map types) throws IOException { @@ -311,16 +300,12 @@ protected void writeEntryTypeDefinitions(Map types) throws IO protected abstract void writeEntryTypeDefinition(CustomEntryType customType) throws IOException; - protected Writer getWriter() { - return writer; - } - /** * Generate keys for all entries that are lacking keys. */ protected List generateBibtexKeys(BibDatabaseContext databaseContext, List entries) { List changes = new ArrayList<>(); - BibtexKeyGenerator keyGenerator = new BibtexKeyGenerator(databaseContext, Globals.prefs.getBibtexKeyPatternPreferences()); + BibtexKeyGenerator keyGenerator = new BibtexKeyGenerator(databaseContext, preferences.getBibtexKeyPatternPreferences()); for (BibEntry bes : entries) { Optional oldKey = bes.getCiteKeyOptional(); if (StringUtil.isBlank(oldKey)) { diff --git a/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java b/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java index 1af38c29e8d..a3811090472 100644 --- a/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java +++ b/src/main/java/org/jabref/logic/exporter/BibtexDatabaseWriter.java @@ -8,7 +8,6 @@ import org.jabref.logic.bibtex.BibEntryWriter; import org.jabref.logic.bibtex.InvalidFieldValueException; import org.jabref.logic.bibtex.LatexFieldFormatter; -import org.jabref.logic.bibtex.LatexFieldFormatterPreferences; import org.jabref.logic.util.OS; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.database.BibDatabaseMode; @@ -25,85 +24,84 @@ public class BibtexDatabaseWriter extends BibDatabaseWriter { private static final String COMMENT_PREFIX = "@Comment"; private static final String PREAMBLE_PREFIX = "@Preamble"; - public BibtexDatabaseWriter(Writer writer) { - super(writer); + public BibtexDatabaseWriter(Writer writer, SavePreferences preferences) { + super(writer, preferences); } @Override protected void writeEpilogue(String epilogue) throws IOException { if (!StringUtil.isNullOrEmpty(epilogue)) { - getWriter().write(OS.NEWLINE); - getWriter().write(epilogue); - getWriter().write(OS.NEWLINE); + writer.write(OS.NEWLINE); + writer.write(epilogue); + writer.write(OS.NEWLINE); } } @Override protected void writeMetaDataItem(Map.Entry metaItem) throws IOException { - getWriter().write(OS.NEWLINE); - getWriter().write(COMMENT_PREFIX + "{"); - getWriter().write(MetaData.META_FLAG); - getWriter().write(metaItem.getKey()); - getWriter().write(":"); - getWriter().write(metaItem.getValue()); - getWriter().write("}"); - getWriter().write(OS.NEWLINE); + writer.write(OS.NEWLINE); + writer.write(COMMENT_PREFIX + "{"); + writer.write(MetaData.META_FLAG); + writer.write(metaItem.getKey()); + writer.write(":"); + writer.write(metaItem.getValue()); + writer.write("}"); + writer.write(OS.NEWLINE); } @Override protected void writePreamble(String preamble) throws IOException { if (!StringUtil.isNullOrEmpty(preamble)) { - getWriter().write(OS.NEWLINE); - getWriter().write(PREAMBLE_PREFIX + "{"); - getWriter().write(preamble); - getWriter().write('}' + OS.NEWLINE); + writer.write(OS.NEWLINE); + writer.write(PREAMBLE_PREFIX + "{"); + writer.write(preamble); + writer.write('}' + OS.NEWLINE); } } @Override - protected void writeString(BibtexString bibtexString, boolean isFirstString, int maxKeyLength, Boolean reformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException { + protected void writeString(BibtexString bibtexString, boolean isFirstString, int maxKeyLength) throws IOException { // If the string has not been modified, write it back as it was - if (!reformatFile && !bibtexString.hasChanged()) { - getWriter().write(bibtexString.getParsedSerialization()); + if (!preferences.isReformatFile() && !bibtexString.hasChanged()) { + writer.write(bibtexString.getParsedSerialization()); return; } // Write user comments String userComments = bibtexString.getUserComments(); if (!userComments.isEmpty()) { - getWriter().write(userComments + OS.NEWLINE); + writer.write(userComments + OS.NEWLINE); } if (isFirstString) { - getWriter().write(OS.NEWLINE); + writer.write(OS.NEWLINE); } - getWriter().write(STRING_PREFIX + "{" + bibtexString.getName() + StringUtil + writer.write(STRING_PREFIX + "{" + bibtexString.getName() + StringUtil .repeatSpaces(maxKeyLength - bibtexString.getName().length()) + " = "); if (bibtexString.getContent().isEmpty()) { - getWriter().write("{}"); + writer.write("{}"); } else { try { - String formatted = new LatexFieldFormatter(latexFieldFormatterPreferences) + String formatted = new LatexFieldFormatter(preferences.getLatexFieldFormatterPreferences()) .format(bibtexString.getContent(), LatexFieldFormatter.BIBTEX_STRING); - getWriter().write(formatted); + writer.write(formatted); } catch (InvalidFieldValueException ex) { throw new IOException(ex); } } - getWriter().write("}" + OS.NEWLINE); + writer.write("}" + OS.NEWLINE); } @Override protected void writeEntryTypeDefinition(CustomEntryType customType) throws IOException { - getWriter().write(OS.NEWLINE); - getWriter().write(COMMENT_PREFIX + "{"); - getWriter().write(customType.getAsString()); - getWriter().write("}"); - getWriter().write(OS.NEWLINE); + writer.write(OS.NEWLINE); + writer.write(COMMENT_PREFIX + "{"); + writer.write(customType.getAsString()); + writer.write("}"); + writer.write(OS.NEWLINE); } @Override @@ -113,14 +111,14 @@ protected void writePrelogue(BibDatabaseContext bibDatabaseContext, Charset enco } // Writes the file encoding information. - getWriter().write("% "); - getWriter().write(SavePreferences.ENCODING_PREFIX + encoding); - getWriter().write(OS.NEWLINE); + writer.write("% "); + writer.write(SavePreferences.ENCODING_PREFIX + encoding); + writer.write(OS.NEWLINE); } @Override protected void writeDatabaseID(String sharedDatabaseID) throws IOException { - getWriter().write("% " + + writer.write("% " + DATABASE_ID_PREFIX + " " + sharedDatabaseID + @@ -128,10 +126,9 @@ protected void writeDatabaseID(String sharedDatabaseID) throws IOException { } @Override - protected void writeEntry(BibEntry entry, BibDatabaseMode mode, Boolean isReformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences) throws IOException { + protected void writeEntry(BibEntry entry, BibDatabaseMode mode) throws IOException { BibEntryWriter bibtexEntryWriter = new BibEntryWriter( - new LatexFieldFormatter(latexFieldFormatterPreferences), true); - bibtexEntryWriter.write(entry, getWriter(), mode, isReformatFile); + new LatexFieldFormatter(preferences.getLatexFieldFormatterPreferences()), true); + bibtexEntryWriter.write(entry, writer, mode, preferences.isReformatFile()); } } diff --git a/src/main/java/org/jabref/logic/exporter/SavePreferences.java b/src/main/java/org/jabref/logic/exporter/SavePreferences.java index 69e3ccafeb1..d3258a26156 100644 --- a/src/main/java/org/jabref/logic/exporter/SavePreferences.java +++ b/src/main/java/org/jabref/logic/exporter/SavePreferences.java @@ -1,9 +1,9 @@ package org.jabref.logic.exporter; import java.nio.charset.Charset; -import java.util.Collections; import org.jabref.logic.bibtex.LatexFieldFormatterPreferences; +import org.jabref.logic.bibtexkeypattern.BibtexKeyPatternPreferences; import org.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern; import org.jabref.model.metadata.SaveOrderConfig; @@ -22,15 +22,12 @@ public class SavePreferences { private final LatexFieldFormatterPreferences latexFieldFormatterPreferences; private final GlobalBibtexKeyPattern globalCiteKeyPattern; private final Boolean generateBibtexKeysBeforeSaving; - - public SavePreferences() { - this(true, null, null, false, DatabaseSaveType.ALL, true, false, new LatexFieldFormatterPreferences(), - new GlobalBibtexKeyPattern(Collections.emptyList()), false); - } + private final BibtexKeyPatternPreferences bibtexKeyPatternPreferences; public SavePreferences(Boolean saveInOriginalOrder, SaveOrderConfig saveOrder, Charset encoding, Boolean makeBackup, DatabaseSaveType saveType, Boolean takeMetadataSaveOrderInAccount, Boolean reformatFile, - LatexFieldFormatterPreferences latexFieldFormatterPreferences, GlobalBibtexKeyPattern globalCiteKeyPattern, Boolean generateBibtexKeysBeforeSaving) { + LatexFieldFormatterPreferences latexFieldFormatterPreferences, GlobalBibtexKeyPattern globalCiteKeyPattern, + Boolean generateBibtexKeysBeforeSaving, BibtexKeyPatternPreferences bibtexKeyPatternPreferences) { this.saveInOriginalOrder = saveInOriginalOrder; this.saveOrder = saveOrder; this.encoding = encoding; @@ -41,9 +38,10 @@ public SavePreferences(Boolean saveInOriginalOrder, SaveOrderConfig saveOrder, C this.latexFieldFormatterPreferences = latexFieldFormatterPreferences; this.globalCiteKeyPattern = globalCiteKeyPattern; this.generateBibtexKeysBeforeSaving = generateBibtexKeysBeforeSaving; + this.bibtexKeyPatternPreferences = bibtexKeyPatternPreferences; } - public Boolean getTakeMetadataSaveOrderInAccount() { + public Boolean takeMetadataSaveOrderInAccount() { return takeMetadataSaveOrderInAccount; } @@ -58,7 +56,7 @@ public boolean isSaveInOriginalOrder() { public SavePreferences withSaveInOriginalOrder(Boolean newSaveInOriginalOrder) { return new SavePreferences(newSaveInOriginalOrder, this.saveOrder, this.encoding, this.makeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving, this.bibtexKeyPatternPreferences); } public boolean makeBackup() { @@ -68,7 +66,7 @@ public boolean makeBackup() { public SavePreferences withMakeBackup(Boolean newMakeBackup) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, this.encoding, newMakeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving, this.bibtexKeyPatternPreferences); } public Charset getEncoding() { @@ -78,7 +76,7 @@ public Charset getEncoding() { public SavePreferences withEncoding(Charset newEncoding) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, newEncoding, this.makeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving, this.bibtexKeyPatternPreferences); } public DatabaseSaveType getSaveType() { @@ -88,7 +86,7 @@ public DatabaseSaveType getSaveType() { public SavePreferences withSaveType(DatabaseSaveType newSaveType) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, this.encoding, this.makeBackup, newSaveType, this.takeMetadataSaveOrderInAccount, this.reformatFile, this.latexFieldFormatterPreferences, - this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving, this.bibtexKeyPatternPreferences); } public Boolean isReformatFile() { @@ -98,7 +96,7 @@ public Boolean isReformatFile() { public SavePreferences withReformatFile(boolean newReformatFile) { return new SavePreferences(this.saveInOriginalOrder, this.saveOrder, this.encoding, this.makeBackup, this.saveType, this.takeMetadataSaveOrderInAccount, newReformatFile, this.latexFieldFormatterPreferences, - this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving); + this.globalCiteKeyPattern, this.generateBibtexKeysBeforeSaving, this.bibtexKeyPatternPreferences); } public Charset getEncodingOrDefault() { @@ -117,6 +115,10 @@ public Boolean generateBibtexKeysBeforeSaving() { return generateBibtexKeysBeforeSaving; } + public BibtexKeyPatternPreferences getBibtexKeyPatternPreferences() { + return bibtexKeyPatternPreferences; + } + public enum DatabaseSaveType { ALL, PLAIN_BIBTEX diff --git a/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java b/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java index 1f88c5640f5..63b43eb4d45 100644 --- a/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java +++ b/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java @@ -1,7 +1,7 @@ package org.jabref.model.metadata; import java.util.ArrayList; -import java.util.Arrays; +import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -14,26 +14,18 @@ public class SaveOrderConfig { private static final String ORIGINAL = "original"; private static final String SPECIFIED = "specified"; - - public boolean saveInOriginalOrder; - - // quick hack for outside modifications - public final SortCriterion[] sortCriteria = new SortCriterion[3]; + private final LinkedList sortCriteria = new LinkedList<>(); + private boolean saveInOriginalOrder; public SaveOrderConfig() { - // fill default values setSaveInOriginalOrder(); - sortCriteria[0] = new SortCriterion(); - sortCriteria[1] = new SortCriterion(); - sortCriteria[2] = new SortCriterion(); } - public SaveOrderConfig(boolean saveInOriginalOrder, SortCriterion first, SortCriterion second, - SortCriterion third) { + public SaveOrderConfig(boolean saveInOriginalOrder, SortCriterion first, SortCriterion second, SortCriterion third) { this.saveInOriginalOrder = saveInOriginalOrder; - sortCriteria[0] = first; - sortCriteria[1] = second; - sortCriteria[2] = third; + sortCriteria.add(first); + sortCriteria.add(second); + sortCriteria.add(third); } private SaveOrderConfig(List data) { @@ -50,72 +42,27 @@ private SaveOrderConfig(List data) { setSaveInSpecifiedOrder(); } - if (data.size() >= 3) { - sortCriteria[0] = new SortCriterion(data.get(1), data.get(2)); - } else { - sortCriteria[0] = new SortCriterion(); - } - if (data.size() >= 5) { - sortCriteria[1] = new SortCriterion(data.get(3), data.get(4)); - } else { - sortCriteria[1] = new SortCriterion(); - } - if (data.size() >= 7) { - sortCriteria[2] = new SortCriterion(data.get(5), data.get(6)); - } else { - sortCriteria[2] = new SortCriterion(); + for (int index = 1; index < data.size(); index = index + 2) { + sortCriteria.addLast(new SortCriterion(data.get(index), data.get(index + 1))); } } + public static SaveOrderConfig parse(List orderedData) { return new SaveOrderConfig(orderedData); } - public static class SortCriterion { - - public String field; - - public boolean descending; - - public SortCriterion() { - this.field = ""; - } - - public SortCriterion(String field, String descending) { - this.field = field; - this.descending = Boolean.parseBoolean(descending); - } - - public SortCriterion(String field, boolean descending) { - this.field = field; - this.descending = descending; - } - @Override - public String toString() { - final StringBuilder sb = new StringBuilder("SortCriterion{"); - sb.append("field='").append(field).append('\''); - sb.append(", descending=").append(descending); - sb.append('}'); - return sb.toString(); - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if ((o == null) || (getClass() != o.getClass())) { - return false; - } - SortCriterion that = (SortCriterion) o; - return Objects.equals(descending, that.descending) && - Objects.equals(field, that.field); - } + public static SaveOrderConfig getDefaultSaveOrder() { + SaveOrderConfig standard = new SaveOrderConfig(); + standard.setSaveInOriginalOrder(); + return standard; + } - @Override - public int hashCode() { - return Objects.hash(field, descending); - } + public boolean saveInOriginalOrder() { + return saveInOriginalOrder; + } + public LinkedList getSortCriteria() { + return sortCriteria; } @Override @@ -125,26 +72,21 @@ public boolean equals(Object o) { } if (o instanceof SaveOrderConfig) { SaveOrderConfig that = (SaveOrderConfig) o; - boolean sortCriteriaEquals = sortCriteria[0].equals(that.sortCriteria[0]) - && sortCriteria[1].equals(that.sortCriteria[1]) && sortCriteria[2].equals(that.sortCriteria[2]); - - return Objects.equals(saveInOriginalOrder, that.saveInOriginalOrder) && sortCriteriaEquals; + return Objects.equals(sortCriteria, that.sortCriteria) && Objects.equals(saveInOriginalOrder, that.saveInOriginalOrder); } return false; } @Override public int hashCode() { - return Objects.hash(saveInOriginalOrder, Arrays.hashCode(sortCriteria)); + return Objects.hash(saveInOriginalOrder, sortCriteria); } @Override public String toString() { - final StringBuilder sb = new StringBuilder("SaveOrderConfig{"); - sb.append("saveInOriginalOrder=").append(saveInOriginalOrder); - sb.append(", sortCriteria=").append(Arrays.toString(sortCriteria)); - sb.append('}'); - return sb.toString(); + return "SaveOrderConfig{" + "saveInOriginalOrder=" + saveInOriginalOrder + + ", sortCriteria=" + sortCriteria + + '}'; } public void setSaveInOriginalOrder() { @@ -166,20 +108,59 @@ public List getAsStringList() { res.add(SPECIFIED); } - res.add(sortCriteria[0].field); - res.add(Boolean.toString(sortCriteria[0].descending)); - res.add(sortCriteria[1].field); - res.add(Boolean.toString(sortCriteria[1].descending)); - res.add(sortCriteria[2].field); - res.add(Boolean.toString(sortCriteria[2].descending)); + for (SortCriterion sortCriterion : sortCriteria) { + res.add(sortCriterion.field); + res.add(Boolean.toString(sortCriterion.descending)); + } return res; } - public static SaveOrderConfig getDefaultSaveOrder() { - SaveOrderConfig standard = new SaveOrderConfig(); - standard.setSaveInOriginalOrder(); - return standard; + public static class SortCriterion { + + public String field; + + public boolean descending; + + public SortCriterion(String field, String descending) { + this.field = field; + this.descending = Boolean.parseBoolean(descending); + } + + public SortCriterion(String field, boolean descending) { + this.field = field; + this.descending = descending; + } + + public SortCriterion() { + + } + + @Override + public String toString() { + return "SortCriterion{" + "field='" + field + '\'' + + ", descending=" + descending + + '}'; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if ((o == null) || (getClass() != o.getClass())) { + return false; + } + SortCriterion that = (SortCriterion) o; + return Objects.equals(descending, that.descending) && + Objects.equals(field, that.field); + } + + @Override + public int hashCode() { + return Objects.hash(field, descending); + } + } } diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 9ba3f30909d..9835a55b428 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1447,7 +1447,8 @@ public SavePreferences loadForExportFromPreferences() { this.getBoolean(JabRefPreferences.REFORMAT_FILE_ON_SAVE_AND_EXPORT), this.getLatexFieldFormatterPreferences(), this.getKeyPattern(), - getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING)); + getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING), + getBibtexKeyPatternPreferences()); } public SavePreferences loadForSaveFromPreferences() { @@ -1461,7 +1462,8 @@ public SavePreferences loadForSaveFromPreferences() { this.getBoolean(JabRefPreferences.REFORMAT_FILE_ON_SAVE_AND_EXPORT), this.getLatexFieldFormatterPreferences(), this.getKeyPattern(), - getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING)); + getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING), + getBibtexKeyPatternPreferences()); } public ExporterFactory getExporterFactory(JournalAbbreviationLoader abbreviationLoader) { @@ -1692,37 +1694,29 @@ public void setRemotePreferences(RemotePreferences remotePreferences) { } public void storeExportSaveOrder(SaveOrderConfig config) { - putBoolean(EXPORT_PRIMARY_SORT_DESCENDING, config.sortCriteria[0].descending); - putBoolean(EXPORT_SECONDARY_SORT_DESCENDING, config.sortCriteria[1].descending); - putBoolean(EXPORT_TERTIARY_SORT_DESCENDING, config.sortCriteria[2].descending); + putBoolean(EXPORT_PRIMARY_SORT_DESCENDING, config.getSortCriteria().get(0).descending); + putBoolean(EXPORT_SECONDARY_SORT_DESCENDING, config.getSortCriteria().get(1).descending); + putBoolean(EXPORT_TERTIARY_SORT_DESCENDING, config.getSortCriteria().get(2).descending); - put(EXPORT_PRIMARY_SORT_FIELD, config.sortCriteria[0].field); - put(EXPORT_SECONDARY_SORT_FIELD, config.sortCriteria[1].field); - put(EXPORT_TERTIARY_SORT_FIELD, config.sortCriteria[2].field); + put(EXPORT_PRIMARY_SORT_FIELD, config.getSortCriteria().get(0).field); + put(EXPORT_SECONDARY_SORT_FIELD, config.getSortCriteria().get(1).field); + put(EXPORT_TERTIARY_SORT_FIELD, config.getSortCriteria().get(2).field); } public SaveOrderConfig loadTableSaveOrder() { - SaveOrderConfig config = new SaveOrderConfig(); - config.sortCriteria[0].field = get(TABLE_PRIMARY_SORT_FIELD); - config.sortCriteria[0].descending = getBoolean(TABLE_PRIMARY_SORT_DESCENDING); - config.sortCriteria[1].field = get(TABLE_SECONDARY_SORT_FIELD); - config.sortCriteria[1].descending = getBoolean(TABLE_SECONDARY_SORT_DESCENDING); - config.sortCriteria[2].field = get(TABLE_TERTIARY_SORT_FIELD); - config.sortCriteria[2].descending = getBoolean(TABLE_TERTIARY_SORT_DESCENDING); - - return config; + return new SaveOrderConfig(true, + new SaveOrderConfig.SortCriterion(get(TABLE_PRIMARY_SORT_FIELD), getBoolean(TABLE_PRIMARY_SORT_DESCENDING)), + new SaveOrderConfig.SortCriterion(get(TABLE_SECONDARY_SORT_FIELD), getBoolean(TABLE_SECONDARY_SORT_DESCENDING)), + new SaveOrderConfig.SortCriterion(get(TABLE_TERTIARY_SORT_FIELD), getBoolean(TABLE_TERTIARY_SORT_DESCENDING)) + ); } public SaveOrderConfig loadExportSaveOrder() { - SaveOrderConfig config = new SaveOrderConfig(); - config.sortCriteria[0].field = get(EXPORT_PRIMARY_SORT_FIELD); - config.sortCriteria[0].descending = getBoolean(EXPORT_PRIMARY_SORT_DESCENDING); - config.sortCriteria[1].field = get(EXPORT_SECONDARY_SORT_FIELD); - config.sortCriteria[1].descending = getBoolean(EXPORT_SECONDARY_SORT_DESCENDING); - config.sortCriteria[2].field = get(EXPORT_TERTIARY_SORT_FIELD); - config.sortCriteria[2].descending = getBoolean(EXPORT_TERTIARY_SORT_DESCENDING); - - return config; + return new SaveOrderConfig(true, + new SaveOrderConfig.SortCriterion(get(EXPORT_PRIMARY_SORT_FIELD), getBoolean(EXPORT_PRIMARY_SORT_DESCENDING)), + new SaveOrderConfig.SortCriterion(get(EXPORT_SECONDARY_SORT_FIELD), getBoolean(EXPORT_SECONDARY_SORT_DESCENDING)), + new SaveOrderConfig.SortCriterion(get(EXPORT_TERTIARY_SORT_FIELD), getBoolean(EXPORT_TERTIARY_SORT_DESCENDING)) + ); } public Character getKeywordDelimiter() { diff --git a/src/test/java/org/jabref/gui/AbstractUITest.java b/src/test/java/org/jabref/gui/AbstractUITest.java index f5982cb4ff1..579729783d2 100644 --- a/src/test/java/org/jabref/gui/AbstractUITest.java +++ b/src/test/java/org/jabref/gui/AbstractUITest.java @@ -49,14 +49,9 @@ protected void onSetUp() { * The backlashes are replaced with forwardslashes b/c assertJ can't type the former one on windows * @param relativePath the relative path to the resource database */ - protected String getAbsolutePath(String relativePath) { + protected String getAbsolutePath(String relativePath) throws URISyntaxException { final URL resource = this.getClass().getClassLoader().getResource(relativePath); - try { - return Paths.get(resource.toURI()).toAbsolutePath().toString().replace("\\", "/"); - } catch (URISyntaxException e) { - e.printStackTrace(); - } - return null; + return Paths.get(resource.toURI()).toAbsolutePath().toString().replace("\\", "/"); } /** diff --git a/src/test/java/org/jabref/gui/EntryTableTest.java b/src/test/java/org/jabref/gui/EntryTableTest.java index 61a4cd4ea91..42dd811c1b3 100644 --- a/src/test/java/org/jabref/gui/EntryTableTest.java +++ b/src/test/java/org/jabref/gui/EntryTableTest.java @@ -26,7 +26,7 @@ public class EntryTableTest extends AbstractUITest{ private final static int TITLE_COLUMN_INDEX = 5; @Test - public void scrollThroughEntryList() { + public void scrollThroughEntryList() throws Exception { String path = getAbsolutePath(TEST_FILE_NAME); importBibIntoNewDatabase(path); diff --git a/src/test/java/org/jabref/gui/UndoTest.java b/src/test/java/org/jabref/gui/UndoTest.java index b8b2fb3eb18..5105f2cdae5 100644 --- a/src/test/java/org/jabref/gui/UndoTest.java +++ b/src/test/java/org/jabref/gui/UndoTest.java @@ -16,7 +16,7 @@ public class UndoTest extends AbstractUITest { @Test - public void undoCutOfMultipleEntries() { + public void undoCutOfMultipleEntries() throws Exception { importBibIntoNewDatabase(getAbsolutePath("testbib/testjabref.bib")); JTableFixture entryTable = mainFrame.table(); diff --git a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java index 29b492f2c61..8b2d9a9476a 100644 --- a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java +++ b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java @@ -12,6 +12,7 @@ import org.jabref.logic.util.OS; import org.jabref.model.database.BibDatabaseMode; import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.BiblatexEntryTypes; import org.jabref.model.entry.LinkedFile; import org.jabref.model.util.DummyFileUpdateMonitor; import org.jabref.model.util.FileUpdateMonitor; @@ -87,7 +88,7 @@ public void writeOtherTypeTest() throws Exception { @Test void writeEntryWithFile() throws Exception { - BibEntry entry = new BibEntry(); + BibEntry entry = new BibEntry(BiblatexEntryTypes.ARTICLE); LinkedFile file = new LinkedFile("test", "/home/uers/test.pdf", "PDF"); entry.addFile(file); diff --git a/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java b/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java index a49da4ec252..1d988120573 100644 --- a/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java +++ b/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java @@ -45,6 +45,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; class BibtexDatabaseWriterTest { @@ -55,11 +56,16 @@ class BibtexDatabaseWriterTest { private BibDatabaseContext bibtexContext; private ImportFormatPreferences importFormatPreferences; private final FileUpdateMonitor fileMonitor = new DummyFileUpdateMonitor(); + private SavePreferences preferences; @BeforeEach void setUp() { stringWriter = new StringWriter(); - databaseWriter = new BibtexDatabaseWriter(stringWriter); + preferences = mock(SavePreferences.class, Answers.RETURNS_DEEP_STUBS); + when(preferences.getSaveOrder()).thenReturn(new SaveOrderConfig()); + when(preferences.getEncoding()).thenReturn(null); + when(preferences.takeMetadataSaveOrderInAccount()).thenReturn(true); + databaseWriter = new BibtexDatabaseWriter(stringWriter, preferences); database = new BibDatabase(); metaData = new MetaData(); @@ -69,24 +75,19 @@ void setUp() { @Test void writeWithNullContextThrowsException() throws Exception { - assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(null, Collections.emptyList(), new SavePreferences())); + assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(null, Collections.emptyList())); } @Test void writeWithNullEntriesThrowsException() throws Exception { - assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(bibtexContext, null, new SavePreferences())); - } - - @Test - void writeWithNullPreferencesThrowsException() throws Exception { - assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), null)); + assertThrows(NullPointerException.class, () -> databaseWriter.savePartOfDatabase(bibtexContext, null)); } @Test void writeEncoding() throws Exception { - SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); + when(preferences.getEncoding()).thenReturn(StandardCharsets.US_ASCII); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE, stringWriter.toString()); } @@ -95,17 +96,17 @@ void writeEncoding() throws Exception { void writePreamble() throws Exception { database.setPreamble("Test preamble"); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Preamble{Test preamble}" + OS.NEWLINE, stringWriter.toString()); } @Test void writePreambleAndEncoding() throws Exception { - SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); + when(preferences.getEncoding()).thenReturn(StandardCharsets.US_ASCII); database.setPreamble("Test preamble"); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + "@Preamble{Test preamble}" + OS.NEWLINE, stringWriter.toString()); @@ -117,7 +118,7 @@ void writeEntry() throws Exception { entry.setType(BibtexEntryTypes.ARTICLE); database.insertEntry(entry); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry)); assertEquals(OS.NEWLINE + "@Article{," + OS.NEWLINE + "}" + OS.NEWLINE + OS.NEWLINE @@ -127,12 +128,12 @@ void writeEntry() throws Exception { @Test void writeEncodingAndEntry() throws Exception { - SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); + when(preferences.getEncoding()).thenReturn(StandardCharsets.US_ASCII); BibEntry entry = new BibEntry(); entry.setType(BibtexEntryTypes.ARTICLE); database.insertEntry(entry); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry)); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + "@Article{," + OS.NEWLINE + "}" @@ -145,17 +146,17 @@ void writeEncodingAndEntry() throws Exception { void writeEpilogue() throws Exception { database.setEpilog("Test epilog"); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "Test epilog" + OS.NEWLINE, stringWriter.toString()); } @Test void writeEpilogueAndEncoding() throws Exception { - SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); + when(preferences.getEncoding()).thenReturn(StandardCharsets.US_ASCII); database.setEpilog("Test epilog"); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + "Test epilog" + OS.NEWLINE, stringWriter.toString()); @@ -167,7 +168,7 @@ void writeMetadata() throws Exception { bibtexKeyPattern.setDefaultValue("test"); metaData.setCiteKeyPattern(bibtexKeyPattern); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: keypatterndefault:test;}" + OS.NEWLINE, stringWriter.toString()); @@ -175,12 +176,12 @@ void writeMetadata() throws Exception { @Test void writeMetadataAndEncoding() throws Exception { - SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); + when(preferences.getEncoding()).thenReturn(StandardCharsets.US_ASCII); DatabaseBibtexKeyPattern bibtexKeyPattern = new DatabaseBibtexKeyPattern(mock(GlobalBibtexKeyPattern.class)); bibtexKeyPattern.setDefaultValue("test"); metaData.setCiteKeyPattern(bibtexKeyPattern); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + @@ -193,7 +194,7 @@ void writeGroups() throws Exception { groupRoot.addSubgroup(new ExplicitGroup("test", GroupHierarchyType.INCLUDING, ',')); metaData.setGroups(groupRoot); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); // @formatter:off assertEquals(OS.NEWLINE @@ -206,13 +207,13 @@ void writeGroups() throws Exception { @Test void writeGroupsAndEncoding() throws Exception { - SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); + when(preferences.getEncoding()).thenReturn(StandardCharsets.US_ASCII); GroupTreeNode groupRoot = GroupTreeNode.fromGroup(new AllEntriesGroup("")); groupRoot.addChild(GroupTreeNode.fromGroup(new ExplicitGroup("test", GroupHierarchyType.INCLUDING, ','))); metaData.setGroups(groupRoot); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); // @formatter:off assertEquals( @@ -229,17 +230,17 @@ void writeGroupsAndEncoding() throws Exception { void writeString() throws Exception { database.addString(new BibtexString("name", "content")); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@String{name = {content}}" + OS.NEWLINE, stringWriter.toString()); } @Test void writeStringAndEncoding() throws Exception { - SavePreferences preferences = new SavePreferences().withEncoding(StandardCharsets.US_ASCII); + when(preferences.getEncoding()).thenReturn(StandardCharsets.US_ASCII); database.addString(new BibtexString("name", "content")); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals("% Encoding: US-ASCII" + OS.NEWLINE + OS.NEWLINE + "@String{name = {content}}" + OS.NEWLINE, stringWriter.toString()); @@ -253,7 +254,7 @@ void writeEntryWithCustomizedTypeAlsoWritesTypeDeclaration() throws Exception { entry.setType("customizedType"); database.insertEntry(entry); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry)); assertEquals( OS.NEWLINE + @@ -273,12 +274,13 @@ void roundtrip() throws Exception { Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); - SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true); + when(preferences.getEncoding()).thenReturn(encoding); + when(preferences.isSaveInOriginalOrder()).thenReturn(true); BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); - try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries()); + try (Scanner scanner = new Scanner(testBibtexFile, encoding.name())) { assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } } @@ -289,11 +291,12 @@ void roundtripWithUserComment() throws Exception { Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); - SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true); + when(preferences.getEncoding()).thenReturn(encoding); + when(preferences.isSaveInOriginalOrder()).thenReturn(true); BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries()); try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } @@ -308,11 +311,12 @@ void roundtripWithUserCommentAndEntryChange() throws Exception { BibEntry entry = result.getDatabase().getEntryByKey("1137631").get(); entry.setField("author", "Mr. Author"); - SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true); + when(preferences.getEncoding()).thenReturn(encoding); + when(preferences.isSaveInOriginalOrder()).thenReturn(true); BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries()); try (Scanner scanner = new Scanner(Paths.get("src/test/resources/testbib/bibWithUserCommentAndEntryChange.bib"), encoding.name())) { assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); @@ -330,11 +334,12 @@ void roundtripWithUserCommentBeforeStringAndChange() throws Exception { string.setContent(string.getContent()); } - SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true); + when(preferences.getEncoding()).thenReturn(encoding); + when(preferences.isSaveInOriginalOrder()).thenReturn(true); BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries()); try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); @@ -347,11 +352,12 @@ void roundtripWithUnknownMetaData() throws Exception { Charset encoding = StandardCharsets.UTF_8; ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding)); - SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true); + when(preferences.getEncoding()).thenReturn(encoding); + when(preferences.isSaveInOriginalOrder()).thenReturn(true); BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries()); try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) { assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString()); } @@ -366,7 +372,7 @@ void writeSavedSerializationOfEntryIfUnchanged() throws Exception { entry.setChanged(false); database.insertEntry(entry); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry)); assertEquals("presaved serialization" + OS.NEWLINE + "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, stringWriter.toString()); @@ -381,8 +387,8 @@ void reformatEntryIfAskedToDoSo() throws Exception { entry.setChanged(false); database.insertEntry(entry); - SavePreferences preferences = new SavePreferences().withReformatFile(true); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry), preferences); + when(preferences.isReformatFile()).thenReturn(true); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.singletonList(entry)); assertEquals(OS.NEWLINE + "@Article{," + OS.NEWLINE + " author = {Mr. author}," + OS.NEWLINE + "}" @@ -398,7 +404,7 @@ void writeSavedSerializationOfStringIfUnchanged() throws Exception { string.setParsedSerialization("serialization"); database.addString(string); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals("serialization", stringWriter.toString()); } @@ -409,8 +415,8 @@ void reformatStringIfAskedToDoSo() throws Exception { string.setParsedSerialization("wrong serialization"); database.addString(string); - SavePreferences preferences = new SavePreferences().withReformatFile(true); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), preferences); + when(preferences.isReformatFile()).thenReturn(true); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@String{name = {content}}" + OS.NEWLINE, stringWriter.toString()); @@ -422,7 +428,7 @@ void writeSaveActions() throws Exception { Collections.singletonList(new FieldFormatterCleanup("title", new LowerCaseFormatter()))); metaData.setSaveActions(saveActions); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: saveActions:enabled;" + OS.NEWLINE + "title[lower_case]" + OS.NEWLINE + ";}" + OS.NEWLINE, stringWriter.toString()); @@ -435,7 +441,7 @@ void writeSaveOrderConfig() throws Exception { new SaveOrderConfig.SortCriterion("abstract", false)); metaData.setSaveOrderConfig(saveOrderConfig); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: saveOrderConfig:specified;author;false;year;true;abstract;false;}" @@ -449,7 +455,7 @@ void writeCustomKeyPattern() throws Exception { pattern.addBibtexKeyPattern("article", "articleTest"); metaData.setCiteKeyPattern(pattern); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: keypattern_article:articleTest;}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: keypatterndefault:test;}" + OS.NEWLINE, @@ -460,7 +466,7 @@ void writeCustomKeyPattern() throws Exception { void writeBiblatexMode() throws Exception { metaData.setMode(BibDatabaseMode.BIBLATEX); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: databaseType:biblatex;}" + OS.NEWLINE, stringWriter.toString()); @@ -470,7 +476,7 @@ void writeBiblatexMode() throws Exception { void writeProtectedFlag() throws Exception { metaData.markAsProtected(); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: protectedFlag:true;}" + OS.NEWLINE, stringWriter.toString()); @@ -481,7 +487,7 @@ void writeFileDirectories() throws Exception { metaData.setDefaultFileDirectory("\\Literature\\"); metaData.setUserFileDirectory("defaultOwner-user", "D:\\Documents"); - databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, Collections.emptyList()); assertEquals(OS.NEWLINE + "@Comment{jabref-meta: fileDirectory:\\\\Literature\\\\;}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: fileDirectory-defaultOwner-user:D:\\\\Documents;}" @@ -490,7 +496,8 @@ void writeFileDirectories() throws Exception { @Test void writeEntriesSorted() throws Exception { - SaveOrderConfig saveOrderConfig = new SaveOrderConfig(false, new SaveOrderConfig.SortCriterion("author", false), + SaveOrderConfig saveOrderConfig = new SaveOrderConfig(false, + new SaveOrderConfig.SortCriterion("author", false), new SaveOrderConfig.SortCriterion("year", true), new SaveOrderConfig.SortCriterion("abstract", false)); metaData.setSaveOrderConfig(saveOrderConfig); @@ -498,12 +505,12 @@ void writeEntriesSorted() throws Exception { BibEntry firstEntry = new BibEntry(); firstEntry.setType(BibtexEntryTypes.ARTICLE); firstEntry.setField("author", "A"); - firstEntry.setField("year", "2000"); + firstEntry.setField("year", "2010"); BibEntry secondEntry = new BibEntry(); secondEntry.setType(BibtexEntryTypes.ARTICLE); secondEntry.setField("author", "A"); - secondEntry.setField("year", "2010"); + secondEntry.setField("year", "2000"); BibEntry thirdEntry = new BibEntry(); thirdEntry.setType(BibtexEntryTypes.ARTICLE); @@ -514,17 +521,17 @@ void writeEntriesSorted() throws Exception { database.insertEntry(thirdEntry); database.insertEntry(firstEntry); - databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries(), new SavePreferences()); + databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries()); assertEquals( OS.NEWLINE + "@Article{," + OS.NEWLINE + " author = {A}," + OS.NEWLINE + - " year = {2000}," + OS.NEWLINE + + " year = {2010}," + OS.NEWLINE + "}" + OS.NEWLINE + OS.NEWLINE + "@Article{," + OS.NEWLINE + " author = {A}," + OS.NEWLINE + - " year = {2010}," + OS.NEWLINE + + " year = {2000}," + OS.NEWLINE + "}" + OS.NEWLINE + OS.NEWLINE + "@Article{," + OS.NEWLINE + " author = {B}," + OS.NEWLINE + @@ -533,8 +540,8 @@ void writeEntriesSorted() throws Exception { "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: saveOrderConfig:specified;author;false;year;true;abstract;false;}" + - OS.NEWLINE - , stringWriter.toString()); + OS.NEWLINE, + stringWriter.toString()); } @Test @@ -558,8 +565,8 @@ void writeEntriesInOriginalOrderWhenNoSaveOrderConfigIsSetInMetadata() throws Ex database.insertEntry(secondEntry); database.insertEntry(thirdEntry); - SavePreferences preferences = new SavePreferences().withSaveInOriginalOrder(false); - databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries(), preferences); + when(preferences.isSaveInOriginalOrder()).thenReturn(false); + databaseWriter.savePartOfDatabase(bibtexContext, database.getEntries()); assertEquals( OS.NEWLINE + @@ -588,11 +595,12 @@ void roundtripWithContentSelectorsAndUmlauts() throws Exception { ParserResult firstParse = new BibtexParser(importFormatPreferences, fileMonitor).parse(new StringReader(fileContent)); - SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true); + when(preferences.getEncoding()).thenReturn(encoding); + when(preferences.isSaveInOriginalOrder()).thenReturn(true); BibDatabaseContext context = new BibDatabaseContext(firstParse.getDatabase(), firstParse.getMetaData(), new Defaults(BibDatabaseMode.BIBTEX)); - databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries(), preferences); + databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries()); assertEquals(fileContent, stringWriter.toString()); } diff --git a/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java b/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java index 10cb4a4d7bb..0da7422a378 100644 --- a/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java +++ b/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java @@ -52,9 +52,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -/** - * Test the BibtexParser - */ class BibtexParserTest { private ImportFormatPreferences importFormatPreferences; @@ -1450,7 +1447,7 @@ void parseOtherTypeTest() throws Exception { void parseRecognizesDatabaseID() throws Exception { String expectedDatabaseID = "q1w2e3r4t5z6"; StringBuilder sharedDatabaseFileContent = new StringBuilder() - .append("% DBID: ").append(expectedDatabaseID) + .append("\\% DBID: ").append(expectedDatabaseID) .append(OS.NEWLINE) .append("@Article{a}"); @@ -1463,8 +1460,8 @@ void parseRecognizesDatabaseID() throws Exception { @Test void parseDoesNotRecognizeDatabaseIDasUserComment() throws Exception { StringBuilder sharedDatabaseFileContent = new StringBuilder() - .append("% Encoding: UTF-8").append(OS.NEWLINE) - .append("% DBID: q1w2e3r4t5z6").append(OS.NEWLINE) + .append("\\% Encoding: UTF-8").append(OS.NEWLINE) + .append("\\% DBID: q1w2e3r4t5z6").append(OS.NEWLINE) .append("@Article{a}"); ParserResult parserResult = parser.parse(new StringReader(sharedDatabaseFileContent.toString())); diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index c099ace2c7c..72df4f9301e 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -2,7 +2,13 @@ import java.sql.ResultSet; import java.sql.SQLException; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.stream.Stream; import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; @@ -24,19 +30,14 @@ @DatabaseTest class DBMSProcessorTest { - private static Stream getTestingDatabaseSystems() { - Stream dbmsTypeStream = TestManager.getDBMSTypeTestParameter().stream(); + private static Stream getTestingDatabaseSystems() throws InvalidDBMSConnectionPropertiesException, SQLException { Collection result = new ArrayList<>(); - dbmsTypeStream.forEach(dbmsType -> { - try { + for (DBMSType dbmsType : TestManager.getDBMSTypeTestParameter()) { result.add(new Object[] { dbmsType, TestConnector.getTestDBMSConnection(dbmsType), DBMSProcessor.getProcessorInstance(TestConnector.getTestDBMSConnection(dbmsType))}); - } catch (SQLException | InvalidDBMSConnectionPropertiesException e) { - e.printStackTrace(); - } - }); + } return result.stream(); } From 643f4618403f0d0b0a5a38105d8d80ae1659174c Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 1 Sep 2018 23:59:30 +0200 Subject: [PATCH 5/9] Remove file lock --- .../gui/collab/DatabaseChangeMonitor.java | 16 --- .../importer/actions/OpenDatabaseAction.java | 31 ----- .../jabref/logic/importer/OpenDatabase.java | 7 - .../jabref/logic/util/io/FileBasedLock.java | 127 ------------------ 4 files changed, 181 deletions(-) delete mode 100644 src/main/java/org/jabref/logic/util/io/FileBasedLock.java diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java index 7c300bcc5d8..701679fed5f 100644 --- a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java +++ b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java @@ -11,7 +11,6 @@ import org.jabref.gui.BasePanel; import org.jabref.gui.SidePaneManager; import org.jabref.gui.SidePaneType; -import org.jabref.logic.util.io.FileBasedLock; import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.util.FileUpdateListener; @@ -61,21 +60,6 @@ public void fileUpdated() { updatedExternally = true; final ChangeScanner scanner = new ChangeScanner(panel.frame(), panel, database.getDatabasePath().orElse(null), tmpFile); - - // Test: running scan automatically in background - if (database.getDatabasePath().isPresent() && !FileBasedLock.waitForFileLock(database.getDatabasePath().get())) { - // The file is locked even after the maximum wait. Do nothing. - LOGGER.error("File updated externally, but change scan failed because the file is locked."); - - // Wait a bit and then try again - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - // Nothing to do - } - fileUpdated(); - return; - } JabRefExecutorService.INSTANCE.executeInterruptableTaskAndWait(scanner); // Adding the sidepane component is Swing work, so we must do this in the Swing diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 49c6e1cb06c..0fdba4fd411 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -3,7 +3,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.nio.file.attribute.FileTime; import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; @@ -36,7 +35,6 @@ import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; import org.jabref.logic.shared.exception.NotASharedDatabaseException; import org.jabref.logic.util.StandardFileType; -import org.jabref.logic.util.io.FileBasedLock; import org.jabref.migrations.FileLinksUpgradeWarning; import org.jabref.model.database.BibDatabase; import org.jabref.model.database.shared.DatabaseNotSupportedException; @@ -199,37 +197,8 @@ private void openTheFile(Path file, boolean raisePanel) { frame.output(Localization.lang("Opening") + ": '" + file + "'"); - String fileName = file.getFileName().toString(); Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, fileToLoad.getParent().toString()); - if (FileBasedLock.hasLockFile(file)) { - Optional modificationTime = FileBasedLock.getLockFileTimeStamp(file); - if ((modificationTime.isPresent()) && ((System.currentTimeMillis() - - modificationTime.get().toMillis()) > FileBasedLock.LOCKFILE_CRITICAL_AGE)) { - // The lock file is fairly old, so we can offer to "steal" the file: - - boolean overWriteFileLockPressed = frame.getDialogService().showConfirmationDialogAndWait(Localization.lang("File locked"), - Localization.lang("Error opening file") + " '" + fileName + "'. " - + Localization.lang("File is locked by another JabRef instance.") + "\n" - + Localization.lang("Do you want to override the file lock?"), - Localization.lang("Overwrite file lock"), - Localization.lang("Cancel")); - - if (overWriteFileLockPressed) { - FileBasedLock.deleteLockFile(file); - } else { - return; - } - } else if (!FileBasedLock.waitForFileLock(file)) { - - frame.getDialogService().showErrorDialogAndWait(Localization.lang("Error"), - Localization.lang("Error opening file") + " '" + fileName + "'. " - + Localization.lang("File is locked by another JabRef instance.")); - - return; - } - } - if (BackupManager.checkForBackupFile(fileToLoad)) { BackupUIManager.showRestoreBackupDialog(frame.getDialogService(), fileToLoad); } diff --git a/src/main/java/org/jabref/logic/importer/OpenDatabase.java b/src/main/java/org/jabref/logic/importer/OpenDatabase.java index ed777ef8211..a47ab79be74 100644 --- a/src/main/java/org/jabref/logic/importer/OpenDatabase.java +++ b/src/main/java/org/jabref/logic/importer/OpenDatabase.java @@ -8,7 +8,6 @@ import org.jabref.logic.importer.fileformat.BibtexImporter; import org.jabref.logic.l10n.Localization; import org.jabref.logic.specialfields.SpecialFieldsUtils; -import org.jabref.logic.util.io.FileBasedLock; import org.jabref.migrations.ConvertLegacyExplicitGroups; import org.jabref.migrations.ConvertMarkingToGroups; import org.jabref.migrations.PostOpenMigration; @@ -44,12 +43,6 @@ public static ParserResult loadDatabase(String name, ImportFormatPreferences imp } try { - if (!FileBasedLock.waitForFileLock(file.toPath())) { - LOGGER.error(Localization.lang("Error opening file") + " '" + name + "'. " - + "File is locked by another JabRef instance."); - return new ParserResult(); - } - ParserResult pr = OpenDatabase.loadDatabase(file, importFormatPreferences, fileMonitor); pr.setFile(file); if (pr.hasWarnings()) { diff --git a/src/main/java/org/jabref/logic/util/io/FileBasedLock.java b/src/main/java/org/jabref/logic/util/io/FileBasedLock.java deleted file mode 100644 index a271879d184..00000000000 --- a/src/main/java/org/jabref/logic/util/io/FileBasedLock.java +++ /dev/null @@ -1,127 +0,0 @@ -package org.jabref.logic.util.io; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.attribute.BasicFileAttributes; -import java.nio.file.attribute.FileTime; -import java.util.Optional; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class FileBasedLock { - /** - * The age in ms of a lock file before JabRef will offer to "steal" the locked file. - */ - public static final long LOCKFILE_CRITICAL_AGE = 60000; - private static final Logger LOGGER = LoggerFactory.getLogger(FileBasedLock.class); - private static final String LOCKFILE_SUFFIX = ".lock"; - - // default retry count for acquiring file lock - private static final int AQUIRE_LOCK_RETRY = 10; - - private FileBasedLock() { - } - - /** - * This method checks whether there is a lock file for the given file. If - * there is, it waits for 500 ms. This is repeated until the lock is gone - * or we have waited the maximum number of times. - * - * @param file The file to check the lock for. - * @param maxWaitCount The maximum number of times to wait. - * @return true if the lock file is gone, false if it is still there. - */ - private static boolean waitForFileLock(Path file, int maxWaitCount) { - // Check if the file is locked by another JabRef user: - int lockCheckCount = 0; - while (hasLockFile(file)) { - - if (lockCheckCount++ == maxWaitCount) { - return false; - } - try { - Thread.sleep(500); - } catch (InterruptedException ignored) { - // Ignored - } - } - return true; - } - - public static boolean waitForFileLock(Path file) { - return waitForFileLock(file, AQUIRE_LOCK_RETRY); - } - - /** - * Check whether a lock file exists for this file. - * - * @param file The file to check. - * @return true if a lock file exists, false otherwise. - */ - public static boolean hasLockFile(Path file) { - Path lockFile = getLockFilePath(file); - return Files.exists(lockFile); - } - - /** - * Find the lock file's last modified time, if it has a lock file. - * - * @param file The file to check. - * @return the last modified time if lock file exists, empty optional otherwise. - */ - public static Optional getLockFileTimeStamp(Path file) { - Path lockFile = getLockFilePath(file); - try { - return Files.exists(lockFile) ? - Optional.of(Files.readAttributes(lockFile, BasicFileAttributes.class).lastModifiedTime()) : - Optional.empty(); - } catch (IOException e) { - return Optional.empty(); - } - } - - /** - * Check if a lock file exists, and delete it if it does. - * - * @return true if the lock file existed, false otherwise. - */ - public static boolean deleteLockFile(Path file) { - Path lockFile = getLockFilePath(file); - if (!Files.exists(lockFile)) { - return false; - } - try { - Files.delete(lockFile); - } catch (IOException e) { - LOGGER.warn("Cannot delete lock file", e); - } - return true; - } - - /** - * Check if a lock file exists, and create it if it doesn't. - * - * @return true if the lock file already existed - * @throws IOException if something happens during creation. - */ - public static boolean createLockFile(Path file) throws IOException { - Path lockFile = getLockFilePath(file); - if (Files.exists(lockFile)) { - return true; - } - - try { - Files.write(lockFile, "0".getBytes()); - } catch (IOException ex) { - LOGGER.error("Error when creating lock file.", ex); - } - lockFile.toFile().deleteOnExit(); - return false; - } - - private static Path getLockFilePath(Path file) { - return file.resolveSibling(file.getFileName() + LOCKFILE_SUFFIX); - } -} From d67a11c87b31ae8a5916197fbc93689af318b088 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sun, 2 Sep 2018 23:48:43 +0200 Subject: [PATCH 6/9] Improve comments --- .../java/org/jabref/logic/exporter/AtomicFileOutputStream.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java index 6d2c6db6ff8..51b6da404ee 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -22,7 +22,7 @@ * In detail, the strategy is to: *

    *
  1. Write to a temporary file (with .tmp suffix) in the same directory as the destination file.
    2. - *
  2. Create a backup of the original file (if it exists) in the same directory.
    3. + *
  3. Create a backup (with .sav suffix) of the original file (if it exists) in the same directory.
    4. *
  4. Move the temporary file to the correct place, overwriting any file that already exists at that location.
    5. *
  5. Delete the backup file.
    6. *
From c7781cd0ff0101b545e8d9c312bb5aef506a5462 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Thu, 13 Sep 2018 19:51:56 +0200 Subject: [PATCH 7/9] Fix build --- .../org/jabref/preferences/JabRefPreferences.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index a504d8adf8e..c37ddb11e9a 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1700,14 +1700,14 @@ public void storeExportSaveOrder(SaveOrderConfig config) { put(EXPORT_TERTIARY_SORT_FIELD, config.getSortCriteria().get(2).field); } - public SaveOrderConfig loadTableSaveOrder() { + private SaveOrderConfig loadTableSaveOrder() { SaveOrderConfig config = new SaveOrderConfig(); - config.sortCriteria[0].field = get(TABLE_PRIMARY_SORT_FIELD); - config.sortCriteria[0].descending = getBoolean(TABLE_PRIMARY_SORT_DESCENDING); - config.sortCriteria[1].field = get(TABLE_SECONDARY_SORT_FIELD); - config.sortCriteria[1].descending = getBoolean(TABLE_SECONDARY_SORT_DESCENDING); - config.sortCriteria[2].field = get(TABLE_TERTIARY_SORT_FIELD); - config.sortCriteria[2].descending = getBoolean(TABLE_TERTIARY_SORT_DESCENDING); + List columns = getStringList(COLUMN_IN_SORT_ORDER); + List sortTypes = getStringList(COlUMN_IN_SORT_ORDER_TYPE).stream().map(SortType::valueOf).map(type -> type == SortType.DESCENDING).collect(Collectors.toList()); + + for (int i = 0; i < columns.size(); i++) { + config.getSortCriteria().add(new SaveOrderConfig.SortCriterion(columns.get(i), sortTypes.get(i))); + } return config; } From 6c36cc7a4928d30c6b84ce0f9df9a7dcb8ad733e Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Thu, 13 Sep 2018 21:01:32 +0200 Subject: [PATCH 8/9] Implement feedback and fix tests --- .../gui/exporter/SaveDatabaseAction.java | 28 ++------- .../exporter/AtomicFileOutputStream.java | 61 ++++++++++++++++--- .../logic/exporter/AtomicFileWriter.java | 6 +- src/main/resources/l10n/JabRef_en.properties | 21 +------ .../logic/bibtex/BibEntryWriterTest.java | 6 +- 5 files changed, 63 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index 7ea864a8c25..b0d8e1bae03 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -3,7 +3,6 @@ import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.UnsupportedCharsetException; -import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; import java.util.Set; @@ -33,7 +32,6 @@ import org.jabref.logic.l10n.Localization; import org.jabref.logic.shared.prefs.SharedDatabasePreferences; import org.jabref.logic.util.StandardFileType; -import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.database.event.ChangePropagation; import org.jabref.model.database.shared.DatabaseLocation; @@ -67,23 +65,8 @@ private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, SavePreferences preferences = Globals.prefs.loadForSaveFromPreferences() .withEncoding(encoding) .withSaveType(saveType); - if (preferences.makeBackup()) { - try { - makeBackup(file); - } catch (IOException exception) { - LOGGER.error("Problems saving during backup creation", exception); - boolean saveWithoutBackup = frame.getDialogService().showConfirmationDialogAndWait( - Localization.lang("Unable to create backup"), - Localization.lang("Save failed during backup creation") + ". " + Localization.lang("Save without backup?"), - Localization.lang("Save without backup")); - - if (!saveWithoutBackup) { - return false; - } - } - } - AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding()); + AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding(), preferences.makeBackup()); BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter, preferences); if (selectedOnly) { @@ -106,10 +89,6 @@ private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, return true; } - private void makeBackup(Path file) throws IOException { - Files.copy(file, FileUtil.addExtension(file, ".bak")); - } - private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset encoding, Set encodingProblems, SavePreferences.DatabaseSaveType saveType) throws SaveException { DialogPane pane = new DialogPane(); VBox vbox = new VBox(); @@ -138,9 +117,10 @@ private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset } private boolean doSave() { + Path targetPath = panel.getBibDatabaseContext().getDatabasePath().get(); try { // Save the database - boolean success = saveDatabase(panel.getBibDatabaseContext().getDatabasePath().get(), false, + boolean success = saveDatabase(targetPath, false, panel.getBibDatabaseContext() .getMetaData() .getEncoding() @@ -172,7 +152,7 @@ private boolean doSave() { } return success; } catch (SaveException ex) { - LOGGER.error("A problem occurred when trying to save the file", ex); + LOGGER.error("A problem occurred when trying to save the file " + targetPath, ex); frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); return false; } diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java index 51b6da404ee..ed90d51872f 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -3,9 +3,13 @@ import java.io.FileOutputStream; import java.io.FilterOutputStream; import java.io.IOException; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.channels.OverlappingFileLockException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.nio.file.attribute.PosixFilePermission; import java.util.EnumSet; import java.util.Set; @@ -22,9 +26,9 @@ * In detail, the strategy is to: *
    *
  1. Write to a temporary file (with .tmp suffix) in the same directory as the destination file.
    2. - *
  2. Create a backup (with .sav suffix) of the original file (if it exists) in the same directory.
    3. + *
  3. Create a backup (with .bak suffix) of the original file (if it exists) in the same directory.
    4. *
  4. Move the temporary file to the correct place, overwriting any file that already exists at that location.
    5. - *
  5. Delete the backup file.
    6. + *
  6. Delete the backup file (if configured to do so).
    7. *
* If all goes well, no temporary or backup files will remain on disk after closing the stream. * @@ -44,32 +48,55 @@ public class AtomicFileOutputStream extends FilterOutputStream { private static final Logger LOGGER = LoggerFactory.getLogger(AtomicFileOutputStream.class); private static final String TEMPORARY_EXTENSION = ".tmp"; - private static final String BACKUP_EXTENSION = ".sav"; + private static final String BACKUP_EXTENSION = ".bak"; /** * The file we want to create/replace. */ private final Path targetFile; + private final FileLock targetFileLock; /** * The file to which writes are redirected to. */ private final Path temporaryFile; + private final FileLock temporaryFileLock; /** * A backup of the target file (if it exists), created when the stream is closed */ private final Path backupFile; + private final boolean keepBackup; /** * Creates a new output stream to write to or replace the file at the specified path. * * @param path the path of the file to write to or replace + * @param keepBackup whether to keep the backup file after a successful write process */ - public AtomicFileOutputStream(Path path) throws IOException { + public AtomicFileOutputStream(Path path, boolean keepBackup) throws IOException { super(Files.newOutputStream(getPathOfTemporaryFile(path))); - targetFile = path; - temporaryFile = getPathOfTemporaryFile(path); - backupFile = getPathOfBackupFile(path); + this.targetFile = path; + this.temporaryFile = getPathOfTemporaryFile(path); + this.backupFile = getPathOfBackupFile(path); + this.keepBackup = keepBackup; + + try { + // Lock files (so that at least not another JabRef instance writes at the same time to the same tmp file) + temporaryFileLock = FileChannel.open(temporaryFile, StandardOpenOption.WRITE).lock(); + targetFileLock = FileChannel.open(targetFile, StandardOpenOption.WRITE, StandardOpenOption.CREATE).lock(); + } catch (OverlappingFileLockException exception) { + throw new IOException("Could not obtain write access. Maybe another instance of JabRef is currently writing to the same file?", exception); + } + } + + /** + * Creates a new output stream to write to or replace the file at the specified path. The backup file is deleted + * when the write was successful. + * + * @param path the path of the file to write to or replace + */ + public AtomicFileOutputStream(Path path) throws IOException { + this(path, false); } private static Path getPathOfTemporaryFile(Path targetFile) { @@ -103,7 +130,7 @@ public void write(byte b[], int off, int len) throws IOException { /** * Closes the write process to the temporary file but does not commit to the target file. */ - public void abort() throws IOException { + public void abort() { try { super.close(); Files.deleteIfExists(temporaryFile); @@ -119,6 +146,18 @@ private void cleanup() { } catch (IOException exception) { LOGGER.debug("Unable to delete file " + temporaryFile, exception); } + + try { + temporaryFileLock.release(); + } catch (IOException exception) { + LOGGER.warn("Unable to release lock on file " + temporaryFile, exception); + } + + try { + targetFileLock.release(); + } catch (IOException exception) { + LOGGER.warn("Unable to release lock on file " + targetFile, exception); + } } // perform the final operations to move the temporary file to its final destination @@ -168,8 +207,10 @@ public void close() throws IOException { } } - // Remove backup file - Files.deleteIfExists(backupFile); + if (!keepBackup) { + // Remove backup file + Files.deleteIfExists(backupFile); + } } finally { // Remove temporary file (but not the backup!) cleanup(); diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java b/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java index febbd4e7d45..19274cbeafa 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java @@ -22,7 +22,11 @@ public class AtomicFileWriter extends OutputStreamWriter { private final Set problemCharacters = new TreeSet<>(); public AtomicFileWriter(Path file, Charset encoding) throws IOException { - super(new AtomicFileOutputStream(file), encoding); + this(file, encoding, false); + } + + public AtomicFileWriter(Path file, Charset encoding, boolean keepBackup) throws IOException { + super(new AtomicFileOutputStream(file, keepBackup), encoding); encoder = encoding.newEncoder(); } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index b5c30ff8deb..bff30cd317d 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -430,13 +430,8 @@ File\ directory\ is\ not\ set\ or\ does\ not\ exist\!=File directory is not set File\ exists=File exists -File\ has\ been\ updated\ externally.\ What\ do\ you\ want\ to\ do?=File has been updated externally. What do you want to do? - File\ not\ found=File not found File\ type=File type - -File\ updated\ externally=File updated externally - filename=filename Files\ opened=Files opened @@ -923,10 +918,6 @@ Save\ library\ as...=Save library as... Save\ entries\ in\ their\ original\ order=Save entries in their original order -Save\ failed=Save failed - -Save\ failed\ during\ backup\ creation=Save failed during backup creation - Saved\ library=Saved library Saved\ selected\ to\ '%0'.=Saved selected to '%0'. @@ -947,8 +938,7 @@ Searching\ for\ files=Searching for files Secondary\ sort\ criterion=Secondary sort criterion Select\ all=Select all - -Select\ encoding=Select encoding +Select\ new\ encoding=Select new encoding Select\ entry\ type=Select entry type @@ -1178,9 +1168,6 @@ Error\ while\ fetching\ from\ %0=Error while fetching from %0 Show\ search\ results\ in\ a\ window=Show search results in a window Show\ global\ search\ results\ in\ a\ window=Show global search results in a window Search\ in\ all\ open\ libraries=Search in all open libraries - -Library\ is\ protected.\ Cannot\ save\ until\ external\ changes\ have\ been\ reviewed.=Library is protected. Cannot save until external changes have been reviewed. -Protected\ library=Protected library Refuse\ to\ save\ the\ library\ before\ external\ changes\ have\ been\ reviewed.=Refuse to save the library before external changes have been reviewed. Library\ protection=Library protection Unable\ to\ save\ library=Unable to save library @@ -1231,9 +1218,6 @@ Formatter\ not\ found\:\ %0=Formatter not found: %0 Clear\ inputarea=Clear inputarea Could\ not\ save,\ file\ locked\ by\ another\ JabRef\ instance.=Could not save, file locked by another JabRef instance. -File\ is\ locked\ by\ another\ JabRef\ instance.=File is locked by another JabRef instance. -Do\ you\ want\ to\ override\ the\ file\ lock?=Do you want to override the file lock? -File\ locked=File locked Current\ tmp\ value=Current tmp value Metadata\ change=Metadata change Changes\ have\ been\ made\ to\ the\ following\ metadata\ elements=Changes have been made to the following metadata elements @@ -1242,7 +1226,6 @@ Generate\ groups\ for\ author\ last\ names=Generate groups for author last names Generate\ groups\ from\ keywords\ in\ a\ BibTeX\ field=Generate groups from keywords in a BibTeX field Enforce\ legal\ characters\ in\ BibTeX\ keys=Enforce legal characters in BibTeX keys -Save\ without\ backup?=Save without backup? Unable\ to\ create\ backup=Unable to create backup Move\ file\ to\ file\ directory=Move file to file directory Rename\ file\ to=Rename file to @@ -2168,8 +2151,6 @@ Restore\ from\ backup=Restore from backup Continue=Continue Generate\ key=Generate key Overwrite\ file=Overwrite file -Overwrite\ file\ lock=Overwrite file lock -Save\ without\ backup=Save without backup Shared\ database\ connection=Shared database connection Could\ not\ connect\ to\ Vim\ server.\ Make\ sure\ that\ Vim\ is\ running\ with\ correct\ server\ name.=Could not connect to Vim server. Make sure that Vim is running with correct server name. diff --git a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java index 8b2d9a9476a..68a31d6eb54 100644 --- a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java +++ b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java @@ -100,13 +100,11 @@ void writeEntryWithFile() throws Exception { + OS.NEWLINE + " file = {test:/home/uers/test.pdf:PDF}," + OS.NEWLINE - + "}" + OS.NEWLINE - + OS.NEWLINE - + "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, stringWriter.toString()); + + "}" + OS.NEWLINE, stringWriter.toString()); } @Test - public void writeReallyunknownTypeTest() throws Exception { + public void writeReallyUnknownTypeTest() throws Exception { String expected = OS.NEWLINE + "@Reallyunknowntype{test," + OS.NEWLINE + " comment = {testentry}," + OS.NEWLINE + "}" + OS.NEWLINE; From 621361a411d1fee11bd4f99fe864b39870ba83d1 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Thu, 13 Sep 2018 21:21:35 +0200 Subject: [PATCH 9/9] Fix build --- .../gui/exporter/SaveDatabaseAction.java | 6 ++--- .../autosaveandbackup/BackupManager.java | 6 ++--- .../exporter/AtomicFileOutputStream.java | 23 ++++++++----------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index b0d8e1bae03..60d43a613cb 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -127,9 +127,6 @@ private boolean doSave() { .orElse(Globals.prefs.getDefaultEncoding()), SavePreferences.DatabaseSaveType.ALL); - // release panel from save status - panel.setSaving(false); - if (success) { panel.updateTimeStamp(); panel.getUndoManager().markUnchanged(); @@ -155,6 +152,9 @@ private boolean doSave() { LOGGER.error("A problem occurred when trying to save the file " + targetPath, ex); frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); return false; + } finally { + // release panel from save status + panel.setSaving(false); } } diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 4e9ead5d3e5..35ffdc314a7 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -123,11 +123,11 @@ private void performBackup(Path backupPath) { new BibtexDatabaseWriter(new AtomicFileWriter(backupPath, savePreferences.getEncoding()), savePreferences) .saveDatabase(bibDatabaseContext); } catch (IOException e) { - logIfCritical(e); + logIfCritical(backupPath, e); } } - private void logIfCritical(IOException e) { + private void logIfCritical(Path backupPath, IOException e) { Throwable innermostCause = e; while (innermostCause.getCause() != null) { innermostCause = innermostCause.getCause(); @@ -136,7 +136,7 @@ private void logIfCritical(IOException e) { // do not print errors in field values into the log during autosave if (!isErrorInField) { - LOGGER.error("Error while saving file.", e); + LOGGER.error("Error while saving to file" + backupPath, e); } } diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java index ed90d51872f..22ceec6004a 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -3,13 +3,11 @@ import java.io.FileOutputStream; import java.io.FilterOutputStream; import java.io.IOException; -import java.nio.channels.FileChannel; import java.nio.channels.FileLock; import java.nio.channels.OverlappingFileLockException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.nio.file.StandardOpenOption; import java.nio.file.attribute.PosixFilePermission; import java.util.EnumSet; import java.util.Set; @@ -54,7 +52,7 @@ public class AtomicFileOutputStream extends FilterOutputStream { * The file we want to create/replace. */ private final Path targetFile; - private final FileLock targetFileLock; + /** * The file to which writes are redirected to. */ @@ -82,10 +80,13 @@ public AtomicFileOutputStream(Path path, boolean keepBackup) throws IOException try { // Lock files (so that at least not another JabRef instance writes at the same time to the same tmp file) - temporaryFileLock = FileChannel.open(temporaryFile, StandardOpenOption.WRITE).lock(); - targetFileLock = FileChannel.open(targetFile, StandardOpenOption.WRITE, StandardOpenOption.CREATE).lock(); + if (out instanceof FileOutputStream) { + temporaryFileLock = ((FileOutputStream) out).getChannel().lock(); + } else { + temporaryFileLock = null; + } } catch (OverlappingFileLockException exception) { - throw new IOException("Could not obtain write access. Maybe another instance of JabRef is currently writing to the same file?", exception); + throw new IOException("Could not obtain write access to " + temporaryFile + ". Maybe another instance of JabRef is currently writing to the same file?", exception); } } @@ -148,16 +149,12 @@ private void cleanup() { } try { - temporaryFileLock.release(); + if (temporaryFileLock != null) { + temporaryFileLock.release(); + } } catch (IOException exception) { LOGGER.warn("Unable to release lock on file " + temporaryFile, exception); } - - try { - targetFileLock.release(); - } catch (IOException exception) { - LOGGER.warn("Unable to release lock on file " + targetFile, exception); - } } // perform the final operations to move the temporary file to its final destination