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

Improve change scanner #5665

Merged
merged 2 commits into from
Nov 28, 2019
Merged

Improve change scanner #5665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a22117
Improve change scanner
tobiasdiez Nov 23, 2019
fbff05b
feedback
tobiasdiez Nov 28, 2019
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
70 changes: 30 additions & 40 deletions src/main/java/org/jabref/gui/collab/ChangeScanner.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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