Fixes: #13673: Uncaught Exception in ThreeWayMerge dialog when looking up Bibliographic information from DOI

[LenHukiro]

Closes #13673

I added a function to check for empty year values in the YearFieldValuePlausibilityComparator, because there were only checks for the formatting of the year.
In there I added a function to check if one of the values that are being compared is empty. That should prevent the empty value exception for either of the values.
The value from the database should never be empty (the right one), but i added a failsafe just in case the value is empty in the database.

I was not sure if i needed to add something to the CHANGELOG.md, but i did just in case.

Steps to test

(on the main branch and this branch)

1. Open Jabref GUI
2. Create or import an article with a working DOI(Library-> add entry/ add entry using... or with the web search feature)
3. Then select the journal entry at the bottom page with a click
4. Empty the "Year" Textfield in the "Required Fields" Tab
5. Switch to the General Tab
6. Input the DOI (If its not already filled)
7. Click the button "Get bibliographic data from DOI"

The button in the General Tab:

The error message before the changes:

The article with the changes:

Mandatory checks

• I own the copyright of the code submitted and I license it under the MIT license
• Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
• Tests created for changes (if applicable)
• Manually tested changed features in running JabRef (always required)
• Screenshots added in PR description (if change is visible to the user)
• 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.

[LenHukiro]

I think that should be everything.
Im still not 100 % sure on the logic in the function "checkEmptyValues" in YearFieldValuePlausibilityComparator, so please correct me if im wrong there.

[HoussemNasri]

Please remove, the enum names are descriptive enough.

[HoussemNasri]

Im still not 100 % sure on the logic in the function "checkEmptyValues" in YearFieldValuePlausibilityComparator, so please correct me if im wrong there.

You are right, the comparaison logic is unnecessarily complicated. We need to do some refactoring. That's out of scope for the issue in hand, and you already addressed the DoD .i.e test case.

Edit: It seems you have misunderstood the root cause of the issue. The problem arises from calling the get() method on an empty Optional; It has nothing to do with empty or blank String values. The Optional will be empty when the validator i.e. YearChecker decides it is invalid.

[HoussemNasri]

This is not the correct fix, please read the stacktrace again and maybe run the debugger to get a better understanding of what's happening.

[trag-bot]

@trag-bot didn't find any issues in the code! ✅✨

[LenHukiro]

Edit: It seems you have misunderstood the root cause of the issue. The problem arises from calling the get() method on an Optional; It has nothing to do with empty or blank String values. The Optional will be empty when the validator i.e. YearChecker decides it is invalid.

YearChecker.checkValue only checks the formatting of the value and returns an empty value, if its correct or empty.
e.g. "" or "1999".
Since it returns empty on a correctly formatted value or an empty value i decided to add a function that checks if the value is empty or not, so it's not empty in the comparison.

[HoussemNasri]

YearChecker.checkValue only checks the formatting of the value and returns an empty value, if its correct or empty.
e.g. "" or "1999".

Please stop calling it an empty value and call it an Optional so I know we are on the same page. An empty Optional does not have anything to do with strings or "". The testcase you added checks for empty strings only.

[LenHukiro]

YearChecker.checkValue only checks the formatting of the value and returns an empty value, if its correct or empty.
e.g. "" or "1999".

Please stop calling it an empty value and call it an Optional so I know we are on the same page. An empty Optional does not have anything to do with strings or "". The testcase you added checks for empty strings only.

My mistake, i will use the correct name from now on.
But to my understanding i would need to check if the value isPresent() on the optional Integers, to prevent the NoSuchElementException in the get(), so i used .IsEmpty() to verify if the optional integers are null.

I guess i was a bit confused, since the YearChecker returns null on the string and later on i checked an optional integer and not a string.

Since this is not the intended solution i will try to find a better one.

[LenHukiro]

I'll close this PR and open a new one, since i need to redo all my changes.
I'll add this PR in the new one so it should show up here.