{"action":"created","comment":{"url":"https://api.github.com/repos/twitter/finatra/pulls/comments/107200234","pull_request_review_id":28158169,"id":107200234,"diff_hunk":"@@ -4,35 +4,74 @@ import com.twitter.finagle.Filter\n import com.twitter.finagle.http.Method._\n import com.twitter.finagle.http.{Method, RouteIndex}\n import com.twitter.inject.Injector\n+import com.twitter.util.Var\n import scala.collection.mutable.ArrayBuffer\n \n-private[http] trait RouteDSL { self =>\n+private[http] case class RouteContext(prefix: String, buildFilter: (Injector) => HttpFilter)\n \n+private trait MutableRouteState {\n+  private[http] val context: RouteContext = RouteContext(\n+    prefix = \"\",\n+    buildFilter = (_) => Filter.identity\n+  )\n+\n+  private[http] lazy val contextRef = Var(context)\n+}\n+\n+private class FilteredDSL[FilterType <: HttpFilter : Manifest] extends RouteDSL {","path":"http/src/main/scala/com/twitter/finatra/http/RouteDSL.scala","position":19,"original_position":19,"commit_id":"d662153cf678cac709526ca0b388961ae9fddd04","original_commit_id":"d662153cf678cac709526ca0b388961ae9fddd04","user":{"login":"chrisbenincasa","id":1640671,"avatar_url":"https://avatars1.githubusercontent.com/u/1640671?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisbenincasa","html_url":"https://github.com/chrisbenincasa","followers_url":"https://api.github.com/users/chrisbenincasa/followers","following_url":"https://api.github.com/users/chrisbenincasa/following{/other_user}","gists_url":"https://api.github.com/users/chrisbenincasa/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisbenincasa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisbenincasa/subscriptions","organizations_url":"https://api.github.com/users/chrisbenincasa/orgs","repos_url":"https://api.github.com/users/chrisbenincasa/repos","events_url":"https://api.github.com/users/chrisbenincasa/events{/privacy}","received_events_url":"https://api.github.com/users/chrisbenincasa/received_events","type":"User","site_admin":false},"body":"Will do. ","created_at":"2017-03-21T16:13:37Z","updated_at":"2017-03-21T16:13:37Z","html_url":"https://github.com/twitter/finatra/pull/393#discussion_r107200234","pull_request_url":"https://api.github.com/repos/twitter/finatra/pulls/393","_links":{"self":{"href":"https://api.github.com/repos/twitter/finatra/pulls/comments/107200234"},"html":{"href":"https://github.com/twitter/finatra/pull/393#discussion_r107200234"},"pull_request":{"href":"https://api.github.com/repos/twitter/finatra/pulls/393"}}},"pull_request":{"url":"https://api.github.com/repos/twitter/finatra/pulls/393","id":110138459,"html_url":"https://github.com/twitter/finatra/pull/393","diff_url":"https://github.com/twitter/finatra/pull/393.diff","patch_url":"https://github.com/twitter/finatra/pull/393.patch","issue_url":"https://api.github.com/repos/twitter/finatra/issues/393","number":393,"state":"open","locked":false,"title":"finatra-http: Increase composability and flexibility of RouteDSL","user":{"login":"chrisbenincasa","id":1640671,"avatar_url":"https://avatars1.githubusercontent.com/u/1640671?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisbenincasa","html_url":"https://github.com/chrisbenincasa","followers_url":"https://api.github.com/users/chrisbenincasa/followers","following_url":"https://api.github.com/users/chrisbenincasa/following{/other_user}","gists_url":"https://api.github.com/users/chrisbenincasa/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisbenincasa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisbenincasa/subscriptions","organizations_url":"https://api.github.com/users/chrisbenincasa/orgs","repos_url":"https://api.github.com/users/chrisbenincasa/repos","events_url":"https://api.github.com/users/chrisbenincasa/events{/privacy}","received_events_url":"https://api.github.com/users/chrisbenincasa/received_events","type":"User","site_admin":false},"body":"Problem:\r\n\r\nToday, the routing DSL supports two composable pieces (prefix and\r\nfilter). These are implemented under different paradigms; filter\r\nproduces a new Route context while prefix takes a block and uses a var\r\nto track the prefix.\r\n\r\nThere are a few issues / unexpected behaviors that result:\r\n* Prefixes are not composable\r\n* Filtered routes inside of prefixes don't work (https://github.com/twitter/finatra/issues/392)\r\n* Filtered prefixes are not supported\r\n* The syntax is rigid between the two\r\n\r\nSolution\r\n\r\nThe changes here revist the structure of RouteDSL and unify the way\r\nroutes are defined in Controllers to increase flexibility and remove\r\nunexpected behavior and bugs. Namely, mutable state is now centralized\r\nto a twitter-util Var cell and an API is laid out for creating\r\n\"specialized\" RouteDSL contexts (FilteredDSL and PrefixedDSL).\r\n\r\nThis allows for a great amount of flexibility for the user when defining\r\nroutes as well as increasing the extensibility of the DSL for future\r\nadditions. For example:\r\n\r\nFiltered prefixes:\r\n```scala\r\nfilter[MyFilter].prefix(\"/v1/prefix\") {\r\n    // ...\r\n}\r\n```\r\n\r\nCascading prefixes:\r\n```scala\r\nprefix(\"/v1\") {\r\n  prefix(\"/foo\") {\r\n    prefix(\"/bar\") {\r\n     // ..\r\n    }\r\n  }\r\n}\r\n```\r\n\r\n\"Freestyle\" filter syntax:\r\n\r\n```scala\r\nfilter[MyFilter] {\r\n  // ...\r\n}\r\n```","created_at":"2017-03-10T14:30:15Z","updated_at":"2017-03-21T16:13:37Z","closed_at":null,"merged_at":null,"merge_commit_sha":"51d2e1241c5a9c0203e40cb8e27d0fc06d8233ae","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/twitter/finatra/pulls/393/commits","review_comments_url":"https://api.github.com/repos/twitter/finatra/pulls/393/comments","review_comment_url":"https://api.github.com/repos/twitter/finatra/pulls/comments{/number}","comments_url":"https://api.github.com/repos/twitter/finatra/issues/393/comments","statuses_url":"https://api.github.com/repos/twitter/finatra/statuses/d662153cf678cac709526ca0b388961ae9fddd04","head":{"label":"chrisbenincasa:route-dsl-refactor","ref":"route-dsl-refactor","sha":"d662153cf678cac709526ca0b388961ae9fddd04","user":{"login":"chrisbenincasa","id":1640671,"avatar_url":"https://avatars1.githubusercontent.com/u/1640671?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisbenincasa","html_url":"https://github.com/chrisbenincasa","followers_url":"https://api.github.com/users/chrisbenincasa/followers","following_url":"https://api.github.com/users/chrisbenincasa/following{/other_user}","gists_url":"https://api.github.com/users/chrisbenincasa/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisbenincasa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisbenincasa/subscriptions","organizations_url":"https://api.github.com/users/chrisbenincasa/orgs","repos_url":"https://api.github.com/users/chrisbenincasa/repos","events_url":"https://api.github.com/users/chrisbenincasa/events{/privacy}","received_events_url":"https://api.github.com/users/chrisbenincasa/received_events","type":"User","site_admin":false},"repo":{"id":84069423,"name":"finatra","full_name":"chrisbenincasa/finatra","owner":{"login":"chrisbenincasa","id":1640671,"avatar_url":"https://avatars1.githubusercontent.com/u/1640671?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisbenincasa","html_url":"https://github.com/chrisbenincasa","followers_url":"https://api.github.com/users/chrisbenincasa/followers","following_url":"https://api.github.com/users/chrisbenincasa/following{/other_user}","gists_url":"https://api.github.com/users/chrisbenincasa/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisbenincasa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisbenincasa/subscriptions","organizations_url":"https://api.github.com/users/chrisbenincasa/orgs","repos_url":"https://api.github.com/users/chrisbenincasa/repos","events_url":"https://api.github.com/users/chrisbenincasa/events{/privacy}","received_events_url":"https://api.github.com/users/chrisbenincasa/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/chrisbenincasa/finatra","description":"Fast, testable, Scala services built on Twitter-Server and Finagle","fork":true,"url":"https://api.github.com/repos/chrisbenincasa/finatra","forks_url":"https://api.github.com/repos/chrisbenincasa/finatra/forks","keys_url":"https://api.github.com/repos/chrisbenincasa/finatra/keys{/key_id}","collaborators_url":"https://api.github.com/repos/chrisbenincasa/finatra/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/chrisbenincasa/finatra/teams","hooks_url":"https://api.github.com/repos/chrisbenincasa/finatra/hooks","issue_events_url":"https://api.github.com/repos/chrisbenincasa/finatra/issues/events{/number}","events_url":"https://api.github.com/repos/chrisbenincasa/finatra/events","assignees_url":"https://api.github.com/repos/chrisbenincasa/finatra/assignees{/user}","branches_url":"https://api.github.com/repos/chrisbenincasa/finatra/branches{/branch}","tags_url":"https://api.github.com/repos/chrisbenincasa/finatra/tags","blobs_url":"https://api.github.com/repos/chrisbenincasa/finatra/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/chrisbenincasa/finatra/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/chrisbenincasa/finatra/git/refs{/sha}","trees_url":"https://api.github.com/repos/chrisbenincasa/finatra/git/trees{/sha}","statuses_url":"https://api.github.com/repos/chrisbenincasa/finatra/statuses/{sha}","languages_url":"https://api.github.com/repos/chrisbenincasa/finatra/languages","stargazers_url":"https://api.github.com/repos/chrisbenincasa/finatra/stargazers","contributors_url":"https://api.github.com/repos/chrisbenincasa/finatra/contributors","subscribers_url":"https://api.github.com/repos/chrisbenincasa/finatra/subscribers","subscription_url":"https://api.github.com/repos/chrisbenincasa/finatra/subscription","commits_url":"https://api.github.com/repos/chrisbenincasa/finatra/commits{/sha}","git_commits_url":"https://api.github.com/repos/chrisbenincasa/finatra/git/commits{/sha}","comments_url":"https://api.github.com/repos/chrisbenincasa/finatra/comments{/number}","issue_comment_url":"https://api.github.com/repos/chrisbenincasa/finatra/issues/comments{/number}","contents_url":"https://api.github.com/repos/chrisbenincasa/finatra/contents/{+path}","compare_url":"https://api.github.com/repos/chrisbenincasa/finatra/compare/{base}...{head}","merges_url":"https://api.github.com/repos/chrisbenincasa/finatra/merges","archive_url":"https://api.github.com/repos/chrisbenincasa/finatra/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/chrisbenincasa/finatra/downloads","issues_url":"https://api.github.com/repos/chrisbenincasa/finatra/issues{/number}","pulls_url":"https://api.github.com/repos/chrisbenincasa/finatra/pulls{/number}","milestones_url":"https://api.github.com/repos/chrisbenincasa/finatra/milestones{/number}","notifications_url":"https://api.github.com/repos/chrisbenincasa/finatra/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/chrisbenincasa/finatra/labels{/name}","releases_url":"https://api.github.com/repos/chrisbenincasa/finatra/releases{/id}","deployments_url":"https://api.github.com/repos/chrisbenincasa/finatra/deployments","created_at":"2017-03-06T12:14:06Z","updated_at":"2017-03-06T12:14:09Z","pushed_at":"2017-03-11T22:55:19Z","git_url":"git://github.com/chrisbenincasa/finatra.git","ssh_url":"git@github.com:chrisbenincasa/finatra.git","clone_url":"https://github.com/chrisbenincasa/finatra.git","svn_url":"https://github.com/chrisbenincasa/finatra","homepage":"https://twitter.github.io/finatra/","size":10324,"stargazers_count":0,"watchers_count":0,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"develop"}},"base":{"label":"twitter:develop","ref":"develop","sha":"27a3d055f6eedf823e7319860663c33d582d19df","user":{"login":"twitter","id":50278,"avatar_url":"https://avatars2.githubusercontent.com/u/50278?v=3","gravatar_id":"","url":"https://api.github.com/users/twitter","html_url":"https://github.com/twitter","followers_url":"https://api.github.com/users/twitter/followers","following_url":"https://api.github.com/users/twitter/following{/other_user}","gists_url":"https://api.github.com/users/twitter/gists{/gist_id}","starred_url":"https://api.github.com/users/twitter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/twitter/subscriptions","organizations_url":"https://api.github.com/users/twitter/orgs","repos_url":"https://api.github.com/users/twitter/repos","events_url":"https://api.github.com/users/twitter/events{/privacy}","received_events_url":"https://api.github.com/users/twitter/received_events","type":"Organization","site_admin":false},"repo":{"id":3465312,"name":"finatra","full_name":"twitter/finatra","owner":{"login":"twitter","id":50278,"avatar_url":"https://avatars2.githubusercontent.com/u/50278?v=3","gravatar_id":"","url":"https://api.github.com/users/twitter","html_url":"https://github.com/twitter","followers_url":"https://api.github.com/users/twitter/followers","following_url":"https://api.github.com/users/twitter/following{/other_user}","gists_url":"https://api.github.com/users/twitter/gists{/gist_id}","starred_url":"https://api.github.com/users/twitter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/twitter/subscriptions","organizations_url":"https://api.github.com/users/twitter/orgs","repos_url":"https://api.github.com/users/twitter/repos","events_url":"https://api.github.com/users/twitter/events{/privacy}","received_events_url":"https://api.github.com/users/twitter/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/twitter/finatra","description":"Fast, testable, Scala services built on Twitter-Server and Finagle","fork":false,"url":"https://api.github.com/repos/twitter/finatra","forks_url":"https://api.github.com/repos/twitter/finatra/forks","keys_url":"https://api.github.com/repos/twitter/finatra/keys{/key_id}","collaborators_url":"https://api.github.com/repos/twitter/finatra/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/twitter/finatra/teams","hooks_url":"https://api.github.com/repos/twitter/finatra/hooks","issue_events_url":"https://api.github.com/repos/twitter/finatra/issues/events{/number}","events_url":"https://api.github.com/repos/twitter/finatra/events","assignees_url":"https://api.github.com/repos/twitter/finatra/assignees{/user}","branches_url":"https://api.github.com/repos/twitter/finatra/branches{/branch}","tags_url":"https://api.github.com/repos/twitter/finatra/tags","blobs_url":"https://api.github.com/repos/twitter/finatra/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/twitter/finatra/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/twitter/finatra/git/refs{/sha}","trees_url":"https://api.github.com/repos/twitter/finatra/git/trees{/sha}","statuses_url":"https://api.github.com/repos/twitter/finatra/statuses/{sha}","languages_url":"https://api.github.com/repos/twitter/finatra/languages","stargazers_url":"https://api.github.com/repos/twitter/finatra/stargazers","contributors_url":"https://api.github.com/repos/twitter/finatra/contributors","subscribers_url":"https://api.github.com/repos/twitter/finatra/subscribers","subscription_url":"https://api.github.com/repos/twitter/finatra/subscription","commits_url":"https://api.github.com/repos/twitter/finatra/commits{/sha}","git_commits_url":"https://api.github.com/repos/twitter/finatra/git/commits{/sha}","comments_url":"https://api.github.com/repos/twitter/finatra/comments{/number}","issue_comment_url":"https://api.github.com/repos/twitter/finatra/issues/comments{/number}","contents_url":"https://api.github.com/repos/twitter/finatra/contents/{+path}","compare_url":"https://api.github.com/repos/twitter/finatra/compare/{base}...{head}","merges_url":"https://api.github.com/repos/twitter/finatra/merges","archive_url":"https://api.github.com/repos/twitter/finatra/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/twitter/finatra/downloads","issues_url":"https://api.github.com/repos/twitter/finatra/issues{/number}","pulls_url":"https://api.github.com/repos/twitter/finatra/pulls{/number}","milestones_url":"https://api.github.com/repos/twitter/finatra/milestones{/number}","notifications_url":"https://api.github.com/repos/twitter/finatra/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/twitter/finatra/labels{/name}","releases_url":"https://api.github.com/repos/twitter/finatra/releases{/id}","deployments_url":"https://api.github.com/repos/twitter/finatra/deployments","created_at":"2012-02-16T23:55:29Z","updated_at":"2017-03-20T17:30:27Z","pushed_at":"2017-03-21T06:31:47Z","git_url":"git://github.com/twitter/finatra.git","ssh_url":"git@github.com:twitter/finatra.git","clone_url":"https://github.com/twitter/finatra.git","svn_url":"https://github.com/twitter/finatra","homepage":"https://twitter.github.io/finatra/","size":11093,"stargazers_count":1539,"watchers_count":1539,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":282,"mirror_url":null,"open_issues_count":13,"forks":282,"open_issues":13,"watchers":1539,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/twitter/finatra/pulls/393"},"html":{"href":"https://github.com/twitter/finatra/pull/393"},"issue":{"href":"https://api.github.com/repos/twitter/finatra/issues/393"},"comments":{"href":"https://api.github.com/repos/twitter/finatra/issues/393/comments"},"review_comments":{"href":"https://api.github.com/repos/twitter/finatra/pulls/393/comments"},"review_comment":{"href":"https://api.github.com/repos/twitter/finatra/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/twitter/finatra/pulls/393/commits"},"statuses":{"href":"https://api.github.com/repos/twitter/finatra/statuses/d662153cf678cac709526ca0b388961ae9fddd04"}}}}
{"actor":{"display_login":"chrisbenincasa"}}