Repository: jabref/jabref
Issue: DOI Fetcher throws NPE if no result was found

JabRef dev version on Windows 10

<!-- Hint: If you use a development version (available at http://builds.jabref.org/master/), ensure that you use the latest one. -->

Steps to reproduce:
1. Entry with DOI for which the fetcher can't find bibtex data, for example: 10.2307/2372901
2. Click "Get BibTeX data from DOI"
3. Results in no warning but an NPE:

```
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
    at java.util.Objects.requireNonNull(Objects.java:203)
    at net.sf.jabref.importer.fetcher.DOItoBibTeXFetcher.getEntryFromDOI(DOItoBibTeXFetcher.java:90)
    at net.sf.jabref.gui.mergeentries.MergeEntryDOIDialog.<init>(MergeEntryDOIDialog.java:75)
    at net.sf.jabref.gui.entryeditor.FieldExtraComponents.lambda$getDoiExtraComponent$339(FieldExtraComponents.java:199)
    at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2022)
    at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2348)
    at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:402)
    at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:259)
    at javax.swing.plaf.basic.BasicButtonListener.mouseReleased(BasicButtonListener.java:252)
    at java.awt.Component.processMouseEvent(Component.java:6535)
    at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
    at java.awt.Component.processEvent(Component.java:6300)
    at java.awt.Container.processEvent(Container.java:2236)
    at java.awt.Component.dispatchEventImpl(Component.java:4891)
    at java.awt.Container.dispatchEventImpl(Container.java:2294)
    at java.awt.Component.dispatchEvent(Component.java:4713)
    at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4888)
    at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4525)
    at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4466)
    at java.awt.Container.dispatchEventImpl(Container.java:2280)
    at java.awt.Window.dispatchEventImpl(Window.java:2750)
    at java.awt.Component.dispatchEvent(Component.java:4713)
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
    at java.awt.EventQueue.access$500(EventQueue.java:97)
    at java.awt.EventQueue$3.run(EventQueue.java:709)
    at java.awt.EventQueue$3.run(EventQueue.java:703)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
    at java.awt.EventQueue$4.run(EventQueue.java:731)
    at java.awt.EventQueue$4.run(EventQueue.java:729)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
    at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)
```

<!-- If applicable, excerpt of the bibliography file, screenshot, and excerpt of log (available in the error console) -->


Changed files:
src/main/java/net/sf/jabref/gui/mergeentries/MergeEntryDOIDialog.java, src/main/java/net/sf/jabref/importer/fetcher/DOItoBibTeXFetcher.java, src/main/java/net/sf/jabref/logic/util/DOI.java, src/main/java/net/sf/jabref/gui/desktop/JabRefDesktop.java, src/test/java/net/sf/jabref/logic/util/DOITest.java, src/main/java/net/sf/jabref/logic/fulltext/DoiResolution.java, src/main/java/net/sf/jabref/importer/fileformat/PdfContentImporter.java, src/main/java/net/sf/jabref/logic/layout/format/DOICheck.java, src/main/java/net/sf/jabref/gui/ClipBoardManager.java

diff --git a/src/main/java/net/sf/jabref/gui/ClipBoardManager.java b/src/main/java/net/sf/jabref/gui/ClipBoardManager.java
index 9071de0a38..fb16c131df 100644
--- a/src/main/java/net/sf/jabref/gui/ClipBoardManager.java
+++ b/src/main/java/net/sf/jabref/gui/ClipBoardManager.java
@@ -32,6 +32,7 @@ import java.io.IOException;
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 
 import net.sf.jabref.importer.fetcher.DOItoBibTeXFetcher;
 import net.sf.jabref.importer.fileformat.BibtexParser;
@@ -105,10 +106,8 @@ public class ClipBoardManager implements ClipboardOwner {
                 // fetch from doi
                 if (DOI.build(data).isPresent()) {
                     LOGGER.info("Found DOI in clipboard");
-                    BibEntry entry = new DOItoBibTeXFetcher().getEntryFromDOI(new DOI(data).getDOI(), null);
-                    if (entry != null) {
-                        result.add(entry);
-                    }
+                    Optional<BibEntry> entry = new DOItoBibTeXFetcher().getEntryFromDOI(new DOI(data).getDOI());
+                    entry.ifPresent(result::add);
                 } else {
                     // parse bibtex string
                     BibtexParser bp = new BibtexParser(new StringReader(data));
diff --git a/src/main/java/net/sf/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/net/sf/jabref/gui/desktop/JabRefDesktop.java
index c6606e12a7..8a577b8dba 100644
--- a/src/main/java/net/sf/jabref/gui/desktop/JabRefDesktop.java
+++ b/src/main/java/net/sf/jabref/gui/desktop/JabRefDesktop.java
@@ -80,7 +80,7 @@ public class JabRefDesktop {
         } else if ("doi".equals(fieldName)) {
             Optional<DOI> doiUrl = DOI.build(link);
             if (doiUrl.isPresent()) {
-                link = doiUrl.get().getURLAsASCIIString();
+                link = doiUrl.get().getURIAsASCIIString();
             }
             // should be opened in browser
             fieldName = "url";
diff --git a/src/main/java/net/sf/jabref/gui/mergeentries/MergeEntryDOIDialog.java b/src/main/java/net/sf/jabref/gui/mergeentries/MergeEntryDOIDialog.java
index 8b7d19c96c..b3df25b126 100644
--- a/src/main/java/net/sf/jabref/gui/mergeentries/MergeEntryDOIDialog.java
+++ b/src/main/java/net/sf/jabref/gui/mergeentries/MergeEntryDOIDialog.java
@@ -40,9 +40,7 @@ import com.jgoodies.forms.layout.FormLayout;
 import com.jgoodies.forms.layout.RowSpec;
 
 /**
- * @author Oscar
- *
- *         Dialog for merging Bibtex entry with data fetched from DOI
+ * Dialog for merging Bibtex entry with data fetched from DOI
  */
 public class MergeEntryDOIDialog extends JDialog {
 
@@ -72,7 +70,7 @@ public class MergeEntryDOIDialog extends JDialog {
 
         this.originalEntry = panel.getSelectedEntries().get(0);
         panel.output(Localization.lang("Fetching info based on DOI"));
-        this.doiEntry = doiFetcher.getEntryFromDOI(this.originalEntry.getField("doi"), null);
+        this.doiEntry = doiFetcher.getEntryFromDOI(this.originalEntry.getField("doi")).orElse(null);
 
         if (this.doiEntry == null) {
             panel.output("");
diff --git a/src/main/java/net/sf/jabref/importer/fetcher/DOItoBibTeXFetcher.java b/src/main/java/net/sf/jabref/importer/fetcher/DOItoBibTeXFetcher.java
index d83c1c7719..5ad05a0eb6 100644
--- a/src/main/java/net/sf/jabref/importer/fetcher/DOItoBibTeXFetcher.java
+++ b/src/main/java/net/sf/jabref/importer/fetcher/DOItoBibTeXFetcher.java
@@ -1,28 +1,10 @@
-/*  Copyright (C) 2014 JabRef contributors.
-    Copyright (C) 2015 Oliver Kopp
-
-    This program is free software: you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation, either version 3 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
 package net.sf.jabref.importer.fetcher;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.MalformedURLException;
-import java.net.URI;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
-import java.util.Objects;
 import java.util.Optional;
 
 import javax.swing.JPanel;
@@ -50,24 +32,21 @@ public class DOItoBibTeXFetcher implements EntryFetcher {
     private final ProtectTermsFormatter protectTermsFormatter = new ProtectTermsFormatter();
     private final UnitsToLatexFormatter unitsToLatexFormatter = new UnitsToLatexFormatter();
 
-
     @Override
     public void stopFetching() {
-        // nothing needed as the fetching is a single HTTP GET
+        // not needed as the fetching is a single HTTP GET
     }
 
     @Override
     public boolean processQuery(String query, ImportInspector inspector, OutputPrinter status) {
         ParserResult parserResult = new ParserResult();
-        BibEntry entry = getEntryFromDOI(query, parserResult);
-        if(parserResult.hasWarnings()) {
+        Optional<BibEntry> entry = getEntryFromDOI(query, parserResult);
+        if (parserResult.hasWarnings()) {
             status.showMessage(parserResult.getErrorMessage());
         }
-        if (entry != null) {
-            inspector.addEntry(entry);
-            return true;
-        }
-        return false;
+        entry.ifPresent(e -> inspector.addEntry(e));
+
+        return entry.isPresent();
     }
 
     @Override
@@ -86,67 +65,73 @@ public class DOItoBibTeXFetcher implements EntryFetcher {
         return null;
     }
 
-    public BibEntry getEntryFromDOI(String doiStr, ParserResult parserResult) {
-        Objects.requireNonNull(parserResult);
-
-        DOI doi;
-        try {
-            doi = new DOI(doiStr);
-        } catch (IllegalArgumentException e) {
-            parserResult.addWarning(Localization.lang("Invalid DOI: '%0'.", doiStr));
-            return null;
-        }
+    public Optional<BibEntry> getEntryFromDOI(String doiStr) {
+        return getEntryFromDOI(doiStr, null);
+    }
 
-        // Send the request
+    public Optional<BibEntry> getEntryFromDOI(String doiStr, ParserResult parserResult) {
+        Optional<DOI> doi = DOI.build(doiStr);
 
-        // construct URL
-        URL url;
-        try {
-            Optional<URI> uri = doi.getURI();
-            if (uri.isPresent()) {
-                url = uri.get().toURL();
-            } else {
-                return null;
+        if (!doi.isPresent()) {
+            if (parserResult != null) {
+                parserResult.addWarning(Localization.lang("Invalid DOI: '%0'.", doiStr));
             }
-        } catch (MalformedURLException e) {
-            LOGGER.warn("Bad URL", e);
-            return null;
+            return Optional.empty();
         }
 
-        String bibtexString = "";
         try {
-            URLDownload dl = new URLDownload(url);
-            dl.addParameters("Accept", "application/x-bibtex");
-            bibtexString = dl.downloadToString(StandardCharsets.UTF_8);
+            URL doiURL = new URL(doi.get().getURIAsASCIIString());
+
+            // BibTeX data
+            URLDownload download = new URLDownload(doiURL);
+            download.addParameters("Accept", "application/x-bibtex");
+            String bibtexString = download.downloadToString(StandardCharsets.UTF_8);
+            bibtexString = cleanupEncoding(bibtexString);
+
+            // BibTeX entry
+            BibEntry entry = BibtexParser.singleFromString(bibtexString);
+
+            if (entry == null) {
+                return Optional.empty();
+            }
+            // Optionally re-format BibTeX entry
+            formatTitleField(entry);
+
+            return Optional.of(entry);
+        } catch (MalformedURLException e) {
+            LOGGER.warn("Bad DOI URL", e);
+            return Optional.empty();
         } catch (FileNotFoundException e) {
-            parserResult.addWarning(Localization.lang("Unknown DOI: '%0'.", doi.getDOI()));
+            if (parserResult != null) {
+                parserResult.addWarning(Localization.lang("Unknown DOI: '%0'.", doi.get().getDOI()));
+            }
             LOGGER.debug("Unknown DOI", e);
-            return null;
+            return Optional.empty();
         } catch (IOException e) {
             LOGGER.warn("Communication problems", e);
-            return null;
+            return Optional.empty();
         }
+    }
 
-        //Usually includes an en-dash in the page range. Char is in cp1252 but not
+    private void formatTitleField(BibEntry entry) {
+        // Optionally add curly brackets around key words to keep the case
+        entry.getFieldOptional("title").ifPresent(title -> {
+            // Unit formatting
+            if (Globals.prefs.getBoolean(JabRefPreferences.USE_UNIT_FORMATTER_ON_SEARCH)) {
+                title = unitsToLatexFormatter.format(title);
+            }
+
+            // Case keeping
+            if (Globals.prefs.getBoolean(JabRefPreferences.USE_CASE_KEEPER_ON_SEARCH)) {
+                title = protectTermsFormatter.format(title);
+            }
+            entry.setField("title", title);
+        });
+    }
+
+    private String cleanupEncoding(String bibtex) {
+        // Usually includes an en-dash in the page range. Char is in cp1252 but not
         // ISO 8859-1 (which is what latex expects). For convenience replace here.
-        bibtexString = bibtexString.replaceAll("(pages=\\\\{[0-9]+)\\u2013([0-9]+\\\\})", "$1--$2");
-        BibEntry entry = BibtexParser.singleFromString(bibtexString);
-
-        if (entry != null) {
-            // Optionally add curly brackets around key words to keep the case
-            entry.getFieldOptional("title").ifPresent(title -> {
-                // Unit formatting
-                if (Globals.prefs.getBoolean(JabRefPreferences.USE_UNIT_FORMATTER_ON_SEARCH)) {
-                    title = unitsToLatexFormatter.format(title);
-                }
-
-                // Case keeping
-                if (Globals.prefs.getBoolean(JabRefPreferences.USE_CASE_KEEPER_ON_SEARCH)) {
-                    title = protectTermsFormatter.format(title);
-                }
-                entry.setField("title", title);
-            });
-        }
-        return entry;
+        return bibtex.replaceAll("(pages=\\\\{[0-9]+)\\u2013([0-9]+\\\\})", "$1--$2");
     }
 }
diff --git a/src/main/java/net/sf/jabref/importer/fileformat/PdfContentImporter.java b/src/main/java/net/sf/jabref/importer/fileformat/PdfContentImporter.java
index a9a412ee97..08a8856998 100644
--- a/src/main/java/net/sf/jabref/importer/fileformat/PdfContentImporter.java
+++ b/src/main/java/net/sf/jabref/importer/fileformat/PdfContentImporter.java
@@ -199,8 +199,8 @@ public class PdfContentImporter extends ImportFormat {
             Optional<DOI> doi = DOI.findInText(firstPageContents);
             if (doi.isPresent()) {
                 ParserResult parserResult = new ParserResult(result);
-                BibEntry entry = DOI_TO_BIBTEX_FETCHER.getEntryFromDOI(doi.get().getDOI(), parserResult);
-                parserResult.getDatabase().insertEntry(entry);
+                Optional<BibEntry> entry = DOI_TO_BIBTEX_FETCHER.getEntryFromDOI(doi.get().getDOI(), parserResult);
+                entry.ifPresent(e -> parserResult.getDatabase().insertEntry(e));
                 return parserResult;
             }
 
diff --git a/src/main/java/net/sf/jabref/logic/fulltext/DoiResolution.java b/src/main/java/net/sf/jabref/logic/fulltext/DoiResolution.java
index 0ee73cfa7a..f4e3098fe7 100644
--- a/src/main/java/net/sf/jabref/logic/fulltext/DoiResolution.java
+++ b/src/main/java/net/sf/jabref/logic/fulltext/DoiResolution.java
@@ -48,7 +48,7 @@ public class DoiResolution implements FullTextFinder {
         Optional<DOI> doi = DOI.build(entry.getField("doi"));
 
         if(doi.isPresent()) {
-            String sciLink = doi.get().getURLAsASCIIString();
+            String sciLink = doi.get().getURIAsASCIIString();
 
             // follow all redirects and scan for a single pdf link
             if (!sciLink.isEmpty()) {
diff --git a/src/main/java/net/sf/jabref/logic/layout/format/DOICheck.java b/src/main/java/net/sf/jabref/logic/layout/format/DOICheck.java
index d3c9d22cc4..4ac3f9ce7c 100644
--- a/src/main/java/net/sf/jabref/logic/layout/format/DOICheck.java
+++ b/src/main/java/net/sf/jabref/logic/layout/format/DOICheck.java
@@ -33,6 +33,6 @@ public class DOICheck implements LayoutFormatter {
         if (result.startsWith("/")) {
             result = result.substring(1);
         }
-        return DOI.build(result).map(DOI::getURLAsASCIIString).orElse(result);
+        return DOI.build(result).map(DOI::getURIAsASCIIString).orElse(result);
     }
 }
diff --git a/src/main/java/net/sf/jabref/logic/util/DOI.java b/src/main/java/net/sf/jabref/logic/util/DOI.java
index 55cbfc614f..08814fa5f4 100644
--- a/src/main/java/net/sf/jabref/logic/util/DOI.java
+++ b/src/main/java/net/sf/jabref/logic/util/DOI.java
@@ -163,7 +163,7 @@ public class DOI {
      *
      * @return an encoded URL representation of the DOI
      */
-    public String getURLAsASCIIString() {
+    public String getURIAsASCIIString() {
         return getURI().map(URI::toASCIIString).orElse("");
     }
 }
diff --git a/src/test/java/net/sf/jabref/logic/util/DOITest.java b/src/test/java/net/sf/jabref/logic/util/DOITest.java
index 859905ca87..b679ca5685 100644
--- a/src/test/java/net/sf/jabref/logic/util/DOITest.java
+++ b/src/test/java/net/sf/jabref/logic/util/DOITest.java
@@ -95,23 +95,23 @@ public class DOITest {
     public void correctlyEncodeDOIs() {
         // See http://www.doi.org/doi_handbook/2_Numbering.html#2.5.2.4
         // % -> (%25)
-        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%25.0001", new DOI("http://doi.org/10.1006/rwei.1999%25.0001").getURLAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%25.0001", new DOI("http://doi.org/10.1006/rwei.1999%25.0001").getURIAsASCIIString());
         // " -> (%22)
-        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%22.0001", new DOI("http://doi.org/10.1006/rwei.1999%22.0001").getURLAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%22.0001", new DOI("http://doi.org/10.1006/rwei.1999%22.0001").getURIAsASCIIString());
         // # -> (%23)
-        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%23.0001", new DOI("http://doi.org/10.1006/rwei.1999%23.0001").getURLAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%23.0001", new DOI("http://doi.org/10.1006/rwei.1999%23.0001").getURIAsASCIIString());
         // SPACE -> (%20)
-        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%20.0001", new DOI("http://doi.org/10.1006/rwei.1999%20.0001").getURLAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%20.0001", new DOI("http://doi.org/10.1006/rwei.1999%20.0001").getURIAsASCIIString());
         // ? -> (%3F)
-        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%3F.0001", new DOI("http://doi.org/10.1006/rwei.1999%3F.0001").getURLAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1006/rwei.1999%3F.0001", new DOI("http://doi.org/10.1006/rwei.1999%3F.0001").getURIAsASCIIString());
     }
 
     @Test
     public void constructCorrectURLForDoi() {
         // add / to RESOLVER url if missing
-        Assert.assertEquals("http://doi.org/10.1006/jmbi.1998.2354", new DOI("10.1006/jmbi.1998.2354").getURLAsASCIIString());
-        Assert.assertEquals("http://doi.org/10.1006/jmbi.1998.2354", new DOI("http://doi.org/10.1006/jmbi.1998.2354").getURLAsASCIIString());
-        Assert.assertEquals("http://doi.org/10.1109/VLHCC.2004.20", new DOI("doi:10.1109/VLHCC.2004.20").getURLAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1006/jmbi.1998.2354", new DOI("10.1006/jmbi.1998.2354").getURIAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1006/jmbi.1998.2354", new DOI("http://doi.org/10.1006/jmbi.1998.2354").getURIAsASCIIString());
+        Assert.assertEquals("http://doi.org/10.1109/VLHCC.2004.20", new DOI("doi:10.1109/VLHCC.2004.20").getURIAsASCIIString());
     }
 
     @Test