Add new parser for MathSciNet search #11055

subhramit · 2024-03-19T20:43:45Z

Closes #10996
Closes #8170
The issue exposed a bug where MathSciNet search gives the following parsing error:

(Replicated locally)

To fix the issue, a new parser has been built for MathSciNet which parses the retrieved JSON results correctly. A test has been added as well.
Files modified:
jabref/src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java
jabref/src/test/java/org/jabref/logic/importer/fetcher/MathSciNet.java
The search now works fine:

Mandatory checks

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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
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.

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

calixtus · 2024-03-19T20:58:34Z

Hi, thanks for your pull request, on first look, your changes are promising. Thanks especially for providing a test with your changes. However, there are some small issues with your codestyle and the MathSciNetTest is failing. Don't worry about the other fetchertests for now.
You can read about setting up your workspace, so it automatically cares about the codestyle here:
https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace/intellij-13-code-style.html
You can also fix the OpenRewrite test by running gradle rewriteRun from the terminal/console.

subhramit · 2024-03-19T21:12:44Z

Hi, thanks for your pull request, on first look, your changes are promising. Thanks especially for providing a test with your changes. However, there are some small issues with your codestyle and the MathSciNetTest is failing. Don't worry about the other fetchertests for now. You can read about setting up your workspace, so it automatically cares about the codestyle here: https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace/intellij-13-code-style.html You can also fix the OpenRewrite test by running gradle rewriteRun from the terminal/console.

Hey, thanks! I missed the codestyle extension. I'll also have a look at the test and get back.

…issue-10996

subhramit · 2024-03-19T22:35:19Z

Done! @calixtus

CHANGELOG.md

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

src/test/java/org/jabref/logic/importer/fetcher/MathSciNetTest.java

Co-authored-by: Oliver Kopp <[email protected]>

…issue-10996

…bref into fix-for-issue-10996

Siedlerchr · 2024-03-20T18:43:01Z

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

+    private Optional<String> getOrNull(JSONObject item, List<String> keys) {
+        Object value = item;
+        for (String key : keys) {
+            if (value instanceof JSONObject) {


Suggested change

if (value instanceof JSONObject) {

if (value instanceof JSONObject obj) {

....

You can use the new instance of pattern matching syntax, which makes the extra casting step necessary
https://docs.oracle.com/en/java/javase/17/language/pattern-matching-instanceof-operator.html#GUID-843060B5-240C-4F47-A7B0-95C42E5B08A7

Hi, I just went through the docs. If I'm not wrong, did you mean that it would make the extra casting step "unnecessary"?

I removed the casts and followed the new syntax. It is odd that IntelliJ is giving me red squiggly lines and a suggestion to cast it back. Will ignore for now.

Okay nope, I added the obj syntax but seems like the cast is necessary, else I'm getting this compilation error:
error: cannot find symbol
value = value.opt(key);
^
symbol: method opt(String)
location: variable value of type Object

See the examples in the linked docs, you need to use value = obj.get... After the if

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

Co-authored-by: Oliver Kopp <[email protected]>

src/test/java/org/jabref/logic/importer/fetcher/MathSciNetTest.java

…or-issue-10996

…bref into fix-for-issue-10996

subhramit · 2024-03-21T01:40:55Z

I've made these new set of changes (wherever feasible, otherwise I've commented on your suggestions) and re-requested for review. I have learnt more from this single pull request regarding production-ready code than I ever have from my three years of CS degree. Thank you so much for your valuable inputs and patience. @koppor @Siedlerchr

Siedlerchr · 2024-03-21T08:20:51Z

src/test/java/org/jabref/logic/importer/fetcher/MathSciNetTest.java

-        Optional<BibEntry> fetchedEntry = fetcher.performSearchById("3537908");
-        assertEquals(Optional.of(ratiuEntry), fetchedEntry);
+    void getParser() throws Exception {
+        String fileName = "/importer/fetcher/jsonTest.json";


Java has a special folder for resources src/main/resoures and for test src/test/resources See the other importer test files there

koppor

Some small code comments

koppor · 2024-03-21T08:33:53Z

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

+        return Optional.of(authorsString);
+    }
+
+    private String getKeywords(JSONObject primaryClass) {


Please also use the Optional<String> thing you did at toAuhtors. Because of consistency.

koppor · 2024-03-21T08:34:55Z

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

+                // Skip author and keywords fields as they are already set
+                if (field == StandardField.AUTHOR || field == StandardField.KEYWORDS) {
+                    continue;
+                }


Just remove StandardField.AUTHOR and StandardField.KEYWORDS from FIELD_MAPPING. Think, this is the only "use" in the code, which savely can be removed.

koppor · 2024-03-21T08:37:11Z

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

+            // Handle articleUrl and mrnumber fields separately, as they are non-nested properties in the JSON and can be retrieved as Strings directly
+            String doi = item.optString("articleUrl", "");
+            if (!doi.isEmpty()) {
+                entry.setField(StandardField.DOI, doi);


Try to route through DOI#parse. If that optional is present, use doi.getNormalized(), otherwise use the doi variable. In That way, the http prefix is removed if it is a valid doi, but the full string is kept in case of a DOI parsing error.

koppor · 2024-03-21T08:37:51Z

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java

+        return Optional.empty();
+    }
+
+    // Method to change character set, to fix output string encoding


Convert to full JavaDoc:

/** * ... text ... */

…issue-10996

…bref into fix-for-issue-10996

subhramit · 2024-03-21T15:16:32Z

Made the third round of changes. @koppor @Siedlerchr
Awaiting more comments if any!

fix tests, rename resources

Siedlerchr · 2024-03-21T22:13:07Z

I took the opportunity to fix some issues:

You probably accidentally removed the BibTex Parser. Some json resutls already deliver
I renamed the resource file
Checked the test and found that the search by id returns a json which contains bibtex that we can direclty throw at our parser

obsolete

subhramit · 2024-03-21T22:20:02Z

I took the opportunity to fix some issues:

You probably accidentally removed the BibTex Parser. Some json resutls already deliver

I renamed the resource file

Checked the test and found that the search by id returns a json which contains bibtex that we can direclty throw at our parser

Thank you so much. (1) is a huge one, don't know if this was because of sleepless coding or a slip of keyboard. (2) will remember this convention. (3) nice catch!

subhramit added 3 commits March 20, 2024 01:54

Fix MathSciNet parser

b22a6ff

Merge branch 'main' into fix-for-issue-10996

8ece060

Conformed with stylechecks

3de80ef

Siedlerchr reviewed Mar 19, 2024

View reviewed changes

src/main/java/org/jabref/logic/importer/fetcher/MathSciNet.java Outdated Show resolved Hide resolved

subhramit added 2 commits March 20, 2024 03:18

Merge branch 'main' of https://github.com/JabRef/jabref into fix-for-…

bff1b11

…issue-10996

Fix Objects import missing

3dc1f55

subhramit force-pushed the fix-for-issue-10996 branch from e421224 to 3dc1f55 Compare March 19, 2024 22:03

Run gradle reWriteRun

ec1b30e

koppor requested changes Mar 20, 2024

View reviewed changes

subhramit and others added 4 commits March 20, 2024 20:06

Update BibEntry type set (via constructor)

95ac88c

Co-authored-by: Oliver Kopp <[email protected]>

Merge branch 'main' of https://github.com/JabRef/jabref into fix-for-…

8b6317d

…issue-10996

Merge branch 'fix-for-issue-10996' of https://github.com/subhramit/ja…

a04b364

…bref into fix-for-issue-10996

Apply review changes

e1e1daa

subhramit force-pushed the fix-for-issue-10996 branch from cbb8a05 to e1e1daa Compare March 20, 2024 18:21

subhramit requested a review from koppor March 20, 2024 18:27

Siedlerchr reviewed Mar 20, 2024

View reviewed changes

koppor requested changes Mar 20, 2024

View reviewed changes

subhramit and others added 3 commits March 21, 2024 02:35

Update value:String to value:Optional<String>

e6a2ec7

Co-authored-by: Oliver Kopp <[email protected]>

Change value setting to lambda form

4997846

Co-authored-by: Oliver Kopp <[email protected]>

Update missing Optional.of()

bd801b6

Co-authored-by: Oliver Kopp <[email protected]>

Siedlerchr reviewed Mar 20, 2024

View reviewed changes

src/test/java/org/jabref/logic/importer/fetcher/MathSciNetTest.java Outdated Show resolved Hide resolved

subhramit and others added 6 commits March 21, 2024 03:27

Update instanceof pattern matching syntax, removed explicit casts

021fd2f

Merge branch 'main' of https://github.com/subhramit/jabref into fix-f…

6d70470

…or-issue-10996

applied second round of review changes

dd32e6a

Merge branch 'JabRef:main' into fix-for-issue-10996

d496968

Merge branch 'fix-for-issue-10996' of https://github.com/subhramit/ja…

aafc8aa

…bref into fix-for-issue-10996

Merge branch 'main' of https://github.com/JabRef/jabref

4e154bd

Merge branch 'main' into fix-for-issue-10996

9a119ba

subhramit requested review from koppor and Siedlerchr March 21, 2024 01:27

Siedlerchr reviewed Mar 21, 2024

View reviewed changes

koppor previously requested changes Mar 21, 2024

View reviewed changes

subhramit and others added 6 commits March 21, 2024 20:24

Merge branch 'JabRef:main' into fix-for-issue-10996

25afe1c

Changes as per third review round

a6299eb

Merge branch 'main' of https://github.com/JabRef/jabref into fix-for-…

d1709ca

…issue-10996

Merge branch 'main' of https://github.com/JabRef/jabref

4eb516b

Merge branch 'main' into fix-for-issue-10996

cde25ee

Merge branch 'fix-for-issue-10996' of https://github.com/subhramit/ja…

95b9288

…bref into fix-for-issue-10996

subhramit requested review from koppor and Siedlerchr March 21, 2024 15:16

Readd bibtex parsing

73c165d

fix tests, rename resources

Siedlerchr enabled auto-merge March 21, 2024 22:16

Siedlerchr approved these changes Mar 21, 2024

View reviewed changes

Siedlerchr added this pull request to the merge queue Mar 21, 2024

Merged via the queue into JabRef:main with commit 295035a Mar 21, 2024
20 of 21 checks passed

subhramit deleted the fix-for-issue-10996 branch March 21, 2024 22:56

subhramit added search fetcher labels Aug 16, 2024

subhramit changed the title ~~Fix for MathSciNet search~~ Add new parser for MathSciNet search Aug 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new parser for MathSciNet search #11055

Add new parser for MathSciNet search #11055

subhramit commented Mar 19, 2024 •

edited

Loading

calixtus commented Mar 19, 2024 •

edited

Loading

subhramit commented Mar 19, 2024

subhramit commented Mar 19, 2024

Siedlerchr Mar 20, 2024

subhramit Mar 20, 2024 •

edited

Loading

subhramit Mar 20, 2024

subhramit Mar 21, 2024

Siedlerchr Mar 21, 2024

subhramit Mar 21, 2024

subhramit commented Mar 21, 2024 •

edited

Loading

Siedlerchr Mar 21, 2024

koppor left a comment

koppor Mar 21, 2024

koppor Mar 21, 2024

koppor Mar 21, 2024

koppor Mar 21, 2024

subhramit commented Mar 21, 2024

Siedlerchr commented Mar 21, 2024

subhramit commented Mar 21, 2024

	if (value instanceof JSONObject) {
	if (value instanceof JSONObject obj) {
	....

Add new parser for MathSciNet search #11055

Add new parser for MathSciNet search #11055

Conversation

subhramit commented Mar 19, 2024 • edited Loading

Mandatory checks

calixtus commented Mar 19, 2024 • edited Loading

subhramit commented Mar 19, 2024

subhramit commented Mar 19, 2024

Choose a reason for hiding this comment

subhramit Mar 20, 2024 • 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

subhramit commented Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

koppor left a 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

subhramit commented Mar 21, 2024

Siedlerchr commented Mar 21, 2024

subhramit commented Mar 21, 2024

subhramit commented Mar 19, 2024 •

edited

Loading

calixtus commented Mar 19, 2024 •

edited

Loading

subhramit Mar 20, 2024 •

edited

Loading

subhramit commented Mar 21, 2024 •

edited

Loading