Fix datbase tests and enable running using GitHub workflows

- fix behavior when no entries have to be inserted during a synch
- fix database tests
- have dbms test test only one database
- DBMS to test is passed via environment variable "DBMS", defaulting to PostgreSQL
- add shared information on Canonical BibTeX entry
- some code improvement
- have checkstyle running on github workflows (not on Travis)

.github/workflows/checkstyle.yml

@@ -0,0 +1,27 @@
+name: checkstyle
+
+on: [push]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout source
+      uses: actions/checkout@v1
+      with:
+        depth: 1
+        submodules: false
+    - name: Set up JDK
+      uses: actions/setup-java@v1
+      with:
+        java-version: 13
+    - uses: actions/cache@v1
+      with:
+        path: ~/.gradle/caches
+        key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+        restore-keys: |
+          ${{ runner.OS }}-gradle-${{ env.cache-name }}-
+          ${{ runner.OS }}-gradle-
+          ${{ runner.OS }}-
+    - name: Run checkstyle test
+      run: ./gradlew checkstyleMain checkstyleTest checkstyleJmh

.github/workflows/test-on-mysql.yml

@@ -0,0 +1,40 @@
+name: Test on MySQL
+
+on: [push]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Shutdown Ubuntu MySQL (SUDO)
+        run: sudo service mysql stop # Shutdown the Default MySQL, "sudo" is necessary, please not remove it
+      - uses: mirromutth/mysql-action@v1.1
+        with:
+          host port: 3800
+          container port: 3307
+          character set server: 'utf8'
+          collation server: 'utf8_general_ci'
+          mysql version: '8.0'
+          mysql database: 'jabref'
+          mysql root password: 'root'
+      - name: Set up JDK
+        uses: actions/setup-java@v1
+        with:
+          java-version: 13
+      - name: Checkout source
+        uses: actions/checkout@v1
+        with:
+          depth: 1
+          submodules: false
+      - uses: actions/cache@v1
+        with:
+          path: ~/.gradle/caches
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: |
+            ${{ runner.OS }}-gradle-${{ env.cache-name }}-
+            ${{ runner.OS }}-gradle-
+            ${{ runner.OS }}-
+      - name: Run database test
+        run: ./gradlew databaseTest
+        env:
+          DBMS: "mysql"

.github/workflows/test-on-postgres.yml

@@ -0,0 +1,40 @@
+name: Test on PostgreSQL
+
+on: [push]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    services:
+      postgres:
+        image: postgres:10.8
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: postgres
+        ports:
+          - 5432:5432
+        # needed because the postgres container does not provide a healthcheck
+        options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
+    steps:
+    - name: Checkout source
+      uses: actions/checkout@v1
+      with:
+        depth: 1
+        submodules: false
+    - name: Set up JDK
+      uses: actions/setup-java@v1
+      with:
+        java-version: 13
+    - uses: actions/cache@v1
+      with:
+        path: ~/.gradle/caches
+        key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+        restore-keys: |
+          ${{ runner.OS }}-gradle-${{ env.cache-name }}-
+          ${{ runner.OS }}-gradle-
+          ${{ runner.OS }}-
+    - name: Run database test
+      run: ./gradlew databaseTest
+      env:
+        DBMS: "postgresql"

.travis.yml

@@ -11,18 +11,12 @@ sudo: required
 git:
   depth: 1
 
-services:
-  - postgresql
-  - mysql
-
 env:
   global:
     - GRADLE_OPTS=-Dorg.gradle.daemon=false
   matrix:
     - TEST_SUITE=check
-    - TEST_SUITE=checkstyle
     - TEST_SUITE=fetcherTest
-    - TEST_SUITE=databaseTest
     - TEST_SUITE=guiTest
     # codecov may fail by itself (see gradle build file)
     # The codecov test itself fails only in case there is an issue with the codecov update.
@@ -34,7 +28,6 @@ matrix:
     fast_finish: true
     allow_failures:
       - env: TEST_SUITE=fetcherTest
-      - env: TEST_SUITE=databaseTest
       - env: TEST_SUITE=guiTest
       - env: DEPENDENCY_UPDATES=check
 
@@ -46,13 +39,8 @@ before_install:
 
 install: true
 
-before_script:
-  - psql -c 'create database jabref;' -U postgres
-  - mysql -u root -e 'create database jabref'
-
 script:
   - if [ "$TEST_SUITE" != "guiTest" ] && [ "$TEST_SUITE" != "checkstyle" ] && [ "$TEST_SUITE" != "codecov" ]; then ./gradlew $TEST_SUITE $OPTIONS -x checkstyleJmh -x checkstyleMain -x checkstyleTest; fi
-  - if [ "$TEST_SUITE" == "checkstyle" ]; then ./gradlew checkstyleMain checkstyleTest checkstyleJmh; fi
   - if [ "$TEST_SUITE" == "guiTest" ]; then ./buildres/gui-tests.sh; fi
   - if [ "$TEST_SUITE" == "codecov" ]; then ./gradlew jacocoTestReport && bash <(curl -s https://codecov.io/bash); fi
   - if [ "$DEPENDENCY_UPDATES" == "check" ]; then ./gradlew -q checkOutdatedDependencies; fi

docs/testing.md

@@ -0,0 +1,17 @@
+# How to test
+
+## Database tests
+
+To quickly host a local PostgreSQL database, execute following statement:
+
+```shell command
+docker run -d -e POSTGRES_USER=postgres -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=postgres -p 5432:5432 --name db postgres:10 postgres -c log_statement=all
+```
+
+Then, all DBMS Tests (annotated with `@org.jabref.testutils.category.DatabaseTest`) run properly.
+
+A MySQL can be started using following command:
+
+``´shell command
+docker run -e MYSQL_ROOT_PASSWORD=root -e MYSQL_DATABASE=jabref -p 3800:3307 mysql:8.0 --port=3307
+```

src/main/java/org/jabref/logic/shared/DBMSConnectionProperties.java

@@ -117,7 +117,13 @@ public void setUseSSL(boolean useSSL) {
     }
 
     public String getUrl() {
-        return type.getUrl(host, port, database);
+        String url = type.getUrl(host, port, database);
+        if (this.type == DBMSType.MYSQL) {
+            // quickfix - see https://mysqlconnector.net/troubleshooting/retrieval-public-key/
+            url += "?allowPublicKeyRetrieval=true";
+            url += "&useSSL=" + this.isUseSSL();
+        }
+        return url;
     }
 
     @Override

src/main/java/org/jabref/logic/shared/DBMSProcessor.java

@@ -66,7 +66,7 @@ public boolean checkBaseIntegrity() throws SQLException {
      *
      * @return <code>true</code> if the structure is old, else <code>false</code>.
      */
-    public boolean checkForPare3Dot6Integrity() throws SQLException {
+    public boolean databaseIsAtMostJabRef35() throws SQLException {
         return checkTableAvailability(
                 "ENTRIES",
                 "ENTRY_GROUP",
@@ -439,7 +439,14 @@ public Optional<BibEntry> getSharedEntry(int sharedID) {
         return Optional.empty();
     }
 
+    /**
+     * Queries the database for shared entries. Optionally, they are filtered by the given list of sharedIds
+     *
+     * @param sharedIDs the list of Ids to filter. If list is empty, then no filter is applied
+     */
     public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {
+        Objects.requireNonNull(sharedIDs);
+
         List<BibEntry> sharedEntries = new ArrayList<>();
 
         StringBuilder query = new StringBuilder();

src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java

@@ -8,6 +8,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.jabref.logic.exporter.BibDatabaseWriter;
 import org.jabref.logic.exporter.MetaDataSerializer;
@@ -146,15 +147,17 @@ public void initializeDatabases() throws DatabaseNotSupportedException {
         try {
             if (!dbmsProcessor.checkBaseIntegrity()) {
                 LOGGER.info("Integrity check failed. Fixing...");
-                dbmsProcessor.setupSharedDatabase();
 
                 // This check should only be performed once on initial database setup.
-                // Calling dbmsProcessor.setupSharedDatabase() lets dbmsProcessor.checkBaseIntegrity() be true.
-                if (dbmsProcessor.checkForPare3Dot6Integrity()) {
+                if (dbmsProcessor.databaseIsAtMostJabRef35()) {
                     throw new DatabaseNotSupportedException();
                 }
+
+                // Calling dbmsProcessor.setupSharedDatabase() lets dbmsProcessor.checkBaseIntegrity() be true.
+                dbmsProcessor.setupSharedDatabase();
             }
         } catch (SQLException e) {
+            LOGGER.error("Could not check intergrity", e);
             throw new IllegalStateException(e);
         }
 
@@ -164,7 +167,7 @@ public void initializeDatabases() throws DatabaseNotSupportedException {
     }
 
     /**
-     * Synchronizes the local database with shared one. Possible update types are removal, update or insert of a {@link
+     * Synchronizes the local database with shared one. Possible update types are: removal, update, or insert of a {@link
      * BibEntry}.
      */
     @Override
@@ -178,13 +181,13 @@ public void synchronizeLocalDatabase() {
 
         // remove old entries locally
         removeNotSharedEntries(localEntries, idVersionMap.keySet());
-        List<Integer> entriesToDrag = new ArrayList<>();
+        List<Integer> entriesToInsertIntoLocalDatabase = new ArrayList<>();
         // compare versions and update local entry if needed
         for (Map.Entry<Integer, Integer> idVersionEntry : idVersionMap.entrySet()) {
-            boolean match = false;
+            boolean remoteEntryMatchingOneLocalEntryFound = false;
             for (BibEntry localEntry : localEntries) {
-                if (idVersionEntry.getKey() == localEntry.getSharedBibEntryData().getSharedID()) {
-                    match = true;
+                if (idVersionEntry.getKey().equals(localEntry.getSharedBibEntryData().getSharedID())) {
+                    remoteEntryMatchingOneLocalEntryFound = true;
                     if (idVersionEntry.getValue() > localEntry.getSharedBibEntryData().getVersion()) {
                         Optional<BibEntry> sharedEntry = dbmsProcessor.getSharedEntry(idVersionEntry.getKey());
                         if (sharedEntry.isPresent()) {
@@ -193,7 +196,7 @@ public void synchronizeLocalDatabase() {
                             localEntry.getSharedBibEntryData()
                                       .setVersion(sharedEntry.get().getSharedBibEntryData().getVersion());
                             sharedEntry.get().getFieldMap().forEach(
-                            // copy remote values to local entry
+                                    // copy remote values to local entry
                                     (field, value) -> localEntry.setField(field, value, EntriesEventSource.SHARED)
                             );
 
@@ -207,13 +210,16 @@ public void synchronizeLocalDatabase() {
                     }
                 }
             }
-            if (!match) {
-                entriesToDrag.add(idVersionEntry.getKey());
+            if (!remoteEntryMatchingOneLocalEntryFound) {
+                entriesToInsertIntoLocalDatabase.add(idVersionEntry.getKey());
             }
         }
 
-        for (BibEntry bibEntry : dbmsProcessor.getSharedEntries(entriesToDrag)) {
-            bibDatabase.insertEntry(bibEntry, EntriesEventSource.SHARED);
+        if (!entriesToInsertIntoLocalDatabase.isEmpty()) {
+            // in case entries should be added into the local database, insert them
+            for (BibEntry bibEntry : dbmsProcessor.getSharedEntries(entriesToInsertIntoLocalDatabase)) {
+                bibDatabase.insertEntry(bibEntry, EntriesEventSource.SHARED);
+            }
         }
     }
 
@@ -224,23 +230,15 @@ public void synchronizeLocalDatabase() {
      * @param sharedIDs    Set of all IDs which are present on shared database
      */
     private void removeNotSharedEntries(List<BibEntry> localEntries, Set<Integer> sharedIDs) {
-        List<BibEntry> entriesToRemove = new ArrayList<>();
-        for (BibEntry localEntry : localEntries) {
-            boolean match = false;
-            for (int sharedID : sharedIDs) {
-                if (localEntry.getSharedBibEntryData().getSharedID() == sharedID) {
-                    match = true;
-                    break;
-                }
-            }
-            if (!match) {
-                entriesToRemove.add(localEntry);
-            }
-        }
+        List<BibEntry> entriesToRemove =
+                localEntries.stream()
+                            .filter(localEntry -> !sharedIDs.contains(localEntry.getSharedBibEntryData().getSharedID()))
+                            .collect(Collectors.toList());
         if (!entriesToRemove.isEmpty()) {
             eventBus.post(new SharedEntriesNotPresentEvent(entriesToRemove));
+            // remove all non-shared entries without triggering listeners
+            bibDatabase.removeEntries(entriesToRemove, EntriesEventSource.SHARED);
         }
-        bibDatabase.removeEntries(entriesToRemove, EntriesEventSource.SHARED); // Should not reach the listeners above.
     }
 
     /**

src/main/java/org/jabref/model/database/shared/DBMSType.java

@@ -51,11 +51,15 @@ public int getDefaultPort() {
     }
 
     public static Optional<DBMSType> fromString(String typeName) {
-        try {
-            return Optional.of(Enum.valueOf(DBMSType.class, typeName.toUpperCase(Locale.ENGLISH)));
-        } catch (IllegalArgumentException exception) {
+        if (typeName == null) {
             return Optional.empty();
+        } else {
+            try {
+                return Optional.of(Enum.valueOf(DBMSType.class, typeName.toUpperCase(Locale.ENGLISH)));
+            } catch (IllegalArgumentException exception) {
+                // IllegalArgumentException is thrown if "typename" is not covered by the Enum
+                return Optional.empty();
+            }
         }
     }
-
 }

src/main/java/org/jabref/model/entry/CanonicalBibEntry.java

@@ -56,6 +56,9 @@ public static String getCanonicalRepresentation(BibEntry entry) {
             String line = String.format("  %s = {%s}", fieldName, String.valueOf(mapFieldToValue.get(fieldName)).replaceAll("\\r\\n", "\n"));
             sj.add(line);
         }
+
+        sj.add(String.format("  _jabref_shared = {sharedId: %d, version: %d}", entry.getSharedBibEntryData().getSharedID(), entry.getSharedBibEntryData().getVersion()));
+
         sb.append(sj);
 
         // append the closing entry bracket

src/main/java/org/jabref/model/entry/SharedBibEntryData.java

@@ -1,7 +1,7 @@
 package org.jabref.model.entry;
 
 /**
- *  Stores all informations needed to manage entries on a shared (SQL) database.
+ *  Stores all information needed to manage entries on a shared (SQL) database.
  */
 public class SharedBibEntryData {
 

src/test/java/org/jabref/logic/shared/DBMSConnectionPropertiesTest.java

@@ -0,0 +1,16 @@
+package org.jabref.logic.shared;
+
+import org.jabref.model.database.shared.DBMSType;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class DBMSConnectionPropertiesTest {
+
+    @Test
+    void urlForMySqlIncludesSslConfig() {
+        DBMSConnectionProperties connectionProperties = new DBMSConnectionProperties(DBMSType.MYSQL, "localhost", 3108, "jabref", "user", "password", false, "");
+        assertEquals("jdbc:mariadb://localhost:3108/jabref?allowPublicKeyRetrieval=true&useSSL=false", connectionProperties.getUrl());
+    }
+}