Shared database synchronized by FocusChangedEvent by m-mauersberger · Pull Request #6771 · JabRef/jabref

m-mauersberger · 2020-08-19T17:49:54Z

This PR is related to #6663 and partly #4461.

Shared database synchronization event in DBMSSynchronizer is triggered by a FocusChangedEvent. This is added similar to FieldChangedEvent but has to be initiated from EntryEditor (?).

I have manually tested the new code - and it does not work. Maybe we can discuss changes through this pull request.

Thank you for your help.

…lled. Focus is changed in EntryEditor. CoarseChangeFilter with no special function -> does not work with shared databases.

Siedlerchr · 2020-08-22T19:08:24Z

+     * @param event {@link FocusChangedEvent} object
+     */
+    @Subscribe
+    public void listen(FocusChangedEvent event) {


I think the problem is that you are listening to an Event from the EventBus which is nowhere fired

Siedlerchr · 2020-08-22T19:09:10Z


-            if (fieldChange.getDelta() > 1 || isEditOnNewField) {
+            if (isEditOnNewField) {
                lastFieldChanged = fieldChange.getField();


See the eventBus.post method, the eventBus post method is reposinsible for firing the event.

m-mauersberger · 2020-08-22T19:29:27Z

I tried firing the event via the BibEntry method setFocusedField. Again, this method is called by EntryEditor via setFocusToField.
But I don't know if this is generally possible.

Perhaps the second commit is a bit irritating, it is only uncommenting something in DBMSSynchronizer... Sorry for that.
The important commit is 87adec3b.

Siedlerchr · 2020-08-22T20:09:26Z

It's a bit complicated with all those listenes.
The EventBus is a local object. In fact it's similar to the classical EventLIstener in GUI. An even its relayed to it's parend until it's handled. The only difference is that we do this relaying here manually.
When you add a new Event in BibEntry you have to relay that to the call chain.

jabref/src/main/java/org/jabref/model/database/BibDatabaseContext.java

Lines 196 to 202 in 3709924

    
           public void convertToSharedDatabase(DatabaseSynchronizer dmbsSynchronizer) { 
        
               this.dbmsSynchronizer = dmbsSynchronizer; 
        
               this.dbmsListener = new CoarseChangeFilter(this); 
        
               dbmsListener.registerListener(dbmsSynchronizer); 
        
               this.location = DatabaseLocation.SHARED;

And the CoraseChangeFilter register itself here:

jabref/src/main/java/org/jabref/model/database/event/CoarseChangeFilter.java

Lines 19 to 23 in 4b39b78

    
           public CoarseChangeFilter(BibDatabaseContext bibDatabaseContext) { 
        
               // Listen for change events 
        
               bibDatabaseContext.getDatabase().registerListener(this); 
        
               bibDatabaseContext.getMetaData().registerListener(this); 
        
               this.context = bibDatabaseContext;

it registers itselt as a listener on BibDAtabase.
And BibDatabase finally register itself on each entry:

jabref/src/main/java/org/jabref/model/database/BibDatabase.java

Lines 198 to 206 in 9d17101

    
           public synchronized void insertEntries(List<BibEntry> newEntries, EntriesEventSource eventSource) { 
        
               Objects.requireNonNull(newEntries); 
        
               for (BibEntry entry : newEntries) { 
        
                   entry.registerListener(this); 
        
               } 
        
               if (newEntries.isEmpty()) { 
        
                   eventBus.post(new EntriesAddedEvent(newEntries, eventSource)); 
        
               } else { 
        
                   eventBus.post(new EntriesAddedEvent(newEntries, newEntries.get(0), eventSource));

So you need to listen for the FocusChangedEvent and relay that

tobiasdiez

Thanks a lot for investigating this issue.

You already made a good analysis of the underlying problem in #6663. Your solution using a new variable tracking the focused field in the BibEntry, has sadly the disadvantage that the data object (the BibEntry) is tightly coupled to the UI state (focused field). That's something we try to avoid as much as possible. So we need to look for a solution that doesn't couple the model and gui. In the end, the problem comes from the fact that this if statement

jabref/src/main/java/org/jabref/model/database/event/CoarseChangeFilter.java

Lines 35 to 38 in 22b1700

    
           if (fieldChange.getDelta() > 1 || isEditOnNewField) { 
        
               lastFieldChanged = fieldChange.getField(); 
        
               eventBus.post(event); 
        
           }

sometimes prevent events from bubbling at all. I think, it would suffice to put an else statement there, which pushes the event say after 3 seconds. If a new small incremental event occurs, then the timer is just restarted. In this way, all events eventually trigger an update to the database. We actually have a class that should be able to do this: https://github.com/JabRef/jabref/blob/6319d05901266f98ac3163a7e66d7f0af72217d6/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java

Can you try to implement something along these lines? That would be great!

Introduced DelayTaskThrottler on CoarseChangeFilter.

m-mauersberger · 2020-08-25T07:34:23Z

Thank you for your help. Finally, I deleted all FocusEventChanged-related stuff and concentrated on CoarseChangeFilter.
There I introduced a DelayTaskThrottler for the eventBus.post(event). Also I revised the conditionals while introducing three conditions:

isNewEdit: Editing right after loading shared database.
isEditOnOtherField: Editing other field = changes on previous field should be synchronized.
isMajorChange: Some editing with much character input.

I found out that a throttled task will not interrupt any input. That means if a FieldChangedEvent is posted with delay, you can write further until database synchronization appears. In the worst case you lose some information because all characters typed in after the event are not synchronized. Furthermore, it appears that when the event is posted during delay JabRef hangs up...

Can you reproduce this problems? In general, it works as intended.

Another idea: What about throttling all FieldChangeEvents in order to not interrupt input process? Then we would have to ensure that all pending synchronization events would be ready before closing the database. We could add an indicator in the GUI for this purpose or add something on closing-database event.

koppor · 2020-08-27T21:50:33Z

Side note: We have an internal discussion ongoing, why not using JavaFX Observables everywhere - instead of the event bus. Not fully checked all aspects until now. Will (hopefully) be done at JabCon. Do you have a "feeling" on that?

m-mauersberger · 2020-08-31T08:29:46Z

I have not much experience with JavaFX Observables but it seems like they are directly bound to GUI elements.
Thus, events maybe could only be fired less in accordance to the GUI... On the other hand it would be an advantage to use GUI events from everywhere with little effort.
I think the CoarseChangeFilter would become obsolete, wouldn't it?
So why not giving Observables a try...

tobiasdiez · 2020-09-01T17:50:08Z

Thanks a lot @m-mauersberger! I've fixed a few very minor things that I noticed and will merge now. Looking forward to your next PR.

* upstream/master: Feature/add ui for query parsing (#6805) Shared database synchronized by FocusChangedEvent (#6771)

m-mauersberger added 2 commits August 19, 2020 14:18

Added FocusChangedEvent, fired in BibEntry when setFocusedField is ca…

87adec3

…lled. Focus is changed in EntryEditor. CoarseChangeFilter with no special function -> does not work with shared databases.

Uncomment FieldChangedEvent listener.

0c895c1

m-mauersberger mentioned this pull request Aug 19, 2020

Shared PostgreSQL database not fully syncing user inputs #6663

Closed

Siedlerchr reviewed Aug 22, 2020

View reviewed changes

tobiasdiez requested changes Aug 23, 2020

View reviewed changes

Removed FocusChangedEvent.java due to improper dependencies.

c53ed56

Introduced DelayTaskThrottler on CoarseChangeFilter.

Delete FocusChangedEvent.java

079f913

tobiasdiez added 4 commits September 1, 2020 18:02

Update EntryEditor.java

a680ecb

Update CoarseChangeFilter.java

346d85a

Update CoarseChangeFilter.java

2fb31f7

Update CoarseChangeFilter.java

b76e7f2

tobiasdiez merged commit c74c557 into JabRef:master Sep 1, 2020

Siedlerchr added a commit that referenced this pull request Sep 1, 2020

Merge remote-tracking branch 'upstream/master' into writebackupToTemp

350f3e9

* upstream/master: Feature/add ui for query parsing (#6805) Shared database synchronized by FocusChangedEvent (#6771)

m-mauersberger deleted the request-focus branch September 3, 2020 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Shared database synchronized by FocusChangedEvent#6771

Shared database synchronized by FocusChangedEvent#6771
tobiasdiez merged 8 commits into
JabRef:masterfrom
m-mauersberger:request-focus

m-mauersberger commented Aug 19, 2020

Uh oh!

Siedlerchr Aug 22, 2020

Uh oh!

Siedlerchr Aug 22, 2020

Uh oh!

m-mauersberger commented Aug 22, 2020

Uh oh!

Siedlerchr commented Aug 22, 2020 •

edited

Loading

Uh oh!

tobiasdiez left a comment

Uh oh!

m-mauersberger commented Aug 25, 2020

Uh oh!

koppor commented Aug 27, 2020

Uh oh!

m-mauersberger commented Aug 31, 2020

Uh oh!

tobiasdiez commented Sep 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	if (fieldChange.getDelta() > 1 \|\| isEditOnNewField) {
	lastFieldChanged = fieldChange.getField();
	eventBus.post(event);
	}

Uh oh!

Conversation

m-mauersberger commented Aug 19, 2020

Uh oh!

Siedlerchr Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

Siedlerchr Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

m-mauersberger commented Aug 22, 2020

Uh oh!

Siedlerchr commented Aug 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

m-mauersberger commented Aug 25, 2020

Uh oh!

koppor commented Aug 27, 2020

Uh oh!

m-mauersberger commented Aug 31, 2020

Uh oh!

tobiasdiez commented Sep 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Siedlerchr commented Aug 22, 2020 •

edited

Loading