Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Completely rework save operation #4309

Merged
merged 11 commits into from
Sep 13, 2018
Merged

Completely rework save operation #4309

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bdf1061
Make FileHistory use Paths instead of strings
tobiasdiez Sep 1, 2018
a9c31b8
Almost completely rewrite save infrastructure
tobiasdiez Sep 1, 2018
256f8be
Merge branch 'master' of https://github.com/JabRef/jabref into rework…
tobiasdiez Sep 1, 2018
dbc9f45
Fix build
tobiasdiez Sep 1, 2018
161a7a0
Fix tests
tobiasdiez Sep 1, 2018
643f461
Remove file lock
tobiasdiez Sep 1, 2018
d67a11c
Improve comments
tobiasdiez Sep 2, 2018
60199a0
Merge branch 'master' of https://github.com/JabRef/jabref into rework…
tobiasdiez Sep 13, 2018
c7781cd
Fix build
tobiasdiez Sep 13, 2018
6c36cc7
Implement feedback and fix tests
tobiasdiez Sep 13, 2018
621361a
Fix build
tobiasdiez Sep 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions src/jmh/java/org/jabref/benchmarks/Benchmarks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@

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;

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;
Expand Down Expand Up @@ -61,11 +60,12 @@ public void init() throws Exception {
entry.setField("rnd", "2" + randomizer.nextInt());
database.insertEntry(entry);
}
BibtexDatabaseWriter<StringSaveSession> 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} \\$";

Expand All @@ -80,11 +80,12 @@ public ParserResult parse() throws IOException {

@Benchmark
public String write() throws Exception {
BibtexDatabaseWriter<StringSaveSession> 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
Expand Down Expand Up @@ -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());
}
Expand Down
70 changes: 25 additions & 45 deletions src/main/java/org/jabref/cli/ArgumentProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -374,27 +372,7 @@ private boolean generateAux(List<ParserResult> 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<SaveSession> 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;
}

Expand All @@ -407,34 +385,36 @@ private boolean generateAux(List<ParserResult> 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<ParserResult> loaded, String[] data) {
if (data.length == 1) {
// This signals that the latest import should be stored in BibTeX
// format to the given file.
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<SaveSession> 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."));
Expand Down
153 changes: 4 additions & 149 deletions src/main/java/org/jabref/gui/BasePanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<SaveSession> 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<FieldChange> 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();
Expand Down Expand Up @@ -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<String> oldKey = bes.getCiteKeyOptional();
if (StringUtil.isBlank(oldKey)) {
Optional<FieldChange> 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.
*/
Expand Down Expand Up @@ -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<Path> 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()));
}
}
}
}
3 changes: 1 addition & 2 deletions src/main/java/org/jabref/gui/JabRefFrame.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading