Skip to content

Commit

Permalink
Improve change scanner (#5665)
Browse files Browse the repository at this point in the history 
Move from 3-way merge (last saved value vs current value in JabRef vs new value in file) to simple 2-way merge (current value in JabRef vs new value in file).
  • Loading branch information
@tobiasdiez @koppor
tobiasdiez authored and koppor committed Nov 28, 2019
1 parent 382c4b2 commit ce0e887
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 191 deletions.
70 changes: 30 additions & 40 deletions src/main/java/org/jabref/gui/collab/ChangeScanner.java
View file
Original file line number Diff line number Diff line change
@@ -1,67 +1,60 @@
package org.jabref.gui.collab;

import java.nio.file.Path;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

import org.jabref.Globals;
import org.jabref.logic.bibtex.DuplicateCheck;
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.importer.ImportFormatPreferences;
import org.jabref.logic.importer.OpenDatabase;
import org.jabref.logic.importer.ParserResult;
import org.jabref.logic.importer.fileformat.BibtexImporter;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.BibtexString;
import org.jabref.model.util.DummyFileUpdateMonitor;

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

public class ChangeScanner {

private final Path referenceFile;
private static final Logger LOGGER = LoggerFactory.getLogger(ChangeScanner.class);
private final BibDatabaseContext database;
private final List<DatabaseChangeViewModel> changes = new ArrayList<>();
private BibDatabaseContext referenceDatabase;

public ChangeScanner(BibDatabaseContext database, Path referenceFile) {
public ChangeScanner(BibDatabaseContext database) {
this.database = database;
this.referenceFile = referenceFile;
}

/**
* Finds the entry in the list best fitting the specified entry. Even if no entries get a score above zero, an entry
* is still returned.
*/
private static BibEntry bestFit(BibEntry targetEntry, List<BibEntry> entries) {
return entries.stream()
.max(Comparator.comparingDouble(candidate -> DuplicateCheck.compareEntriesStrictly(targetEntry, candidate)))
.orElse(null);
}

public List<DatabaseChangeViewModel> scanForChanges() {
database.getDatabasePath().ifPresent(diskdb -> {
// Parse the temporary file.
ImportFormatPreferences importFormatPreferences = Globals.prefs.getImportFormatPreferences();
ParserResult result = OpenDatabase.loadDatabase(referenceFile.toAbsolutePath().toString(), importFormatPreferences, Globals.getFileUpdateMonitor());
referenceDatabase = result.getDatabaseContext();
if (database.getDatabasePath().isEmpty()) {
return Collections.emptyList();
}

try {
List<DatabaseChangeViewModel> changes = new ArrayList<>();

// Parse the modified file.
result = OpenDatabase.loadDatabase(diskdb.toAbsolutePath().toString(), importFormatPreferences, Globals.getFileUpdateMonitor());
// Parse the modified file
ImportFormatPreferences importFormatPreferences = Globals.prefs.getImportFormatPreferences();
ParserResult result = new BibtexImporter(importFormatPreferences, new DummyFileUpdateMonitor())
.importDatabase(database.getDatabasePath().get(), importFormatPreferences.getEncoding());
BibDatabaseContext databaseOnDisk = result.getDatabaseContext();

// Start looking at changes.
BibDatabaseDiff differences = BibDatabaseDiff.compare(referenceDatabase, databaseOnDisk);
BibDatabaseDiff differences = BibDatabaseDiff.compare(database, databaseOnDisk);
differences.getMetaDataDifferences().ifPresent(diff -> {
changes.add(new MetaDataChangeViewModel(diff));
diff.getGroupDifferences().ifPresent(groupDiff -> changes.add(new GroupChangeViewModel(groupDiff)));
});
differences.getPreambleDifferences().ifPresent(diff -> changes.add(new PreambleChangeViewModel(diff)));
differences.getBibStringDifferences().forEach(diff -> changes.add(createBibStringDiff(diff)));
differences.getEntryDifferences().forEach(diff -> changes.add(createBibEntryDiff(diff)));
});
return changes;
return changes;
} catch (IOException e) {
LOGGER.warn("Error while parsing changed file.", e);
return Collections.emptyList();
}
}

private DatabaseChangeViewModel createBibStringDiff(BibStringDiff diff) {
Expand All @@ -70,17 +63,14 @@ private DatabaseChangeViewModel createBibStringDiff(BibStringDiff diff) {
}

if (diff.getNewString() == null) {
Optional<BibtexString> current = database.getDatabase().getStringByName(diff.getOriginalString().getName());
return new StringRemoveChangeViewModel(diff.getOriginalString(), current.orElse(null));
return new StringRemoveChangeViewModel(diff.getOriginalString());
}

if (diff.getOriginalString().getName().equals(diff.getNewString().getName())) {
Optional<BibtexString> current = database.getDatabase().getStringByName(diff.getOriginalString().getName());
return new StringChangeViewModel(current.orElse(null), diff.getOriginalString(), diff.getNewString().getContent());
return new StringChangeViewModel(diff.getOriginalString(), diff.getNewString());
}

Optional<BibtexString> current = database.getDatabase().getStringByName(diff.getOriginalString().getName());
return new StringNameChangeViewModel(current.orElse(null), diff.getOriginalString(), current.map(BibtexString::getName).orElse(""), diff.getNewString().getName());
return new StringNameChangeViewModel(diff.getOriginalString(), diff.getNewString());
}

private DatabaseChangeViewModel createBibEntryDiff(BibEntryDiff diff) {
Expand All @@ -89,9 +79,9 @@ private DatabaseChangeViewModel createBibEntryDiff(BibEntryDiff diff) {
}

if (diff.getNewEntry() == null) {
return new EntryDeleteChangeViewModel(bestFit(diff.getOriginalEntry(), database.getEntries()), diff.getOriginalEntry());
return new EntryDeleteChangeViewModel(diff.getOriginalEntry());
}

return new EntryChangeViewModel(bestFit(diff.getOriginalEntry(), database.getEntries()), diff.getOriginalEntry(), diff.getNewEntry());
return new EntryChangeViewModel(diff.getOriginalEntry(), diff.getNewEntry());
}
}
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor file
@Override
public void fileUpdated() {
// File on disk has changed, thus look for notable changes and notify listeners in case there are such changes
ChangeScanner scanner = new ChangeScanner(database, referenceFile);
ChangeScanner scanner = new ChangeScanner(database);
BackgroundTask.wrap(scanner::scanForChanges)
.onSuccess(changes -> {
if (!changes.isEmpty()) {
Expand Down
87 changes: 33 additions & 54 deletions src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@

import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableFieldChange;
import org.jabref.logic.bibtex.DuplicateCheck;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.FieldChange;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.model.strings.StringUtil;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -28,43 +29,28 @@ class EntryChangeViewModel extends DatabaseChangeViewModel {

private final List<FieldChangeViewModel> fieldChanges = new ArrayList<>();

public EntryChangeViewModel(BibEntry memEntry, BibEntry tmpEntry, BibEntry diskEntry) {
public EntryChangeViewModel(BibEntry entry, BibEntry newEntry) {
super();
name = tmpEntry.getCiteKeyOptional()
.map(key -> Localization.lang("Modified entry") + ": '" + key + '\'')
.orElse(Localization.lang("Modified entry"));

// We know that tmpEntry is not equal to diskEntry. Check if it has been modified
// locally as well, since last tempfile was saved.
boolean isModifiedLocally = (DuplicateCheck.compareEntriesStrictly(memEntry, tmpEntry) <= 1);

// Another (unlikely?) possibility is that both disk and mem version has been modified
// in the same way. Check for this, too.
boolean modificationsAgree = (DuplicateCheck.compareEntriesStrictly(memEntry, diskEntry) > 1);

LOGGER.debug("Modified entry: " + memEntry.getCiteKeyOptional().orElse("<no BibTeX key set>")
+ "\n Modified locally: " + isModifiedLocally + " Modifications agree: " + modificationsAgree);
name = entry.getCiteKeyOptional()
.map(key -> Localization.lang("Modified entry") + ": '" + key + '\'')
.orElse(Localization.lang("Modified entry"));

Set<Field> allFields = new TreeSet<>(Comparator.comparing(Field::getName));
allFields.addAll(memEntry.getFields());
allFields.addAll(tmpEntry.getFields());
allFields.addAll(diskEntry.getFields());
allFields.addAll(entry.getFields());
allFields.addAll(newEntry.getFields());

for (Field field : allFields) {
Optional<String> mem = memEntry.getField(field);
Optional<String> tmp = tmpEntry.getField(field);
Optional<String> disk = diskEntry.getField(field);
Optional<String> value = entry.getField(field);
Optional<String> newValue = newEntry.getField(field);

if ((tmp.isPresent()) && (disk.isPresent())) {
if (!tmp.equals(disk)) {
if (value.isPresent() && newValue.isPresent()) {
if (!value.equals(newValue)) {
// Modified externally.
fieldChanges.add(new FieldChangeViewModel(field, memEntry, tmpEntry, mem.orElse(null), tmp.get(), disk.get()));
fieldChanges.add(new FieldChangeViewModel(entry, field, value.get(), newValue.get()));
}
} else if (((!tmp.isPresent()) && (disk.isPresent()) && !disk.get().isEmpty())
|| ((!disk.isPresent()) && (tmp.isPresent()) && !tmp.get().isEmpty()
&& (mem.isPresent()) && !mem.get().isEmpty())) {
// Added externally.
fieldChanges.add(new FieldChangeViewModel(field, memEntry, tmpEntry, mem.orElse(null), tmp.orElse(null), disk.orElse(null)));
} else {
// Added/removed externally.
fieldChanges.add(new FieldChangeViewModel(entry, field, value.orElse(null), newValue.orElse(null)));
}
}
}
Expand All @@ -80,7 +66,7 @@ public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {

@Override
public Node description() {
VBox container = new VBox();
VBox container = new VBox(10);
Label header = new Label(name);
header.getStyleClass().add("sectionHeader");
container.getChildren().add(header);
Expand All @@ -95,49 +81,42 @@ public Node description() {
static class FieldChangeViewModel extends DatabaseChangeViewModel {

private final BibEntry entry;
private final BibEntry tmpEntry;
private final Field field;
private final String inMem;
private final String onTmp;
private final String onDisk;
private final String value;
private final String newValue;

public FieldChangeViewModel(Field field, BibEntry memEntry, BibEntry tmpEntry, String inMem, String onTmp, String onDisk) {
public FieldChangeViewModel(BibEntry entry, Field field, String value, String newValue) {
super(field.getName());
entry = memEntry;
this.tmpEntry = tmpEntry;
this.entry = entry;
this.field = field;
this.inMem = inMem;
this.onTmp = onTmp;
this.onDisk = onDisk;
this.value = value;
this.newValue = newValue;
}

@Override
public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
if (onDisk == null) {
entry.clearField(field);
Optional<FieldChange> change;
if (StringUtil.isBlank(newValue)) {
change = entry.clearField(field);
} else {
entry.setField(field, onDisk);
change = entry.setField(field, newValue);
}
undoEdit.addEdit(new UndoableFieldChange(entry, field, inMem, onDisk));

change.map(UndoableFieldChange::new).ifPresent(undoEdit::addEdit);
}

@Override
public Node description() {
VBox container = new VBox();
container.getChildren().add(new Label(Localization.lang("Modification of field") + " " + field));
container.getChildren().add(new Label(Localization.lang("Modification of field") + " " + field.getDisplayName()));

if ((onDisk != null) && !onDisk.isEmpty()) {
container.getChildren().add(new Label(Localization.lang("Value set externally") + ": " + onDisk));
if (StringUtil.isNotBlank(newValue)) {
container.getChildren().add(new Label(Localization.lang("Value set externally") + ": " + newValue));
} else {
container.getChildren().add(new Label(Localization.lang("Value cleared externally")));
}

if ((inMem != null) && !inMem.isEmpty()) {
container.getChildren().add(new Label(Localization.lang("Current value") + ": " + inMem));
}
if ((onTmp != null) && !onTmp.isEmpty()) {
container.getChildren().add(new Label(Localization.lang("Current tmp value") + ": " + onTmp));
}
container.getChildren().add(new Label(Localization.lang("Current value") + ": " + value));

return container;
}
Expand Down
25 changes: 6 additions & 19 deletions src/main/java/org/jabref/gui/collab/EntryDeleteChangeViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import org.jabref.gui.preview.PreviewViewer;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableRemoveEntries;
import org.jabref.logic.bibtex.DuplicateCheck;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
Expand All @@ -18,35 +17,23 @@
class EntryDeleteChangeViewModel extends DatabaseChangeViewModel {

private static final Logger LOGGER = LoggerFactory.getLogger(EntryDeleteChangeViewModel.class);
private final BibEntry memEntry;
private final BibEntry tmpEntry;
private final BibEntry entry;

public EntryDeleteChangeViewModel(BibEntry memEntry, BibEntry tmpEntry) {
public EntryDeleteChangeViewModel(BibEntry entry) {
super(Localization.lang("Deleted entry"));
this.memEntry = memEntry;
this.tmpEntry = tmpEntry;

// Compare the deleted entry in memory with the one in the tmpfile. The
// entry could have been removed in memory.
double matchWithTmp = DuplicateCheck.compareEntriesStrictly(memEntry, tmpEntry);

// Check if it has been modified locally, since last tempfile was saved.
boolean isModifiedLocally = (matchWithTmp <= 1);

LOGGER.debug("Modified entry: " + memEntry.getCiteKeyOptional().orElse("<no BibTeX key set>")
+ "\n Modified locally: " + isModifiedLocally);
this.entry = entry;
}

@Override
public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
database.getDatabase().removeEntry(memEntry);
undoEdit.addEdit(new UndoableRemoveEntries(database.getDatabase(), memEntry));
database.getDatabase().removeEntry(entry);
undoEdit.addEdit(new UndoableRemoveEntries(database.getDatabase(), entry));
}

@Override
public Node description() {
PreviewViewer previewViewer = new PreviewViewer(new BibDatabaseContext(), JabRefGUI.getMainFrame().getDialogService(), Globals.stateManager);
previewViewer.setEntry(memEntry);
previewViewer.setEntry(entry);
return previewViewer;
}
}
Loading

0 comments on commit ce0e887

Please sign in to comment.