From 153a674fda6085a338a390788e74985e071f1d4d Mon Sep 17 00:00:00 2001 From: Benjamin Geer Date: Mon, 15 Mar 2021 09:28:01 +0100 Subject: [PATCH] feat(api-v2): Improve error message when an XSLT transformation file is not found (DSP-1404) (#1831) --- .../responders/v1/StandoffResponderV1.scala | 10 ++- .../responders/v2/ResourcesResponderV2.scala | 18 ++++- .../v2/ResponderWithStandoffV2.scala | 14 +++- .../webapi/store/iiif/SipiConnector.scala | 68 +++++++++++-------- .../it/v1/KnoraSipiIntegrationV1ITSpec.scala | 2 +- 5 files changed, 77 insertions(+), 35 deletions(-) diff --git a/webapi/src/main/scala/org/knora/webapi/responders/v1/StandoffResponderV1.scala b/webapi/src/main/scala/org/knora/webapi/responders/v1/StandoffResponderV1.scala index 6a5fd916de..50d89a1722 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/v1/StandoffResponderV1.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/v1/StandoffResponderV1.scala @@ -23,6 +23,7 @@ import java.util.UUID import akka.pattern._ import org.knora.webapi._ +import org.knora.webapi.exceptions.{NotFoundException, SipiException} import org.knora.webapi.feature.FeatureFactoryConfig import org.knora.webapi.messages.IriConversions._ import org.knora.webapi.messages.StringFormatter @@ -72,7 +73,7 @@ class StandoffResponderV1(responderData: ResponderData) extends Responder(respon featureFactoryConfig: FeatureFactoryConfig, userProfile: UserADM): Future[GetXSLTransformationResponseV1] = { - for { + val xslTransformationFuture = for { xsltTransformation <- (responderManager ? GetXSLTransformationRequestV2( xsltTextRepresentationIri = xslTransformationIri, featureFactoryConfig = featureFactoryConfig, @@ -82,6 +83,13 @@ class StandoffResponderV1(responderData: ResponderData) extends Responder(respon GetXSLTransformationResponseV1( xslt = xsltTransformation.xslt ) + + xslTransformationFuture.recover { + case notFound: NotFoundException => + throw SipiException(s"XSL transformation $xslTransformationIri not found: ${notFound.message}") + + case other => throw other + } } /** diff --git a/webapi/src/main/scala/org/knora/webapi/responders/v2/ResourcesResponderV2.scala b/webapi/src/main/scala/org/knora/webapi/responders/v2/ResourcesResponderV2.scala index fae0e92e50..b837d1fe4f 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/v2/ResourcesResponderV2.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/v2/ResourcesResponderV2.scala @@ -1770,11 +1770,18 @@ class ResourcesResponderV2(responderData: ResponderData) extends ResponderWithSt requestingUser = requestingUser ) - for { + val xslTransformationFuture = for { xslTransformation: GetXSLTransformationResponseV2 <- (responderManager ? teiHeaderXsltRequest) .mapTo[GetXSLTransformationResponseV2] } yield Some(xslTransformation.xslt) + xslTransformationFuture.recover { + case notFound: NotFoundException => + throw SipiException(s"TEI header XSL transformation <$headerIri> not found: ${notFound.message}") + + case other => throw other + } + case None => Future(None) } @@ -1819,11 +1826,18 @@ class ResourcesResponderV2(responderData: ResponderData) extends ResponderWithSt requestingUser = requestingUser ) - for { + val xslTransformationFuture = for { xslTransformation: GetXSLTransformationResponseV2 <- (responderManager ? teiBodyXsltRequest) .mapTo[GetXSLTransformationResponseV2] } yield xslTransformation.xslt + xslTransformationFuture.recover { + case notFound: NotFoundException => + throw SipiException( + s"Default XSL transformation <${teiMapping.mapping.defaultXSLTransformation.get}> not found for mapping <${teiMapping.mappingIri}>: ${notFound.message}") + + case other => throw other + } case None => throw BadRequestException(s"Default XSL Transformation expected for mapping $otherMapping") } } diff --git a/webapi/src/main/scala/org/knora/webapi/responders/v2/ResponderWithStandoffV2.scala b/webapi/src/main/scala/org/knora/webapi/responders/v2/ResponderWithStandoffV2.scala index 681bc47ab9..8a195a09ec 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/v2/ResponderWithStandoffV2.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/v2/ResponderWithStandoffV2.scala @@ -19,8 +19,10 @@ package org.knora.webapi.responders.v2 +import akka.http.scaladsl.util.FastFuture import akka.pattern._ import org.knora.webapi.IRI +import org.knora.webapi.exceptions.{NotFoundException, SipiException} import org.knora.webapi.feature.FeatureFactoryConfig import org.knora.webapi.messages.admin.responder.usersmessages.UserADM import org.knora.webapi.messages.util.ConstructResponseUtilV2.{MappingAndXSLTransformation, ResourceWithValueRdfData} @@ -79,15 +81,23 @@ abstract class ResponderWithStandoffV2(responderData: ResponderData) extends Res for { // if given, get the default XSL transformation xsltOption: Option[String] <- if (mapping.mapping.defaultXSLTransformation.nonEmpty) { - for { + val xslTransformationFuture = for { xslTransformation: GetXSLTransformationResponseV2 <- (responderManager ? GetXSLTransformationRequestV2( mapping.mapping.defaultXSLTransformation.get, featureFactoryConfig = featureFactoryConfig, requestingUser = requestingUser )).mapTo[GetXSLTransformationResponseV2] } yield Some(xslTransformation.xslt) + + xslTransformationFuture.recover { + case notFound: NotFoundException => + throw SipiException( + s"Default XSL transformation <${mapping.mapping.defaultXSLTransformation.get}> not found for mapping <${mapping.mappingIri}>: ${notFound.message}") + + case other => throw other + } } else { - Future(None) + FastFuture.successful(None) } } yield mapping.mappingIri -> MappingAndXSLTransformation(mapping = mapping.mapping, diff --git a/webapi/src/main/scala/org/knora/webapi/store/iiif/SipiConnector.scala b/webapi/src/main/scala/org/knora/webapi/store/iiif/SipiConnector.scala index 65b67d6274..d5ba47a335 100644 --- a/webapi/src/main/scala/org/knora/webapi/store/iiif/SipiConnector.scala +++ b/webapi/src/main/scala/org/knora/webapi/store/iiif/SipiConnector.scala @@ -31,7 +31,7 @@ import org.apache.http.impl.client.{CloseableHttpClient, HttpClients} import org.apache.http.message.BasicNameValuePair import org.apache.http.util.EntityUtils import org.apache.http.{Consts, HttpHost, HttpRequest, NameValuePair} -import org.knora.webapi.exceptions.{BadRequestException, SipiException} +import org.knora.webapi.exceptions.{BadRequestException, NotFoundException, SipiException} import org.knora.webapi.messages.StringFormatter import org.knora.webapi.messages.store.sipimessages._ import org.knora.webapi.messages.v2.responder.SuccessResponseV2 @@ -221,14 +221,24 @@ class SipiConnector extends Actor with ActorLogging { } yield SipiGetTextFileResponse(responseStr) sipiResponseTry.recover { + case notFoundException: NotFoundException => + throw NotFoundException( + s"Unable to get file ${textFileRequest.fileUrl} from Sipi as requested by ${textFileRequest.senderName}: ${notFoundException.message}") + case badRequestException: BadRequestException => throw SipiException( s"Unable to get file ${textFileRequest.fileUrl} from Sipi as requested by ${textFileRequest.senderName}: ${badRequestException.message}") + case sipiException: SipiException => throw SipiException( s"Unable to get file ${textFileRequest.fileUrl} from Sipi as requested by ${textFileRequest.senderName}: ${sipiException.message}", sipiException.cause ) + + case other => + throw SipiException( + s"Unable to get file ${textFileRequest.fileUrl} from Sipi as requested by ${textFileRequest.senderName}: ${other.getMessage}" + ) } } @@ -254,45 +264,45 @@ class SipiConnector extends Actor with ActorLogging { */ private def doSipiRequest(request: HttpRequest): Try[String] = { val httpContext: HttpClientContext = HttpClientContext.create + var maybeResponse: Option[CloseableHttpResponse] = None val sipiResponseTry = Try { - var maybeResponse: Option[CloseableHttpResponse] = None - - try { - maybeResponse = Some(httpClient.execute(targetHost, request, httpContext)) + maybeResponse = Some(httpClient.execute(targetHost, request, httpContext)) - val responseEntityStr: String = Option(maybeResponse.get.getEntity) match { - case Some(responseEntity) => EntityUtils.toString(responseEntity) - case None => "" - } - - val statusCode: Int = maybeResponse.get.getStatusLine.getStatusCode - val statusCategory: Int = statusCode / 100 + val responseEntityStr: String = Option(maybeResponse.get.getEntity) match { + case Some(responseEntity) => EntityUtils.toString(responseEntity) + case None => "" + } - // Was the request successful? - if (statusCategory == 2) { - // Yes. - responseEntityStr + val statusCode: Int = maybeResponse.get.getStatusLine.getStatusCode + val statusCategory: Int = statusCode / 100 + + // Was the request successful? + if (statusCategory == 2) { + // Yes. + responseEntityStr + } else { + // No. Throw an appropriate exception. + val sipiErrorMsg = SipiUtil.getSipiErrorMessage(responseEntityStr) + + if (statusCode == 404) { + throw NotFoundException(sipiErrorMsg) + } else if (statusCategory == 4) { + throw BadRequestException(s"Sipi responded with HTTP status code $statusCode: $sipiErrorMsg") } else { - // No. Throw an appropriate exception. - val sipiErrorMsg = SipiUtil.getSipiErrorMessage(responseEntityStr) - - if (statusCategory == 4) { - throw BadRequestException(s"Sipi responded with HTTP status code $statusCode: $sipiErrorMsg") - } else { - throw SipiException(s"Sipi responded with HTTP status code $statusCode: $sipiErrorMsg") - } - } - } finally { - maybeResponse match { - case Some(response) => response.close() - case None => () + throw SipiException(s"Sipi responded with HTTP status code $statusCode: $sipiErrorMsg") } } } + maybeResponse match { + case Some(response) => response.close() + case None => () + } + sipiResponseTry.recover { case badRequestException: BadRequestException => throw badRequestException + case notFoundException: NotFoundException => throw notFoundException case sipiException: SipiException => throw sipiException case e: Exception => throw SipiException("Failed to connect to Sipi", e, log) } diff --git a/webapi/src/test/scala/org/knora/webapi/it/v1/KnoraSipiIntegrationV1ITSpec.scala b/webapi/src/test/scala/org/knora/webapi/it/v1/KnoraSipiIntegrationV1ITSpec.scala index d4d68000e5..651b8d8c74 100644 --- a/webapi/src/test/scala/org/knora/webapi/it/v1/KnoraSipiIntegrationV1ITSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/it/v1/KnoraSipiIntegrationV1ITSpec.scala @@ -729,7 +729,7 @@ class KnoraSipiIntegrationV1ITSpec Await.result(response.entity.toStrict(2.seconds).map(_.data.decodeString("UTF-8")), 2.seconds) assert(responseBodyStr.contains("Unable to get file")) assert(responseBodyStr.contains("as requested by org.knora.webapi.responders.v2.StandoffResponderV2")) - assert(responseBodyStr.contains("Sipi responded with HTTP status code 404")) + assert(responseBodyStr.contains("Not Found")) } "create a resource with a PDF file attached" in {