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.

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 code of BibDataBaseDiff #11355

Merged
merged 7 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/collab/DatabaseChange.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void setAccepted(boolean accepted) {

/**
* Convenience method for accepting changes
* */
*/
public void accept() {
setAccepted(true);
}
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/jabref/gui/collab/DatabaseChangeList.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ private static DatabaseChange createBibStringDiff(BibDatabaseContext originalDat
}

private static DatabaseChange createBibEntryDiff(BibDatabaseContext originalDatabase, DatabaseChangeResolverFactory databaseChangeResolverFactory, BibEntryDiff diff) {
if (diff.getOriginalEntry() == null) {
return new EntryAdd(diff.getNewEntry(), originalDatabase, databaseChangeResolverFactory);
if (diff.originalEntry() == null) {
return new EntryAdd(diff.newEntry(), originalDatabase, databaseChangeResolverFactory);
}

if (diff.getNewEntry() == null) {
return new EntryDelete(diff.getOriginalEntry(), originalDatabase, databaseChangeResolverFactory);
if (diff.newEntry() == null) {
return new EntryDelete(diff.originalEntry(), originalDatabase, databaseChangeResolverFactory);
}

return new EntryChange(diff.getOriginalEntry(), diff.getNewEntry(), originalDatabase, databaseChangeResolverFactory);
return new EntryChange(diff.originalEntry(), diff.newEntry(), originalDatabase, databaseChangeResolverFactory);
}
}
13 changes: 11 additions & 2 deletions src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public class DatabaseChangeMonitor implements FileUpdateListener {
private final TaskExecutor taskExecutor;
private final DialogService dialogService;
private final PreferencesService preferencesService;
private final LibraryTab.DatabaseNotification notificationPane;
private final StateManager stateManager;
private LibraryTab saveState;

public DatabaseChangeMonitor(BibDatabaseContext database,
Expand All @@ -49,6 +51,8 @@ public DatabaseChangeMonitor(BibDatabaseContext database,
this.taskExecutor = taskExecutor;
this.dialogService = dialogService;
this.preferencesService = preferencesService;
this.notificationPane = notificationPane;
this.stateManager = stateManager;

this.listeners = new ArrayList<>();

Expand All @@ -60,7 +64,12 @@ public DatabaseChangeMonitor(BibDatabaseContext database,
}
});

addListener(changes -> notificationPane.notify(
addListener(this::notifyOnChange);
}

private void notifyOnChange(List<DatabaseChange> changes) {
// The changes come from {@link org.jabref.gui.collab.DatabaseChangeList.compareAndGetChanges}
notificationPane.notify(
IconTheme.JabRefIcons.SAVE.getGraphicNode(),
Localization.lang("The library has been modified by another program."),
List.of(new Action(Localization.lang("Dismiss changes"), event -> notificationPane.hide()),
Expand All @@ -82,7 +91,7 @@ public DatabaseChangeMonitor(BibDatabaseContext database,
}
notificationPane.hide();
})),
Duration.ZERO));
Duration.ZERO);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,50 @@
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.entry.field.StandardField;

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

public class BibDatabaseDiff {

private static final Logger LOGGER = LoggerFactory.getLogger(BibDatabaseDiff.class);

private static final double MATCH_THRESHOLD = 0.4;
private final Optional<MetaDataDiff> metaDataDiff;
private final Optional<PreambleDiff> preambleDiff;
private final List<BibStringDiff> bibStringDiffs;
private final List<BibEntryDiff> entryDiffs;

private BibDatabaseDiff(BibDatabaseContext originalDatabase, BibDatabaseContext newDatabase, boolean includeEmptyEntries) {
private BibDatabaseDiff(BibDatabaseContext originalDatabase, BibDatabaseContext newDatabase) {
metaDataDiff = MetaDataDiff.compare(originalDatabase.getMetaData(), newDatabase.getMetaData());
preambleDiff = PreambleDiff.compare(originalDatabase, newDatabase);
bibStringDiffs = BibStringDiff.compare(originalDatabase.getDatabase(), newDatabase.getDatabase());
entryDiffs = getBibEntryDiffs(originalDatabase, newDatabase);
if (LOGGER.isDebugEnabled() && !isEmpty()) {
LOGGER.debug("Differences detected");
metaDataDiff.ifPresent(diff -> LOGGER.debug("Metadata differences: {}", diff));
preambleDiff.ifPresent(diff -> LOGGER.debug("Premble differences: {}", diff));
LOGGER.debug("BibString differences: {}", bibStringDiffs);
LOGGER.debug("Entry differences: {}", entryDiffs);
}
}

private boolean isEmpty() {
return !metaDataDiff.isPresent() && !preambleDiff.isPresent() && bibStringDiffs.isEmpty() && entryDiffs.isEmpty();
}

private List<BibEntryDiff> getBibEntryDiffs(BibDatabaseContext originalDatabase, BibDatabaseContext newDatabase) {
final List<BibEntryDiff> entryDiffs;
// Sort both databases according to a common sort key.
EntryComparator comparator = getEntryComparator();
List<BibEntry> originalEntriesSorted = originalDatabase.getDatabase().getEntriesSorted(comparator);
List<BibEntry> newEntriesSorted = newDatabase.getDatabase().getEntriesSorted(comparator);

if (!includeEmptyEntries) {
originalEntriesSorted.removeIf(BibEntry::isEmpty);
newEntriesSorted.removeIf(BibEntry::isEmpty);
}
// Ignore empty entries
originalEntriesSorted.removeIf(BibEntry::isEmpty);
newEntriesSorted.removeIf(BibEntry::isEmpty);

entryDiffs = compareEntries(originalEntriesSorted, newEntriesSorted, originalDatabase.getMode());
return entryDiffs;
}

private static EntryComparator getEntryComparator() {
Expand All @@ -56,7 +76,7 @@ private static List<BibEntryDiff> compareEntries(List<BibEntry> originalEntries,

// Create a HashSet where we can put references to entries in the new
// database that we have matched. This is to avoid matching them twice.
Set<Integer> used = new HashSet<>(newEntries.size());
Set<Integer> matchedEntries = new HashSet<>(newEntries.size());
Set<BibEntry> notMatched = new HashSet<>(originalEntries.size());

// Loop through the entries of the original database, looking for exact matches in the new one.
Expand All @@ -65,10 +85,10 @@ private static List<BibEntryDiff> compareEntries(List<BibEntry> originalEntries,
mainLoop:
for (BibEntry originalEntry : originalEntries) {
for (int i = 0; i < newEntries.size(); i++) {
if (!used.contains(i)) {
if (!matchedEntries.contains(i)) {
double score = DuplicateCheck.compareEntriesStrictly(originalEntry, newEntries.get(i));
if (score > 1) {
used.add(i);
matchedEntries.add(i);
continue mainLoop;
}
}
Expand All @@ -85,7 +105,7 @@ private static List<BibEntryDiff> compareEntries(List<BibEntry> originalEntries,
double bestMatch = 0;
int bestMatchIndex = 0;
for (int i = 0; i < newEntries.size(); i++) {
if (!used.contains(i)) {
if (!matchedEntries.contains(i)) {
double score = DuplicateCheck.compareEntriesStrictly(originalEntry, newEntries.get(i));
if (score > bestMatch) {
bestMatch = score;
Expand All @@ -97,7 +117,7 @@ private static List<BibEntryDiff> compareEntries(List<BibEntry> originalEntries,
if (bestMatch > MATCH_THRESHOLD
|| hasEqualCitationKey(originalEntry, bestEntry)
|| duplicateCheck.isDuplicate(originalEntry, bestEntry, mode)) {
used.add(bestMatchIndex);
matchedEntries.add(bestMatchIndex);
differences.add(new BibEntryDiff(originalEntry, newEntries.get(bestMatchIndex)));
} else {
differences.add(new BibEntryDiff(originalEntry, null));
Expand All @@ -106,7 +126,7 @@ private static List<BibEntryDiff> compareEntries(List<BibEntry> originalEntries,

// Finally, look if there are still untouched entries in the new database. These may have been added.
for (int i = 0; i < newEntries.size(); i++) {
if (!used.contains(i)) {
if (!matchedEntries.contains(i)) {
differences.add(new BibEntryDiff(null, newEntries.get(i)));
}
}
Expand All @@ -119,7 +139,7 @@ private static boolean hasEqualCitationKey(BibEntry oneEntry, BibEntry twoEntry)
}

public static BibDatabaseDiff compare(BibDatabaseContext base, BibDatabaseContext changed) {
return new BibDatabaseDiff(base, changed, false);
return new BibDatabaseDiff(base, changed);
}

public Optional<MetaDataDiff> getMetaDataDifferences() {
Expand Down
25 changes: 11 additions & 14 deletions src/main/java/org/jabref/logic/bibtex/comparator/BibEntryDiff.java
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
package org.jabref.logic.bibtex.comparator;

import org.jabref.model.entry.BibEntry;

public class BibEntryDiff {
private final BibEntry originalEntry;
private final BibEntry newEntry;
import java.util.StringJoiner;

public BibEntryDiff(BibEntry originalEntry, BibEntry newEntry) {
this.originalEntry = originalEntry;
this.newEntry = newEntry;
}
import org.jabref.model.entry.BibEntry;

public BibEntry getOriginalEntry() {
return originalEntry;
}
public record BibEntryDiff(
BibEntry originalEntry,
BibEntry newEntry) {

public BibEntry getNewEntry() {
return newEntry;
@Override
public String toString() {
return new StringJoiner(",\n", BibEntryDiff.class.getSimpleName() + "[", "]")
.add("originalEntry=" + originalEntry)
.add("newEntry=" + newEntry)
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ private void addToListIfDiff(List<Difference> changes, DifferenceType difference
if (isDefaultContentSelectors(originalContentSelectors) && isDefaultContentSelectors(newContentSelectors)) {
return;
}
}
if (differenceType == DifferenceType.GROUPS) {
} else if (differenceType == DifferenceType.GROUPS) {
Optional<GroupTreeNode> originalGroups = (Optional<GroupTreeNode>) originalObject;
Optional<GroupTreeNode> newGroups = (Optional<GroupTreeNode>) newObject;
if (isDefaultGroup(originalGroups) && isDefaultGroup(newGroups)) {
Expand Down Expand Up @@ -137,6 +136,7 @@ public String toString() {
"groupDiff=" + groupDiff +
", originalMetaData=" + originalMetaData +
", newMetaData=" + getNewMetaData() +
", getDifferences()=" + getDifferences(new GlobalCitationKeyPatterns(CitationKeyPattern.NULL_CITATION_KEY_PATTERN)) +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,12 @@ public boolean equals(Object other) {
public int hashCode() {
return Objects.hash(originalPreamble, newPreamble);
}

@Override
public String toString() {
return "PreambleDiff{" +
"originalPreamble='" + originalPreamble + '\'' +
", newPreamble='" + newPreamble + '\'' +
'}';
}
}
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/model/entry/BibEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ public boolean equals(Object o) {
*/
@Override
public int hashCode() {
return Objects.hash(commentsBeforeEntry, type.getValue(), fields);
return Objects.hash(type.getValue(), fields, commentsBeforeEntry);
}

public void registerListener(Object object) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ void compareOfTwoDifferentEntriesWithDifferentDataReportsDifferences() throws Ex
// two different entries between the databases
assertEquals(2, diff.getEntryDifferences().size(), "incorrect amount of different entries");

assertEquals(entryOne, diff.getEntryDifferences().getFirst().getOriginalEntry(), "there is another value as originalEntry");
assertNull(diff.getEntryDifferences().getFirst().getNewEntry(), "newEntry is not null");
assertEquals(entryTwo, diff.getEntryDifferences().get(1).getNewEntry(), "there is another value as newEntry");
assertNull(diff.getEntryDifferences().get(1).getOriginalEntry(), "originalEntry is not null");
assertEquals(entryOne, diff.getEntryDifferences().getFirst().originalEntry(), "there is another value as originalEntry");
assertNull(diff.getEntryDifferences().getFirst().newEntry(), "newEntry is not null");
assertEquals(entryTwo, diff.getEntryDifferences().get(1).newEntry(), "there is another value as newEntry");
assertNull(diff.getEntryDifferences().get(1).originalEntry(), "originalEntry is not null");
}

@Test
Expand All @@ -104,12 +104,12 @@ void compareOfThreeDifferentEntriesWithDifferentDataReportsDifferences() throws
// three different entries between the databases
assertEquals(3, diff.getEntryDifferences().size(), "incorrect amount of different entries");

assertEquals(entryOne, diff.getEntryDifferences().getFirst().getOriginalEntry(), "there is another value as originalEntry");
assertNull(diff.getEntryDifferences().getFirst().getNewEntry(), "newEntry is not null");
assertEquals(entryTwo, diff.getEntryDifferences().get(1).getNewEntry(), "there is another value as newEntry");
assertNull(diff.getEntryDifferences().get(1).getOriginalEntry(), "originalEntry is not null");
assertEquals(entryThree, diff.getEntryDifferences().get(2).getNewEntry(), "there is another value as newEntry [2]");
assertNull(diff.getEntryDifferences().get(2).getOriginalEntry(), "originalEntry is not null [2]");
assertEquals(entryOne, diff.getEntryDifferences().getFirst().originalEntry(), "there is another value as originalEntry");
assertNull(diff.getEntryDifferences().getFirst().newEntry(), "newEntry is not null");
assertEquals(entryTwo, diff.getEntryDifferences().get(1).newEntry(), "there is another value as newEntry");
assertNull(diff.getEntryDifferences().get(1).originalEntry(), "originalEntry is not null");
assertEquals(entryThree, diff.getEntryDifferences().get(2).newEntry(), "there is another value as newEntry [2]");
assertNull(diff.getEntryDifferences().get(2).originalEntry(), "originalEntry is not null [2]");
}

@Test
Expand All @@ -130,8 +130,8 @@ void compareOfTwoEntriesWithEqualCitationKeysShouldReportsOneDifference() {
// two different entries between the databases
assertEquals(1, diff.getEntryDifferences().size(), "incorrect amount of different entries");

assertEquals(entryOne, diff.getEntryDifferences().getFirst().getOriginalEntry(), "there is another value as originalEntry");
assertEquals(entryTwo, diff.getEntryDifferences().getFirst().getNewEntry(), "there is another value as newEntry");
assertEquals(entryOne, diff.getEntryDifferences().getFirst().originalEntry(), "there is another value as originalEntry");
assertEquals(entryTwo, diff.getEntryDifferences().getFirst().newEntry(), "there is another value as newEntry");
}

private BibDatabaseDiff compareEntries(BibEntry entryOne, BibEntry entryTwo) {
Expand Down
Loading