GitHub PullRequestReviewCommentEvent at 2014-11-25 16:58:17
Repository: akka
Actor: bantonsson
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/akka/akka/pulls/comments/20876747","id":20876747,"diff_hunk":"@@ -40,88 +46,115 @@ private[http] class HttpServerPipeline(settings: ServerSettings, log: LoggingAda\n     val userIn = Sink.publisher[HttpRequest]\n     val userOut = Source.subscriber[HttpResponse]\n \n+    val oneHundredContinueSource = Source[OneHundredContinue.type](Props[OneHundredContinueSourceActor])\n+    var oneHundredContinueRef: Option[ActorRef] = None // FIXME: unnecessary after fixing #16168\n+\n+    // each connection uses a single (private) request parser instance for all its requests\n+    // which builds a cache of all header instances seen on that connection\n+    val requestParser = rootParser.createSharedCopy(() ⇒ oneHundredContinueRef)\n+\n     val pipeline = FlowGraph { implicit b ⇒\n-      val bypassFanout = Broadcast[(RequestOutput, Source[RequestOutput])](\"bypassFanout\")\n-      val bypassFanin = Merge[Any](\"merge\")\n+      val bypassFanout = Broadcast[RequestOutput](\"bypassFanout\")\n+      val bypassMerge = new BypassMerge\n+\n+      val requestParsing = Flow[ByteString].transform(\"rootParser\", () ⇒ requestParser)\n \n-      val rootParsePipeline =\n-        Flow[ByteString]\n-          .transform(\"rootParser\", () ⇒ rootParser.copyWith(warnOnIllegalHeader))\n+      val requestPreparation =\n+        Flow[RequestOutput]\n           .splitWhen(x ⇒ x.isInstanceOf[MessageStart] || x == MessageEnd)\n           .headAndTail\n+          .collect {\n+            case (RequestStart(method, uri, protocol, headers, createEntity, _, _), entityParts) ⇒\n+              val effectiveUri = HttpRequest.effectiveUri(uri, headers, securedConnection = false, settings.defaultHostHeader)\n+              val effectiveMethod = if (method == HttpMethods.HEAD && settings.transparentHeadRequests) HttpMethods.GET else method\n+              HttpRequest(effectiveMethod, effectiveUri, headers, createEntity(entityParts), protocol)\n+\n+            case (MessageStartError(_, info), _) ⇒ throw new Http.StreamException(info)\n+          }\n \n       val rendererPipeline =\n-        Flow[Any]\n-          .transform(\"applyApplicationBypass\", () ⇒ applyApplicationBypass)\n+        Flow[ResponseRenderingContext]\n           .transform(\"renderer\", () ⇒ responseRendererFactory.newRenderer)\n           .flatten(FlattenStrategy.concat)\n           .transform(\"errorLogger\", () ⇒ errorLogger(log, \"Outgoing response stream error\"))\n \n-      val requestTweaking = Flow[(RequestOutput, Source[RequestOutput])].collect {\n-        case (RequestStart(method, uri, protocol, headers, createEntity, _), entityParts) ⇒\n-          val effectiveUri = HttpRequest.effectiveUri(uri, headers, securedConnection = false, settings.defaultHostHeader)\n-          val effectiveMethod = if (method == HttpMethods.HEAD && settings.transparentHeadRequests) HttpMethods.GET else method\n-          HttpRequest(effectiveMethod, effectiveUri, headers, createEntity(entityParts), protocol)\n-      }\n-\n-      val bypass =\n-        Flow[(RequestOutput, Source[RequestOutput])]\n-          .collect[MessageStart with RequestOutput] { case (x: MessageStart, _) ⇒ x }\n-\n       //FIXME: the graph is unnecessary after fixing #15957\n-      networkIn ~> rootParsePipeline ~> bypassFanout ~> requestTweaking ~> userIn\n-      bypassFanout ~> bypass ~> bypassFanin\n-      userOut ~> bypassFanin ~> rendererPipeline ~> networkOut\n+      networkIn ~> requestParsing ~> bypassFanout ~> requestPreparation ~> userIn\n+      bypassFanout ~> bypassMerge.bypassInput\n+      userOut ~> bypassMerge.applicationInput ~> rendererPipeline ~> networkOut\n+      oneHundredContinueSource ~> bypassMerge.oneHundredContinueInput\n \n     }.run()\n+    oneHundredContinueRef = Some(pipeline.get(oneHundredContinueSource))\n \n     Http.IncomingConnection(tcpConn.remoteAddress, pipeline.get(userIn), pipeline.get(userOut))\n   }\n \n-  /**\n-   * Combines the HttpResponse coming in from the application with the ParserOutput.RequestStart\n-   * produced by the request parser into a ResponseRenderingContext.\n-   * If the parser produced a ParserOutput.ParseError the error response is immediately dispatched to downstream.\n-   */\n-  def applyApplicationBypass =\n-    new PushStage[Any, ResponseRenderingContext] {\n-      var applicationResponse: HttpResponse = _\n-      var requestStart: RequestStart = _\n-\n-      override def onPush(elem: Any, ctx: Context[ResponseRenderingContext]): Directive = elem match {\n-        case response: HttpResponse ⇒\n-          requestStart match {\n-            case null ⇒\n-              applicationResponse = response\n-              ctx.pull()\n-            case x: RequestStart ⇒\n-              requestStart = null\n-              ctx.push(dispatch(x, response))\n-          }\n+  class BypassMerge extends FlexiMerge[ResponseRenderingContext](\"BypassMerge\") {\n+    import FlexiMerge._\n+    val bypassInput = createInputPort[RequestOutput]()\n+    val oneHundredContinueInput = createInputPort[OneHundredContinue.type]()\n+    val applicationInput = createInputPort[HttpResponse]()\n \n-        case requestStart: RequestStart ⇒\n-          applicationResponse match {\n-            case null ⇒\n-              this.requestStart = requestStart\n-              ctx.pull()\n-            case response ⇒\n-              applicationResponse = null\n-              ctx.push(dispatch(requestStart, response))\n-          }\n+    def createMergeLogic() = new MergeLogic[ResponseRenderingContext] {\n+      override def inputHandles(inputCount: Int) = {\n+        require(inputCount == 3, s\"BypassMerge must have 3 connected inputs, was $inputCount\")\n+        Vector(bypassInput, oneHundredContinueInput, applicationInput)\n+      }\n \n-        case ParseError(status, info) ⇒\n-          ctx.push(errorResponse(status, info))\n+      override def initialState = readBypass\n+\n+      val handleMessageStart: (MergeLogicContext, InputHandle, Any) ⇒ State[Any] = {\n+        case (ctx, _, requestStart: RequestStart)      ⇒ waitingForResponseOrStreamError(requestStart)\n+        case (ctx, _, MessageStartError(status, info)) ⇒ finishWithError(ctx, \"request\", status, info)\n+        case x                                         ⇒ throw new IllegalStateException(s\"Unexpected element `$x`, expected MessageStart\")\n       }\n \n-      def dispatch(requestStart: RequestStart, response: HttpResponse): ResponseRenderingContext = {\n-        import requestStart._\n-        ResponseRenderingContext(response, method, protocol, closeAfterResponseCompletion)\n+      val readBypass = State[Any](Read(bypassInput))(handleMessageStart)\n+\n+      def waitingForResponseOrStreamError(requestStart: RequestStart): State[Any] =\n+        State[Any](ReadAny(bypassInput, oneHundredContinueInput, applicationInput)) {\n+          case (ctx, _, response: HttpResponse)         ⇒ sendResponse(ctx, requestStart, response, readBypass)\n+          case (ctx, _, nextMessageStart: MessageStart) ⇒ waitingForResponse(requestStart, nextMessageStart)","path":"akka-http-core/src/main/scala/akka/http/engine/server/HttpServerPipeline.scala","position":173,"original_position":174,"commit_id":"6ac58c2425530da89ca157627d7343aeb1861afa","original_commit_id":"be9c63eefd9eac382e4d55032169fd56e0720e26","user":{"login":"bantonsson","id":805830,"avatar_url":"https://avatars.githubusercontent.com/u/805830?v=3","gravatar_id":"","url":"https://api.github.com/users/bantonsson","html_url":"https://github.com/bantonsson","followers_url":"https://api.github.com/users/bantonsson/followers","following_url":"https://api.github.com/users/bantonsson/following{/other_user}","gists_url":"https://api.github.com/users/bantonsson/gists{/gist_id}","starred_url":"https://api.github.com/users/bantonsson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bantonsson/subscriptions","organizations_url":"https://api.github.com/users/bantonsson/orgs","repos_url":"https://api.github.com/users/bantonsson/repos","events_url":"https://api.github.com/users/bantonsson/events{/privacy}","received_events_url":"https://api.github.com/users/bantonsson/received_events","type":"User","site_admin":false},"body":"Check. I also assumed that the request entity failure would propagate in some way, and that would be the convenience part.","created_at":"2014-11-25T16:58:17Z","updated_at":"2014-11-25T16:58:17Z","html_url":"https://github.com/akka/akka/pull/16337#discussion_r20876747","pull_request_url":"https://api.github.com/repos/akka/akka/pulls/16337","_links":{"self":{"href":"https://api.github.com/repos/akka/akka/pulls/comments/20876747"},"html":{"href":"https://github.com/akka/akka/pull/16337#discussion_r20876747"},"pull_request":{"href":"https://api.github.com/repos/akka/akka/pulls/16337"}}},"pull_request":{"url":"https://api.github.com/repos/akka/akka/pulls/16337","id":24693900,"html_url":"https://github.com/akka/akka/pull/16337","diff_url":"https://github.com/akka/akka/pull/16337.diff","patch_url":"https://github.com/akka/akka/pull/16337.patch","issue_url":"https://api.github.com/repos/akka/akka/issues/16337","number":16337,"state":"open","locked":false,"title":"Add server-side support for `Expect: 100-continue`, other improvements","user":{"login":"sirthias","id":135347,"avatar_url":"https://avatars.githubusercontent.com/u/135347?v=3","gravatar_id":"","url":"https://api.github.com/users/sirthias","html_url":"https://github.com/sirthias","followers_url":"https://api.github.com/users/sirthias/followers","following_url":"https://api.github.com/users/sirthias/following{/other_user}","gists_url":"https://api.github.com/users/sirthias/gists{/gist_id}","starred_url":"https://api.github.com/users/sirthias/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sirthias/subscriptions","organizations_url":"https://api.github.com/users/sirthias/orgs","repos_url":"https://api.github.com/users/sirthias/repos","events_url":"https://api.github.com/users/sirthias/events{/privacy}","received_events_url":"https://api.github.com/users/sirthias/received_events","type":"User","site_admin":false},"body":"This PR improves a number of things in akka-http-core and adds server-side support for `Expect: 100-continue` (#15799).\r\n\r\nThe client-side support depends on #16336 and is therefore postponed for the time being.","created_at":"2014-11-19T15:02:06Z","updated_at":"2014-11-25T16:58:17Z","closed_at":null,"merged_at":null,"merge_commit_sha":"881b3d1995b0dab40b4d3babd7f7039f878fa992","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/akka/akka/pulls/16337/commits","review_comments_url":"https://api.github.com/repos/akka/akka/pulls/16337/comments","review_comment_url":"https://api.github.com/repos/akka/akka/pulls/comments/{number}","comments_url":"https://api.github.com/repos/akka/akka/issues/16337/comments","statuses_url":"https://api.github.com/repos/akka/akka/statuses/6ac58c2425530da89ca157627d7343aeb1861afa","head":{"label":"spray:wip-15799-mathias","ref":"wip-15799-mathias","sha":"6ac58c2425530da89ca157627d7343aeb1861afa","user":{"login":"spray","id":688516,"avatar_url":"https://avatars.githubusercontent.com/u/688516?v=3","gravatar_id":"","url":"https://api.github.com/users/spray","html_url":"https://github.com/spray","followers_url":"https://api.github.com/users/spray/followers","following_url":"https://api.github.com/users/spray/following{/other_user}","gists_url":"https://api.github.com/users/spray/gists{/gist_id}","starred_url":"https://api.github.com/users/spray/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spray/subscriptions","organizations_url":"https://api.github.com/users/spray/orgs","repos_url":"https://api.github.com/users/spray/repos","events_url":"https://api.github.com/users/spray/events{/privacy}","received_events_url":"https://api.github.com/users/spray/received_events","type":"Organization","site_admin":false},"repo":{"id":3809985,"name":"akka","full_name":"spray/akka","owner":{"login":"spray","id":688516,"avatar_url":"https://avatars.githubusercontent.com/u/688516?v=3","gravatar_id":"","url":"https://api.github.com/users/spray","html_url":"https://github.com/spray","followers_url":"https://api.github.com/users/spray/followers","following_url":"https://api.github.com/users/spray/following{/other_user}","gists_url":"https://api.github.com/users/spray/gists{/gist_id}","starred_url":"https://api.github.com/users/spray/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spray/subscriptions","organizations_url":"https://api.github.com/users/spray/orgs","repos_url":"https://api.github.com/users/spray/repos","events_url":"https://api.github.com/users/spray/events{/privacy}","received_events_url":"https://api.github.com/users/spray/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/spray/akka","description":"Akka Project","fork":true,"url":"https://api.github.com/repos/spray/akka","forks_url":"https://api.github.com/repos/spray/akka/forks","keys_url":"https://api.github.com/repos/spray/akka/keys{/key_id}","collaborators_url":"https://api.github.com/repos/spray/akka/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/spray/akka/teams","hooks_url":"https://api.github.com/repos/spray/akka/hooks","issue_events_url":"https://api.github.com/repos/spray/akka/issues/events{/number}","events_url":"https://api.github.com/repos/spray/akka/events","assignees_url":"https://api.github.com/repos/spray/akka/assignees{/user}","branches_url":"https://api.github.com/repos/spray/akka/branches{/branch}","tags_url":"https://api.github.com/repos/spray/akka/tags","blobs_url":"https://api.github.com/repos/spray/akka/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/spray/akka/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/spray/akka/git/refs{/sha}","trees_url":"https://api.github.com/repos/spray/akka/git/trees{/sha}","statuses_url":"https://api.github.com/repos/spray/akka/statuses/{sha}","languages_url":"https://api.github.com/repos/spray/akka/languages","stargazers_url":"https://api.github.com/repos/spray/akka/stargazers","contributors_url":"https://api.github.com/repos/spray/akka/contributors","subscribers_url":"https://api.github.com/repos/spray/akka/subscribers","subscription_url":"https://api.github.com/repos/spray/akka/subscription","commits_url":"https://api.github.com/repos/spray/akka/commits{/sha}","git_commits_url":"https://api.github.com/repos/spray/akka/git/commits{/sha}","comments_url":"https://api.github.com/repos/spray/akka/comments{/number}","issue_comment_url":"https://api.github.com/repos/spray/akka/issues/comments/{number}","contents_url":"https://api.github.com/repos/spray/akka/contents/{+path}","compare_url":"https://api.github.com/repos/spray/akka/compare/{base}...{head}","merges_url":"https://api.github.com/repos/spray/akka/merges","archive_url":"https://api.github.com/repos/spray/akka/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/spray/akka/downloads","issues_url":"https://api.github.com/repos/spray/akka/issues{/number}","pulls_url":"https://api.github.com/repos/spray/akka/pulls{/number}","milestones_url":"https://api.github.com/repos/spray/akka/milestones{/number}","notifications_url":"https://api.github.com/repos/spray/akka/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/spray/akka/labels{/name}","releases_url":"https://api.github.com/repos/spray/akka/releases{/id}","created_at":"2012-03-23T15:57:31Z","updated_at":"2014-10-15T18:53:07Z","pushed_at":"2014-11-25T16:53:48Z","git_url":"git://github.com/spray/akka.git","ssh_url":"git@github.com:spray/akka.git","clone_url":"https://github.com/spray/akka.git","svn_url":"https://github.com/spray/akka","homepage":"http://akka.io","size":154777,"stargazers_count":7,"watchers_count":7,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":7,"default_branch":"master"}},"base":{"label":"akka:release-2.3-dev","ref":"release-2.3-dev","sha":"abb9e88c6588e4971738ab2d7a6d9f4434dd0ea4","user":{"login":"akka","id":1496237,"avatar_url":"https://avatars.githubusercontent.com/u/1496237?v=3","gravatar_id":"","url":"https://api.github.com/users/akka","html_url":"https://github.com/akka","followers_url":"https://api.github.com/users/akka/followers","following_url":"https://api.github.com/users/akka/following{/other_user}","gists_url":"https://api.github.com/users/akka/gists{/gist_id}","starred_url":"https://api.github.com/users/akka/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akka/subscriptions","organizations_url":"https://api.github.com/users/akka/orgs","repos_url":"https://api.github.com/users/akka/repos","events_url":"https://api.github.com/users/akka/events{/privacy}","received_events_url":"https://api.github.com/users/akka/received_events","type":"Organization","site_admin":false},"repo":{"id":130013,"name":"akka","full_name":"akka/akka","owner":{"login":"akka","id":1496237,"avatar_url":"https://avatars.githubusercontent.com/u/1496237?v=3","gravatar_id":"","url":"https://api.github.com/users/akka","html_url":"https://github.com/akka","followers_url":"https://api.github.com/users/akka/followers","following_url":"https://api.github.com/users/akka/following{/other_user}","gists_url":"https://api.github.com/users/akka/gists{/gist_id}","starred_url":"https://api.github.com/users/akka/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akka/subscriptions","organizations_url":"https://api.github.com/users/akka/orgs","repos_url":"https://api.github.com/users/akka/repos","events_url":"https://api.github.com/users/akka/events{/privacy}","received_events_url":"https://api.github.com/users/akka/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/akka/akka","description":"Akka Project","fork":false,"url":"https://api.github.com/repos/akka/akka","forks_url":"https://api.github.com/repos/akka/akka/forks","keys_url":"https://api.github.com/repos/akka/akka/keys{/key_id}","collaborators_url":"https://api.github.com/repos/akka/akka/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/akka/akka/teams","hooks_url":"https://api.github.com/repos/akka/akka/hooks","issue_events_url":"https://api.github.com/repos/akka/akka/issues/events{/number}","events_url":"https://api.github.com/repos/akka/akka/events","assignees_url":"https://api.github.com/repos/akka/akka/assignees{/user}","branches_url":"https://api.github.com/repos/akka/akka/branches{/branch}","tags_url":"https://api.github.com/repos/akka/akka/tags","blobs_url":"https://api.github.com/repos/akka/akka/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/akka/akka/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/akka/akka/git/refs{/sha}","trees_url":"https://api.github.com/repos/akka/akka/git/trees{/sha}","statuses_url":"https://api.github.com/repos/akka/akka/statuses/{sha}","languages_url":"https://api.github.com/repos/akka/akka/languages","stargazers_url":"https://api.github.com/repos/akka/akka/stargazers","contributors_url":"https://api.github.com/repos/akka/akka/contributors","subscribers_url":"https://api.github.com/repos/akka/akka/subscribers","subscription_url":"https://api.github.com/repos/akka/akka/subscription","commits_url":"https://api.github.com/repos/akka/akka/commits{/sha}","git_commits_url":"https://api.github.com/repos/akka/akka/git/commits{/sha}","comments_url":"https://api.github.com/repos/akka/akka/comments{/number}","issue_comment_url":"https://api.github.com/repos/akka/akka/issues/comments/{number}","contents_url":"https://api.github.com/repos/akka/akka/contents/{+path}","compare_url":"https://api.github.com/repos/akka/akka/compare/{base}...{head}","merges_url":"https://api.github.com/repos/akka/akka/merges","archive_url":"https://api.github.com/repos/akka/akka/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/akka/akka/downloads","issues_url":"https://api.github.com/repos/akka/akka/issues{/number}","pulls_url":"https://api.github.com/repos/akka/akka/pulls{/number}","milestones_url":"https://api.github.com/repos/akka/akka/milestones{/number}","notifications_url":"https://api.github.com/repos/akka/akka/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/akka/akka/labels{/name}","releases_url":"https://api.github.com/repos/akka/akka/releases{/id}","created_at":"2009-02-16T12:51:54Z","updated_at":"2014-11-24T18:59:08Z","pushed_at":"2014-11-25T16:32:49Z","git_url":"git://github.com/akka/akka.git","ssh_url":"git@github.com:akka/akka.git","clone_url":"https://github.com/akka/akka.git","svn_url":"https://github.com/akka/akka","homepage":"http://akka.io","size":244766,"stargazers_count":3222,"watchers_count":3222,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":904,"mirror_url":null,"open_issues_count":494,"forks":904,"open_issues":494,"watchers":3222,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/akka/akka/pulls/16337"},"html":{"href":"https://github.com/akka/akka/pull/16337"},"issue":{"href":"https://api.github.com/repos/akka/akka/issues/16337"},"comments":{"href":"https://api.github.com/repos/akka/akka/issues/16337/comments"},"review_comments":{"href":"https://api.github.com/repos/akka/akka/pulls/16337/comments"},"review_comment":{"href":"https://api.github.com/repos/akka/akka/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/akka/akka/pulls/16337/commits"},"statuses":{"href":"https://api.github.com/repos/akka/akka/statuses/6ac58c2425530da89ca157627d7343aeb1861afa"}}}}