[WIP] provide inspectable routes with minimal changes #2966
Conversation
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
akka-ci
added
validating
|
Test FAILed. Pull request validation reportMima FailuresOther Failed Tasksakka-http / Compile / ./ headerCheck failed because of There are files without headers! akka-http-tests / Test / ./ headerCheck failed because of There are files without headers! |
jrudolph
commented
Feb 13, 2020
| @@ -23,17 +23,18 @@ import scala.concurrent.{ ExecutionContextExecutor, Future } | |||
| private[http] class RequestContextImpl( | |||
| val request: HttpRequest, | |||
| val unmatchedPath: Uri.Path, | |||
| val tokenValues: Map[ExtractionToken[_], Any], | |||
There was a problem hiding this comment.
This could be a request attribute as well once they are managed but since it is specific to routing maybe it fits here well.
akka-ci
added
validating
jrudolph
force-pushed
the
feature/inspectable-routes
branch
from
3b59ee8 to
551f060
Compare
akka-ci
added
needs-attention
jrudolph
commented
Feb 13, 2020
| new ParamMagnet { | ||
| type Out = pdef.Out | ||
| def apply() = pdef(value) | ||
| def apply() = pdef(value) named "parameters" |
There was a problem hiding this comment.
Ultimately, that's not enough and we want to add separate information about each of the parameters.
jrudolph
commented
Feb 13, 2020
| private val _patch : Directive0 = method(PATCH) | ||
| private val _post : Directive0 = method(POST) | ||
| private val _put : Directive0 = method(PUT) | ||
| private val _delete : Directive0 = method(DELETE) named "delete" |
There was a problem hiding this comment.
These ones override the name from the def method(HttpMethod) one above.
akka-ci
added
tested
|
Test PASSed. |
jrudolph
force-pushed
the
feature/inspectable-routes
branch
from
551f060 to
2f3c25b
Compare
akka-ci
added
validating
jrudolph
commented
Apr 30, 2020
| case dr: DirectiveRoute => leaves(dr.child, prefix :+ dr) | ||
| case last => Vector(RoutePath(prefix, last)) | ||
| } | ||
| leaves(route, Vector.empty).foreach(println) |
There was a problem hiding this comment.
What this prints currently is this:
[info] pathPrefix(<function1>) -> pathEnd -> post -> class akka.http.scaladsl.server.StandardRoute$$anon$1
[info] pathPrefix(<function1>) -> pathEnd -> put -> class akka.http.scaladsl.server.StandardRoute$$anon$1
[info] pathPrefix(<function1>) -> path(<function1>) -> get -> class akka.http.scaladsl.server.DynamicDirective$$$Lambda$310/1221027335
[info] pathPrefix(<function1>) -> path(<function1>) -> delete -> class akka.http.scaladsl.server.DynamicDirective$$$Lambda$310/1221027335
That means, we can already somewhat enumerate the chain of directives per leaf route. We don't have enough metadata available to look into the actual path matchers. That should be the next step.
jrudolph
commented
Apr 30, 2020
| } | ||
| }(ctx) | ||
|
|
||
| implicit class AddStatic[T](d: Directive1[T]) { |
There was a problem hiding this comment.
It seems, this one isn't necessary at all. Adding implicits like the above addByNameNullaryApply and addDirective1Apply means that we can change the semantics of route building with a single import that brings those implicits into scope.
jrudolph
commented
Apr 30, 2020
| private val _options: Directive0 = method(OPTIONS) named "options" | ||
| private val _patch : Directive0 = method(PATCH) named "patch" | ||
| private val _post : Directive0 = method(POST) named "post" | ||
| private val _put : Directive0 = method(PUT) named "put" |
There was a problem hiding this comment.
Ultimately, these named decorators are just a toy. What we should provide instead is something like meta which allows to decorate a Directive with typed meta information.
akka-ci
added
needs-attention
|
Test FAILed. Pull request validation reportOther Failed Tasksakka-http / Compile / ./ headerCheck failed because of There are files without headers! akka-http-tests / Test / ./ headerCheck failed because of There are files without headers! |
akka-ci
added
validating
jrudolph
commented
Apr 30, 2020
| complete(apiDescForRoute(forRoute)) | ||
|
|
||
| private def apiDescForRoute(route: Route): OpenApi = { | ||
| case class RoutePath(segments: Seq[DirectiveRoute], last: Route) { |
There was a problem hiding this comment.
So, what we get here per leaf route is an ordered chain of all (the metadata of) the directives that are applied for that leaf route. Once we expose enough metadata, we should be able to generate openapi specifications but also do "static" (= at route building time) analytics of the routing tree. Even with this simple prototype we could prevent that people would be accidentally nesting path or method directives.
|
Test PASSed. |
jrudolph
force-pushed
the
feature/inspectable-routes
branch
from
193d255 to
9f86e0c
Compare
akka-ci
added
validating
|
Test FAILed. !!! Couldn't read commit file !!! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refs #201, #2956
In a previous try, I tried to create a full AST of possible route structures. However, I don't think that's necessary. What we should strive for is