Include https://bibtex.chimbori.com/ as fallback for the ebook.de fetcher #2374
Conversation
koppor
force-pushed
the
isbnfetcherupdate
branch
3 times, most recently
from
183c1c2 to
630e054
Compare
koppor
added
the
status: ready-for-review
koppor
force-pushed
the
isbnfetcherupdate
branch
from
630e054 to
6b78b2d
Compare
| @Override | ||
| public Parser getParser() { | ||
| return new BibtexParser(importFormatPreferences); | ||
| IsbnViaEbookDeFetcher isbnViaEbookDeFetcher = new IsbnViaEbookDeFetcher(importFormatPreferences); |
There was a problem hiding this comment.
I don't like the architecture that a fetcher controls other fetcher. All of them should be completely independent and their working-together should be controlled at a higher level. Moreover, I would allow the user to choose between "ISBN (ebook.de)" and "ISBN (Chimbori/Amazon)", or is the quality of ebook's metadata really superior?
There was a problem hiding this comment.
I think, it should be transparent to the user, so that he does not need think, what fetcher he wants to use. We could also offer all three fetchers to the user so that if he really wants to choose.
It is the deal with Chimbori that we use ebook.de first to reduce load on his server. The deal was made in personal emails I exchanged with the author of Chimbori
|
|
||
| HttpResponse<String> postResponse; | ||
| try { | ||
| postResponse = Unirest.post("https://bibtex.chimbori.com/isbn-bibtex") |
There was a problem hiding this comment.
We should rewrite IdBasedParserFetcher so that it also supports POST actions (and maybe uses Unirest for it).
There was a problem hiding this comment.
Not sure whether it is worth the effort - I think, we currently have only one exception for that.
|
|
||
| @Override | ||
| public void doPostCleanup(BibEntry entry) { | ||
| // We MUST NOT clean the URL. this is the deal with ebook.de |
There was a problem hiding this comment.
I don't see that we have such an agreement with ebook.de
There was a problem hiding this comment.
I can forward you the emails I exchanged with them.
|
Update: Fixes #2343 |
koppor
force-pushed
the
isbnfetcherupdate
branch
2 times, most recently
from
8f765b3 to
74b5527
Compare
- Introduce AbstractIsbnFetcher - Name the fetchers differently
koppor
force-pushed
the
isbnfetcherupdate
branch
from
74b5527 to
99410fb
Compare
| import net.sf.jabref.logic.l10n.Localization; | ||
| import net.sf.jabref.logic.util.ISBN; | ||
|
|
||
| public abstract class AbstractIsbnFetcher implements IdBasedParserFetcher { |
There was a problem hiding this comment.
Here IdBasedFetcher should be sufficient and then IsbnViaEbookDeFetcher implements IdBasedParserFetcher (in addition to AbstractIsbnFetcher)
There was a problem hiding this comment.
In IsbnViaChimboriFetcher, I need both doPostCleanup() and getParser() which are not offered by IdBasedFetcher.
Don't want to introduce a new intermediate class...
tobiasdiez
removed
the
status: ready-for-review
This was our conversation regarding the version that I had self-hosted, before I set up the dedicated service at bibtex.chimbori.com. Feel free to send all your traffic to this new endpoint! |
The fetcher offered by ebook.de does not offer bibtex for all valid ISBNs. This PR adds https://bibtex.chimbori.com/ as fallback for the ebook.de fetcher
Long discussion at: #684 (comment)
I created a meta IsbnFetcher and two separate fetchers for ebook.de and chimbori. I readded the URL field as this is the deal with both ebook.de and chimbori.
gradle localizationUpdate?Fixes #684