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

Refactor EntryEvents - removal part #5410

Merged
merged 36 commits into from
Nov 11, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
80e8377
Initial changes to remove entry functionality
abepolk Oct 8, 2019
48cb7fd
Fix logic error in KeyChangeListener
abepolk Oct 8, 2019
923b40e
Fix logic error in BasePanel
abepolk Oct 8, 2019
a0ffcb5
Changes to tests
abepolk Oct 9, 2019
b08ae88
Fix typo
abepolk Oct 9, 2019
3a830a8
Move remove entry calls to batch version
abepolk Oct 11, 2019
4e57a46
Un-propagate for loop in KeyChangeListener
abepolk Oct 15, 2019
b990e0d
Finalize changes to SharedEntriesNotPresentEvent
abepolk Oct 15, 2019
3122e67
Fix bug
abepolk Oct 15, 2019
aac840c
Fix other compile errors
abepolk Oct 15, 2019
90c27f1
Fix bug and update tests
abepolk Oct 20, 2019
a0cafc4
Fix tests
abepolk Oct 20, 2019
868deed
Merge master
abepolk Oct 20, 2019
2afb868
Fix test omission
abepolk Oct 20, 2019
460f44a
Update l10n
abepolk Oct 20, 2019
f11389e
For loop to citationStyle
abepolk Oct 20, 2019
f535a0b
Add comment for method not working
abepolk Oct 23, 2019
77ff450
Clarify var name
abepolk Oct 23, 2019
b0805cc
Allow single entry for undo
abepolk Oct 23, 2019
7ac7a1c
Replace compound edit undo with normal undo in BasePanel
abepolk Oct 24, 2019
8255ad2
Typo
abepolk Oct 25, 2019
3b8dfea
Simplify loop in DBMSSynchronizer
abepolk Oct 27, 2019
5f1b811
Use if instead of stream
abepolk Oct 28, 2019
c41dc17
Pluralize Javadoc
abepolk Oct 30, 2019
bb88484
EntryEvent -> EntriesEvent in Javadoc, comments, and var names
abepolk Oct 30, 2019
4c97c64
Make imports explicit in BasePanel
abepolk Oct 30, 2019
30d8d51
Merge master
abepolk Oct 30, 2019
5cf5965
Final EntriesRemovedEvent fixes
abepolk Nov 1, 2019
1df2869
Fix checkStyleMain
abepolk Nov 3, 2019
0f5d76f
Merge branch 'master' into refactor_group_entryevents
abepolk Nov 3, 2019
13abf0c
More checkStyle fixes
abepolk Nov 3, 2019
17f04b0
More fixes
abepolk Nov 10, 2019
214e958
Move List coercion into DuplicateSearchResult method
abepolk Nov 10, 2019
a90c44e
Fix typo
abepolk Nov 11, 2019
648c5da
Fix pesky BibDatabaseTest error with setStrings
abepolk Nov 11, 2019
b0f585a
Fixed BibDatabase Javadoc
abepolk Nov 11, 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
26 changes: 10 additions & 16 deletions src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@
import org.jabref.gui.specialfields.SpecialFieldDatabaseChangeListener;
import org.jabref.gui.specialfields.SpecialFieldValueViewModel;
import org.jabref.gui.specialfields.SpecialFieldViewModel;
import org.jabref.gui.undo.CountingUndoManager;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableFieldChange;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableRemoveEntry;
import org.jabref.gui.undo.*;
abepolk marked this conversation as resolved.
Show resolved Hide resolved
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.worker.SendAsEMailAction;
import org.jabref.logic.citationstyle.CitationStyleCache;
Expand All @@ -80,8 +76,8 @@
import org.jabref.model.database.KeyCollisionException;
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
import org.jabref.model.database.event.CoarseChangeFilter;
import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.database.event.EntryAddedEvent;
import org.jabref.model.database.event.EntryRemovedEvent;
import org.jabref.model.database.shared.DatabaseLocation;
import org.jabref.model.database.shared.DatabaseSynchronizer;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -409,11 +405,9 @@ private void delete(boolean cut, List<BibEntry> entries) {
} else {
compound = new NamedCompound((entries.size() > 1 ? Localization.lang("delete entries") : Localization.lang("delete entry")));
}
for (BibEntry entry : entries) {
compound.addEdit(new UndoableRemoveEntry(bibDatabaseContext.getDatabase(), entry));
bibDatabaseContext.getDatabase().removeEntry(entry);
ensureNotShowingBottomPanel(entry);
}
compound.addEdit(new UndoableRemoveEntries(bibDatabaseContext.getDatabase(), entries));
bibDatabaseContext.getDatabase().removeEntries(entries);
ensureNotShowingBottomPanel(entries);
compound.end();
getUndoManager().addEdit(compound);

Expand Down Expand Up @@ -886,8 +880,8 @@ public void entryEditorClosing() {
/**
* Closes the entry editor if it is showing the given entry.
*/
private void ensureNotShowingBottomPanel(BibEntry entry) {
if (((mode == BasePanelMode.SHOWING_EDITOR) && (entryEditor.getEntry() == entry))) {
private void ensureNotShowingBottomPanel(List<BibEntry> entriesToCheck) {
if ((mode == BasePanelMode.SHOWING_EDITOR) && (entriesToCheck.contains(entryEditor.getEntry()))) {
closeBottomPane();
}
}
Expand Down Expand Up @@ -1145,8 +1139,8 @@ public void listen(EntryAddedEvent addedEntryEvent) {
private class EntryRemovedListener {

@Subscribe
public void listen(EntryRemovedEvent entryRemovedEvent) {
ensureNotShowingBottomPanel(entryRemovedEvent.getBibEntry());
public void listen(EntriesRemovedEvent entriesRemovedEvent) {
ensureNotShowingBottomPanel(entriesRemovedEvent.getBibEntries());
}
}

Expand Down Expand Up @@ -1184,7 +1178,7 @@ public void listen(EntryChangedEvent entryChangedEvent) {
}

@Subscribe
public void listen(EntryRemovedEvent removedEntryEvent) {
public void listen(EntriesRemovedEvent removedEntriesEvent) {
// IMO only used to update the status (found X entries)
DefaultTaskExecutor.runInJavaFXThread(() -> frame.getGlobalSearchBar().performSearch());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,43 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.List;

/**
* This class represents the removal of an entry. The constructor needs
* references to the database, the entry, and the map of open entry editors.
* The latter to be able to close the entry's editor if it is opened after
* an undo, and the removal is then undone.
*/
public class UndoableRemoveEntry extends AbstractUndoableJabRefEdit {
public class UndoableRemoveEntries extends AbstractUndoableJabRefEdit {

private static final Logger LOGGER = LoggerFactory.getLogger(UndoableRemoveEntry.class);
private final BibDatabase base;
private final BibEntry entry;
private final List<BibEntry> entries;

public UndoableRemoveEntry(BibDatabase base, BibEntry entry) {
public UndoableRemoveEntries(BibDatabase base, List<BibEntry> entries) {
this.base = base;
this.entry = entry;
this.entries = entries;
}

@Override
public String getPresentationName() {
return Localization.lang("remove entry %0",
StringUtil.boldHTML(entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
if (entries.size() > 1) {
return Localization.lang("remove entries");
}
else if (entries.size() == 1) {
return Localization.lang("remove entry %0",
StringUtil.boldHTML(entries.get(0).getCiteKeyOptional().orElse(Localization.lang("undefined"))));
}
else {
return null;
}
}

@Override
public void undo() {
super.undo();
base.insertEntry(entry, EntryEventSource.UNDO);
base.insertEntries(entries, EntryEventSource.UNDO);
}

@Override
Expand All @@ -44,7 +54,7 @@ public void redo() {

// Redo the change.
try {
base.removeEntry(entry);
base.removeEntries(entries);
} catch (Throwable ex) {
LOGGER.warn("Problem to redo `remove entry`", ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import java.util.Objects;

import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.EntryRemovedEvent;
import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.event.EntryChangedEvent;

Expand Down Expand Up @@ -65,8 +65,8 @@ public void listen(EntryChangedEvent entryChangedEvent) {
* removes the citation of the removed entry as it's not needed anymore
*/
@Subscribe
public void listen(EntryRemovedEvent entryRemovedEvent) {
citationStyleCache.invalidate(entryRemovedEvent.getBibEntry());
public void listen(EntriesRemovedEvent entriesRemovedEvent) {
abepolk marked this conversation as resolved.
Show resolved Hide resolved
citationStyleCache.invalidateAll(entriesRemovedEvent.getBibEntries());
abepolk marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
14 changes: 10 additions & 4 deletions src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@
import org.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.database.event.EntryAddedEvent;
import org.jabref.model.database.event.EntryRemovedEvent;
import org.jabref.model.database.shared.DatabaseConnection;
import org.jabref.model.database.shared.DatabaseConnectionProperties;
import org.jabref.model.database.shared.DatabaseNotSupportedException;
import org.jabref.model.database.shared.DatabaseSynchronizer;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.event.EntriesEvent;
import org.jabref.model.entry.event.EntryEvent;
import org.jabref.model.entry.event.EntryEventSource;
import org.jabref.model.entry.event.FieldChangedEvent;
Expand Down Expand Up @@ -108,12 +109,17 @@ public void listen(FieldChangedEvent event) {
*
* @param event {@link EntryRemovedEvent} object
*/

// This has not been made parellel yet - hence the for loop - that will take more effort
@Subscribe
public void listen(EntryRemovedEvent event) {
public void listen(EntriesRemovedEvent event) {
// While synchronizing the local database (see synchronizeLocalDatabase() below), some EntryEvents may be posted.
// In this case DBSynchronizer should not try to delete the bibEntry entry again (but it would not harm).
if (isEventSourceAccepted(event) && checkCurrentConnection()) {
dbmsProcessor.removeEntry(event.getBibEntry());
List<BibEntry> entries = event.getBibEntries();
for (BibEntry entry : entries) {
dbmsProcessor.removeEntry(entry);
}
synchronizeLocalMetaData();
synchronizeLocalDatabase(); // Pull changes for the case that there where some
}
Expand Down Expand Up @@ -350,7 +356,7 @@ public boolean checkCurrentConnection() {
* @param event An {@link EntryEvent}
* @return <code>true</code> if the event is able to trigger operations in {@link DBMSSynchronizer}, else <code>false</code>
*/
public boolean isEventSourceAccepted(EntryEvent event) {
public boolean isEventSourceAccepted(EntriesEvent event) {
EntryEventSource eventSource = event.getEntryEventSource();
return ((eventSource == EntryEventSource.LOCAL) || (eventSource == EntryEventSource.UNDO));
}
Expand Down
21 changes: 13 additions & 8 deletions src/main/java/org/jabref/model/database/BibDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;

import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.database.event.EntryAddedEvent;
import org.jabref.model.database.event.EntryRemovedEvent;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.BibtexString;
import org.jabref.model.entry.Month;
Expand Down Expand Up @@ -216,7 +216,7 @@ public synchronized void insertEntries(List<BibEntry> entries) throws KeyCollisi
insertEntries(entries, EntryEventSource.LOCAL);
}

private synchronized void insertEntries(List<BibEntry> newEntries, EntryEventSource eventSource) throws KeyCollisionException {
public synchronized void insertEntries(List<BibEntry> newEntries, EntryEventSource eventSource) throws KeyCollisionException {
Objects.requireNonNull(newEntries);

for (BibEntry entry : newEntries) {
Expand All @@ -238,8 +238,8 @@ private synchronized void insertEntries(List<BibEntry> newEntries, EntryEventSou
* The Entry is removed based on the id {@link BibEntry#id}
* @param toBeDeleted Entry to delete
*/
public synchronized void removeEntry(BibEntry toBeDeleted) {
removeEntry(toBeDeleted, EntryEventSource.LOCAL);
public synchronized void removeEntries(List<BibEntry> toBeDeleted) {
removeEntries(toBeDeleted, EntryEventSource.LOCAL);
}

/**
Expand All @@ -249,13 +249,18 @@ public synchronized void removeEntry(BibEntry toBeDeleted) {
* @param toBeDeleted Entry to delete
* @param eventSource Source the event is sent from
*/
public synchronized void removeEntry(BibEntry toBeDeleted, EntryEventSource eventSource) {
public synchronized void removeEntries(List<BibEntry> toBeDeleted, EntryEventSource eventSource) {
Objects.requireNonNull(toBeDeleted);
//Require not empty?
abepolk marked this conversation as resolved.
Show resolved Hide resolved

boolean anyRemoved = entries.removeIf(entry -> entry.getId().equals(toBeDeleted.getId()));
List<String> ids = new ArrayList<>();
for (BibEntry entry : toBeDeleted) {
ids.add(entry.getId());
}
boolean anyRemoved = entries.removeIf(entry -> ids.contains(entry.getId()));
if (anyRemoved) {
internalIDs.remove(toBeDeleted.getId());
eventBus.post(new EntryRemovedEvent(toBeDeleted, eventSource));
internalIDs.removeAll(ids);
eventBus.post(new EntriesRemovedEvent(toBeDeleted, eventSource));
}
}

Expand Down
12 changes: 7 additions & 5 deletions src/main/java/org/jabref/model/database/DuplicationChecker.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package org.jabref.model.database;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.database.event.EntryAddedEvent;
import org.jabref.model.database.event.EntryRemovedEvent;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.event.FieldChangedEvent;
import org.jabref.model.entry.field.InternalField;
Expand Down Expand Up @@ -92,10 +93,11 @@ public void listen(FieldChangedEvent fieldChangedEvent) {
}

@Subscribe
public void listen(EntryRemovedEvent entryRemovedEvent) {
Optional<String> citeKey = entryRemovedEvent.getBibEntry().getCiteKeyOptional();
if (citeKey.isPresent()) {
removeKeyFromSet(citeKey.get());
public void listen(EntriesRemovedEvent entriesRemovedEvent) {
//This should be done differently, maybe with a stream
List<BibEntry> entries = entriesRemovedEvent.getBibEntries();
for (BibEntry entry : entries) {
entry.getCiteKeyOptional().ifPresent(this::removeKeyFromSet);
}
}

Expand Down
45 changes: 29 additions & 16 deletions src/main/java/org/jabref/model/database/KeyChangeListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;

import org.jabref.model.database.event.EntryRemovedEvent;
import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.event.FieldChangedEvent;
import org.jabref.model.entry.field.Field;
Expand Down Expand Up @@ -32,39 +33,51 @@ public void listen(FieldChangedEvent event) {
}

@Subscribe
public void listen(EntryRemovedEvent event) {
event.getBibEntry().getCiteKeyOptional().ifPresent(oldKey -> updateEntryLinks(null, oldKey));
public void listen(EntriesRemovedEvent event) {
List<BibEntry> entries = event.getBibEntries();
List<String> oldKeys = new ArrayList<>();
for (BibEntry entry : entries) {
Optional<String> citeKey = entry.getCiteKeyOptional();
if (citeKey.isPresent()) {
oldKeys.add(citeKey);
}
}
updateEntryLinks(null, oldKeys);
}

private void updateEntryLinks(String newKey, String oldKey) {
private void updateEntryLinks(String newKey, List<String> oldKeys) {
for (BibEntry entry : database.getEntries()) {
for (Field field : FieldFactory.getKeyFields()) {
entry.getField(field).ifPresent(fieldContent -> {
if (field.getProperties().contains(FieldProperty.SINGLE_ENTRY_LINK)) {
replaceSingleKeyInField(newKey, oldKey, entry, field, fieldContent);
replaceSingleKeyInField(newKey, oldKeys, entry, field, fieldContent);
} else { // MULTIPLE_ENTRY_LINK
replaceKeyInMultiplesKeyField(newKey, oldKey, entry, field, fieldContent);
replaceKeyInMultiplesKeyField(newKey, oldKeys, entry, field, fieldContent);
}
});
}
}
}

private void replaceKeyInMultiplesKeyField(String newKey, String oldKey, BibEntry entry, Field field, String fieldContent) {
// These methods may have to be renamed
private void replaceKeyInMultiplesKeyField(String newKey, List<String> oldKeys, BibEntry entry, Field field, String fieldContent) {
List<String> keys = new ArrayList<>(Arrays.asList(fieldContent.split(",")));
int index = keys.indexOf(oldKey);
if (index != -1) {
if (newKey == null) {
keys.remove(index);
} else {
keys.set(index, newKey);
for (String key : oldKeys) {
int index = keys.indexOf(key);
if (index != -1) {
if (newKey == null) {
keys.remove(index);
} else {
keys.set(index, newKey);
}
entry.setField(field, String.join(",", keys));
}
entry.setField(field, String.join(",", keys));
}
}

private void replaceSingleKeyInField(String newKey, String oldKey, BibEntry entry, Field field, String fieldContent) {
if (fieldContent.equals(oldKey)) {
private void replaceSingleKeyInField(String newKey, List<String> oldKeys, BibEntry entry, Field field, String fieldContent) {
int index = oldKeys.indexOf(fieldContent);
if (index != -1) {
if (newKey == null) {
entry.clearField(field);
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,32 @@
package org.jabref.model.database.event;

import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.event.EntriesEvent;
import org.jabref.model.entry.event.EntryEvent;
import org.jabref.model.entry.event.EntryEventSource;

import java.util.List;

/**
* <code>RemovedEntryEvent</code> is fired when a <code>BibEntry</code> was removed
* from the database.
*/

public class EntryRemovedEvent extends EntryEvent {
public class EntriesRemovedEvent extends EntriesEvent {

/**
* @param bibEntry <code>BibEntry</code> object which has been removed.
*/
public EntryRemovedEvent(BibEntry bibEntry) {
super(bibEntry);
public EntriesRemovedEvent(List<BibEntry> bibEntries) {
super(bibEntries);
}

/**
* @param bibEntry <code>BibEntry</code> object which has been removed.
* @param location Location affected by this event
*/
public EntryRemovedEvent(BibEntry bibEntry, EntryEventSource location) {
super(bibEntry, location);
public EntriesRemovedEvent(List<BibEntry> bibEntries, EntryEventSource location) {
super(bibEntries, location);
}

}
Loading