Skip to content

Commit

Permalink
Helper to test request Accept content types
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrf committed Apr 5, 2012
1 parent 51cb4ab commit 2ec79c5
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 1 deletion.
11 changes: 11 additions & 0 deletions framework/src/play/src/main/java/play/mvc/Http.java
Expand Up @@ -164,6 +164,17 @@ public abstract static class Request {
*/
public abstract List<play.i18n.Lang> acceptLanguages();

/**
* @return The media types set in the request Accept header, not sorted in any particular order.
*/
public abstract List<String> accept();

/**
* Check if this request accepts a given media type.
* @returns true if <code>mediaType</code> is in the Accept header, otherwise false
*/
public abstract boolean accepts(String mediaType);

/**
* The query string content.
*/
Expand Down
Expand Up @@ -16,7 +16,7 @@ import play.api.http._
* }
* }}}
*/
trait Controller extends Results with BodyParsers with Status with HeaderNames with ContentTypes {
trait Controller extends Results with BodyParsers with Status with HeaderNames with ContentTypes with RequestExtractors {

/**
* Provides an empty `Action` implementation: the result is a standard ‘Not implemented yet’ result page.
Expand Down
19 changes: 19 additions & 0 deletions framework/src/play/src/main/scala/play/api/mvc/Http.scala
Expand Up @@ -60,6 +60,25 @@ package play.api.mvc {
}
}

/**
* @return The media types set in the request Accept header, not sorted in any particular order.
*/
lazy val accept: Seq[String] = {
for {
acceptHeader <- headers.get(play.api.http.HeaderNames.ACCEPT).toSeq
value <- acceptHeader.split(",")
contentType <- value.split(";").headOption
} yield contentType
}

/**
* Check if this request accepts a given media type.
* @returns true if `mediaType` matches the Accept header, otherwise false
*/
def accepts(mediaType: String): Boolean = {
accept.contains(mediaType) || accept.contains("*/*") || accept.contains(mediaType.takeWhile(_ != '/') + "/*")
}

/**
* The HTTP cookies.
*/
Expand Down
@@ -0,0 +1,59 @@
package play.api.mvc

trait RequestExtractors extends AcceptExtractors {

/**
* Convenient extractor allowing to apply two extractors.
* Example of use:
* {{{
* request match {
* case Accepts.Json() & Accepts.Html() => "This request accepts both JSON and HTML"
* }
* }}}
*/
object & {
def unapply(request: RequestHeader): Option[(RequestHeader, RequestHeader)] = Some(request, request)
}

}

/**
* Define a set of extractors allowing to pattern match on the Accept HTTP header of a request
*/
trait AcceptExtractors {

/**
* Common extractors to check if a request accepts JSON, Html, etc.
* Example of use:
* {{{
* request match {
* case Accepts.Json() => Ok(toJson(value))
* case _ => Ok(views.html.show(value))
* }
* }}}
*/
object Accepts {
val Json = Accepting("application/json")
val Html = Accepting("text/html")
val Xml = Accepting("application/xml")
val JavaScript = Accepting("application/javascript")
}

}

/**
* Convenient class to generate extractors checking if a given mime type matches the Accept header of a request.
* Example of use:
* {{{
* val AcceptsMp3 = Accepting("audio/mp3")
* }}}
* Then:
* {{{
* request match {
* case AcceptsMp3() => ...
* }
* }}}
*/
case class Accepting(val mimeType: String) {
def unapply(request: RequestHeader): Boolean = request.accepts(mimeType)
}
Expand Up @@ -70,6 +70,10 @@ trait JavaHelpers {
req.queryString.mapValues(_.toArray).asJava
}

def accept = req.accept.asJava

def accepts(mediaType: String) = req.accepts(mediaType)

def cookies = new JCookies {
def get(name: String) = (for (cookie <- req.cookies.get(name))
yield new JCookie(cookie.name, cookie.value, cookie.maxAge, cookie.path, cookie.domain.getOrElse(null), cookie.secure, cookie.httpOnly)).getOrElse(null)
Expand Down Expand Up @@ -111,6 +115,10 @@ trait JavaHelpers {

def acceptLanguages = req.acceptLanguages.map(new play.i18n.Lang(_)).asJava

def accept = req.accept.asJava

def accepts(mediaType: String) = req.accepts(mediaType)

def queryString = {
req.queryString.mapValues(_.toArray).asJava
}
Expand Down
2 changes: 2 additions & 0 deletions framework/src/play/src/test/scala/play/data/FormSpec.scala
Expand Up @@ -11,6 +11,8 @@ class DummyRequest(data: Map[String, Array[String]]) extends play.mvc.Http.Reque
def path() = "test"
def host() = "localhost"
def acceptLanguages = new java.util.ArrayList[play.i18n.Lang]
def accept = List("text/html").asJava
def accepts(mediaType: String) = false
def headers() = new java.util.HashMap[String, Array[String]]()
def body() = new Http.RequestBody {
override def asFormUrlEncoded = data.asJava
Expand Down
Expand Up @@ -93,4 +93,12 @@ object Application extends Controller {
Ok("fromPath=%s fromQueryString=%s".format(fromPath, fromQueryString))
}

def accept = Action { request =>
request match {
case Accepts.Json() => Ok("json")
case Accepts.Html() => Ok("html")
case _ => BadRequest
}
}

}
9 changes: 9 additions & 0 deletions framework/test/integrationtest/app/controllers/JavaApi.java
Expand Up @@ -71,5 +71,14 @@ public static Result jsonpJava(String callback) {
return ok(jsonp(callback, json));
}

public static Result accept() {
if (request().accepts("application/json")) {
return ok("json");
} else if (request().accepts("text/html")) {
return ok("html");
} else {
return badRequest();
}
}
}

3 changes: 3 additions & 0 deletions framework/test/integrationtest/conf/routes
Expand Up @@ -36,6 +36,9 @@ GET /urldecode/:p controllers.Application.urldecode(p, q)
GET /javascript-routes controllers.Application.javascriptRoutes()
GET /javascript-test controllers.Application.javascriptTest(name)

GET /accept controllers.Application.accept()
GET /accept-java controllers.JavaApi.accept()

# Map static resources from the /public folder to the /public path
GET /public/*file controllers.Assets.at(path="/public", file)

30 changes: 30 additions & 0 deletions framework/test/integrationtest/test/ApplicationSpec.scala
Expand Up @@ -159,6 +159,36 @@ class ApplicationSpec extends Specification {
}
}

"test Accept header mime-types" in {
import play.api.http.HeaderNames._
"Scala API" in {
running(FakeApplication()) {
val url = controllers.routes.Application.accept().url
val Some(result) = routeAndCall(FakeRequest(GET, url).withHeaders(ACCEPT -> "text/html,application/xml;q=0.5"))
contentAsString(result) must equalTo ("html")

val Some(result2) = routeAndCall(FakeRequest(GET, url).withHeaders(ACCEPT -> "text/*"))
contentAsString(result2) must equalTo ("html")

val Some(result3) = routeAndCall(FakeRequest(GET, url).withHeaders(ACCEPT -> "application/json"))
contentAsString(result3) must equalTo ("json")
}
}
"Java API" in {
running(FakeApplication()) {
val url = controllers.routes.JavaApi.accept().url
val Some(result) = routeAndCall(FakeRequest(GET, url).withHeaders(ACCEPT -> "text/html,application/xml;q=0.5"))
contentAsString(result) must equalTo ("html")

val Some(result2) = routeAndCall(FakeRequest(GET, url).withHeaders(ACCEPT -> "text/*"))
contentAsString(result2) must equalTo ("html")

val Some(result3) = routeAndCall(FakeRequest(GET, url).withHeaders(ACCEPT -> "application/json"))
contentAsString(result3) must equalTo ("json")
}
}
}

}

}

7 comments on commit 2ec79c5

@aukevanleeuwen
Copy link

Choose a reason for hiding this comment

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

I guess browsers often send a */* accepts header as fallback:

Accept:text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8

Which means that a construct like:

case Accepts.Json() => Ok("json")

Will usually match as well...

Edit:
The qvalue of the accept header is not taken into account as well. Which is part of the problem I guess. Instead of writing a single match statement you would have to loop over the accept types (in order of qvalue) and see if we can actually serve this content. If none of that is matched, you'd still want to have a fallback I guess.
I'd make a pull request but I can't really come up with a nice way of writing this. Since my Scala isn't up to par with yours, maybe you guys have an idea yourself?

@julienrf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, Scala pattern matching tries the cases in the order they are defined, so you can write the following:

request match {
  case Accepts.Html() => Ok("html")
  case Accepts.Json() => Ok("json")
  case _ => BadRequest
}

So browser’s requests will always be caught by the first case.

@aukevanleeuwen
Copy link

Choose a reason for hiding this comment

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

Still, if I would rather have Json over XML (or the other way around) this would be a problem. Don't get me wrong, this is a good feature to have. However I think that in general the client is the driving force between what kind of content he would have. Determined not only by the type, but also the qvalue. Now it's the controller that determines the order instead.
I kind of feel this is such a thing where you would afterwards think: I wish I would have done the initial implementation differently because now we have API breaking changes (for example).

Again, just trying to give some positive criticism here.

@GigKahuna
Copy link

Choose a reason for hiding this comment

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

I agree with aukevanleeuwen - really, the pattern matching should take into account the qvalues in determining what content type to return. Are there any plans to support this?

@julienrf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, but I don't think it is possible with pattern matching. I'm however working on another way to solve this problem.

@tobnee
Copy link
Contributor

@tobnee tobnee commented on 2ec79c5 Feb 1, 2013

Choose a reason for hiding this comment

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

I have the same impression than Julien. The pattern matching is pretty much a local thing. In order to evaluate based on the q-values your need a global solution. Based on the new helper functions in 2.1 I build this (https://gist.github.com/4594246). It works but the syntax is quite ugly. Still it could be improved with a marco wrapping the statement into a function (making the statement lazily evaluated).

@julienrf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please sign in to comment.