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.

Sign up for GitHub

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

Jump to bottom

feat(sipi): Improve error message if XSL file not found #1590

Merged
merged 7 commits into from Feb 7, 2020
Merged

feat(sipi): Improve error message if XSL file not found #1590

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
46c77d0
feat(sipi): Improve error message if XSL file not found.
Feb 5, 2020
cbc8e25
Merge branch 'develop' into wip/1580-xsl-not-found
Feb 5, 2020
3f58d6f
style(sipi): Optimise imports.
Feb 5, 2020
029291d
fix(sipi): Preserve nested exception.
Feb 5, 2020
2c5ed5c
test(sipi): Add test for missing XSL transformation.
Feb 6, 2020
080dd44
feat(sipi): Rename method, make error message less specific.
Feb 7, 2020
0f34ace
feat(sipi): Include sender in text file request message.
Feb 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions webapi/_test_data/all_data/anything-data.ttl
View file
Open in desktop
Expand Up @@ -1695,3 +1695,25 @@
knora-base:hasRootNode <http://rdfh.ch/lists/0001/treeList>;
knora-base:listNodePosition 1;
rdfs:label "Tree list node 11"@en .

<http://rdfh.ch/0801/608NfPLCRpeYnkXKABC5mg> a knora-base:XSLTransformation;
knora-base:attachedToProject <http://rdfh.ch/projects/yTerZGyxjZVqFMNNKXCDPF>;
knora-base:attachedToUser <http://rdfh.ch/users/root>;
knora-base:creationDate "2020-02-06T11:55:47.860023Z"^^xsd:dateTime;
knora-base:hasPermissions "CR knora-admin:Creator|M knora-admin:ProjectMember|V knora-admin:KnownUser|RV knora-admin:UnknownUser";
knora-base:hasTextFileValue <http://rdfh.ch/0801/608NfPLCRpeYnkXKABC5mg/values/geQNPU4USB61YhNHHarQ5A>;
knora-base:isDeleted false;
rdfs:label "BEOL header XSLT" .

<http://rdfh.ch/0801/608NfPLCRpeYnkXKABC5mg/values/geQNPU4USB61YhNHHarQ5A> a knora-base:TextFileValue;
knora-base:attachedToUser <http://rdfh.ch/users/root>;
knora-base:hasPermissions "CR knora-admin:Creator|M knora-admin:ProjectMember|V knora-admin:KnownUser|RV knora-admin:UnknownUser";
knora-base:internalFilename "missing.xsl";
knora-base:internalMimeType "text/xml";
knora-base:isDeleted false;
knora-base:originalFilename "header.xsl";
knora-base:originalMimeType "text/xml";
knora-base:valueCreationDate "2020-02-06T11:55:47.860023Z"^^xsd:dateTime;
knora-base:valueHasOrder 0;
knora-base:valueHasString "header.xsl";
knora-base:valueHasUUID "geQNPU4USB61YhNHHarQ5A" .
24 changes: 21 additions & 3 deletions webapi/src/it/scala/org/knora/webapi/e2e/v1/KnoraSipiIntegrationV1ITSpec.scala
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@ import akka.http.scaladsl.model.{HttpEntity, _}
import com.typesafe.config.{Config, ConfigFactory}
import org.knora.webapi._
import org.knora.webapi.messages.store.triplestoremessages.{RdfDataObject, TriplestoreJsonProtocol}
import org.knora.webapi.util.jsonld.{JsonLDDocument, JsonLDString}
import org.knora.webapi.util.{FileUtil, MutableTestIri}
import org.xmlunit.builder.{DiffBuilder, Input}
import org.xmlunit.diff.Diff
Expand Down Expand Up @@ -74,6 +75,7 @@ class KnoraSipiIntegrationV1ITSpec extends ITKnoraLiveSpec(KnoraSipiIntegrationV
private val pathToBEOLLetterMapping = "_test_data/test_route/texts/beol/testLetter/beolMapping.xml"
private val pathToBEOLBulkXML = "_test_data/test_route/texts/beol/testLetter/bulk.xml"
private val letterIri = new MutableTestIri
private val gravsearchTemplateIri = new MutableTestIri

/**
* Adds the IRI of a XSL transformation to the given mapping.
Expand Down Expand Up @@ -725,12 +727,12 @@ class KnoraSipiIntegrationV1ITSpec extends ITKnoraLiveSpec(KnoraSipiIntegrationV

val gravsearchTemplateJSON: JsObject = getResponseJson(gravsearchTemplateRequest)

val gravsearchTemplateIri: IRI = gravsearchTemplateJSON.fields.get("res_id") match {
gravsearchTemplateIri.set(gravsearchTemplateJSON.fields.get("res_id") match {

case Some(JsString(gravsearchIri)) => gravsearchIri

case _ => throw InvalidApiJsonException("expected IRI for Gravsearch template")
}
})

// create an XSL transformation
val headerParams = JsObject(
Expand Down Expand Up @@ -774,7 +776,7 @@ class KnoraSipiIntegrationV1ITSpec extends ITKnoraLiveSpec(KnoraSipiIntegrationV
val letterTEIRequest: HttpRequest = Get(baseApiUrl + "/v2/tei/" + URLEncoder.encode(letterIri.get, "UTF-8") +
"?textProperty=" + URLEncoder.encode("http://0.0.0.0:3333/ontology/0801/beol/v2#hasText", "UTF-8") +
"&mappingIri=" + URLEncoder.encode("http://rdfh.ch/projects/yTerZGyxjZVqFMNNKXCDPF/mappings/BEOLToTEI", "UTF-8") +
"&gravsearchTemplateIri=" + URLEncoder.encode(gravsearchTemplateIri, "UTF-8") +
"&gravsearchTemplateIri=" + URLEncoder.encode(gravsearchTemplateIri.get, "UTF-8") +
"&teiHeaderXSLTIri=" + URLEncoder.encode(headerXSLTIri, "UTF-8")
)

Expand Down Expand Up @@ -833,6 +835,22 @@ class KnoraSipiIntegrationV1ITSpec extends ITKnoraLiveSpec(KnoraSipiIntegrationV
xmlDiff.hasDifferences should be(false)

}

"provide a helpful error message if an XSLT file is not found" in {
val missingHeaderXSLTIri = "http://rdfh.ch/0801/608NfPLCRpeYnkXKABC5mg"

val letterTEIRequest: HttpRequest = Get(baseApiUrl + "/v2/tei/" + URLEncoder.encode(letterIri.get, "UTF-8") +
"?textProperty=" + URLEncoder.encode("http://0.0.0.0:3333/ontology/0801/beol/v2#hasText", "UTF-8") +
"&mappingIri=" + URLEncoder.encode("http://rdfh.ch/projects/yTerZGyxjZVqFMNNKXCDPF/mappings/BEOLToTEI", "UTF-8") +
"&gravsearchTemplateIri=" + URLEncoder.encode(gravsearchTemplateIri.get, "UTF-8") +
"&teiHeaderXSLTIri=" + URLEncoder.encode(missingHeaderXSLTIri, "UTF-8")
)

val response: HttpResponse = singleAwaitingRequest(letterTEIRequest)
assert(response.status.intValue == 500)
val responseBodyStr: String = Await.result(response.entity.toStrict(2.seconds).map(_.data.decodeString("UTF-8")), 2.seconds)
assert(responseBodyStr.contains("Unable to get XSL transformation file"))
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion webapi/src/main/scala/org/knora/webapi/store/iiif/SipiConnector.scala
View file
Open in desktop
Expand Up @@ -321,9 +321,14 @@ class SipiConnector extends Actor with ActorLogging {
// ask Sipi to return the XSL transformation
val request = new HttpGet(xsltFileUrl)

for {
val sipiResponseTry: Try[SipiGetTextFileResponse] = for {
responseStr <- doSipiRequest(request)
} yield SipiGetTextFileResponse(responseStr)

sipiResponseTry.recover {
case badRequestException: BadRequestException => throw SipiException(s"Unable to get XSL transformation file $xsltFileUrl from Sipi: ${badRequestException.message}")
Copy link
Contributor

Choose a reason for hiding this comment

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

sipiGetXsltTransformationRequestV2 is called in two cases: for XSLT used by StandoffResponderV2 to turn XML into HTML and for XSLT used the TEI transformer in ResourcesResponderV2. So I guess testing it with the TEI route is fine.

However, SipiGetTextFileRequest is as the message sent to SipiConnector that then internally calls sipiGetXsltTransformationRequestV2 seems strange to me. Shouldn't either the message be more specific or the method more generic?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the future, Knora might require other text files from Sipi than XSL transformations and then the error message thrown by sipiGetXsltTransformationRequestV2 might be misleading.

Copy link
Author

Choose a reason for hiding this comment

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

Quite true, I'll fix that.

Copy link
Author

Choose a reason for hiding this comment

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

Done in 080dd44.

case sipiException: SipiException => throw SipiException(s"Unable to get XSL transformation file $xsltFileUrl from Sipi: ${sipiException.message}", sipiException.cause)
}
}

/**
Expand Down