Fixes making paths of linked files relative (web urls will not be touched anymore)

[systemoperator]

Fixes #5861
Description:

• Cleaning up entries with "Make paths of linked files relative (if possible)" broke web URLs and resulted in various other issues subsequently. This PR fixes this issue.
  It has been tested for local files and web urls.

• Change in CHANGELOG.md described (if applicable)

• Manually tested changed features in running JabRef (always required)

• Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

[Siedlerchr]

Thank you very much for your contribution!
fix looks good!

[systemoperator]

Is there still something I need to do or is everything fine, because one check failed?

[Siedlerchr]

you can ignore the failing check. It's just the upload of the binaries which sometimes fails.
Our Code Quality Policy requires a review from a second reviewer for external developers before it can be merged.

I think you also should add a Changelog entry since it's at least an error from the beta/previous version

[tobiasdiez]

Thanks for your PR. The change looks good to me and I've only one small remark where an existing method can be reused.

[tobiasdiez]

The class LinkedFile already contains a method that does this job:

jabref/src/main/java/org/jabref/model/entry/LinkedFile.java

Lines 162 to 164 in 8520833

	public boolean isOnlineLink() {
	return isOnlineLink(link.get());
	}

Can you please use it. I would also suggest to add the normalization trim().toLowerCase() there, which I find a nice addition.

[systemoperator]

Concerning isOnlineLink(): I would recommend removing "toCheck.contains("www.")", because a path to a local file might contain this pattern coincidentally. What do you say?

[calixtus]

Would a proper implementation of rfc 3986 be an overkill? Maybe this could be achieved with a proper regex?
https://tools.ietf.org/html/rfc3986

Spotted this
https://stackoverflow.com/questions/30847/regex-to-validate-uris#30858

[systemoperator]

@calixtus Checking a path as follows could be a simple alternative https://www.geeksforgeeks.org/check-if-url-is-valid-or-not-in-java/ But nevertheless, I also see advantages with the current approach, since it also detects urls as web urls, even if they are potentially broken in some way. This was the reason, why I created it like that. I think the main goal is to identify a web url rather than checking its validity.

[Siedlerchr]

@systemoperator That should work using new URL() in practice and I think should be sufficient for most cases as an indicator

[systemoperator]

I e.g. think about the following scenario: Given are broken web urls like:

• https://www. google.at/ <- mind the unintended space character
• https://www..google.at/ <- mind the unintended second period
• ...

I think, in this screnario, it is more important to identify whether it is a web url or not, rather than making it more strict, saying checking whether it is a VALID web url, because in the case, that it would be a invalid web url (as e.g. shown above) then in this scenario the url would again get cleaned up and thus it will become even more "destoryed", as it would be interpreted as a local file url again and many other issues would again show up.

On the other hand, if a broken url gets identified as a web url, then it will be processed properly and everything will work out, but when opening the url, it just cannot be loaded.

In my opinion, this method is all about identifying a web url, not validating one. (That's why it is called isOnlineLink() rather than isValidOnlineLink().)

[calixtus]

Somewhere I think, you're some kind of right too.

On the other hand, where do we want to decide, what's still a acceptable and what not? What about "file://" (Im aware that normally a file path is printed without this here, but it still could happen). We also had a few months ago someone here using JabRef as database for genetics stuff and not for books (proofing the mutability of JabRef).
Please don't misunderstand me, I get your point and I don't exactly know the right answer...

[systemoperator]

Before implementing it, I thought about many of those things. With the current implementation a file link starting with "file://" will not be interpreted as a web url anyway, so this would be fine as well.

[calixtus]

At least the javadoc should then be adapted... ;-)

[systemoperator]

Of course. I would like to hear @tobiasdiez's recommendation about that as well. Then I will implement it as desired. :)

[tobiasdiez]

I think the current implementation is a good compromise. In the ideal world, the URL is valid and a proper validation as @calixtus would be preferable. However, the data quality we get from some fetchers is suboptimal so that it is a good idea to error on the save side (e.g. better identify to many paths as web urls). I suggest to merge now and come back to it later if users complain / provide problematic use cases.