This is an automated email from the ASF dual-hosted git repository.
mhenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new d9c6197 Remove path projections on web actions. (#4415)
d9c6197 is described below
commit d9c61978c53e77ffe08e51493802495e81a387de
Author: rodric rabbah <[email protected]>
AuthorDate: Thu Dec 12 11:45:32 2019 -0500
Remove path projections on web actions. (#4415)
---
.../openwhisk/core/controller/WebActions.scala | 86 +++++++++++-----------
.../core/cli/test/WskWebActionsTests.scala | 21 +++---
.../core/controller/test/WebActionsApiTests.scala | 65 ++++------------
3 files changed, 67 insertions(+), 105 deletions(-)
diff --git
a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/WebActions.scala
b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/WebActions.scala
index a784f33..182881f 100644
---
a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/WebActions.scala
+++
b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/WebActions.scala
@@ -146,11 +146,11 @@ protected[core] object WhiskWebActionsApi extends
Directives {
private val mediaTranscoders = {
// extensions are expected to contain only [a-z]
Seq(
- MediaExtension(".http", None, false, resultAsHttp _),
- MediaExtension(".json", None, true, resultAsJson _),
- MediaExtension(".html", Some(List("html")), true, resultAsHtml _),
- MediaExtension(".svg", Some(List("svg")), true, resultAsSvg _),
- MediaExtension(".text", Some(List("text")), true, resultAsText _))
+ MediaExtension(".http", resultAsHttp _),
+ MediaExtension(".json", resultAsJson _),
+ MediaExtension(".html", resultAsHtml _),
+ MediaExtension(".svg", resultAsSvg _),
+ MediaExtension(".text", resultAsText _))
}
private val defaultMediaTranscoder: MediaExtension =
mediaTranscoders.find(_.extension == ".http").get
@@ -177,35 +177,52 @@ protected[core] object WhiskWebActionsApi extends
Directives {
/**
* Supported extensions, their default projection and transcoder to complete
a request.
*
- * @param extension the supported media types for action response
- * @param defaultProjection the default media extensions for action
projection
- * @param transcoder the HTTP decoder and terminator for the extension
+ * @param extension the supported media types for action response
+ * @param transcoder the HTTP decoder and terminator for the extension
*/
protected case class MediaExtension(extension: String,
- defaultProjection: Option[List[String]],
- projectionAllowed: Boolean,
transcoder: (JsValue, TransactionId,
WebApiDirectives) => Route) {
val extensionLength = extension.length
}
- private def resultAsHtml(result: JsValue, transid: TransactionId, rp:
WebApiDirectives) = result match {
- case JsString(html) =>
complete(HttpEntity(ContentTypes.`text/html(UTF-8)`, html))
- case _ => terminate(BadRequest,
Messages.invalidMedia(`text/html`))(transid, jsonPrettyPrinter)
+ private def resultAsHtml(result: JsValue, transid: TransactionId, rp:
WebApiDirectives) = {
+ val htmlResult = result match {
+ case JsObject(fields) =>
fields.get("body").orElse(fields.get("html")).getOrElse(JsNull)
+ case _ => result
+ }
+
+ htmlResult match {
+ case JsString(html) =>
complete(HttpEntity(ContentTypes.`text/html(UTF-8)`, html))
+ case _ => terminate(BadRequest,
Messages.invalidMedia(`text/html`))(transid, jsonPrettyPrinter)
+ }
}
- private def resultAsSvg(result: JsValue, transid: TransactionId, rp:
WebApiDirectives) = result match {
- case JsString(svg) => complete(HttpEntity(`image/svg+xml`, svg.getBytes))
- case _ => terminate(BadRequest,
Messages.invalidMedia(`image/svg+xml`))(transid, jsonPrettyPrinter)
+ private def resultAsSvg(result: JsValue, transid: TransactionId, rp:
WebApiDirectives) = {
+ val svgResult = result match {
+ case JsObject(fields) =>
fields.get("body").orElse(fields.get("svg")).getOrElse(JsNull)
+ case _ => result
+ }
+
+ svgResult match {
+ case JsString(svg) => complete(HttpEntity(`image/svg+xml`, svg.getBytes))
+ case _ => terminate(BadRequest,
Messages.invalidMedia(`image/svg+xml`))(transid, jsonPrettyPrinter)
+ }
}
private def resultAsText(result: JsValue, transid: TransactionId, rp:
WebApiDirectives) = {
- result match {
- case r: JsObject => complete(OK, r.prettyPrint)
- case r: JsArray => complete(OK, r.prettyPrint)
- case JsString(s) => complete(OK, s)
- case JsBoolean(b) => complete(OK, b.toString)
- case JsNumber(n) => complete(OK, n.toString)
- case JsNull => complete(OK, JsNull.toString)
+ val txtResult = result match {
+ case JsObject(fields) => fields.get("body").orElse(fields.get("text"))
+ case _ => Some(result)
+ }
+
+ txtResult match {
+ case Some(r: JsObject) => complete(OK, r.prettyPrint)
+ case Some(r: JsArray) => complete(OK, r.prettyPrint)
+ case Some(JsString(s)) => complete(OK, s)
+ case Some(JsBoolean(b)) => complete(OK, b.toString)
+ case Some(JsNumber(n)) => complete(OK, n.toString)
+ case Some(JsNull) => complete(OK, JsNull.toString)
+ case _ => terminate(NotFound,
Messages.propertyNotFound)(transid, jsonPrettyPrinter)
}
}
@@ -617,11 +634,10 @@ trait WhiskWebActionsApi
}
}
- completeRequest(queuedActivation, projectResultField(context,
responseType), responseType)
+ completeRequest(queuedActivation, responseType)
}
private def completeRequest(queuedActivation: Future[Either[ActivationId,
WhiskActivation]],
- projectResultField: => List[String],
responseType: MediaExtension)(implicit transid:
TransactionId) = {
onComplete(queuedActivation) {
case Success(Right(activation)) =>
@@ -630,7 +646,7 @@ trait WhiskWebActionsApi
if (activation.response.isSuccess ||
activation.response.isApplicationError) {
val resultPath = if (activation.response.isSuccess) {
- projectResultField
+ List.empty
} else {
// the activation produced an error response: therefore ignore
// the requested projection and unwrap the error instead
@@ -735,27 +751,11 @@ trait WhiskWebActionsApi
}
/**
- * Determines the result projection path, if any.
- *
- * @return optional list of projections
- */
- private def projectResultField(context: Context, responseType:
MediaExtension): List[String] = {
- val projection = if (responseType.projectionAllowed) {
- Option(context.path)
- .filter(_.nonEmpty)
- .map(_.split("/").filter(_.nonEmpty).toList)
- .orElse(responseType.defaultProjection)
- } else responseType.defaultProjection
-
- projection.getOrElse(List.empty)
- }
-
- /**
* Check if "require-whisk-auth" authentication is needed, and if so,
authenticate the request
* NOTE: Only number or string JSON "require-whisk-auth" annotation values
are supported
*
* @param annotations - web action annotations
- * @param reqHeaders - web action invocation request headers
+ * @param reqHeaders - web action invocation request headers
* @return Option[Boolean]
* None if annotations does not include require-whisk-auth (i.e.
auth test not needed)
* Some(true) if annotations includes require-whisk-auth and it's
value matches the request header `X-Require-Whisk-Auth` value
diff --git
a/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskWebActionsTests.scala
b/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskWebActionsTests.scala
index fb6783d..85008c3 100644
---
a/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskWebActionsTests.scala
+++
b/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskWebActionsTests.scala
@@ -17,7 +17,6 @@
package org.apache.openwhisk.core.cli.test
-import java.nio.charset.StandardCharsets
import java.util.Base64
import scala.util.Failure
@@ -88,7 +87,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
val name = "webaction"
val file = Some(TestUtils.getTestActionFilename("echo.js"))
val host = getServiceURL()
- val requestPath = host +
s"$testRoutePath/$namespace/default/$name.text/a?a="
+ val requestPath = host +
s"$testRoutePath/$namespace/default/$name.json/a?a="
val padAmount = MAX_URL_LENGTH - requestPath.length
assetHelper.withCleaner(wsk.action, name) { (action, _) =>
@@ -111,7 +110,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
withClue(s"response code: $responseCode, url length: ${url.length},
pad amount: ${pad.length}, url: $url") {
responseCode shouldBe code
if (code == 200) {
- response.body.asString shouldBe pad
+
response.body.asString.parseJson.asJsObject.fields("a").convertTo[String]
shouldBe pad
} else {
response.body.asString should include("414 Request-URI Too
Large") // from nginx
}
@@ -127,7 +126,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
val name = "webaction"
val file = Some(TestUtils.getTestActionFilename("echo.js"))
val host = getServiceURL()
- val url = s"$host$testRoutePath/$namespace/default/$name.text/__ow_user"
+ val url = s"$host$testRoutePath/$namespace/default/$name.json"
assetHelper.withCleaner(wsk.action, name) { (action, _) =>
action.create(name, file, web = Some("true"), annotations =
Map("require-whisk-auth" -> true.toJson))
@@ -145,7 +144,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
.get(url)
authorizedResponse.statusCode shouldBe 200
- authorizedResponse.body.asString shouldBe namespace
+
authorizedResponse.body.asString.parseJson.asJsObject.fields("__ow_user").convertTo[String]
shouldBe namespace
}
it should "ensure that CORS header is preserved for custom options" in
withAssetCleaner(wskprops) {
@@ -216,7 +215,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
val file = Some(TestUtils.getTestActionFilename("echo.js"))
val bodyContent = "This is the body"
val host = getServiceURL()
- val url =
s"$host$testRoutePath/$namespace/default/webaction.text/__ow_body"
+ val url = s"$host$testRoutePath/$namespace/default/webaction.json"
assetHelper.withCleaner(wsk.action, name) { (action, _) =>
action.create(name, file, web = Some("true"))
@@ -224,11 +223,11 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
val paramRes = RestAssured.given().contentType("text/html").param("key",
"value").config(sslconfig).post(url)
paramRes.statusCode shouldBe 200
- new String(paramRes.body.asByteArray, StandardCharsets.UTF_8) shouldBe
"key=value"
+
paramRes.body.asString().parseJson.asJsObject.fields("__ow_body").convertTo[String]
shouldBe "key=value"
val bodyRes =
RestAssured.given().contentType("text/html").body(bodyContent).config(sslconfig).post(url)
bodyRes.statusCode shouldBe 200
- new String(bodyRes.body.asByteArray, StandardCharsets.UTF_8) shouldBe
bodyContent
+
bodyRes.body.asString().parseJson.asJsObject.fields("__ow_body").convertTo[String]
shouldBe bodyContent
}
it should "reject invocation of web action with invalid accept header" in
withAssetCleaner(wskprops) {
@@ -368,7 +367,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
action.create(actionName, file, web = Some(true.toString))(wp)
}
- val url = getServiceApiHost(vanitySubdomain, true) +
s"/default/$actionName.text/a?a=A"
+ val url = getServiceApiHost(vanitySubdomain, true) +
s"/default/$actionName.json/a?a=A"
println(s"url: $url")
// try the rest assured path first, failing that, try curl with explicit
resolve
@@ -376,7 +375,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
val response = RestAssured.given().config(sslconfig).get(url)
val responseCode = response.statusCode
responseCode shouldBe 200
- response.body.asString shouldBe "A"
+
response.body.asString.parseJson.asJsObject.fields("a").convertTo[String]
shouldBe "A"
} match {
case Failure(t) =>
println(s"RestAssured path failed, trying curl: $t")
@@ -390,7 +389,7 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
val cmd = Seq("curl", "-k", url, "--resolve", s"$host:$ip")
val (stdout, stderr, exitCode) = SimpleExec.syncRunCmd(cmd)
withClue(s"\n$stderr\n") {
- stdout shouldBe "A"
+ stdout.parseJson.asJsObject.fields("a").convertTo[String] shouldBe
"A"
exitCode shouldBe 0
}
diff --git
a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/WebActionsApiTests.scala
b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/WebActionsApiTests.scala
index fcbd460..c580de3 100644
---
a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/WebActionsApiTests.scala
+++
b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/WebActionsApiTests.scala
@@ -824,7 +824,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
- it should s"project a field from the result object (auth?
${creds.isDefined})" in {
+ it should s"not project a field from the result object (auth?
${creds.isDefined})" in {
implicit val tid = transid()
Seq(s"$systemId/proxy/export_c.json/content").foreach { path =>
@@ -833,7 +833,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
status should be(OK)
- val response = responseAs[JsObject]
+ val response = responseAs[JsObject].fields("content")
response shouldBe metaPayload(
m.method.name.toLowerCase,
JsObject.empty,
@@ -843,38 +843,6 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
}
-
- Seq(
- s"$systemId/proxy/export_c.text/content/z",
- s"$systemId/proxy/export_c.text/content/z/",
- s"$systemId/proxy/export_c.text/content/z//").foreach { path =>
- allowedMethods.foreach { m =>
- invocationsAllowed += 1
-
- m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
- status should be(OK)
- val response = responseAs[String]
- response shouldBe "Z"
- }
- }
- }
- }
-
- it should s"reject when projecting a result which does not match expected
type (auth? ${creds.isDefined})" in {
- implicit val tid = transid()
-
- // these project a result which does not match expected type
- Seq(s"$systemId/proxy/export_c.json/a").foreach { path =>
- allowedMethods.foreach { m =>
- invocationsAllowed += 1
- actionResult = Some(JsObject("a" -> JsString("b")))
-
- m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
- status should be(BadRequest)
- confirmErrorWithTid(responseAs[JsObject],
Some(Messages.invalidMedia(MediaTypes.`application/json`)))
- }
- }
- }
}
it should s"reject when projecting a field from the result object that
does not exist (auth? ${creds.isDefined})" in {
@@ -1038,10 +1006,10 @@ trait WebActionsApiBaseTests extends
ControllerTestCommon with BeforeAndAfterEac
Seq(JsObject("a" -> "A".toJson), JsArray("a".toJson), JsString("a"),
JsTrue, JsNumber(1), JsNull)
.foreach { jsval =>
- val path = s"$systemId/proxy/export_c.text/res"
+ val path = s"$systemId/proxy/export_c.text"
allowedMethods.foreach { m =>
invocationsAllowed += 1
- actionResult = Some(JsObject("res" -> jsval))
+ actionResult = Some(JsObject("body" -> jsval))
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
responseAs[String] shouldBe {
@@ -1472,20 +1440,15 @@ trait WebActionsApiBaseTests extends
ControllerTestCommon with BeforeAndAfterEac
it should s"support formdata (auth? ${creds.isDefined})" in {
implicit val tid = transid()
- Seq(s"$systemId/proxy/export_c.text/content/field1",
s"$systemId/proxy/export_c.text/content/field2").foreach {
- path =>
- val form = FormData(Map("field1" -> "value1", "field2" -> "value2"))
- invocationsAllowed += 2
-
- Post(s"$testRoutePath/$path", form.toEntity) ~>
Route.seal(routes(creds)) ~> check {
- status should be(OK)
- responseAs[String] should (be("value1") or be("value2"))
- }
+ Seq(s"$systemId/proxy/export_c.json").foreach { path =>
+ val form = FormData(Map("field1" -> "value1", "field2" -> "value2"))
+ invocationsAllowed += 1
- Post(s"$testRoutePath/$path", form.toEntity) ~>
Route.seal(routes(creds)) ~> check {
- status should be(OK)
- responseAs[String] should (be("value1") or be("value2"))
- }
+ Post(s"$testRoutePath/$path", form.toEntity) ~>
Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ responseAs[JsObject].fields("content").asJsObject.fields("field1")
shouldBe JsString("value1")
+ responseAs[JsObject].fields("content").asJsObject.fields("field2")
shouldBe JsString("value2")
+ }
}
}
@@ -1952,7 +1915,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
it should s"support json (including +json subtypes) (auth?
${creds.isDefined})" in {
implicit val tid = transid()
- val path = s"$systemId/proxy/export_c.text/content/field1"
+ val path = s"$systemId/proxy/export_c.json"
val entity = JsObject("field1" -> "value1".toJson)
Seq(
@@ -1961,7 +1924,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
invocationsAllowed += 1
Post(s"$testRoutePath/$path", HttpEntity(ct, entity.compactPrint)) ~>
Route.seal(routes(creds)) ~> check {
status should be(OK)
- responseAs[String] shouldBe "value1"
+ responseAs[JsObject].fields("content").asJsObject.fields("field1")
shouldBe entity.fields("field1")
}
}
}
