Merge branch '156-errors' of https://github.com/hbz/lobid-gnd

Resolves #156 See http://lobid.org/gnd/search?q=%2B%2Btest
hbz · Sep 21, 2018 · f9f28d6 · f9f28d6
2 parents 78dc48b + 68ce911
commit f9f28d6
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 31 deletions.
diff --git a/app/controllers/HomeController.java b/app/controllers/HomeController.java
@@ -178,8 +178,8 @@ public Result authority(String id, String format) {
 		Format responseFormat = Accept.formatFor(format, request().acceptedTypes());
 		if (responseFormat == null || responseFormat == Accept.Format.JSON_LINES
 				|| format != null && format.contains(":")) {
-			return unsupportedMediaType(String.format("Unsupported for single resource: format=%s, accept=%s", format,
-					request().acceptedTypes()));
+			return unsupportedMediaType(views.html.error.render(id, String.format(
+					"Unsupported for single resource: format=%s, accept=%s", format, request().acceptedTypes())));
 		}
 		try {
 			switch (responseFormat) {
@@ -199,7 +199,7 @@ public Result authority(String id, String format) {
 			}
 		} catch (Exception e) {
 			Logger.error("Could not create response", e);
-			return internalServerError(e.getMessage());
+			return internalServerError(views.html.error.render(id, e.getMessage()));
 		}
 	}
 
@@ -297,31 +297,37 @@ public Result search(String q, String filter, int from, int size, String format)
 		Format responseFormat = Accept.formatFor(format, request().acceptedTypes());
 		if (responseFormat == null || Stream.of(RdfFormat.values()).map(RdfFormat::getParam)
 				.anyMatch(f -> f.equals(responseFormat.queryParamString))) {
-			return unsupportedMediaType(
-					String.format("Unsupported for search: format=%s, accept=%s", format, request().acceptedTypes()));
+			return unsupportedMediaType(views.html.error.render(q,
+					String.format("Unsupported for search: format=%s, accept=%s", format, request().acceptedTypes())));
 		}
 		String queryString = (q == null || q.isEmpty()) ? "*" : q;
-		SearchResponse response = index.query(queryString, filter, from, size);
-		response().setHeader("Access-Control-Allow-Origin", "*");
-		String[] formatAndConfig = format == null ? new String[] {} : format.split(":");
-		boolean returnSuggestions = formatAndConfig.length == 2;
-		if (returnSuggestions) {
-			List<Map<String, Object>> hits = Arrays.asList(response.getHits().getHits()).stream()
-					.map(hit -> hit.getSource()).collect(Collectors.toList());
-			return withCallback(toSuggestions(Json.toJson(hits), formatAndConfig[1]));
-		}
-		switch (responseFormat) {
-		case HTML: {
-			return htmlSearch(q, filter, from, size, responseFormat.queryParamString, response);
-		}
-		case JSON_LINES: {
-			response().setHeader("Content-Disposition",
-					String.format("attachment; filename=\"lobid-gnd-bulk-%s.jsonl\"", System.currentTimeMillis()));
-			return jsonLines(queryString, filter, response);
-		}
-		default: {
-			return ok(returnAsJson(q, response)).as(config("index.content"));
-		}
+		try {
+			SearchResponse response = index.query(queryString, filter, from, size);
+			response().setHeader("Access-Control-Allow-Origin", "*");
+			String[] formatAndConfig = format == null ? new String[] {} : format.split(":");
+			boolean returnSuggestions = formatAndConfig.length == 2;
+			if (returnSuggestions) {
+				List<Map<String, Object>> hits = Arrays.asList(response.getHits().getHits()).stream()
+						.map(hit -> hit.getSource()).collect(Collectors.toList());
+				return withCallback(toSuggestions(Json.toJson(hits), formatAndConfig[1]));
+			}
+			switch (responseFormat) {
+			case HTML: {
+				return htmlSearch(q, filter, from, size, responseFormat.queryParamString, response);
+			}
+			case JSON_LINES: {
+				response().setHeader("Content-Disposition",
+						String.format("attachment; filename=\"lobid-gnd-bulk-%s.jsonl\"", System.currentTimeMillis()));
+				return jsonLines(queryString, filter, response);
+			}
+			default: {
+				return ok(returnAsJson(q, response)).as(config("index.content"));
+			}
+			}
+		} catch (Throwable t) {
+			String message = t.getMessage() + (t.getCause() != null ? ", cause: " + t.getCause().getMessage() : "");
+			Logger.error("Error: {}", message);
+			return internalServerError(views.html.error.render(q, "Error: " + message));
 		}
 	}
 
@@ -361,7 +367,7 @@ public ByteString next() {
 
 	private Result htmlSearch(String q, String type, int from, int size, String format, SearchResponse response) {
 		return ok(views.html.search.render(q, type, from, size, returnAsJson(q, response),
-				response.getHits().getTotalHits()));
+				response == null ? 0 : response.getHits().getTotalHits()));
 	}
 
 	private static Result withCallback(final String json) {
@@ -434,6 +440,9 @@ public static String currentUri() {
 	}
 
 	private static String returnAsJson(String q, SearchResponse queryResponse) {
+		if (queryResponse == null) {
+			return Json.newObject().toString();
+		}
 		List<Map<String, Object>> hits = Arrays.asList(queryResponse.getHits().getHits()).stream()
 				.map(hit -> hit.getSource()).collect(Collectors.toList());
 		ObjectNode object = Json.newObject();

diff --git a/app/views/error.scala.html b/app/views/error.scala.html
@@ -0,0 +1,10 @@
+@* Copyright 2018 Fabian Steeg, hbz. Licensed under the GPLv2 *@
+
+@(q: String, message: String)
+
+@main(q, "Error") { 
+	<div class="panel panel-danger footer">
+		<div class='panel-heading'>Fehler</div>
+		<div class='panel-body'>@message</div>
+	</div>
+}
diff --git a/test/controllers/AcceptIntegrationTest.java b/test/controllers/AcceptIntegrationTest.java
@@ -49,8 +49,8 @@ public static Collection<Object[]> data() {
 			{ fakeRequest(GET, "/gnd/search?q=*").header("Accept", "*/*"), /*->*/ "application/json" },
 			{ fakeRequest(GET, "/gnd/search?q=*&format="), /*->*/ "application/json" },
 			{ fakeRequest(GET, "/gnd/search?q=*&format=json"), /*->*/ "application/json" },
-			{ fakeRequest(GET, "/gnd/search?q=*&format=whatever"), /*->*/ "text/plain" },
-			{ fakeRequest(GET, "/gnd/search?q=*").header("Accept", "text/plain"), /*->*/ "text/plain" },
+			{ fakeRequest(GET, "/gnd/search?q=*&format=whatever"), /*->*/ "text/html" },
+			{ fakeRequest(GET, "/gnd/search?q=*").header("Accept", "text/plain"), /*->*/ "text/html" },
 			// search, bulk format: JSON lines
 			{ fakeRequest(GET, "/gnd/search?q=*").header("Accept", "application/x-jsonlines"), /*->*/ "application/x-jsonlines" },
 			{ fakeRequest(GET, "/gnd/search?format=jsonl"), /*->*/ "application/x-jsonlines" },
@@ -65,8 +65,8 @@ public static Collection<Object[]> data() {
 			{ fakeRequest(GET, "/gnd/118820591"), /*->*/ "application/json" },
 			{ fakeRequest(GET, "/gnd/118820591?format="), /*->*/ "application/json" },
 			{ fakeRequest(GET, "/gnd/118820591?format=json"), /*->*/ "application/json" },
-			{ fakeRequest(GET, "/gnd/118820591?format=whatever"), /*->*/ "text/plain" },
-			{ fakeRequest(GET, "/gnd/118820591?format=whatever").header("Accept", "text/html"), /*->*/ "text/plain" },
+			{ fakeRequest(GET, "/gnd/118820591?format=whatever"), /*->*/ "text/html" },
+			{ fakeRequest(GET, "/gnd/118820591?format=whatever").header("Accept", "text/html"), /*->*/ "text/html" },
 			{ fakeRequest(GET, "/gnd/118820591").header("Accept", "text/plain"), /*->*/ "application/n-triples" },
 			// get, other formats as query param:
 			{ fakeRequest(GET, "/gnd/118820591?format=html"), /*->*/ "text/html" },

diff --git a/test/controllers/HomeControllerTest.java b/test/controllers/HomeControllerTest.java
@@ -33,6 +33,7 @@ public static Collection<Object[]> data() {
 				{ routes.HomeController.search("*", "", 0, 10, "json").toString(), Status.OK },
 				{ routes.HomeController.search("*", "", 0, 10, "jsonl").toString(), Status.OK },
 				{ routes.HomeController.search("*", "", 0, 10, "json:suggest").toString(), Status.OK },
+				{ routes.HomeController.search("++test", "", 0, 10, "html").toString(), Status.INTERNAL_SERVER_ERROR },
 				{ routes.HomeController.search("*", "", 0, 10, "jsonfoo").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
 				{ routes.HomeController.search("*", "", 0, 10, "ttl").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
 				{ routes.HomeController.search("*", "", 0, 10, "rdf").toString(), Status.UNSUPPORTED_MEDIA_TYPE },

diff --git a/test/modules/IndexQueryTest.java b/test/modules/IndexQueryTest.java
@@ -7,6 +7,7 @@
 import java.io.FileNotFoundException;
 import java.util.Set;
 
+import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchResponse;
 import org.junit.Assert;
 import org.junit.Test;
@@ -122,4 +123,9 @@ public void testNoStemming() {
 		Assert.assertEquals(1, index.query("namenlosen").getHits().getTotalHits());
 	}
 
+	@Test(expected = SearchPhaseExecutionException.class)
+	public void testInvalidQuery() {
+		index.query("++test");
+	}
+
 }