Skip to content
This repository has been archived by the owner on Sep 12, 2021. It is now read-only.

Default fallbacks should be media type-aware #28

Merged
merged 1 commit into from Jan 19, 2014

Conversation

fernandoacorreia
Copy link
Contributor

The default fallbacks for unauthorized and forbidden requests should use content negotiation to produce a response with a content type adequate to the Accept request header.

At least the most common media types should be supported: text/plain, text/html, application/json and application/xml.

See this discussion.

@akkie
Copy link
Contributor

akkie commented Jan 7, 2014

We could set the ACCEPT header in the action for the endpoint.

def protectedAction = SecuredAction("application/json") { implicit request =>
  Ok("Full access")
}

So the user can also implement the local fallback as example:

override def notAuthenticated(request: RequestHeader): Option[Future[SimpleResult]] = {
  Some(Future.successful(
    render {
      case Accepts.Json() => Forbidden(Json.obj("result" -> "Forbidden"))
      case Accepts.Html() => Forbidden("Forbidden")
    }
  ))
}

And this works for the content negotiation and the endpoint approach.

What do you think?

@fernandoacorreia
Copy link
Contributor Author

Relevant comment by @akkie:

A default implementation should be placed here and here. But the global or local fallback should only be implemented by the user.

@fernandoacorreia
Copy link
Contributor Author

I'm not sure I understand this suggestion:

We could set the ACCEPT header in the action for the endpoint.

def protectedAction = SecuredAction("application/json") { implicit request =>
  Ok("Full access")
}

As I understand, the user can implement the 2nd code fragment anyway, supporting the content types which are relevant to the application.

And the default handlers would support the most common content types. The user handler might even delegate to the default handler in case of an unknown or not supported content type.

@akkie
Copy link
Contributor

akkie commented Jan 7, 2014

As per definition the endpoint approach should only have one media type. If I call the endpoint /site/api/profile.json the media type should be application/json even if the client sends an other media type in the ACCEPT header. With this solution we define the media type for an action and override the media type send by the client.

@fernandoacorreia
Copy link
Contributor Author

I see. The Accept specification says:

If no Accept header field is present, then it is assumed that the client accepts all media types. If an Accept header field is present, and if the server cannot send a response which is acceptable according to the combined Accept field value, then the server SHOULD send a 406 (not acceptable) response.

So for an endpoint which produces results of a single type (for instance, JSON), it will check the Accept header. If it is empty or if it includes application/json, it will produce the response with the appropriate Content-Type header; otherwise it should return status 406.

This is done on the application code and I don't think it would be appropriate to override the Accept header sent by the client.

I'm familiar with handling these requirements, so if you agree I could write some code that I think will clarify what I'm saying.

@akkie
Copy link
Contributor

akkie commented Jan 7, 2014

Yes, I agree this is the better solution.

@fernandoacorreia
Copy link
Contributor Author

This commit is an initial approach to implement the default, media type-aware fallbacks. It's not complete yet but it should give you an idea of the direction it's going.

After finishing the default handlers, I plan to write some tests for situations in which these default handlers will and will not be invoked, along with different kinds of media type-aware user actions.

@akkie
Copy link
Contributor

akkie commented Jan 19, 2014

We should create an example in the wiki for that.

@fernandoacorreia
Copy link
Contributor Author

OK. I'll do that after I finish this issue.

@fernandoacorreia
Copy link
Contributor Author

@akkie please review

fernandoacorreia added a commit that referenced this pull request Jan 19, 2014
Default fallbacks should be media type-aware
@fernandoacorreia fernandoacorreia merged commit 6d7f20e into mohiva:master Jan 19, 2014
@fernandoacorreia fernandoacorreia deleted the default-fallbacks branch January 19, 2014 18:07
@fernandoacorreia
Copy link
Contributor Author

Thank you for the review. Corrected and merged.

@akkie
Copy link
Contributor

akkie commented Jan 27, 2014

@fernandoacorreia Do you remember to document this functionality. Maybe we should replace my AJAX example as suggested in #16.

@fernandoacorreia
Copy link
Contributor Author

Thank you for bringing this to my attention. I'll work on that.

@akkie
Copy link
Contributor

akkie commented Jan 27, 2014

Perfect! Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants