Implement the new Play 2.4 messages API

[akkie]

See: https://www.playframework.com/documentation/2.4.x/ScalaI18N

[rfranco]

@akkie is it possible to do that without spray the Messages for every place? Because Messages can be extract from the RequestHeader.

maybe one solution could be convert DefaultEndpointHandler to the Play ErrorHandler and inject the MessagesApi

WDYT?

[akkie]

The Lang which is contained in the messages can be retrieved from many different sources. The default in Play is to extract it from the request. This can be easily done with the request2Messages method in the I18nSupport method. There are now two possibilities to override this default behavior:

• Retrieve the Lang also from request:

  The Lang can also be retrieved from another request header(Cookie header, Proxy sets a custom X-LANG header, ...). This can be implemented by overriding the request2Messages method in the I18nSupport trait. Following your suggestion, a user must now additionally add the overridden request2Messages method in scope, everywhere a Messages instance is used. With my suggestion, the user must only add it once at the controller level.

• Retrieve the language from another source

  It is also possible that a user has the need to retrieve the Lang from a database. With your suggestion the user must query the Lang from the database, everywhere a Messages instance is used. I'm also not sure if it is easily possible to pass the database dependencies around. With my suggestion, the user must only add it once at the controller level.

In my opinion, as the default behavior, it is better to define the lang in the entry point(Controller) and then pass it around to the other components. If a user needs another Lang in the sub components, he can still override it with one of the different methods. But it is very unlikely that a Lang changes in a single request cycle.

[rfranco]

My concern is that now need to every Controler that extends Silhouette is mandatory to inject Environment and MessageApi, so my sugestion is minimize the dependencies.

There are 4 place where have Messages as param, Authorization, Event, SecuredSettings and DefaultEndpointHandler, for me none of those make sense to have the Messages.

The only one that use Messages is the DefaultEndpointHandler that could be change to a ErrorHandler.

I agree that the best point to set Lang is the Controller but really need to be mandatory?

[akkie]

There are 4 place where have Messages as param, Authorization, Event, SecuredSettings and DefaultEndpointHandler, for me none of those make sense to have the Messages.

The SecuredSettings needs the Messages because it can deal with templates. For the Authorization trait it could be possible to restrict access based on a locale. And for the events it may be useful to handle events based on the lang or send localized emails.

One big problem with Secure Social was, that it wasn't possible to internationalize your application. This was one of the main reasons I've forked the library and created Silhouette. I see your concern, but I think omitting the Messages from the API is a step backward. Maybe we could find another solution for this problem.

The only one that use Messages is the DefaultEndpointHandler that could be change to a ErrorHandler.

I'm not sure if it easily possible to replace the DefaultEndpointHandler with an ErrorHandler. The DefaultEndpointHandler is responsible to display the result for the current content type. Where the ErrorHandler is responsible to handle exceptions. I'm also not sure if it's possible to have multiple error handlers. Otherwise the user must compose it's own error handler based of different error handlers. We cannot create a package in the global package and therefore we must set play.http.errorHandler in the config. But what happens if a user overrides this entry?

[akkie]

@rfranco What do you think about something like this?

[rfranco]

@akkie it's looks ok but i don't know if that is realy necessary create a wrapper for Messages, because Message still depends on MessagesApi maybe the Enviromnent can hold a MessagesApi

trait Environment[I <: Identity, T <: Authenticator] {
  def identityService: IdentityService[I]
  def authenticatorService: AuthenticatorService[T]
  def requestProviders: Seq[RequestProvider]
  def eventBus: EventBus
  def messagesApi: MessageApi
}

trait Silhouette[I <: Identity, A <: Authenticator] extends Controller with Logger with play.api.i18n.I18nSupport  {
 lazy val messagesApi: MessagesApi = env.messagesApi
}

WDYT?

[akkie]

Let me put it this way: Silhouette depends on the Play messages support. So why not! I'll change it and update the pull request. Thanks, for the suggestion!

[akkie]

@rfranco Please could you review it again?

[rfranco]

Is it necessary Messages as param?

because it's possible to do that

class I18nAuthorization (implicit messagesApi: MessagesApi) extends Authorization[User] with I18nSupport {

  def isAuthorized(identity: User)(implicit request: RequestHeader): Unit = {
    val messages: Messages = request2Messages
    ...
  }
}

[akkie]

@rfranco

#335 (comment)

I agree that the best point to set Lang is the Controller

I thought we agreed that it's the best to define the Messages in the controller so that the user must not override the logic in different places in case of different lang sources. Inside a Silhouette controller the Messages instance will be resolved implicitly. So there is no extra effort for the user to pass the Messages instance.

[rfranco]

@akkie Ok, i agreed, looks good to go!