Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetcher OttoBib #5125

Closed
wants to merge 10 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import org.slf4j.LoggerFactory;

/**
* Fetcher for ISBN trying ebook.de first and then chimbori.com
* Fetcher for ISBN trying ebook.de first, chimbori.com and then ottobib
*/
public class IsbnFetcher implements EntryBasedFetcher, IdBasedFetcher {

Expand Down Expand Up @@ -47,12 +47,19 @@ public Optional<BibEntry> performSearchById(String identifier) throws FetcherExc

IsbnViaEbookDeFetcher isbnViaEbookDeFetcher = new IsbnViaEbookDeFetcher(importFormatPreferences);
Optional<BibEntry> bibEntry = isbnViaEbookDeFetcher.performSearchById(identifier);

// nothing found at ebook.de, try chimbori.com
if (!bibEntry.isPresent()) {
LOGGER.debug("No entry found at ebook.de try chimbori.com");
IsbnViaChimboriFetcher isbnViaChimboriFetcher = new IsbnViaChimboriFetcher(importFormatPreferences);
bibEntry = isbnViaChimboriFetcher.performSearchById(identifier);
}

//nothing found at ebook.de and chimbori.com, try ottobib
if (!bibEntry.isPresent()) {
LOGGER.debug("No entry found at ebook.de and chimbori.com try ottobib");
IsbnViaOttoBibFetcher isbnViaOttoBibFetcher = new IsbnViaOttoBibFetcher(importFormatPreferences);
bibEntry = isbnViaOttoBibFetcher.performSearchById(identifier);
}

return bibEntry;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package org.jabref.logic.importer.fetcher;

import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.List;
import java.util.Optional;

import org.jabref.logic.importer.FetcherException;
import org.jabref.logic.importer.ImportFormatPreferences;
import org.jabref.logic.importer.ParseException;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.strings.StringUtil;

import com.mashape.unirest.http.HttpResponse;
import com.mashape.unirest.http.Unirest;
import com.mashape.unirest.http.exceptions.UnirestException;

/**
* Fetcher for ISBN using https://www.ottobib.com
*/
public class IsbnViaOttoBibFetcher extends AbstractIsbnFetcher {

public IsbnViaOttoBibFetcher(ImportFormatPreferences importFormatPreferences) {
super(importFormatPreferences);
}

@Override
public String getName() {
return "ISBN (OttoBib)";
}

/**
* @return null, because the identifier is passed using form data. This method is not used.
*/
@Override
public URL getURLForID(String identifier) throws URISyntaxException, MalformedURLException, FetcherException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove these exceptions. They can never be thrown.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LinusDietz You are right that those exceptions can never be thrown, but the Exception declaration is coming from the interface IdBasedParserFetcher

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it not possible to implement the ottobib fetcher by properly overwriting getURLForID as e.g here:

@Override
public URL getURLForID(String identifier) throws URISyntaxException, MalformedURLException, FetcherException {
this.ensureThatIsbnIsValid(identifier);
URIBuilder uriBuilder = new URIBuilder(BASE_URL);
uriBuilder.addParameter("isbn", identifier);
return uriBuilder.build().toURL();
}

I don't really see why the performSearchById has to be overwritten (which is more work as you have to manually post the request and parse the response).

return null;
}

@Override
public Optional<BibEntry> performSearchById(String identifier) throws FetcherException {
if (StringUtil.isBlank(identifier)) {
return Optional.empty();
}

this.ensureThatIsbnIsValid(identifier);

HttpResponse<String> postResponse;

String BASE_URL = "https://www.ottobib.com/isbn/" + identifier + "/bibtex";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would propose to make BASE_URL final.

Also, I would prefer a String.format(): String.format("https://www.ottobib.com/isbn/%s/bibtex", identifier);


try {
postResponse = Unirest.post(BASE_URL)
.asString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put this into the line before

} catch (UnirestException e) {
throw new FetcherException("Could not retrieve data from ottobib.com", e);
}
if (postResponse.getStatus() != 200) {
throw new FetcherException("Error while retrieving data from ottobib.com: " + postResponse.getBody());
}

List<BibEntry> fetchedEntries;
try {
fetchedEntries = getParser().parseEntries(postResponse.getRawBody());
} catch (ParseException e) {
throw new FetcherException("An internal parser error occurred", e);
}
if (fetchedEntries.isEmpty()) {
return Optional.empty();
} else if (fetchedEntries.size() > 1) {
LOGGER.info("Fetcher " + getName() + "found more than one result for identifier " + identifier
+ ". We will use the first entry.");
}

BibEntry entry = fetchedEntries.get(0);

// ottobib does not return an ISBN.
entry.setField("isbn", identifier);

doPostCleanup(entry);

return Optional.of(entry);
}

@Override
public void doPostCleanup(BibEntry entry) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove that method if it's not used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the method I was not using. I'll do the unit test soon.


}

}
2 changes: 2 additions & 0 deletions src/test/java/org/jabref/logic/importer/WebFetchersTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.jabref.logic.importer.fetcher.AbstractIsbnFetcher;
import org.jabref.logic.importer.fetcher.IsbnViaChimboriFetcher;
import org.jabref.logic.importer.fetcher.IsbnViaEbookDeFetcher;
import org.jabref.logic.importer.fetcher.IsbnViaOttoBibFetcher;
import org.jabref.logic.importer.fetcher.MrDLibFetcher;

import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -36,6 +37,7 @@ void getIdBasedFetchersReturnsAllFetcherDerivingFromIdBasedFetcher() throws Exce
// Remove special ISBN fetcher since we don't want to expose them to the user
expected.remove(IsbnViaChimboriFetcher.class);
expected.remove(IsbnViaEbookDeFetcher.class);
expected.remove(IsbnViaOttoBibFetcher.class);
assertEquals(expected, getClasses(idFetchers));
}

Expand Down