Fixes Shared Database: Changes filtering in CoarseChangeFilter to attribute property

[m-mauersberger]

WIth further investigations on the already closed issue #6663 I changed the function of CoarseChangeFilter.java. Following problems I encountered with the actual version:

• The database sometimes hangs up when an actual event (directly via eventBus.post) triggers database synchronization at the same time as a delayed event (delayed via delayPost.schedule). Apparently, this should be avoided.
• Delaying an event post via DelayTaskThrottler causes the subsequent actions on event listening (e.g. in DBMSSynchronizer) to run on a new thread concurrently. If I have a large shared library (~5600 entries), it takes some seconds to pull changes. A delayed event that induces database synchronization ignores all field changes within synchronization time. These are then lost.
• There is no other option fetching non-saved field changes because these events are completely filtered out. DBMSSynchronizer does not see them at all.

My new proposal includes an additional attribute filteredOut in BibDatabaseContextChangedEvent. You can set it if the event may be filtered out by a listening method. The corresponding constructor has to be called in every child class (e.g. MetaDataChangedEvent or EntriesEvent). The according functionality has been updated in AutosaveManager and BackupManager.

How can the remaining field changes be saved that are marked by filteredOut == true? I tried to introduce a lastEntryChanged attribute in DBMSSynchronizer that indicate which BibEntry these unsaved changes are bound to. At every event that has been listened to last entry changes are synchronized via pullWithLastEntry. Finally, unsaved field changes are also caught at closing the shared database. So DBMSSynchronizer.closeSharedDatabase calls pullLastEntryChanges. This method only synchronizes the database if unsaved field changes still remain.

Maybe you could look at the proposals. The main reason for that changes is that delayed event post does not work with synchronization of large shared databases.

PS: DelayTaskThrottler has been extended by a method to immediately run a previously scheduled task. This had be done in accordance to the possibility of the executor service ScheduledThreadPoolExecutor.

• Change in CHANGELOG.md described (if applicable)
• Tests created for changes (if applicable)
• Manually tested changed features in running JabRef (always required)
• Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

[koppor]

I can follow your line of argumentation.

The comments should go to a new file remote-storage.md in https://github.com/JabRef/jabref/tree/master/docs/advanced-reading.

Can you please also look at the failing tests (checkstyle and database).

I am not sure whether following failures are related to your changes - or whether you can just fix them :)

Another thing: Is the magic number 30 covered by any test case or can you think of a test setup of an automated test?

[m-mauersberger]

Thank you very much for review!

I tried to get in line with your change requests changing some lines in CoarseChangeFilter.java and DBMSSynchronizer.java.

• In CoarseChangeFilter I had a misinterpretation of major changes. Now I restored the value 1 as before Shared PostgreSQL database not fully syncing user inputs #6663 - and I understood that it was meant to represent pasting and deleting text implicitly. After some manual tests I added a check whether a new BibEntry is edited. If you edited the same field on another entry, there was no change recognized at all.

• In DBMSSynchronizer I added pullWithLastEntry to every entry-related event in order to consider non-saved changes in fields before adding or removing another entry. I must admit that I do not fully understand why new entries are added after (listen(EntriesAddedEvent event))while entries are removed before metadata and database synchronization (listen(EntriesRemovedEvent event)). Furthermore, lastEntryChanged is assigned as non-empty after adding an entry although it was empty before (ensured by pullWithLastEntry). Maybe some FieldChangedEvents are posted after adding an entry?

Unfortunately I could not resolve the failing database tests. SharedEntriesNotPresentEvents and UpdateRefusedEvents are fired when synchronizing database or accordingly shared entries. Maybe it has something to do with the correct order of synchronization commands (as mentioned above)?

Following manual tests worked when using the database for myself:

Proper synchronization after:

• Add entry.
• Remove entry.
• Add entry after typing something without synchronization.
• Remove entry after typing something without synchronization.
• Typing on the same entry in another field.
• Typing on another entry in the same field.
• Typing on another entry in another field.
• Pasting something more than 1 character.

I would be glad if you could help me fix the remaining failed tests. Maybe you have an idea about that.

[koppor]

Thank you for the work. This is a huge step forward. Thank you for documenting your tests. I hope, we can inspect why test cases fail.