Skip to content

Commit

Permalink
Fix DuplicationChecker and key generator (#2151)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chriba @koppor
chriba authored and koppor committed Oct 11, 2016
1 parent af71abf commit c1c8c76
Show file tree
Hide file tree
Showing 11 changed files with 239 additions and 247 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- Customized importer files need to be slightly changed since the class `ImportFormat` was renamed to `Importer`

### Fixed
- Fixed [#2089](https://github.com/JabRef/jabref/issues/2089): Fixed faulty cite key generation
- Fixed [#2092](https://github.com/JabRef/jabref/issues/2092): "None"-button in date picker clears the date field
- Fixed [#1993](https://github.com/JabRef/jabref/issues/1993): Various optimizations regarding search performance
- Fixed [koppor#160](https://github.com/koppor/jabref/issues/160): Tooltips now working in the main table
Expand Down
94 changes: 37 additions & 57 deletions src/main/java/net/sf/jabref/gui/BasePanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -113,6 +112,7 @@
import net.sf.jabref.logic.util.io.FileUtil;
import net.sf.jabref.logic.util.io.RegExpFileSearch;
import net.sf.jabref.model.FieldChange;
import net.sf.jabref.model.bibtexkeypattern.AbstractBibtexKeyPattern;
import net.sf.jabref.model.database.BibDatabase;
import net.sf.jabref.model.database.BibDatabaseContext;
import net.sf.jabref.model.database.DatabaseLocation;
Expand Down Expand Up @@ -435,65 +435,47 @@ public void init() {
// Run second, on a different thread:
@Override
public void run() {
BibEntry bes;

// First check if any entries have keys set already. If so, possibly remove
// them from consideration, or warn about overwriting keys.
// This is a partial clone of net.sf.jabref.gui.entryeditor.EntryEditor.GenerateKeyAction.actionPerformed(ActionEvent)
for (final Iterator<BibEntry> i = entries.iterator(); i.hasNext();) {
bes = i.next();
if (bes.hasCiteKey()) {
if (Globals.prefs.getBoolean(JabRefPreferences.AVOID_OVERWRITING_KEY)) {
// Remove the entry, because its key is already set:
i.remove();
} else if (Globals.prefs.getBoolean(JabRefPreferences.WARN_BEFORE_OVERWRITING_KEY)) {
// Ask if the user wants to cancel the operation:
CheckBoxMessage cbm = new CheckBoxMessage(
Localization.lang("One or more keys will be overwritten. Continue?"),
Localization.lang("Disable this confirmation dialog"), false);
final int answer = JOptionPane.showConfirmDialog(frame, cbm,
Localization.lang("Overwrite keys"), JOptionPane.YES_NO_OPTION);
if (cbm.isSelected()) {
Globals.prefs.putBoolean(JabRefPreferences.WARN_BEFORE_OVERWRITING_KEY, false);
}
if (answer == JOptionPane.NO_OPTION) {
// Ok, break off the operation.
canceled = true;
return;
}
// No need to check more entries, because the user has already confirmed
// that it's ok to overwrite keys:
break;
// We don't want to generate keys for entries which already have one thus remove the entries
if (Globals.prefs.getBoolean(JabRefPreferences.AVOID_OVERWRITING_KEY)) {
entries.removeIf(BibEntry::hasCiteKey);

// if we're going to override some cite keys warn the user about it
} else if (Globals.prefs.getBoolean(JabRefPreferences.WARN_BEFORE_OVERWRITING_KEY)) {
if (entries.parallelStream().anyMatch(BibEntry::hasCiteKey)) {
CheckBoxMessage cbm = new CheckBoxMessage(
Localization.lang("One or more keys will be overwritten. Continue?"),
Localization.lang("Disable this confirmation dialog"), false);
final int answer = JOptionPane.showConfirmDialog(frame, cbm,
Localization.lang("Overwrite keys"), JOptionPane.YES_NO_OPTION);
Globals.prefs.putBoolean(JabRefPreferences.WARN_BEFORE_OVERWRITING_KEY, !cbm.isSelected());

// The user doesn't want to overide cite keys
if (answer == JOptionPane.NO_OPTION) {
canceled = true;
return;
}
}
}

Map<BibEntry, String> oldvals = new HashMap<>();
// Iterate again, removing already set keys. This is skipped if overwriting
// is disabled, since all entries with keys set will have been removed.
if (!Globals.prefs.getBoolean(JabRefPreferences.AVOID_OVERWRITING_KEY)) {
for (BibEntry entry : entries) {
bes = entry;
// Store the old value:
oldvals.put(bes, bes.getCiteKeyOptional().orElse(null));
bes.clearCiteKey();
}
}

// generate the new cite keys for each entry
final NamedCompound ce = new NamedCompound(Localization.lang("Autogenerate BibTeX keys"));

// Finally, set the new keys:
AbstractBibtexKeyPattern citeKeyPattern = bibDatabaseContext.getMetaData()
.getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern());
for (BibEntry entry : entries) {
bes = entry;
BibtexKeyPatternUtil.makeLabel(bibDatabaseContext.getMetaData()
.getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
bibDatabaseContext.getDatabase(),
bes, Globals.prefs.getBibtexKeyPatternPreferences());
ce.addEdit(new UndoableKeyChange(bibDatabaseContext.getDatabase(), bes, oldvals.get(bes),
bes.getCiteKeyOptional().orElse(null)));
String oldCiteKey = entry.getCiteKeyOptional().orElse("");
BibtexKeyPatternUtil.makeLabel(citeKeyPattern, bibDatabaseContext.getDatabase(),
entry, Globals.prefs.getBibtexKeyPatternPreferences());
String newCiteKey = entry.getCiteKeyOptional().orElse("");
if (!oldCiteKey.equals(newCiteKey)) {
ce.addEdit(new UndoableKeyChange(entry, oldCiteKey, newCiteKey));
}
}
ce.end();
getUndoManager().addEdit(ce);

// register the undo event only if new cite keys were generated
if (ce.hasEdits()) {
getUndoManager().addEdit(ce);
}
}

// Run third, on EDT:
Expand Down Expand Up @@ -1929,7 +1911,6 @@ public boolean showDeleteConfirmationDialog(int numberOfEntries) {
public void autoGenerateKeysBeforeSaving() {
if (Globals.prefs.getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING)) {
NamedCompound ce = new NamedCompound(Localization.lang("Autogenerate BibTeX keys"));
boolean any = false;

for (BibEntry bes : bibDatabaseContext.getDatabase().getEntries()) {
Optional<String> oldKey = bes.getCiteKeyOptional();
Expand All @@ -1938,13 +1919,12 @@ public void autoGenerateKeysBeforeSaving() {
.getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
bibDatabaseContext.getDatabase(),
bes, Globals.prefs.getBibtexKeyPatternPreferences());
ce.addEdit(new UndoableKeyChange(bibDatabaseContext.getDatabase(), bes, null,
bes.getCiteKeyOptional().get())); // Cite key is set here
any = true;
ce.addEdit(new UndoableKeyChange(bes, oldKey.orElse(""), bes.getCiteKeyOptional().get()));
}
}

// Store undo information, if any:
if (any) {
if (ce.hasEdits()) {
ce.end();
getUndoManager().addEdit(ce);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/sf/jabref/gui/bibtexkeypattern/SearchFixDuplicateLabels.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void update() {
.getCiteKeyPattern(Globals.prefs.getBibtexKeyPatternPreferences().getKeyPattern()),
panel.getDatabase(), entry,
Globals.prefs.getBibtexKeyPatternPreferences());
ce.addEdit(new UndoableKeyChange(panel.getDatabase(), entry, oldKey, entry.getCiteKeyOptional().get()));
ce.addEdit(new UndoableKeyChange(entry, oldKey, entry.getCiteKeyOptional().get()));
}
ce.end();
panel.getUndoManager().addEdit(ce);
Expand Down
14 changes: 6 additions & 8 deletions src/main/java/net/sf/jabref/gui/entryeditor/EntryEditor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,12 +778,9 @@ private boolean storeSource() {
BibEntry newEntry = database.getEntries().get(0);
String newKey = newEntry.getCiteKeyOptional().orElse(null);
boolean entryChanged = false;
boolean duplicateWarning = false;
boolean emptyWarning = (newKey == null) || newKey.isEmpty();

if (panel.getDatabase().setCiteKeyForEntry(entry, newKey)) {
duplicateWarning = true;
}
entry.setCiteKey(newKey);

// First, remove fields that the user has removed.
for (Entry<String, String> field : entry.getFieldMap().entrySet()) {
Expand Down Expand Up @@ -828,7 +825,7 @@ private boolean storeSource() {

panel.getUndoManager().addEdit(compound);

if (duplicateWarning) {
if (panel.getDatabase().getDuplicationChecker().isDuplicateCiteKeyExisting(entry)) {
warnDuplicateBibtexkey();
} else if (emptyWarning) {
warnEmptyBibtexkey();
Expand Down Expand Up @@ -1108,11 +1105,12 @@ public void actionPerformed(ActionEvent event) {
return;
}

boolean isDuplicate = panel.getDatabase().setCiteKeyForEntry(entry, newValue);
entry.setCiteKey(newValue);

if (newValue == null) {
warnEmptyBibtexkey();
} else {
boolean isDuplicate = panel.getDatabase().getDuplicationChecker().isDuplicateCiteKeyExisting(entry);
if (isDuplicate) {
warnDuplicateBibtexkey();
} else {
Expand All @@ -1121,7 +1119,7 @@ public void actionPerformed(ActionEvent event) {
}

// Add an UndoableKeyChange to the baseframe's undoManager.
UndoableKeyChange undoableKeyChange = new UndoableKeyChange(panel.getDatabase(), entry, oldValue, newValue);
UndoableKeyChange undoableKeyChange = new UndoableKeyChange(entry, oldValue, newValue);
if (updateTimeStampIsSet()) {
NamedCompound ce = new NamedCompound(undoableKeyChange.getPresentationName());
ce.addEdit(undoableKeyChange);
Expand Down Expand Up @@ -1341,7 +1339,7 @@ public void actionPerformed(ActionEvent e) {

// Store undo information:
panel.getUndoManager().addEdit(
new UndoableKeyChange(panel.getDatabase(), entry, oldValue.orElse(null),
new UndoableKeyChange(entry, oldValue.orElse(null),
entry.getCiteKeyOptional().get())); // Cite key always set here

// here we update the field
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/sf/jabref/gui/openoffice/OpenOfficePanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ private boolean checkThatEntriesHaveKeys(List<BibEntry> entries) {
prefs);
// Add undo change
undoCompound.addEdit(
new UndoableKeyChange(panel.getDatabase(), entry, null, entry.getCiteKeyOptional().get()));
new UndoableKeyChange(entry, null, entry.getCiteKeyOptional().get()));
}
}
undoCompound.end();
Expand Down
19 changes: 3 additions & 16 deletions src/main/java/net/sf/jabref/gui/undo/UndoableKeyChange.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package net.sf.jabref.gui.undo;

import net.sf.jabref.logic.l10n.Localization;
import net.sf.jabref.model.database.BibDatabase;
import net.sf.jabref.model.entry.BibEntry;
import net.sf.jabref.model.strings.StringUtil;

Expand All @@ -13,14 +12,11 @@
public class UndoableKeyChange extends AbstractUndoableJabRefEdit {

private final BibEntry entry;
private final BibDatabase base;
private final String oldValue;
private final String newValue;


public UndoableKeyChange(BibDatabase base, BibEntry entry,
String oldValue, String newValue) {
this.base = base;
public UndoableKeyChange(BibEntry entry, String oldValue, String newValue) {
this.entry = entry;
this.oldValue = oldValue;
this.newValue = newValue;
Expand All @@ -31,27 +27,18 @@ public String getPresentationName() {
return Localization.lang("change key from %0 to %1",
StringUtil.boldHTML(oldValue, Localization.lang("undefined")),
StringUtil.boldHTML(newValue, Localization.lang("undefined")));

}

@Override
public void undo() {
super.undo();

// Revert the change.
set(oldValue);
entry.setCiteKey(oldValue);
}

@Override
public void redo() {
super.redo();

// Redo the change.
set(newValue);
}

private void set(String to) {
base.setCiteKeyForEntry(entry, to);
entry.setCiteKey(newValue);
}

}
44 changes: 10 additions & 34 deletions src/main/java/net/sf/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ public static void makeLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabas
}

String oldKey = entry.getCiteKeyOptional().orElse(null);
int occurrences = database.getNumberOfKeyOccurrences(key);
int occurrences = database.getDuplicationChecker().getNumberOfKeyOccurrences(key);

if (Objects.equals(oldKey, key)) {
occurrences--; // No change, so we can accept one dupe.
Expand All @@ -439,48 +439,24 @@ public static void makeLabel(AbstractBibtexKeyPattern citeKeyPattern, BibDatabas
boolean firstLetterA = bibtexKeyPatternPreferences.isFirstLetterA();

if (!alwaysAddLetter && (occurrences == 0)) {
// No dupes found, so we can just go ahead.
if (!key.equals(oldKey)) {
if (database.containsEntryWithId(entry.getId())) {
database.setCiteKeyForEntry(entry, key);
} else {
// entry does not (yet) exist in the database, just update the entry
entry.setCiteKey(key);
}
}

entry.setCiteKey(key);
} else {
// The key is already in use, so we must modify it.
int number = 0;
if (!alwaysAddLetter && !firstLetterA) {
number = 1;
}

String moddedKey = key + getAddition(number);
occurrences = database.getNumberOfKeyOccurrences(moddedKey);

if (Objects.equals(oldKey, moddedKey)) {
occurrences--;
}
int number = !alwaysAddLetter && !firstLetterA ? 1 : 0;
String moddedKey;

while (occurrences > 0) {
number++;
do {
moddedKey = key + getAddition(number);
number++;

occurrences = database.getNumberOfKeyOccurrences(moddedKey);
occurrences = database.getDuplicationChecker().getNumberOfKeyOccurrences(moddedKey);
// only happens if #getAddition() is buggy
if (Objects.equals(oldKey, moddedKey)) {
occurrences--;
}
}
} while (occurrences > 0);

if (!moddedKey.equals(oldKey)) {
if (database.containsEntryWithId(entry.getId())) {
database.setCiteKeyForEntry(entry, moddedKey);
} else {
// entry does not (yet) exist in the database, just update the entry
entry.setCiteKey(moddedKey);
}
}
entry.setCiteKey(moddedKey);
}
}

Expand Down
Loading

0 comments on commit c1c8c76

Please sign in to comment.