Implementation of shared database support #1451

obraliar · 2016-05-26T03:25:22Z

Implementation of shared database support for MySQL, PostgreSQL and Oracle database systems.

Made changes:

Old sqlpackage removed. (This request aims to replace the old system completely)
Automatic change push to the server via EventBus. (Support for all kinds of EntryEvents, see AddEventSystem #1028)
Full synchronization support for MySQL, PostgreSQL and Oracle
(manual and semi-automated pull)
Conflict avoidance by version control and Optimistic Offline Lock
Efficient pull using version numbers
Synchronization of metadata with conflicts avoiding mechanisms
Metadata appliance
Groups synchronization
Mode synchronization (mode switch on the fly possible)
Adjusted tab and window titling
New localization entries
UI dialog for opening new or existing shared database
UI dialogs for resolving conflicts between BibEntries
Key bindings
Connection loss handling
Switch to offline working mode
Driver availibilty check (e.g. for the dropdown list)
Tests for all classes localized in shared package
Synchronization tests

Screenshots:

Change in CHANGELOG.md described
Tests created for changes
Screenshots added (for bigger UI changes)

obraliar · 2016-05-26T03:25:59Z

Issue: #970

Siedlerchr · 2016-05-26T09:43:05Z

src/main/java/net/sf/jabref/gui/JabRefFrame.java

@@ -483,6 +483,7 @@ public void actionPerformed(ActionEvent e) {
    private final List<Object> openDatabaseOnlyActions = new LinkedList<>();
    private final List<Object> severalDatabasesOnlyActions = new LinkedList<>();
    private final List<Object> openAndSavedDatabasesOnlyActions = new LinkedList<>();
+    private final List<Object> remoteDatabasesOnlyActions = new LinkedList<>();


Could you please investigate if its possible to use a more concrete typ here and above then object?
Using Object in a generic type is making the whole concept of generics ad absurdum.

Those lists are used to add actions. Unfortunately there are differnt types of them, such as AbstractAction, GeneralActions which extends MnemonicAwareAction or just the interface Action.
I think this could be a good stuff for a new PR.

Siedlerchr · 2016-05-26T09:57:26Z

Looks good overall, just some minor remarks 👍 Please also add some new Database tests.

simonharrer · 2016-05-26T10:08:26Z

Why do you want to include Oracle? What is the reasoning here? Is this a DB that is likely to be used in this context? I am not really sure as Oracle is expensive and hard to maintain.

I am against changing the events and adding a location to them. Why must this be differentiated in the first place?

All the metadata must be stored in the DB as well, including biblatex or bibtex mode (this is not something the user should change). Also do not forget the save actions, as they have an influence on the values in the DB as well.

What happens if a change locally cannot be stored in the DB as there are conflicts?

koppor · 2016-05-26T12:30:20Z

Me really wants Oracle. Some bigger institutes use Oracle and they don't want to run other databases. At least, this is, what I understood.

I will contact them again and ask how important Oracle is.

Currently, it seems that the hard part of the Oracle implementation is already done. The synchronization via events seems to be easy - see implementation hints at #970.

simonharrer · 2016-05-26T12:52:04Z

Ok, then please remove the lib file and use a file from maven repositories for the ojdbc.jar

obraliar · 2016-05-28T16:59:23Z

@simonharrer

Why must this be differentiated in the first place?

DBSynchronizer class listens to EntryEvents like EntryAddedEvent and pushes the new entry to the server. Such an event is sent from the method BibDatabase.insertEntry. The problem is that DBSynchronizer (and other classes in remote package) itself also use local methods such as BibDatabase.insertEntry or BibEntry.setField to update the local database. In this case the sent event should not trigger the SQL inserting/updating/deleting methods again (see https://git.io/vrHOw). But as I described in the code comments, if we remove the EventLocation it would not harm. It only causes a little redundancy.

All the metadata must be stored in the DB as well, including biblatex or bibtex mode...

Okay, this is going to be implemented.

Also do not forget the save actions, as they have an influence on the values in the DB as well.

Unfortunately I didn' t understand that completely. Now if you work on remote database you've still the possibility to save the file locally. Otherwise the changes are pushed by EntryEvents. Are there some other data which are involved within the saving process?

What happens if a change locally cannot be stored in the DB as there are conflicts?

In this mode the newest version is held by remote database. Changes are going to be pushed automatically and every time you push an entry your local database is also going to be updated (this process is going to be accelerated via hashing etc). So there is no scenario a conflict can occur except connection problems. But in this case the editor should restrict any editings (TODO). Or two people are working at the same time at one field. In this case the last push will win.

simonharrer · 2016-05-29T07:46:43Z

The save actions control stuff like that a specific field must be lower or upper case letters. Think about it: person A always uses lower case letters for the title, and person B uses always upper case letters for the title - what effect will this have on the database sync when basically the same data is used? They will create a lot of traffic if they work concurrently as the save of one will overwrite all the title fields of the other, and it will create conflicts. You can only overcome this if both know the correct save actions, and that is the reason why the save actions are stored in the metadata.

Regarding resolving conflicts: sounds ok to be practical. It feels wrong, however, from my background with using version control systems.

tobiasdiez · 2016-06-04T11:37:43Z

I think it would be very profitable to unite the code for file-based exports with the code for sql-exports. For example, I really like the immediately sync changes to the sql database feature, which also would be nice for bib-files (similar to intellij's autosave). On the other hand some features which are currently missing for sql exports (like save actions or how external changes are handled) are already present for bib files.
I did some refactorings in #1472 to make this unification easier.

obraliar · 2016-06-04T13:32:56Z

Currently I'm working on save actions. As I see the remote synchronization differs a lot from the local saving of metadata and defaults (think about the relational structure). Unifiying the code would dissolve the remote package and a lot of things would have to be moved/passed through to BibtexParser/BibtexDatabaseWriter which would blow up those classes. Furthermore it would cost a lot of time which is not present.

Currently I'm going to implement this in remote package. If there is some time capacity left after the first release, we could make some considerations to implement this and other customer wishes.

obraliar · 2016-06-24T20:44:24Z

@simonharrer @koppor It seems that ojdbc.jar is not longer available in maven repositories.
(see http://central.maven.org/maven2/ojdbc/ojdbc/14/)

compile group: 'ojdbc', name: 'ojdbc', version: '14' in build.gradle causes:

Execution failed for task ':tasks'.
> Could not resolve all dependencies for configuration ':runtime'.
   > Could not find ojdbc.jar (ojdbc:ojdbc:14).
     Searched in the following locations:
         https://jcenter.bintray.com/ojdbc/ojdbc/14/ojdbc-14.jar

simonharrer · 2016-08-03T07:04:33Z

src/main/java/net/sf/jabref/shared/DBMSProcessor.java

+        .append(" WHERE ")
+        .append(escape("SHARED_ID"))
+        .append(" = ")
+        .append(sharedID);


Always use prepared statement, please.

Siedlerchr · 2016-08-04T15:35:37Z

src/main/java/net/sf/jabref/shared/DBMSType.java

-                return Optional.ofNullable(dbmsType);
-            }
+        try {
+            return Optional.of(Enum.valueOf(DBMSType.class, typeName.toUpperCase()));


Good! But I recommend adding Locale.ENGLISH in the toUpperCase method.
Here is a nice explanation why it is sometimes necessary (the infamous Turkish Locale bug)
http://mattryall.net/blog/2009/02/the-infamous-turkish-locale-bug

koppor · 2016-08-04T19:13:34Z

Conflict resolution with Optimistic Offline Lock 👍

After resolving the minor issues from above, I vote for merging to prevent the continuous merging conflicts.

Things to be solved after merging (Starting from Thursday, 2016-08-18): - [ ] Database properties dialog: - [ ] Do not show "Save sort order" - database sorts arbitrarily 😇 - [ ] Do not show "Database protection" - makes no sense in a shared database setting - [ ] Session restore: JabRef should open all `[shared]` tabs after startup - [ ] Store used connections in the "open remote database dialog"

Follow up at #1703

Support for MySQL, PostgreSQL and Oracle database systems. Full entry synchronisation. Full meta data synchronization. Group synchrnoization. Semi-automatic/event based synchronization. Version control system. Conflicts resolving machanisms. User interfaces for different situations. Extensive synchronization and class tests.

* master: (22 commits) Do not cite entries without a key in OpenOffice/LibreOffice (JabRef#1682) (JabRef#1683) Got rid of unused preferences (JabRef#1672) Move labelpattern (JabRef#1626) Implementation of shared database support (full system). (JabRef#1451) Removed bst from architecture tests JabRef#1699 Update PULL_REQUEST_TEMPLATE.md French localization: Menu: Translation of an empty string French localization: Jabref_fr: empty strings translated Updated string similarity version (JabRef#1698) Minify description of release process Announce switch from GPL to MIT in CONTRIBUTING.md and README.md Some cleanups to initialize empty MetaData at fewer positions (JabRef#1696) Automatic group names are converted from LaTeX to Unicode (JabRef#1684) Update ISSUE_TEMPLATE.md (JabRef#1686) Removed (false) NPE issue reported by FindBugs More Swedish translations (JabRef#1691) Updated wiremock version (JabRef#1690) Some minor code cleanups (JabRef#1689) Removed dependencies of Globals.prefs in some tests (JabRef#1688) Lookup BibEntry from ISBN and merge information (JabRef#1621) ... # Conflicts: # src/main/java/net/sf/jabref/gui/BasePanel.java # src/main/java/net/sf/jabref/importer/ImportMenuItem.java # src/main/resources/l10n/JabRef_da.properties # src/main/resources/l10n/JabRef_de.properties # src/main/resources/l10n/JabRef_en.properties # src/main/resources/l10n/JabRef_es.properties # src/main/resources/l10n/JabRef_fa.properties # src/main/resources/l10n/JabRef_fr.properties # src/main/resources/l10n/JabRef_in.properties # src/main/resources/l10n/JabRef_it.properties # src/main/resources/l10n/JabRef_ja.properties # src/main/resources/l10n/JabRef_nl.properties # src/main/resources/l10n/JabRef_no.properties # src/main/resources/l10n/JabRef_pt_BR.properties # src/main/resources/l10n/JabRef_ru.properties # src/main/resources/l10n/JabRef_sv.properties # src/main/resources/l10n/JabRef_tr.properties # src/main/resources/l10n/JabRef_vi.properties # src/main/resources/l10n/JabRef_zh.properties

Support for MySQL, PostgreSQL and Oracle database systems. Full entry synchronisation. Full meta data synchronization. Group synchrnoization. Semi-automatic/event based synchronization. Version control system. Conflicts resolving machanisms. User interfaces for different situations. Extensive synchronization and class tests.

tobiasdiez · 2016-09-09T23:07:57Z

@obraliar Could you please answer http://discourse.jabref.org/t/new-sql-structure/197/4 ? Thanks!

obraliar force-pushed the SharedDatabaseSupport branch 2 times, most recently from e79c5ff to add5fb3 Compare May 26, 2016 03:38

Siedlerchr reviewed May 26, 2016
View reviewed changes

tobiasdiez added the stupro-ready-for-internal-review label May 26, 2016

obraliar force-pushed the SharedDatabaseSupport branch from add5fb3 to e7dcebc Compare May 28, 2016 16:59

Braunch added the stupro label May 30, 2016

obraliar force-pushed the SharedDatabaseSupport branch 7 times, most recently from d47d740 to ee64d6a Compare June 3, 2016 20:08

tobiasdiez mentioned this pull request Jun 4, 2016

Refactor writing of bib files #1472

Merged

3 tasks

koppor mentioned this pull request Jun 6, 2016

Fix #1476 NPE when importing from SQL DB because of missing DatabaseMode #1477

Merged

obraliar force-pushed the SharedDatabaseSupport branch 3 times, most recently from e092e5c to bc26166 Compare June 23, 2016 18:43

simonharrer reviewed Aug 3, 2016
View reviewed changes

obraliar force-pushed the SharedDatabaseSupport branch from f60328c to c17631b Compare August 4, 2016 11:46

Siedlerchr reviewed Aug 4, 2016
View reviewed changes

obraliar force-pushed the SharedDatabaseSupport branch 5 times, most recently from e201f7f to bb37704 Compare August 6, 2016 11:55

koppor added the status: devcall label Aug 7, 2016

obraliar force-pushed the SharedDatabaseSupport branch from bb37704 to 891387e Compare August 8, 2016 18:43

obraliar force-pushed the SharedDatabaseSupport branch from 891387e to 72d929f Compare August 8, 2016 18:55

koppor merged commit 5d8b15c into JabRef:master Aug 9, 2016

koppor mentioned this pull request Aug 9, 2016

Improve shared database feature #1703

Closed

6 tasks

koppor removed the status: devcall label Aug 9, 2016

koppor changed the title ~~[WIP] Implementation of shared database support~~ Implementation of shared database support Aug 9, 2016

This was referenced Aug 9, 2016

Create blog entry for shared database support JabRef/blog.jabref.org#1

Open

Enable testing of Oracle on TravisCI koppor/jabref#151

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of shared database support #1451

Implementation of shared database support #1451

obraliar commented May 26, 2016 •

edited

Loading

obraliar commented May 26, 2016 •

edited

Loading

Siedlerchr May 26, 2016

obraliar May 28, 2016 •

edited

Loading

Siedlerchr commented May 26, 2016

simonharrer commented May 26, 2016

koppor commented May 26, 2016 •

edited

Loading

simonharrer commented May 26, 2016

obraliar commented May 28, 2016 •

edited

Loading

simonharrer commented May 29, 2016

tobiasdiez commented Jun 4, 2016

obraliar commented Jun 4, 2016

obraliar commented Jun 24, 2016

simonharrer Aug 3, 2016

obraliar Aug 4, 2016

Siedlerchr Aug 4, 2016

koppor commented Aug 4, 2016 •

edited

Loading

tobiasdiez commented Sep 9, 2016

Implementation of shared database support #1451

Implementation of shared database support #1451

Conversation

obraliar commented May 26, 2016 • edited Loading

obraliar commented May 26, 2016 • edited Loading

Siedlerchr May 26, 2016

Choose a reason for hiding this comment

obraliar May 28, 2016 • edited Loading

Choose a reason for hiding this comment

Siedlerchr commented May 26, 2016

simonharrer commented May 26, 2016

koppor commented May 26, 2016 • edited Loading

simonharrer commented May 26, 2016

obraliar commented May 28, 2016 • edited Loading

simonharrer commented May 29, 2016

tobiasdiez commented Jun 4, 2016

obraliar commented Jun 4, 2016

obraliar commented Jun 24, 2016

simonharrer Aug 3, 2016

Choose a reason for hiding this comment

obraliar Aug 4, 2016

Choose a reason for hiding this comment

Siedlerchr Aug 4, 2016

Choose a reason for hiding this comment

koppor commented Aug 4, 2016 • edited Loading

tobiasdiez commented Sep 9, 2016

obraliar commented May 26, 2016 •

edited

Loading

obraliar commented May 26, 2016 •

edited

Loading

obraliar May 28, 2016 •

edited

Loading

koppor commented May 26, 2016 •

edited

Loading

obraliar commented May 28, 2016 •

edited

Loading

koppor commented Aug 4, 2016 •

edited

Loading