Fixed the DOI connection Error by Automatically Removing all Special Characters at the end of DOI #8215

jiezheng5 · 2021-11-07T06:08:03Z

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

jiezheng5 · 2021-11-07T09:07:21Z

I fixed all the checkstyle errors.

Siedlerchr · 2021-11-07T13:44:31Z

src/main/java/org/jabref/model/entry/identifier/DOI.java

+            }
+        }
+
+//        char[] lcharDoi = doiStr.toCharArray();


remove commented out code

This is not really in the spirit of open source contribution. You requested to work on this issue after me and I have started working on it.

This is not really in the spirit of open source contribution. You requested to work on this issue after me and I have started working on it.

I started working on this issue 3 weeks ago. Then I saw your request, I thought maybe I should also post my progress. Do we have to get permission before we can work on any open issues? This is my first time contributing to open source, I am sorry that if I have done something wrong.

thanks,

remove commented out code

thanks very much for the suggestions, will do.

and I have started working on it.

@mrcstan May we see your approach? Maybe it differs significantly of this one - and we can make use of your ideas, too?

…issue8127

koppor · 2021-11-08T05:20:01Z

.idea/runConfigurations/JabRef_Main.xml

@@ -2,7 +2,7 @@
  <configuration default="false" name="JabRef Main" type="Application" factoryName="Application" singleton="true">
    <option name="MAIN_CLASS_NAME" value="org.jabref.gui.JabRefMain" />
    <module name="JabRef.main" />
-    <option name="VM_PARAMETERS" value="--patch-module test=fastparse_2.12-1.0.0.jar --patch-module test2=fastparse-utils_2.12-1.0.0.jar --patch-module test3=sourcecode_2.12-0.1.4.jar --patch-module org.jabref=build/resources/main --add-exports javafx.controls/com.sun.javafx.scene.control=org.jabref --add-exports org.controlsfx.controls/impl.org.controlsfx.skin=org.jabref --add-opens javafx.controls/javafx.scene.control=org.jabref --add-opens org.controlsfx.controls/org.controlsfx.control.textfield=org.jabref --add-exports javafx.graphics/com.sun.javafx.scene=org.controlsfx.controls --add-exports javafx.graphics/com.sun.javafx.scene.traversal=org.controlsfx.controls --add-exports javafx.graphics/com.sun.javafx.css=org.controlsfx.controls --add-exports javafx.controls/com.sun.javafx.scene.control.behavior=org.controlsfx.controls --add-exports javafx.controls/com.sun.javafx.scene.control=org.controlsfx.controls --add-exports javafx.controls/com.sun.javafx.scene.control.inputmap=org.controlsfx.controls --add-exports javafx.base/com.sun.javafx.event=org.controlsfx.controls --add-exports javafx.base/com.sun.javafx.collections=org.controlsfx.controls --add-exports javafx.base/com.sun.javafx.runtime=org.controlsfx.controls --add-exports javafx.web/com.sun.webkit=org.controlsfx.controls --add-exports javafx.graphics/com.sun.javafx.css=org.controlsfx.controls --add-opens javafx.controls/javafx.scene.control.skin=org.controlsfx.controls --add-opens javafx.graphics/javafx.scene=org.controlsfx.controls --add-exports com.oracle.truffle.regex/com.oracle.truffle.regex=org.graalvm.truffle --add-exports javafx.graphics/com.sun.javafx.stage=com.jfoenix --add-exports javafx.controls/com.sun.javafx.scene.control.behavior=com.jfoenix" />
+    <option name="VM_PARAMETERS" value="--patch-module test=fastparse_2.12-1.0.0.jar --patch-module test2=fastparse-utils_2.12-1.0.0.jar --patch-module test3=sourcecode_2.12-0.1.4.jar --patch-module org.jabref=build/resources/main --add-exports javafx.controls/com.sun.javafx.scene.control=org.jabref --add-exports org.controlsfx.controls/impl.org.controlsfx.skin=org.jabref --add-opens javafx.controls/javafx.scene.control=org.jabref --add-opens org.controlsfx.controls/org.controlsfx.control.textfield=org.jabref --add-exports javafx.graphics/com.sun.javafx.scene=org.controlsfx.controls --add-exports javafx.graphics/com.sun.javafx.scene.traversal=org.controlsfx.controls --add-exports javafx.graphics/com.sun.javafx.css=org.controlsfx.controls --add-exports javafx.controls/com.sun.javafx.scene.control.behavior=org.controlsfx.controls --add-exports javafx.controls/com.sun.javafx.scene.control=org.controlsfx.controls --add-exports javafx.controls/com.sun.javafx.scene.control.inputmap=org.controlsfx.controls --add-exports javafx.base/com.sun.javafx.event=org.controlsfx.controls --add-exports javafx.base/com.sun.javafx.collections=org.controlsfx.controls --add-exports javafx.base/com.sun.javafx.runtime=org.controlsfx.controls --add-exports javafx.web/com.sun.webkit=org.controlsfx.controls --add-exports javafx.graphics/com.sun.javafx.css=org.controlsfx.controls --add-opens javafx.controls/javafx.scene.control.skin=org.controlsfx.controls --add-opens javafx.graphics/javafx.scene=org.controlsfx.controls --add-exports com.oracle.truffle.regex/com.oracle.truffle.regex=org.graalvm.truffle --add-exports javafx.graphics/com.sun.javafx.stage=com.jfoenix --add-exports javafx.controls/com.sun.javafx.scene.control.behavior=com.jfoenix -ea --add-exports=javafx.controls/com.sun.javafx.scene.control=org.jabref --add-exports=org.controlsfx.controls/impl.org.controlsfx.skin=org.jabref --add-exports javafx.graphics/com.sun.javafx.application=ALL-UNNAMED --add-exports javafx.graphics/com.sun.javafx.stage=ALL-UNNAMED --add-exports javafx.graphics/com.sun.javafx.stage=com.jfoenix" />


May I ask of the reasons why this file mas modified? IntelliJ runs fine as is.

May I ask of the reasons why this file mas modified? IntelliJ runs fine as is.

Hi, Kopper:

Thanks very much for the feedback. I think IntelliJ automatically did it. I had lots of trouble running using IntelliJ both on windows and Linux. So I just used gradlew to run, which worked perfectly.

Thanks

koppor · 2021-11-08T05:20:15Z

CHANGELOG.md

@@ -44,6 +44,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

 ### Fixed

+- We fixed an issue where an invalid DOI should not show "Connection error" [#8127](https://github.com/JabRef/jabref/issues/8127)
+- We fixed an issue where an exception ocurred when a linked online file was edited in the entry editor [#8008](https://github.com/JabRef/jabref/issues/8008)


Please remove this duplicted line

sure, will do. Thanks very much.

koppor · 2021-11-08T05:20:26Z

src/main/java/org/jabref/model/entry/identifier/DOI.java

@@ -106,6 +106,10 @@ public DOI(String doi) {
        // Remove whitespace
        String trimmedDoi = doi.trim();

+        // jie add-------------------------


Please do not add author markers

thanks very much for your patience, I forgot to remove it after debugging. will do.

have a great night.

koppor · 2021-11-08T05:21:26Z

src/main/java/org/jabref/model/entry/identifier/DOI.java

+    /**
+     * Removes all special characters at the end of the DOI string
+     *
+     * @param doiStr the DOI/Short DOI string
+     * @return an DOI string with the special character at the end removed
+     */
+    public static String removeScharDOI(String doiStr) {
+        // jie add
+        char[] lcharDoi = doiStr.toCharArray();
+        int i = lcharDoi.length - 1;
+        // valid DOI characters are a-z(97-122), A-Z(65-90), 0-9(48-57), -(45), .(46), _(95), ;(59), ( (40), ) (41), /(47)
+        for (; i >= 0; i--) {
+            if ((lcharDoi[i] >= 45 && lcharDoi[i] <= 47) ||
+                    (lcharDoi[i] >= 48 && lcharDoi[i] <= 57) ||
+                    (lcharDoi[i] >= 65 && lcharDoi[i] <= 90) ||
+                    (lcharDoi[i] >= 97 && lcharDoi[i] <= 122) ||
+                    lcharDoi[i] == 95 ||
+                    lcharDoi[i] == 59 ||
+                    lcharDoi[i] == 40 ||
+                    lcharDoi[i] == 41
+            ) {
+                // System.out.println(i);
+                break;
+            }
+        }


This approach feels a bit hacky. I thought that it is about DOI parsing with the RegEx. Could you please test the RegEx whether it removes the bad character?

ok, great suggestion. I will refactor the code and test the RegEx.

greatly appreciated all the suggestions

koppor · 2021-11-08T20:32:42Z

Fixes #8127

koppor · 2021-11-08T20:53:26Z

With more than 3 core developers, we looked at this issue in today's dev call. We found out, that there is DOI.findInText which resolves the issue --> #8227

@jiezheng5 Could you add your test cases to that PR? I would like to ask you to add a new test section below // findDoiInsideArbitraryText with your test cases of this branch.

koppor · 2021-11-08T20:55:34Z

I am closing this one, as we think, the tests should be merged with #8227

mrcstan · 2021-11-08T21:32:58Z

I guess I just missed this one. I created a PR that uses regexp #8288. Seems like it has been fixed elsewhere. Let me know if I can contribute anything else.

jiezheng5 · 2021-11-09T00:16:38Z

With more than 3 core developers, we looked at this issue in today's dev call. We found out, that there is DOI.findInText which resolves the issue --> #8227

@jiezheng5 Could you add your test cases to that PR? I would like to ask you to add a new test section below // findDoiInsideArbitraryText with your test cases of this branch.

my pleasure. will do.

jiezheng5 added 7 commits November 6, 2021 22:24

Fixed the DOI connection Error issue: Issue 8127

98887a9

Update CHANGELOG.md

5e1ad61

Update CHANGELOG.md

418699d

Update CHANGELOG.md

4addd9f

Merge branch 'main' into issue8127

5a3a142

fixing checkstyle error

1cb2f23

fixing stylecheck error by adding space before and after +/-

e49a1d2

jiezheng5 marked this pull request as ready for review November 7, 2021 09:06

jiezheng5 mentioned this pull request Nov 7, 2021

Invalid DOI should not show "Connection error" #8127

Closed

2 tasks

Siedlerchr reviewed Nov 7, 2021

View reviewed changes

jiezheng5 added 7 commits November 8, 2021 01:00

deleted all commented out code

302e758

deleting commented code

51ce4f2

reverted to e49a1d2 then deleted commented code

9c5cb5e

fixed url in ChangeLog.md

2068e13

adding spaces between +/- and 1

a4b8d61

modified changelog.md

c279415

Merge branch 'issue8127' of https://github.com/jiezheng5/jabref into …

aeb7630

…issue8127

koppor requested changes Nov 8, 2021

View reviewed changes

koppor mentioned this pull request Nov 8, 2021

DOI determining now uses "DOI.findInText" (and not "DOI.parse") #8227

Closed

5 tasks

koppor closed this Nov 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed the DOI connection Error by Automatically Removing all Special Characters at the end of DOI #8215

Fixed the DOI connection Error by Automatically Removing all Special Characters at the end of DOI #8215

jiezheng5 commented Nov 7, 2021 •

edited

Loading

jiezheng5 commented Nov 7, 2021

Siedlerchr Nov 7, 2021

mrcstan Nov 7, 2021

jiezheng5 Nov 8, 2021 •

edited

Loading

jiezheng5 Nov 8, 2021

koppor Nov 8, 2021

koppor Nov 8, 2021

jiezheng5 Nov 9, 2021

koppor Nov 8, 2021

jiezheng5 Nov 9, 2021

koppor Nov 8, 2021

jiezheng5 Nov 9, 2021

koppor Nov 8, 2021

jiezheng5 Nov 9, 2021

koppor commented Nov 8, 2021

koppor commented Nov 8, 2021

koppor commented Nov 8, 2021

mrcstan commented Nov 8, 2021

jiezheng5 commented Nov 9, 2021

Fixed the DOI connection Error by Automatically Removing all Special Characters at the end of DOI #8215

Fixed the DOI connection Error by Automatically Removing all Special Characters at the end of DOI #8215

Conversation

jiezheng5 commented Nov 7, 2021 • edited Loading

jiezheng5 commented Nov 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiezheng5 Nov 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented Nov 8, 2021

koppor commented Nov 8, 2021

koppor commented Nov 8, 2021

mrcstan commented Nov 8, 2021

jiezheng5 commented Nov 9, 2021

jiezheng5 commented Nov 7, 2021 •

edited

Loading

jiezheng5 Nov 8, 2021 •

edited

Loading