Skip to content

Commit

Permalink
SLUB: Get Results by Id for Lent and Reserved Items (#598)
Browse files Browse the repository at this point in the history
* SLUB: Put prefix "id" before id identifier

to prepare for different kinds of identifiers

* SLUB: Test for getResultById

This test is not ideal as the test should just verify that parseResultById
will be called with correct id but not actually call it. So it should be

Mockito.doReturn(DetailedItem()).`when`(slub).parseResultById(Matchers.any(), Matchers.any())
...
verify(slub).parseResultById("id/123", JSONObject(response))

but this throws an IllegalStateException: Mockito.any() must not be null because of
the not nullable Kotlin function signature of parseResultById. None of the solutions
from  https://stackoverflow.com/questions/30305217/is-it-possible-to-use-mockito-in-kotlin
work. See also mockito/mockito#1255.
The only possible solution seems to be switching to MockK.

* SLUB: Get actual id from JSON instead of using id passed in as argument

to call of getResultById and hence parseResultById as the actual identifier
obtained in the process may differ from the one used to call the method.

* SLUB: make getResultById work for bc identifiers

Get id identifier from bc identifier by intercepting redirect from bc to id. This requires
to unset client's followRedirections property by building a new client based on the client
of OkHttpBaseApi.

* SLUB: test code cleanup

- remove empty primary constructor
- remove redundant qualifier name
- optimize imports

* SLUB: fallback in getResultById for legacy id identifiers without prefix

which could result from items added to favorites list before commit 31d6006.
Also add final '/' to identifier when checking (in case a legacy identifier
starts with the characters 'id' which are not a prefix in this case).
Make implicit support for 'rsn' identifiers (currently not used) explicit.
  • Loading branch information
StefRe committed Nov 3, 2020
1 parent f7eb613 commit c36c0f4
Show file tree
Hide file tree
Showing 3 changed files with 233 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,70 @@ public String httpPost(String url, RequestBody data, String encoding, boolean ig
}
}

/**
* Perform a HTTP HEAD request to a given URL
*
* @param url URL to fetch
* @param name Name of the header field
* @param defaultValue Default value of this header field
* @param ignore_errors If true, status codes above 400 do not raise an exception
* @param client Http client to use
* @return Answer content
* @throws NotReachableException Thrown when server returns a HTTP status code greater or equal
* than 400.
*/
public String httpHead(String url, String name, String defaultValue, boolean ignore_errors,
OkHttpClient client) throws IOException {
Request request = new Request.Builder()
.url(cleanUrl(url))
.header("Accept", "*/*")
.header("User-Agent", getUserAgent())
.head()
.build();

try {
Response response = client.newCall(request).execute();

if (!ignore_errors && response.code() >= 400) {
throw new NotReachableException(response.message());
}

return response.header(name, defaultValue);
} catch (javax.net.ssl.SSLPeerUnverifiedException e) {
logHttpError(e);
throw new SSLSecurityException(e.getMessage());
} catch (javax.net.ssl.SSLException e) {
// Can be "Not trusted server certificate" or can be a
// aborted/interrupted handshake/connection
if (e.getMessage().contains("timed out")
|| e.getMessage().contains("reset by")) {
logHttpError(e);
throw new NotReachableException(e.getMessage());
} else {
logHttpError(e);
throw new SSLSecurityException(e.getMessage());
}
} catch (InterruptedIOException e) {
logHttpError(e);
throw new NotReachableException(e.getMessage());
} catch (UnknownHostException e) {
throw new NotReachableException(e.getMessage());
} catch (IOException e) {
if (e.getMessage() != null
&& e.getMessage().contains("Request aborted")) {
logHttpError(e);
throw new NotReachableException(e.getMessage());
} else {
throw e;
}
}
}

public String httpHead(String url, String name, String defaultValue, OkHttpClient client)
throws IOException {
return httpHead(url, name, defaultValue, false, client);
}

public CompletableFuture<Response> asyncPost(String url, RequestBody data,
final boolean ignore_errors) {
Request request = new Request.Builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ open class SLUB : OkHttpBaseApi() {
}
type = mediaTypes[it.getJSONArray("format").optString(0)]
?: SearchResult.MediaType.NONE
id = it.getString("id")
id = "id/${it.getString("id")}"
}
}
//TODO: get status (one request per item!)
Expand All @@ -159,19 +159,33 @@ open class SLUB : OkHttpBaseApi() {

override fun getResultById(id: String, homebranch: String?): DetailedItem {
val json: JSONObject
val url = when {
id.startsWith("id/") ->
"$baseurl/$id/"
id.startsWith("bc/") || id.startsWith("rsn/") ->
http_client.newBuilder()
.followRedirects(false)
.build()
.run {
httpHead("$baseurl/$id/",
"Location",
"",
this)
}
else -> // legacy case: id identifier without prefix
"$baseurl/id/$id/"
} + "?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app"
try {
json = JSONObject(httpGet(
"$baseurl/id/$id/?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app",
ENCODING))
json = JSONObject(httpGet(url, ENCODING))
} catch (e: JSONException) {
throw OpacApi.OpacErrorException(stringProvider.getFormattedString(
StringProvider.UNKNOWN_ERROR_WITH_DESCRIPTION,
"search returned malformed JSON object: ${e.message}"))
}
return parseResultById(id, json)
return parseResultById(json)
}

internal fun parseResultById(id: String, json: JSONObject): DetailedItem {
internal fun parseResultById(json: JSONObject): DetailedItem {
val dateFormat = DateTimeFormat.forPattern("dd.MM.yyyy")
var hasReservableCopies = false
fun getCopies(copiesArray: JSONArray, df: DateTimeFormatter): List<Copy> =
Expand Down Expand Up @@ -201,7 +215,7 @@ open class SLUB : OkHttpBaseApi() {
}
}
return DetailedItem().apply {
this.id = id
this.id = "id/${json.getString("id")}"
val record = json.getJSONObject("record")
for (key in record.keys()) {
var value = when (val v = record.get(key as String)) {
Expand All @@ -212,7 +226,9 @@ open class SLUB : OkHttpBaseApi() {
is String -> arrayItem
is JSONObject -> arrayItem.optString("title").also {
// if item is part of multiple collections, collectionsId holds the last one
collectionId = arrayItem.optString("id", null)
arrayItem.optString("id", null)?.let {
collectionId = "id/$it"
}
}
else -> null
}
Expand Down Expand Up @@ -420,7 +436,7 @@ open class SLUB : OkHttpBaseApi() {
reservationsList.add(ReservedItem().apply {
title = it.optString("about")
author = it.optJSONArray("X_author")?.optString(0)
//id = it.optString("label") // TODO: get details from here via /bc --> redirects to /id, from there get the proper id
id = "bc/${it.optString("label")}"
format = it.optString("X_medientyp")
status = when (type) { // TODO: maybe we need time (LocalDateTime) too make an educated guess on actual ready date for stack requests
"hold" -> stringProvider.getFormattedString(StringProvider.HOLD,
Expand Down Expand Up @@ -469,7 +485,7 @@ open class SLUB : OkHttpBaseApi() {
author = it.optJSONArray("X_author")?.optString(0)
setDeadline(it.optString("X_date_due"))
format = it.optString("X_medientyp")
//id = it.optString("label") // TODO: get details from here via /bc --> redirects to /id, from there get the proper id
id = "bc/${it.optString("label")}"
barcode = it.optString("X_barcode")
if (it.optInt("X_is_renewable") == 1) { // TODO: X_is_flrenewable for ill items
isRenewable = true
Expand Down Expand Up @@ -517,7 +533,7 @@ open class SLUB : OkHttpBaseApi() {
}

override fun getShareUrl(id: String?, title: String?): String {
return "$baseurl/id/$id"
return "$baseurl/$id"
}

override fun getSupportFlags(): Int {
Expand Down
Loading

0 comments on commit c36c0f4

Please sign in to comment.