Port Controller from Spray to Akka #2218
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| # default application configuration file for akka | ||
| include "logging" | ||
|
|
||
| akka.http { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,56 +19,56 @@ package whisk.http | |
|
|
||
| import scala.concurrent.duration.DurationInt | ||
| import scala.language.postfixOps | ||
| import scala.collection.immutable.Seq | ||
| import scala.concurrent.Future | ||
|
|
||
| import akka.actor.Actor | ||
| import akka.actor.ActorSystem | ||
| import akka.actor.Props | ||
| import akka.event.Logging | ||
| import akka.japi.Creator | ||
| import akka.util.Timeout | ||
| import akka.http.scaladsl.server.Directives | ||
| import akka.http.scaladsl.server.directives.DebuggingDirectives | ||
| import akka.http.scaladsl.server.directives.LogEntry | ||
| import akka.http.scaladsl.server.Route | ||
| import akka.http.scaladsl.model.HttpRequest | ||
| import akka.http.scaladsl.model._ | ||
| import akka.http.scaladsl.server.RejectionHandler | ||
| import akka.http.scaladsl.server.UnacceptedResponseContentTypeRejection | ||
| import akka.http.scaladsl.server.RouteResult.Rejected | ||
| import akka.http.scaladsl.Http | ||
|
|
||
| import spray.json._ | ||
|
|
||
| import whisk.common.LogMarker | ||
| import whisk.common.LogMarkerToken | ||
| import whisk.common.Logging | ||
| import whisk.common.LoggingMarkers | ||
| import whisk.common.TransactionCounter | ||
| import whisk.common.TransactionId | ||
| import akka.stream.ActorMaterializer | ||
|
|
||
| /** | ||
| * This trait extends the Akka Directives and Actor with logging and transaction counting | ||
| * facilities common to all OpenWhisk REST services. | ||
| */ | ||
| trait BasicHttpService extends Directives with Actor with TransactionCounter { | ||
| implicit def logging: Logging | ||
| implicit val materializer = ActorMaterializer() | ||
| implicit val actorSystem = context.system | ||
| implicit val executionContext = actorSystem.dispatcher | ||
|
|
||
| val port: Int | ||
|
|
||
| /** Rejection handler to terminate connection on a bad request. Delegates to Akka handler. */ | ||
| implicit def customRejectionHandler(implicit transid: TransactionId) = | ||
| RejectionHandler.default.mapRejectionResponse { | ||
| case res @ HttpResponse(_, _, ent: HttpEntity.Strict, _) => | ||
| val error = ErrorResponse(ent.data.utf8String, transid).toJson | ||
| res.copy(entity = HttpEntity(ContentTypes.`application/json`, error.compactPrint)) | ||
| case x => x | ||
| } | ||
|
|
||
| /** | ||
| * Gets the routes implemented by the HTTP service. | ||
|
|
@@ -86,39 +86,47 @@ trait BasicHttpService extends HttpService with TransactionCounter { | |
| */ | ||
| def loglevelForRoute(route: String): Logging.LogLevel = Logging.InfoLevel | ||
|
|
||
| /** Rejection handler to terminate connection on a bad request. Delegates to Akka handler. */ | ||
| val prioritizeRejections = recoverRejections { rejections => | ||
| val priorityRejection = rejections.find { | ||
| case rejection: UnacceptedResponseContentTypeRejection => true | ||
| case _ => false | ||
| } | ||
|
There was a problem hiding this comment. Idiomatically these would be: val priorityRejection = rejections.find { case _: UnacceptedResponseContentTypeRejection => true }
priorityRejection.map(rejection => Rejected(Seq(rejection))).getOrElse(Rejected(rejections))
|
||
|
|
||
| priorityRejection.map(rejection => Rejected(Seq(rejection))).getOrElse(Rejected(rejections)) | ||
| } | ||
|
|
||
| /** | ||
| * Receives a message and runs the router. | ||
| */ | ||
| def route: Route = { | ||
| assignId { implicit transid => | ||
| handleRejections(customRejectionHandler) { | ||
|
There was a problem hiding this comment. @dubee why did the directive order change? |
||
| prioritizeRejections { | ||
| DebuggingDirectives.logRequest(logRequestInfo _) { | ||
| DebuggingDirectives.logRequestResult(logResponseInfo _) { | ||
| toStrictEntity(1.second) { | ||
| routes | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| def receive = { | ||
| case _ => | ||
| } | ||
|
There was a problem hiding this comment. why is this needed? Is an Actor required to implement it? There was a problem hiding this comment. Yes, the Actor requires receive to be overridden. Tried not inheriting from the Actor, but it looks like we need access to the Actor context in controller.scala There was a problem hiding this comment. can you point out where that is? There was a problem hiding this comment. There was a problem hiding this comment. i'm fairly sure we can eliminate this but it's a bit of a change - I'll send a PR against your branch or we can deal with it later. |
||
|
|
||
| /** Assigns transaction id to every request. */ | ||
| protected val assignId = extract(_ => transid()) | ||
|
|
||
|
There was a problem hiding this comment. is my understanding correct that the prioritization above lowers the priority on the auth rejection? |
||
|
There was a problem hiding this comment. are we losing the rejection log message? There was a problem hiding this comment. i'm ok dropping this - it's of nominal value. |
||
| /** Generates log entry for every request. */ | ||
| protected def logRequestInfo(req: HttpRequest)(implicit tid: TransactionId): LogEntry = { | ||
| val m = req.method.toString | ||
| val p = req.uri.path.toString | ||
| val q = req.uri.query().toString | ||
|
There was a problem hiding this comment. is calling query side effecting? There was a problem hiding this comment. Seems that the code won't compile without the parentheses. There was a problem hiding this comment. Can't seem to comment on your above comment for |
||
| val l = loglevelForRoute(p) | ||
| LogEntry(s"[$tid] $m $p $q", l) | ||
| } | ||
|
|
@@ -137,29 +145,19 @@ trait BasicHttpService extends HttpService with TransactionCounter { | |
| Some(LogEntry(s"[$tid] [$name] $marker", l)) | ||
| case _ => None // other kind of responses | ||
| } | ||
|
|
||
| val bindingFuture = { | ||
| Http().bindAndHandle(route, "0.0.0.0", port) | ||
| } | ||
|
|
||
| def shutdown(): Future[Unit] = { | ||
| bindingFuture.flatMap(_.unbind()).map(_ => ()) | ||
| } | ||
| } | ||
|
|
||
| object BasicHttpService extends Directives { | ||
| def startService[T <: Actor](system: ActorSystem, name: String, interface: String, service: Creator[T]) = { | ||
| val actor = system.actorOf(Props.create(service), s"$name-service") | ||
| implicit val timeout = Timeout(5 seconds) | ||
|
There was a problem hiding this comment. this in particular was used for the auth failure mode #2607. we can delete CustomRejection since it will be unused now - assuming we keep this behavior. |
||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of those should be removable to complete the pull-request, modulo spray-json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the above, spray-caching hasn't been ported to Akka yet. There is an issue open for it here: akka/akka-http#213. Will look at replacing spray-client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like spray-client is used by HttpClient.scala, but nothing uses the HttpClient code, so I deleted it.