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

* upstream/master: Fix NPE in OpenAccessDoi (#5994) Move generate BibTeX keys to Edit menu (#5987) Fixed display of groups and links column in MainTable (#5985) Fixed loosing group if the most bottom one in list is dragged on itself (#5983) Dividers (#5975) Add initial section on code quality (#5991) Create .sonarcloud.properties Add new IntelliJ video tutorial to docs. (#5984) Fix checkstyle Optimize stream() code Use application/x-bibtex for Grobid instance
JabRef · Feb 23, 2020 · 6e11c08 · 6e11c08
2 parents 81c51c3 + d38f813
commit 6e11c08
Show file tree

Hide file tree

Showing 13 changed files with 109 additions and 71 deletions.
diff --git a/.sonarcloud.properties b/.sonarcloud.properties
@@ -0,0 +1 @@
+
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,7 +13,7 @@ We refer to [GitHub issues](/~https://github.com/JabRef/jabref/issues) by using `#
 
 ### Changed
 
-- We reintroduced the possibility to extract references from plain text (using GROBID) [#5614](/~https://github.com/JabRef/jabref/pull/5614) 
+- We reintroduced the possibility to extract references from plain text (using GROBID) [#5614](/~https://github.com/JabRef/jabref/pull/5614)
 - We changed the open office panel to show buttons in rows of three instead of going straight down to save space as the button expanded out to take up unnecessary horizontal space. [#5479](/~https://github.com/JabRef/jabref/issues/5479)
 - We cleaned up the group add/edit dialog. [#5826](/~https://github.com/JabRef/jabref/pull/5826)
 - We reintroduced the index column. [#5844](/~https://github.com/JabRef/jabref/pull/5844)
@@ -42,7 +42,9 @@ We refer to [GitHub issues](/~https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue in the optics of the library properties, that cropped the dialog on scaled displays. [#5969](/~https://github.com/JabRef/jabref/issues/5969)
 - We fixed an issue where changing the type of an entry did not update the main table. [#5906](/~https://github.com/JabRef/jabref/issues/5906)
 - We fixed an issue where opening a library from the recent libraries menu was not possible. [#5939](/~https://github.com/JabRef/jabref/issues/5939)
+- We fixed an issue where the most bottom group in the list got lost, if it was dragged on itself. [#5983](/~https://github.com/JabRef/jabref/issues/5983)
 - We fixed an issue where changing entry type doesn't always work when biblatex source is shown. [#5905](/~https://github.com/JabRef/jabref/issues/5905)
+- We fixed an issue where the group and the link column were not updated after changing the entry in the main table. [#5985](/~https://github.com/JabRef/jabref/issues/5985)
 
 ### Removed
 

diff --git a/docs/code-quality.md b/docs/code-quality.md
@@ -0,0 +1,12 @@
+# Code Quality
+
+We monitor the general source code quality at three places:
+
+* [Teamscale](https://www.cqse.eu/de/produkte/teamscale/landing/) is a popular German product analyzing code quality. The analysis results are available at <https://demo.teamscale.com/dashboard.html#show//?id=kinnen%2FJabref%20Overview>.
+* [codacy](https://www.codacy.com/) is a hosted service to monitor code quality. The code quality analysis for JabRef is available at <https://www.codacy.com/app/simonharrer/jabref/dashboard>.
+* [codecov](https://codecov.io/) is a solution to check code coverage of test cases. The code coverage metrics for JabRef are available at <https://codecov.io/github/JabRef/jabref>.
+
+We strongly recommend to read following two books on code quality:
+
+* [Java by Comparison](java.by-comparison.com/) is a book by three JabRef developers which focuses on code improvements close to single statements. It is fast to read and one gains much information from each recommendation discussed in the book.
+* [Effective Java](https://www.oreilly.com/library/view/effective-java-3rd/9780134686097/) is the standard book for advanced Java programming. Did you know that `enum` is the [recommended way to enforce a singleton instance of a class](https://learning.oreilly.com/library/view/effective-java-3rd/9780134686097/ch2.xhtml#lev3)? Did you know that one should [refer to objects by their interfaces](https://learning.oreilly.com/library/view/effective-java-3rd/9780134686097/ch9.xhtml#lev64)?
diff --git a/docs/guidelines-for-setting-up-a-local-workspace.md b/docs/guidelines-for-setting-up-a-local-workspace.md
@@ -1,12 +1,12 @@
-# Setup a local workspace
+# Set up a local workspace
 
 This guide explains how to set up your environment for development of JabRef. It includes information about prerequisites, configuring your IDE, and running JabRef locally to verify your setup.
 
-For a complete step-by-step guide (using IntellJ as the IDE), have a look at the following video instructions:
+For a complete step-by-step guide for Linux using IntellJ IDEA as the IDE, have a look at the following video instructions:
 
-<p align="center">
-  <a href="http://www.youtube.com/watch?v=FeQpygT0314"><img src="http://img.youtube.com/vi/FeQpygT0314/0.jpg" /></a>
-</p>
+<a href="https://youtu.be/JkFVJ6p0urw">
+<img src="https://img.youtube.com/vi/JkFVJ6p0urw/mqdefault.jpg" alt="JabRef development video tutorial"/>
+</a>
 
 ## Prerequisites
 

diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java
@@ -713,6 +713,7 @@ private MenuBar createMenu() {
                 new SeparatorMenuItem(),
 
                 factory.createMenuItem(StandardActions.REPLACE_ALL, new OldDatabaseCommandWrapper(Actions.REPLACE_ALL, this, stateManager)),
+                factory.createMenuItem(StandardActions.GENERATE_CITE_KEYS, new OldDatabaseCommandWrapper(Actions.MAKE_KEY, this, stateManager)),
 
                 new SeparatorMenuItem(),
 
@@ -721,13 +722,13 @@ private MenuBar createMenu() {
 
         if (Globals.prefs.getBoolean(JabRefPreferences.SPECIALFIELDSENABLED)) {
             edit.getItems().addAll(
+                    new SeparatorMenuItem(),
                     SpecialFieldMenuItemFactory.createSpecialFieldMenuForActiveDatabase(SpecialField.RANKING, factory, undoManager),
                     SpecialFieldMenuItemFactory.getSpecialFieldSingleItemForActiveDatabase(SpecialField.RELEVANCE, factory),
                     SpecialFieldMenuItemFactory.getSpecialFieldSingleItemForActiveDatabase(SpecialField.QUALITY, factory),
                     SpecialFieldMenuItemFactory.getSpecialFieldSingleItemForActiveDatabase(SpecialField.PRINTED, factory),
                     SpecialFieldMenuItemFactory.createSpecialFieldMenuForActiveDatabase(SpecialField.PRIORITY, factory, undoManager),
-                    SpecialFieldMenuItemFactory.createSpecialFieldMenuForActiveDatabase(SpecialField.READ_STATUS, factory, undoManager),
-                    new SeparatorMenuItem()
+                    SpecialFieldMenuItemFactory.createSpecialFieldMenuForActiveDatabase(SpecialField.READ_STATUS, factory, undoManager)
             );
         }
 
@@ -785,7 +786,6 @@ private MenuBar createMenu() {
 
                 new SeparatorMenuItem(),
 
-                factory.createMenuItem(StandardActions.GENERATE_CITE_KEYS, new OldDatabaseCommandWrapper(Actions.MAKE_KEY, this, stateManager)),
                 factory.createMenuItem(StandardActions.SEND_AS_EMAIL, new OldDatabaseCommandWrapper(Actions.SEND_AS_EMAIL, this, stateManager)),
                 pushToApplicationMenuItem,
 

diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java
@@ -291,10 +291,14 @@ public Optional<GroupTreeNode> getParent() {
     }
 
     public void draggedOn(GroupNodeViewModel target, DroppingMouseLocation mouseLocation) {
+        // No action, if the target is the same as the source
+        if (this.equals(target)) {
+            return;
+        }
+
         Optional<GroupTreeNode> targetParent = target.getParent();
         if (targetParent.isPresent()) {
             int targetIndex = target.getPositionInParent();
-
             // In case we want to move an item in the same parent
             // and the item is moved down, we need to adjust the target index
             if (targetParent.equals(getParent())) {

diff --git a/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java b/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java
@@ -7,6 +7,7 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import javafx.beans.binding.Bindings;
 import javafx.beans.binding.ObjectBinding;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.value.ObservableValue;
@@ -18,6 +19,7 @@
 import org.jabref.model.entry.FileFieldParser;
 import org.jabref.model.entry.LinkedFile;
 import org.jabref.model.entry.field.Field;
+import org.jabref.model.entry.field.InternalField;
 import org.jabref.model.entry.field.SpecialField;
 import org.jabref.model.entry.field.StandardField;
 import org.jabref.model.groups.AbstractGroup;
@@ -52,31 +54,33 @@ public ObservableValue<List<LinkedFile>> getLinkedFiles() {
         return EasyBind.map(getField(StandardField.FILE), FileFieldParser::parse);
     }
 
-    public ObservableValue<Map<Field,String>> getLinkedIdentifiers() {
-        SimpleObjectProperty<Map<Field,String>> linkedIdentifiers = new SimpleObjectProperty<>(new HashMap<>());
-
-        entry.getField(StandardField.URL).ifPresent(value -> linkedIdentifiers.getValue().put(StandardField.URL, value));
-        entry.getField(StandardField.DOI).ifPresent(value -> linkedIdentifiers.getValue().put(StandardField.DOI, value));
-        entry.getField(StandardField.URI).ifPresent(value -> linkedIdentifiers.getValue().put(StandardField.URI, value));
-        entry.getField(StandardField.EPRINT).ifPresent(value -> linkedIdentifiers.getValue().put(StandardField.EPRINT, value));
-
-        return linkedIdentifiers;
+    public ObservableValue<Map<Field, String>> getLinkedIdentifiers() {
+        return Bindings.createObjectBinding(() -> {
+                    Map<Field, String> linkedIdentifiers = new HashMap<>();
+                    entry.getField(StandardField.URL).ifPresent(value -> linkedIdentifiers.put(StandardField.URL, value));
+                    entry.getField(StandardField.DOI).ifPresent(value -> linkedIdentifiers.put(StandardField.DOI, value));
+                    entry.getField(StandardField.URI).ifPresent(value -> linkedIdentifiers.put(StandardField.URI, value));
+                    entry.getField(StandardField.EPRINT).ifPresent(value -> linkedIdentifiers.put(StandardField.EPRINT, value));
+                    return linkedIdentifiers;
+                },
+                getEntry().getFieldBinding(StandardField.URL),
+                getEntry().getFieldBinding(StandardField.DOI),
+                getEntry().getFieldBinding(StandardField.URI),
+                getEntry().getFieldBinding(StandardField.EPRINT));
     }
 
     public ObservableValue<List<AbstractGroup>> getMatchedGroups(BibDatabaseContext database) {
-        SimpleObjectProperty<List<AbstractGroup>> matchedGroups = new SimpleObjectProperty<>(Collections.emptyList());
-
-        Optional<GroupTreeNode> root = database.getMetaData()
-                                               .getGroups();
+        Optional<GroupTreeNode> root = database.getMetaData().getGroups();
         if (root.isPresent()) {
-            List<AbstractGroup> groups = root.get().getMatchingGroups(entry)
-                                             .stream()
-                                             .map(GroupTreeNode::getGroup)
-                                             .collect(Collectors.toList());
-            groups.remove(root.get().getGroup());
-            matchedGroups.setValue(groups);
+            return EasyBind.map(entry.getFieldBinding(InternalField.GROUPS), field -> {
+                List<AbstractGroup> groups = root.get().getMatchingGroups(entry)
+                                                 .stream()
+                                                 .map(GroupTreeNode::getGroup)
+                                                 .collect(Collectors.toList());
+                groups.remove(root.get().getGroup());
+                return groups;
+            });
         }
-
-        return matchedGroups;
+        return new SimpleObjectProperty<>(Collections.emptyList());
     }
 }
diff --git a/src/main/java/org/jabref/logic/importer/fetcher/DoiFetcher.java b/src/main/java/org/jabref/logic/importer/fetcher/DoiFetcher.java
@@ -15,6 +15,7 @@
 import org.jabref.logic.importer.ImportFormatPreferences;
 import org.jabref.logic.importer.ParseException;
 import org.jabref.logic.importer.fileformat.BibtexParser;
+import org.jabref.logic.importer.util.MediaTypes;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.net.URLDownload;
 import org.jabref.model.cleanup.FieldFormatterCleanup;
@@ -52,7 +53,7 @@ public Optional<BibEntry> performSearchById(String identifier) throws FetcherExc
 
                 // BibTeX data
                 URLDownload download = new URLDownload(doiURL);
-                download.addHeader("Accept", "application/x-bibtex");
+                download.addHeader("Accept", MediaTypes.APPLICATION_BIBTEX);
                 String bibtexString = download.asString();
 
                 // BibTeX entry

diff --git a/src/main/java/org/jabref/logic/importer/fetcher/GrobidCitationFetcher.java b/src/main/java/org/jabref/logic/importer/fetcher/GrobidCitationFetcher.java
@@ -1,9 +1,7 @@
 package org.jabref.logic.importer.fetcher;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -32,16 +30,17 @@ public GrobidCitationFetcher(ImportFormatPreferences importFormatPreferences) {
     }
 
     /**
-     * Passes request to grobid server, using consolidateCitations option to improve result.
-     * Takes a while, since the server has to look up the entry.
-     * @return A BibTeX-String if extraction is successful and an empty String otherwise.
+     * Passes request to grobid server, using consolidateCitations option to improve result. Takes a while, since the
+     * server has to look up the entry.
+     *
+     * @return A BibTeX string if extraction is successful
      */
-    private String parseUsingGrobid(String plainText) {
+    private Optional<String> parseUsingGrobid(String plainText) {
         try {
-            return grobidService.processCitation(plainText, GrobidService.ConsolidateCitations.WITH_METADATA);
+            return Optional.of(grobidService.processCitation(plainText, GrobidService.ConsolidateCitations.WITH_METADATA));
         } catch (IOException e) {
             LOGGER.debug("Could not process citation", e);
-            return "";
+            return Optional.empty();
         }
     }
 
@@ -56,18 +55,15 @@ private Optional<BibEntry> parseBibToBibEntry(String bibtexString) {
 
     @Override
     public List<BibEntry> performSearch(String query) {
-        List<String> plainReferences = Arrays.stream( query.split( "\\r\\r+|\\n\\n+|\\r\\n(\\r\\n)+" ) )
-              .map(String::trim)
-              .filter(str -> !str.isBlank())
-              .collect(Collectors.toCollection(ArrayList::new));
-        if (plainReferences.isEmpty()) {
-            return Collections.emptyList();
-        } else {
-            return plainReferences.stream()
-                  .map(reference -> parseBibToBibEntry(parseUsingGrobid(reference)))
-                  .flatMap(Optional::stream)
-                  .collect(Collectors.toList());
-        }
+        return Arrays
+                .stream(query.split("\\r\\r+|\\n\\n+|\\r\\n(\\r\\n)+"))
+                .map(String::trim)
+                .filter(str -> !str.isBlank())
+                .map(reference -> parseUsingGrobid(reference))
+                .flatMap(Optional::stream)
+                .map(reference -> parseBibToBibEntry(reference))
+                .flatMap(Optional::stream)
+                .collect(Collectors.toList());
     }
 
     @Override

diff --git a/src/main/java/org/jabref/logic/importer/fetcher/OpenAccessDoi.java b/src/main/java/org/jabref/logic/importer/fetcher/OpenAccessDoi.java
@@ -11,18 +11,20 @@
 import org.jabref.model.entry.field.StandardField;
 import org.jabref.model.entry.identifier.DOI;
 
-import kong.unirest.HttpResponse;
-import kong.unirest.JsonNode;
 import kong.unirest.Unirest;
 import kong.unirest.UnirestException;
-import kong.unirest.json.JSONObject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * A fulltext fetcher that uses <a href="https://oadoi.org/">oaDOI</a>.
  *
  * @implSpec API is documented at http://unpaywall.org/api/v2
  */
 public class OpenAccessDoi implements FulltextFetcher {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(FulltextFetcher.class);
+
     private static String API_URL = "https://api.oadoi.org/v2/";
 
     @Override
@@ -47,17 +49,24 @@ public TrustLevel getTrustLevel() {
         return TrustLevel.META_SEARCH;
     }
 
-    public Optional<URL> findFullText(DOI doi) throws UnirestException, MalformedURLException {
-        HttpResponse<JsonNode> jsonResponse = Unirest.get(API_URL + doi.getDOI() + "?email=developers@jabref.org")
-                                                     .header("accept", "application/json")
-                                                     .asJson();
-        JSONObject root = jsonResponse.getBody().getObject();
-        Optional<String> url = Optional.ofNullable(root.optJSONObject("best_oa_location"))
-                .map(location -> location.optString("url"));
-        if (url.isPresent()) {
-            return Optional.of(new URL(url.get()));
-        } else {
-            return Optional.empty();
-        }
+    public Optional<URL> findFullText(DOI doi) throws UnirestException {
+        return Optional.of(Unirest.get(API_URL + doi.getDOI() + "?email=developers@jabref.org")
+                                  .header("accept", "application/json")
+                                  .asJson())
+                       .map(response -> response.getBody())
+                       .filter(body -> body != null)
+                       .map(body -> body.getObject())
+                       .filter(root -> root != null)
+                       .map(root -> root.optJSONObject("best_oa_location"))
+                       .filter(object -> object != null)
+                       .map(location -> location.optString("url"))
+                       .flatMap(url -> {
+                           try {
+                               return Optional.of(new URL(url));
+                           } catch (MalformedURLException e) {
+                               LOGGER.debug("Could not determine URL to fetch full text from", e);
+                               return Optional.empty();
+                           }
+                       });
     }
 }
diff --git a/src/main/java/org/jabref/logic/importer/util/GrobidService.java b/src/main/java/org/jabref/logic/importer/util/GrobidService.java
@@ -38,13 +38,16 @@ public GrobidService(String grobidServerURL) {
     }
 
     /**
-     * @return A BibTeX-String if extraction is successful and an IOException otherwise.
+     * Calls the Grobid server for converting the citation into BibTeX
+     *
+     * @return A plain BibTeX string (generated by the Grobid server)
+     * @throws IOException if an I/O excecption during the call ocurred or no BibTeX entry could be determiend
      */
     public String processCitation(String rawCitation, ConsolidateCitations consolidateCitations) throws IOException {
         rawCitation = URLEncoder.encode(rawCitation, StandardCharsets.UTF_8);
         URLDownload urlDownload = new URLDownload(grobidServerURL
                 + "/api/processCitation");
-        //urlDownload.addHeader("Accept", "application/x-bibtex"); //TODO: Uncomment as soon as the default GROBID server is used.
+        urlDownload.addHeader("Accept", MediaTypes.APPLICATION_BIBTEX);
         urlDownload.setPostData("citations=" + rawCitation + "&consolidateCitations=" + consolidateCitations);
         String httpResponse = urlDownload.asString();
 

diff --git a/src/main/java/org/jabref/logic/importer/util/MediaTypes.java b/src/main/java/org/jabref/logic/importer/util/MediaTypes.java
@@ -0,0 +1,8 @@
+package org.jabref.logic.importer.util;
+
+/**
+ * Stores MediaTypes required by JabRef (which are not availble in used libraries)
+ */
+public class MediaTypes {
+    public static final String APPLICATION_BIBTEX = "application/x-bibtex";
+}
diff --git a/src/test/java/org/jabref/logic/importer/fetcher/OpenAccessDoiTest.java b/src/test/java/org/jabref/logic/importer/fetcher/OpenAccessDoiTest.java
@@ -28,14 +28,12 @@ void setUp() {
     @Test
     void findByDOI() throws IOException {
         entry.setField(StandardField.DOI, "10.1038/nature12373");
-
         assertEquals(Optional.of(new URL("https://dash.harvard.edu/bitstream/1/12285462/1/Nanometer-Scale%20Thermometry.pdf")), finder.findFullText(entry));
     }
 
     @Test
     void notFoundByDOI() throws IOException {
         entry.setField(StandardField.DOI, "10.1186/unknown-doi");
-
         assertEquals(Optional.empty(), finder.findFullText(entry));
     }
 }