Skip to content

Commit

Permalink
SLUB: Add references to other SLUB catalog items as volumes
Browse files Browse the repository at this point in the history 
This is a workaround as the only possibility to add links to other detailed
items from a detailed item. It's possible as we can display volumes and
copies at the same time. References are some kind of details which is why
the display order of copies and volumes was swapped: first show volumes
(which may be references) right after the details and only then copies.
Increase volumes text view to maximum two lines high to be able to put
the title of the reference and the reference itself on different lines
for better readability.

Change resolution of bc or rsn and the new libero identifiers to following
head redirects and retrieving the final url to get the id identifier. This
requires one request more than the old method (get location parameter from
non-redirecting head request) but is much shorter to implement.

IDs of references have a single query component (e.g. ?libero_mab21364775)
and no key=value pairs. The URLEncodedUtils.format implementation used by
Android has a bug and adds a trailing equal sign (=) to the query component
which leads to infinite redirects, see opacapp#612 for details. The prevent this
we use cleanUrl only if there are any = signs in the url, otherwise we use
the raw url in head requests.
  • Loading branch information
@StefRe
StefRe committed Feb 28, 2021
1 parent 2040420 commit e897129
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 71 deletions.
19 changes: 5 additions & 14 deletions opacclient/libopac/src/main/java/de/geeksfactory/opacclient/apis/OkHttpBaseApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,31 +226,27 @@ 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
* @return Response
* @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 {
public Response httpHead(String url, boolean ignore_errors) throws IOException {
Request request = new Request.Builder()
.url(cleanUrl(url))
.url(url.contains("=") ? cleanUrl(url) : url)
.header("Accept", "*/*")
.header("User-Agent", getUserAgent())
.head()
.build();

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

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

return response.header(name, defaultValue);
return response;
} catch (javax.net.ssl.SSLPeerUnverifiedException e) {
logHttpError(e);
throw new SSLSecurityException(e.getMessage());
Expand Down Expand Up @@ -281,11 +277,6 @@ public String httpHead(String url, String name, String defaultValue, boolean ign
}
}

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
36 changes: 17 additions & 19 deletions opacclient/libopac/src/main/java/de/geeksfactory/opacclient/apis/SLUB.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,9 @@ open class SLUB : OkHttpBaseApi() {
id.startsWith("id/") ->
"$baseurl/$id/"
id.startsWith("bc/") || id.startsWith("rsn/") ->
http_client.newBuilder()
.followRedirects(false)
.build()
.run {
httpHead("$baseurl/$id/",
"Location",
"",
this)
}
httpHead("$baseurl/$id/", false).request().url().toString()
id.startsWith("http://slubdd.de/katalog?libero_mab") ->
httpHead(id, false).request().url().toString()
else -> // legacy case: id identifier without prefix
"$baseurl/id/$id/"
} + "?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app"
Expand Down Expand Up @@ -260,7 +254,7 @@ open class SLUB : OkHttpBaseApi() {
cover = this
}
}
// links and references
// links
for (link in listOf("linksRelated", "linksAccess", "linksGeneral")) {
json.getJSONArray(link).forEach<JSONObject> {
// assuming that only on of material, note or hostlabel is set
Expand All @@ -270,10 +264,6 @@ open class SLUB : OkHttpBaseApi() {
addDetail(Detail(key, it.optString("uri")))
}
}
json.getJSONArray("references").forEach<JSONObject> {
// TODO: usually links to old SLUB catalogue, does it make sense to add the link?
addDetail(Detail(it.optString("text"), "${it.optString("name")} (${it.optString("target")})"))
}
// copies
val cps = json.get("copies")
if (cps is JSONArray) {
Expand All @@ -290,11 +280,19 @@ open class SLUB : OkHttpBaseApi() {
}
isReservable = hasReservableCopies
// volumes
volumes = json.optJSONObject("parts")?.optJSONArray("records")?.map<JSONObject, Volume> {
Volume(it.optString("id"),
"${it.optString("part")} ${Parser.unescapeEntities(it.optString("name"), false)}")

} ?: emptyList()
json.getJSONObject("parts").optJSONArray("records")?.forEach<JSONObject> {
volumes.add(Volume(it.optString("id"),
"${it.optString("part")} ${Parser.unescapeEntities(it.optString("name"), false)}"))
}
// references
json.getJSONArray("references").forEach<JSONObject> {
val link = it.optString("link")
if (link.startsWith("http://slubdd.de/katalog?libero_mab")){
addVolume(Volume(link, "${it.optString("text")}:\n${it.optString("name")}"))
} else {
addDetail(Detail(it.optString("text"), "${it.optString("name")} (${it.optString("target")})"))
}
}
}
}

Expand Down
64 changes: 32 additions & 32 deletions opacclient/libopac/src/test/java/de/geeksfactory/opacclient/apis/SLUBTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ import de.geeksfactory.opacclient.objects.*
import de.geeksfactory.opacclient.searchfields.DropdownSearchField
import de.geeksfactory.opacclient.searchfields.SearchQuery
import de.geeksfactory.opacclient.searchfields.TextSearchField
import okhttp3.FormBody
import okhttp3.OkHttpClient
import okhttp3.RequestBody
import okhttp3.*
import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.*
import org.joda.time.LocalDate
Expand Down Expand Up @@ -376,7 +374,7 @@ class SLUBSearchTest : BaseHtmlTest() {
addDetail(Detail("Inhaltsverzeichnis", "http://www.gbv.de/dms/tib-ub-hannover/727434322.pdf"))
addDetail(Detail("Inhaltstext", "http://deposit.d-nb.de/cgi-bin/dokserv?id=4155321&prov=M&dok_var=1&dok_ext=htm"))
addDetail(Detail("Zugang zur Ressource (via ProQuest Ebook Central)", "http://wwwdb.dbod.de/login?url=http://slub.eblib.com/patron/FullRecord.aspx?p=1575685"))
addDetail(Detail("Online-Ausgabe", "Tamm, Michael: JUnit-Profiwissen (SLUB)"))
addVolume(Volume("http://slubdd.de/katalog?libero_mab216187885", "Online-Ausgabe:\nTamm, Michael: JUnit-Profiwissen"))
}

val item = slub.parseResultById(json)
Expand Down Expand Up @@ -822,12 +820,16 @@ class SLUBSearchMockTest(@Suppress("unused") private val name: String,

class SLUBGetResultByIdMockTest : BaseHtmlTest() {
private val slub = spy(SLUB::class.java)

private class ClientDoesntFollowRedirects : ArgumentMatcher<OkHttpClient?>() {
override fun matches(argument: Any): Boolean {
return !(argument as OkHttpClient).followRedirects()
}
}
private val mockHeadResponse = Response.Builder()
.request(Request.Builder()
.url("https://test.de/id/123/")
.build())
.protocol(Protocol.HTTP_2)
.code(200)
.message("")
.build()
val mockGetResponse = """{"record":{"title":"The title"},"id":"123","thumbnail":"","links":[],"linksRelated":[],
|"linksAccess":[],"linksGeneral":[],"references":[],"copies":[],"parts":{}}""".trimMargin()

init {
slub.init(Library().apply {
Expand All @@ -848,52 +850,50 @@ class SLUBGetResultByIdMockTest : BaseHtmlTest() {

@Test
fun testIdIdentifier() {
val response = """{"record":{"title":"The title"},"id":"123","thumbnail":"","links":[],"linksRelated":[],
|"linksAccess":[],"linksGeneral":[],"references":[],"copies":[],"parts":{}}""".trimMargin()
doReturn(response).`when`(slub).httpGet(Matchers.any(), Matchers.any())
doReturn(mockGetResponse).`when`(slub).httpGet(Matchers.any(), Matchers.any())
val actual = slub.getResultById("id/123", null)
verify(slub).httpGet("https://test.de/id/123/?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app", "UTF-8")
verify(slub, never()).httpHead(any(), any(), any(), any())
verify(slub, never()).httpHead(any(), anyBoolean())
assertEquals("id/123", actual.id)
}

@Test
fun testBcIdentifier() {
doReturn("https://test.de/id/123/").`when`(slub).httpHead(Matchers.any(),
Matchers.any(), Matchers.any(), Matchers.any())
val response = """{"record":{"title":"The title"},"id":"123","thumbnail":"","links":[],"linksRelated":[],
|"linksAccess":[],"linksGeneral":[],"references":[],"copies":[],"parts":{}}""".trimMargin()
doReturn(response).`when`(slub).httpGet(Matchers.any(), Matchers.any())
doReturn(mockHeadResponse).`when`(slub).httpHead(Matchers.any(), Matchers.anyBoolean())
doReturn(mockGetResponse).`when`(slub).httpGet(Matchers.any(), Matchers.any())
val actual = slub.getResultById("bc/456", null)
verify(slub).httpHead(eq("https://test.de/bc/456/"), eq("Location"), eq(""),
argThat(ClientDoesntFollowRedirects()))
verify(slub).httpHead(eq("https://test.de/bc/456/"), eq(false))
verify(slub).httpGet("https://test.de/id/123/?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app", "UTF-8")
assertEquals("id/123", actual.id)
}

@Test
fun testRsnIdentifier() {
doReturn("https://test.de/id/123/").`when`(slub).httpHead(Matchers.any(),
Matchers.any(), Matchers.any(), Matchers.any())
val response = """{"record":{"title":"The title"},"id":"123","thumbnail":"","links":[],"linksRelated":[],
|"linksAccess":[],"linksGeneral":[],"references":[],"copies":[],"parts":{}}""".trimMargin()
doReturn(response).`when`(slub).httpGet(Matchers.any(), Matchers.any())
doReturn(mockHeadResponse).`when`(slub).httpHead(Matchers.any(), Matchers.anyBoolean())
doReturn(mockGetResponse).`when`(slub).httpGet(Matchers.any(), Matchers.any())
val actual = slub.getResultById("rsn/456", null)
verify(slub).httpHead(eq("https://test.de/rsn/456/"), eq("Location"), eq(""),
argThat(ClientDoesntFollowRedirects()))
verify(slub).httpHead(eq("https://test.de/rsn/456/"), eq(false))
verify(slub).httpGet("https://test.de/id/123/?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app", "UTF-8")
assertEquals("id/123", actual.id)
}

@Test
fun testLibeorIdentifier() {
doReturn(mockHeadResponse).`when`(slub).httpHead(Matchers.any(), Matchers.anyBoolean())
doReturn(mockGetResponse).`when`(slub).httpGet(Matchers.any(), Matchers.any())
val actual = slub.getResultById("http://slubdd.de/katalog?libero_mab456", null)
verify(slub).httpHead(eq("http://slubdd.de/katalog?libero_mab456"), eq(false))
verify(slub).httpGet("https://test.de/id/123/?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app", "UTF-8")
assertEquals("id/123", actual.id)
}

@Test
fun testLegacyIdentifier() {
// id without prefix, e.g. from old favorites list
val response = """{"record":{"title":"The title"},"id":"123","thumbnail":"","links":[],"linksRelated":[],
|"linksAccess":[],"linksGeneral":[],"references":[],"copies":[],"parts":{}}""".trimMargin()
doReturn(response).`when`(slub).httpGet(Matchers.any(), Matchers.any())
doReturn(mockGetResponse).`when`(slub).httpGet(Matchers.any(), Matchers.any())
val actual = slub.getResultById("123", null)
verify(slub).httpGet("https://test.de/id/123/?type=1369315142&tx_find_find[format]=data&tx_find_find[data-format]=app", "UTF-8")
verify(slub, never()).httpHead(any(), any(), any(), any())
verify(slub, never()).httpHead(any(), anyBoolean())
assertEquals("id/123", actual.id)
}
}
Expand Down
11 changes: 6 additions & 5 deletions ...opacapp/src/main/java/de/geeksfactory/opacclient/frontend/SearchResultDetailFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,11 +382,6 @@ protected void display() {
addSubhead(joiner, R.string.details_head);
joiner.add(new JoinableAdapter(new DetailsAdapter(item.getDetails(), getActivity())));

if (item.getCopies().size() != 0) {
addSubhead(joiner, R.string.copies_head);
joiner.add(new JoinableAdapter(new CopiesAdapter(item.getCopies(), getActivity())));
}

if (item.getVolumesearch() != null) {
addSubhead(joiner, R.string.volumes);
joiner.add(new JoinableLayout(
Expand All @@ -405,6 +400,12 @@ public void onClick(View v) {
addSubhead(joiner, R.string.volumes);
joiner.add(new JoinableAdapter(new VolumesAdapter(item.getVolumes(), getActivity())));
}

if (item.getCopies().size() != 0) {
addSubhead(joiner, R.string.copies_head);
joiner.add(new JoinableAdapter(new CopiesAdapter(item.getCopies(), getActivity())));
}

rvDetails.setAdapter(joiner.getAdapter());

if (id == null || id.equals("")) {
Expand Down
3 changes: 2 additions & 1 deletion opacclient/opacapp/src/main/res/layout/listitem_volume.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:ellipsize="end"
android:singleLine="true"/>
android:maxLines="2"
android:singleLine="false"/>

</LinearLayout>

0 comments on commit e897129

Please sign in to comment.