From 6f68c78b30c26cb7e32ad918dc976796f1c04d64 Mon Sep 17 00:00:00 2001 From: vendethiel Date: Thu, 29 Jul 2021 17:18:06 +0200 Subject: [PATCH 01/11] Link generation from route definition --- lib/Cro/HTTP/RouteSignatureToSub.pm6 | 104 ++++++++++++++++++++++++ lib/Cro/HTTP/Router.pm6 | 64 +++++++++++---- t/http-router-named-urls.t | 117 +++++++++++++++++++++++++++ 3 files changed, 269 insertions(+), 16 deletions(-) create mode 100644 lib/Cro/HTTP/RouteSignatureToSub.pm6 create mode 100644 t/http-router-named-urls.t diff --git a/lib/Cro/HTTP/RouteSignatureToSub.pm6 b/lib/Cro/HTTP/RouteSignatureToSub.pm6 new file mode 100644 index 00000000..e9a4790f --- /dev/null +++ b/lib/Cro/HTTP/RouteSignatureToSub.pm6 @@ -0,0 +1,104 @@ +unit module Cro::HTTP::RouteSignatureToSub; +use Cro::Uri :encode-percents; + +class RouteSignatureURLHolder { + has Callable $.fn is required; + + method CALL-ME(|c) { self.absolute(|c) } + method relative(|c) { $!fn(|c) } + method absolute(|c) { '/' ~ $!fn(|c) } + method url(|c) { + my $root-url = $*CRO-ROOT-URL or die 'No CRO-ROOT-URL configured'; + $root-url ~ ($root-url.ends-with('/') ?? '' !! '/') ~ $!fn(|c) + } +} + +sub route-signature-to-sub(Signature $s) is export { + RouteSignatureURLHolder.new(fn => signature-to-sub($s)) +} + +sub signature-to-sub(Signature $s) { + sub extract-static-part(Parameter $p) { + if $p.constraint_list == 1 && $p.constraint_list[0] ~~ Str { + return $p.constraint_list[0] + } + } + + my @path-parts; + my @fn-parts; + my $has-slurpy; + my $has-slurpy-named; + my %allowed-named; + my %required-named; + my @default; + my $min-args = 0; + for $s.params.kv -> $i, $param { + if $param.positional { + with extract-static-part $param -> $part { + @path-parts[$i] = $part; + } else { + ++$min-args; + @fn-parts.push: $i; + if $param.optional { + @default.push: $_ with $param.default + } + } + } elsif $param.named { + if $param.slurpy { + $has-slurpy-named = True; + next; + } + + %allowed-named{$param.usage-name} = True; + unless $param.optional { + %required-named{$param.usage-name} = True + } + } elsif $param.slurpy { + $has-slurpy = True; + } + # otherwise it's a Capture, which the router doesn't allow + } + my $allowed-nameds = %allowed-named.keys.Set; + my $required-nameds = %required-named.keys.Set; + + -> *@args, *%nameds { + if @args < $min-args { + die "Not enough arguments"; + } + + my @result = @path-parts; + my @available-default = @default; + for @fn-parts -> $i { + if @args { + @result[$i] = @args.shift + } elsif @available-default { + @result[$i] = @available-default.shift + } + # Otherwise, an optional wasn't filled, leave empty + } + + if @args && !$has-slurpy { + die "Extraneous arguments"; + } + + if !$has-slurpy-named { + my $passed-nameds = %nameds.keys.Set; + my $missing-nameds = $required-nameds (-) $passed-nameds; + my $extra-nameds = $passed-nameds (-) $allowed-nameds; + if $missing-nameds || $extra-nameds { + my @parts = ( + |("Missing named arguments: " ~ $missing-nameds.keys.sort.join(', ') if $missing-nameds), + |("Extraneous named arguments: " ~ $extra-nameds.keys.sort.join(', ') if $extra-nameds) + ); + die @parts.join('. ') ~ '.'; + } + } + + @result.append: @args; + my $result = @result.join: '/'; + if %nameds { + $result ~= '?' ~ %nameds.sort(*.key).map({ encode-percents(.key) ~ "=" ~ encode-percents(.value.Str) }).join('&'); + } + $result + } +} diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index e19c78d0..922b40dc 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -12,6 +12,7 @@ use Cro::HTTP::Request; use Cro::HTTP::Response; use Cro::UnhandledErrorReporter; use IO::Path::ChildSecure; +use Cro::HTTP::RouteSignatureToSub; class X::Cro::HTTP::Router::OnlyInRouteBlock is Exception { has Str $.what is required; @@ -32,6 +33,12 @@ class X::Cro::HTTP::Router::NoRequestBodyMatch is Exception { "error; if you're seeing it, you may have an over-general error handling)" } } +class X::Cro::HTTP::Router::DuplicateLinkName is Exception { + has Str $.key is required; + method message() { + "Conflicting link name: $.key" + } +} class X::Cro::HTTP::Router::ConfusedCapture is Exception { has $.body; @@ -136,14 +143,15 @@ module Cro::HTTP::Router { my class RouteHandler does Handler { has Str $.method; + has Str $.name; has &.implementation; has Hash[Array, Cro::HTTP::Router::PluginKey] $.plugin-config; has Hash[Array, Cro::HTTP::Router::PluginKey] $.flattened-plugin-config; method copy-adding(:@prefix, :@body-parsers!, :@body-serializers!, :@before-matched!, :@after-matched!, :@around!, - Hash[Array, Cro::HTTP::Router::PluginKey] :$plugin-config) { + Hash[Array, Cro::HTTP::Router::PluginKey] :$plugin-config, :$name-prefix) { self.bless: - :$!method, :&!implementation, + :$!method, :&!implementation, |(name => ($name-prefix // '') ~ $!name with $!name), :prefix[flat @prefix, @!prefix], :body-parsers[flat @!body-parsers, @body-parsers], :body-serializers[flat @!body-serializers, @body-serializers], @@ -280,6 +288,7 @@ module Cro::HTTP::Router { } } + has Str $.name; has Handler @.handlers; has Cro::BodyParser @.body-parsers; has Cro::BodySerializer @.body-serializers; @@ -292,6 +301,7 @@ module Cro::HTTP::Router { has $!path-matcher; has @!handlers-to-add; # Closures to defer adding, so they get all the middleware has Array %!plugin-config{Cro::HTTP::Router::PluginKey}; + has %.urls; method consumes() { Cro::HTTP::Request } method produces() { Cro::HTTP::Response } @@ -353,10 +363,10 @@ module Cro::HTTP::Router { } } - method add-handler(Str $method, &implementation --> Nil) { + method add-handler(Str $method, &implementation, Str :$name --> Nil) { @!handlers-to-add.push: { @!handlers.push(RouteHandler.new(:$method, :&implementation, :@!before-matched, :@!after-matched, - :@!around, :%!plugin-config)); + :@!around, :%!plugin-config, :$name)); } } @@ -368,8 +378,8 @@ module Cro::HTTP::Router { @!body-serializers.push($serializer); } - method add-include(@prefix, RouteSet $includee) { - @!includes.push({ :@prefix, :$includee }); + method add-include(@prefix, RouteSet $includee, Str :$name-prefix) { + @!includes.push({ :@prefix, :$includee, :$name-prefix }); } method add-before($middleware) { @@ -417,13 +427,27 @@ module Cro::HTTP::Router { .body-parsers = @!body-parsers; .body-serializers = @!body-serializers; } - for @!includes -> (:@prefix, :$includee) { + for @!includes -> (:@prefix, :$includee, :$name-prefix) { for $includee.handlers() { @!handlers.push: .copy-adding(:@prefix, :@!body-parsers, :@!body-serializers, - :@!before-matched, :@!after-matched, :@!around, :%!plugin-config); + :@!before-matched, :@!after-matched, :@!around, :%!plugin-config, :$name-prefix); } } self!generate-route-matcher(); + self!generate-urls(); + } + + method !generate-urls() { + my %urls; + my $prefix = $.name // ""; + for @.handlers -> $handler { + if $handler ~~ RouteHandler && $handler.name.defined { + my $key = $prefix ~ $handler.name; + die X::Cro::HTTP::Router::DuplicateLinkName.new(:$key) if %urls{$key}:exists; + %urls{$key} = route-signature-to-sub($handler.signature); + } + } + %.urls = %urls } method !generate-route-matcher(--> Nil) { @@ -689,14 +713,16 @@ module Cro::HTTP::Router { #| Define a set of routes. Expects to receive a block, which will be evaluated #| to set up the routing definition. - sub route(&route-definition) is export { - my $*CRO-ROUTE-SET = RouteSet.new; + multi route(&route-definition, Str :$name) is export { + my $*CRO-ROUTE-SET = RouteSet.new(:$name); route-definition(); $*CRO-ROUTE-SET.definition-complete(); my @before = $*CRO-ROUTE-SET.before; my @after = $*CRO-ROUTE-SET.after; if @before || @after { - return Cro.compose(|@before, $*CRO-ROUTE-SET, |@after, :for-connection); + return Cro.compose(|@before, $*CRO-ROUTE-SET, |@after, :for-connection) but role { + method route-prefix { $name } + }; } else { $*CRO-ROUTE-SET; } @@ -704,14 +730,14 @@ module Cro::HTTP::Router { #| Add a handler for a HTTP GET request. The signature of the handler will be #| used to determine the routing. - multi get(&handler --> Nil) is export { - $*CRO-ROUTE-SET.add-handler('GET', &handler); + multi sub get(&handler, Str :$name --> Nil) is export { + $*CRO-ROUTE-SET.add-handler('GET', &handler, :$name); } #| Add a handler for a HTTP POST request. The signature of the handler will be #| used to determine the routing. - multi post(&handler --> Nil) is export { - $*CRO-ROUTE-SET.add-handler('POST', &handler); + multi post(&handler, Str :$name --> Nil) is export { + $*CRO-ROUTE-SET.add-handler('POST', &handler, :$name); } #| Add a handler for a HTTP PUT request. The signature of the handler will be @@ -1249,7 +1275,13 @@ module Cro::HTTP::Router { #| Add a request handler for the specified HTTP method. This is useful #| when there is no shortcut function available for the HTTP method. - sub http($method, &handler --> Nil) is export { + multi http($name, $method, &handler --> Nil) is export { + $*CRO-ROUTE-SET.add-handler($method, &handler, :$name); + } + + #| Add a request handler for the specified HTTP method. This is useful + #| when there is no shortcut function available for the HTTP method. + multi http($method, &handler --> Nil) is export { $*CRO-ROUTE-SET.add-handler($method, &handler); } diff --git a/t/http-router-named-urls.t b/t/http-router-named-urls.t new file mode 100644 index 00000000..4452ac98 --- /dev/null +++ b/t/http-router-named-urls.t @@ -0,0 +1,117 @@ +use Cro; +use Cro::HTTP::Request; +use Cro::HTTP::Router; +use Test; + +{ + my $app = route { + get -> {}; + } + is-deeply $app.urls, %(), "No named urls"; +} + +{ + my $app = route :name, { + get -> {}; + } + is-deeply $app.urls, %(), "No named urls with a prefix"; +} + +{ + my $*CRO-ROOT-URL = 'https://foobar.com'; + my $app = route { + get :name, -> {}; + } + is-deeply $app.urls.keys, ('home',), "A named url with no prefix"; + is $app.urls(), '/'; + is $app.urls.relative, ''; + is $app.urls.absolute, '/'; + is $app.urls.url, 'https://foobar.com/'; +} + +{ + my $app = route :name, { + get :name, -> {}; + } + is-deeply $app.urls.keys, ('main-home',), "A named url with a prefix"; + is $app.urls(), '/'; +} + +throws-like { + route { + get :name, -> {}; + get :name, -> {}; + } +}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: home"; + +throws-like { + route :name, { + get :name, -> {}; + get :name, -> {}; + } +}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: main-home"; + +{ + my $app = route :name, { + include route { + get :name, -> {}; + } + } + is-deeply $app.urls.keys, ('main-home',), "A named url in an include with a prefix"; +} + +{ + my $app = route { + get :name, -> 'hello', $name { }; + } + is $app.urls('world'), '/hello/world'; + throws-like { $app.urls() }, Exception, message => "Not enough arguments"; + throws-like { $app.urls('a', 'b') }, Exception, message => "Extraneous arguments"; +} + +{ + my $app = route { + get :name, -> :$a, :$b { }; + } + is $app.urls(:a(1), :b(2)), '/?a=1&b=2'; + is $app.urls(:a(1)), '/?a=1'; + is $app.urls(:b(2)), '/?b=2'; + throws-like { $app.urls(1) }, Exception, message => "Extraneous arguments"; + throws-like { $app.urls(:c(3)) }, Exception, message => "Extraneous named arguments: c."; + throws-like { $app.urls(:a(1), :c(3)) }, Exception, message => "Extraneous named arguments: c."; +} + +{ + my $app = route { + get :name, -> :$a!, :$b! { }; + } + is $app.urls(:a(1), :b(2)), '/?a=1&b=2'; + throws-like { $app.urls(:a(1)) }, Exception, message => "Missing named arguments: b."; + throws-like { $app.urls(:b(2)) }, Exception, message => "Missing named arguments: a."; + throws-like { $app.urls(1) }, Exception, message => "Extraneous arguments"; + throws-like { $app.urls(:c(3)) }, Exception, message => "Missing named arguments: a, b. Extraneous named arguments: c."; + throws-like { $app.urls(:a(1), :c(3)) }, Exception, message => "Missing named arguments: b. Extraneous named arguments: c."; +} + +{ + my $app = route { + get :name, -> 'css', +a { }; + } + is $app.urls(), '/css'; + is $app.urls('x', 'y', 'z'), '/css/x/y/z'; +} + +{ + my $app = route { + get :name, -> *@a, *%b { }; + } + is $app.urls(), '/', 'Splat with no args at all'; + is $app.urls('x', 'y', 'z'), '/x/y/z', 'Splat with no named args'; + is $app.urls(:a(1), :b(2), :c(3)), '/?a=1&b=2&c=3', 'Splat with no pos args'; + is $app.urls('x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; +} + +# TODO before/after +# TODO include should not trigger name conflicts + +done-testing; \ No newline at end of file From e014103ce1761b760d06b468926ffe7b2482d1cc Mon Sep 17 00:00:00 2001 From: Altai-man Date: Wed, 4 Aug 2021 19:40:00 +0200 Subject: [PATCH 02/11] Update META6.json with new files --- META6.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/META6.json b/META6.json index 8c59d739..6fad8d91 100644 --- a/META6.json +++ b/META6.json @@ -63,6 +63,7 @@ "Cro::HTTP::ResponseParser": "lib/Cro/HTTP/ResponseParser.pm6", "Cro::HTTP::ResponseSerializer": "lib/Cro/HTTP/ResponseSerializer.pm6", "Cro::HTTP::ReverseProxy": "lib/Cro/HTTP/ReverseProxy.pm6", + "Cro::HTTP::RouteSignatureToSub": "lib/Cro/HTTP/RouteSignatureToSub.pm6", "Cro::HTTP::Router": "lib/Cro/HTTP/Router.pm6", "Cro::HTTP::Server": "lib/Cro/HTTP/Server.pm6", "Cro::HTTP::Session::IdGenerator": "lib/Cro/HTTP/Session/IdGenerator.pm6", @@ -80,4 +81,4 @@ "Server" ], "source-url": "https://github.com/croservices/cro-http.git" -} \ No newline at end of file +} From 278bd0c42e6695289f6955293668b836b2061176 Mon Sep 17 00:00:00 2001 From: Altai-man Date: Fri, 13 Aug 2021 19:38:46 +0200 Subject: [PATCH 03/11] Streamline route naming story Resolve issues we had due to information loss at handlers being flattened after inclusion, make tests pass again as the conflict detection is more strict and at the same time smart now. --- lib/Cro/HTTP/Router.pm6 | 35 ++++++++++---- t/http-router-named-urls.t | 94 ++++++++++++++++++++++++++++++++------ 2 files changed, 104 insertions(+), 25 deletions(-) diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index 922b40dc..0a91bf84 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -147,11 +147,12 @@ module Cro::HTTP::Router { has &.implementation; has Hash[Array, Cro::HTTP::Router::PluginKey] $.plugin-config; has Hash[Array, Cro::HTTP::Router::PluginKey] $.flattened-plugin-config; + has Bool $.from-include; method copy-adding(:@prefix, :@body-parsers!, :@body-serializers!, :@before-matched!, :@after-matched!, :@around!, - Hash[Array, Cro::HTTP::Router::PluginKey] :$plugin-config, :$name-prefix) { + Hash[Array, Cro::HTTP::Router::PluginKey] :$plugin-config, :$name-prefix, :$from-include!) { self.bless: - :$!method, :&!implementation, |(name => ($name-prefix // '') ~ $!name with $!name), + :$!method, :&!implementation, |(name => ($name-prefix ?? "$name-prefix." !! '') ~ $!name with $!name), :prefix[flat @prefix, @!prefix], :body-parsers[flat @!body-parsers, @body-parsers], :body-serializers[flat @!body-serializers, @body-serializers], @@ -159,7 +160,8 @@ module Cro::HTTP::Router { :after-matched[flat @!after-matched, @after-matched], :around[flat @!around, @around], :$!plugin-config, - :flattened-plugin-config(merge-plugin-config($plugin-config, $!flattened-plugin-config // $!plugin-config)) + :flattened-plugin-config(merge-plugin-config($plugin-config, $!flattened-plugin-config // $!plugin-config)), + :$from-include } sub merge-plugin-config($outer, $inner) { @@ -320,6 +322,7 @@ module Cro::HTTP::Router { with $routing-outcome { my ($handler-idx, $args) = .ast; my $handler := @!handlers[$handler-idx]; + my %*URLS = %!urls; whenever $handler.invoke($request, $args) -> $response { emit $response; QUIT { @@ -366,7 +369,7 @@ module Cro::HTTP::Router { method add-handler(Str $method, &implementation, Str :$name --> Nil) { @!handlers-to-add.push: { @!handlers.push(RouteHandler.new(:$method, :&implementation, :@!before-matched, :@!after-matched, - :@!around, :%!plugin-config, :$name)); + :@!around, :%!plugin-config, :$name, :!from-include)); } } @@ -430,7 +433,7 @@ module Cro::HTTP::Router { for @!includes -> (:@prefix, :$includee, :$name-prefix) { for $includee.handlers() { @!handlers.push: .copy-adding(:@prefix, :@!body-parsers, :@!body-serializers, - :@!before-matched, :@!after-matched, :@!around, :%!plugin-config, :$name-prefix); + :@!before-matched, :@!after-matched, :@!around, :%!plugin-config, :$name-prefix, :from-include); } } self!generate-route-matcher(); @@ -440,9 +443,11 @@ module Cro::HTTP::Router { method !generate-urls() { my %urls; my $prefix = $.name // ""; + $prefix ~= '.' if $prefix; for @.handlers -> $handler { if $handler ~~ RouteHandler && $handler.name.defined { - my $key = $prefix ~ $handler.name; + my $key = $handler.from-include ?? $handler.name !! $prefix ~ $handler.name; + next if $handler.from-include and not $key.contains('.'); die X::Cro::HTTP::Router::DuplicateLinkName.new(:$key) if %urls{$key}:exists; %urls{$key} = route-signature-to-sub($handler.signature); } @@ -595,7 +600,7 @@ module Cro::HTTP::Router { # Turned nameds into unpacks. for @named -> $param { - my $target-name = $param.named_names[0]; + my $target-name = $param.slurpy ?? $param.name !! $param.named_names[0]; my ($exists, $lookup) = do given $param { when Cookie { '$req.has-cookie(Q[' ~ $target-name ~ '])', @@ -775,17 +780,17 @@ module Cro::HTTP::Router { sub include(*@includees, *%includees --> Nil) is export { for @includees { when RouteSet { - $*CRO-ROUTE-SET.add-include([], $_); + $*CRO-ROUTE-SET.add-include([], $_, name-prefix => $_.name); } when Pair { my ($prefix, $routes) = .kv; if $routes ~~ RouteSet { given $prefix { when Str { - $*CRO-ROUTE-SET.add-include([$prefix], $routes); + $*CRO-ROUTE-SET.add-include([$prefix], $routes, name-prefix => $routes.name); } when Iterable { - $*CRO-ROUTE-SET.add-include($prefix, $routes); + $*CRO-ROUTE-SET.add-include($prefix, $routes, name-prefix => $routes.name); } default { die "An 'include' prefix may be a Str or Iterable, but not " ~ .^name; @@ -1357,6 +1362,16 @@ module Cro::HTTP::Router { } } + sub make-link($route-name, *@args, *%args) is export { + my $maker = %*URLS{$route-name} // Nil; + with $maker { + return $_(|@args, |%args); + } else { + warn "Called the make-link subroutine with $route-name but no such route defined"; + ""; + } + } + #| Register a router plugin. The provided ID is for debugging purposes. #| Returns a plugin key object which can be used for further interactions #| with the router plugin infrastructure. diff --git a/t/http-router-named-urls.t b/t/http-router-named-urls.t index 4452ac98..8ea469be 100644 --- a/t/http-router-named-urls.t +++ b/t/http-router-named-urls.t @@ -30,11 +30,11 @@ use Test; } { - my $app = route :name, { + my $app = route :name
, { get :name, -> {}; } - is-deeply $app.urls.keys, ('main-home',), "A named url with a prefix"; - is $app.urls(), '/'; + is-deeply $app.urls.keys, ('main.home',), "A named url with a prefix"; + is $app.urls(), '/'; } throws-like { @@ -45,20 +45,34 @@ throws-like { }, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: home"; throws-like { - route :name, { + route :name
, { get :name, -> {}; get :name, -> {}; } -}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: main-home"; +}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: main.home"; -{ - my $app = route :name, { - include route { - get :name, -> {}; - } - } - is-deeply $app.urls.keys, ('main-home',), "A named url in an include with a prefix"; -} +# XXX the test intention is bogus? +# We basically have two sorts of cases: +# * A route is not from include for sure, so we append its possible name to possible names of routes +# * A route is from include, so we entrust it to settle the name for itself (because the addressing happens from each individual route bottom-top way +# For the code below, it assumes we should squash the hierarchy when the include is anonymous, but this is forbidden because we explicitly allow: +# my $app = route { +# include route { +# get :name, -> {} +# } +# include route { +# get :name, -> {} +# } +# } +# to exist, implying we do not peek into "anonymous" includes +#{ +# my $app = route :name
, { +# include route { +# get :name, -> {}; +# } +# } +# is-deeply $app.urls.keys, ('main.home',), "A named url in an include with a prefix"; +#} { my $app = route { @@ -111,7 +125,57 @@ throws-like { is $app.urls('x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; } -# TODO before/after -# TODO include should not trigger name conflicts +{ + lives-ok { + my $app = route { + include route { + get :name, -> {}; + } + include route { + get :name, -> {}; + } + } + }, 'Conflict check is per-route block 1'; +} + +{ + lives-ok { + my $app = route { + get :name, -> {}; + include route { + get :name, -> {}; + get :name, -> {}; + } + include route :name, { + get :name, -> {}; + get :name, -> {}; + } + } + }, 'Conflict check is per-route block 2'; +} + +{ + lives-ok { + my $app = route { + include route :name, { + get :name, -> {}; + } + include route :name, { + get :name, -> {}; + } + } + }, 'Conflict check is by route name'; +} + +throws-like { + my $app = route { + include route :name, { + get :name, -> {}; + } + include route :name, { + get :name, -> {}; + } + } +}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: foo.home"; done-testing; \ No newline at end of file From 28f7af10104fe3ef4ad2f65335448e6cd27451ca Mon Sep 17 00:00:00 2001 From: Altai-man Date: Mon, 23 Aug 2021 19:12:41 +0200 Subject: [PATCH 04/11] Fix construction of URLs, properly resolve names in inner blocks Here we do three things: * Before that, URLs constructed for a route had no idea they can be used in some outer route block, so they generated `/foo` instead of `/included-prefix/foo`. To overcome this, we add an url-prefix attribute when we include a handler we rewrite it to an appropriate value. * Before that, we suffered at referencing the correct names due to route flattening. Inner anonymous routes had their handlers not present in the outermost hash, and given the hash was implemented as an attribute on the route set, they were in fact unable to refer to proper names. To overcome this, instaed of using an attribute we rely on the plugin system which allows us to separately store the data of inner route blocks. * This commit also adapts tests to accommodate to removal of the urls attribute available before. There is now a fallback method, though it's possible it will be removed as well in next commits due to its redundancy. --- lib/Cro/HTTP/RouteSignatureToSub.pm6 | 9 +- lib/Cro/HTTP/Router.pm6 | 34 ++++-- t/http-router-named-urls.t | 150 ++++++++++++++++----------- 3 files changed, 119 insertions(+), 74 deletions(-) diff --git a/lib/Cro/HTTP/RouteSignatureToSub.pm6 b/lib/Cro/HTTP/RouteSignatureToSub.pm6 index e9a4790f..fb84db12 100644 --- a/lib/Cro/HTTP/RouteSignatureToSub.pm6 +++ b/lib/Cro/HTTP/RouteSignatureToSub.pm6 @@ -3,18 +3,19 @@ use Cro::Uri :encode-percents; class RouteSignatureURLHolder { has Callable $.fn is required; + has Str $.prefix; method CALL-ME(|c) { self.absolute(|c) } method relative(|c) { $!fn(|c) } - method absolute(|c) { '/' ~ $!fn(|c) } + method absolute(|c) { '/' ~ ($!prefix ?? $!prefix ~ '/' !! '') ~ $!fn(|c) } method url(|c) { my $root-url = $*CRO-ROOT-URL or die 'No CRO-ROOT-URL configured'; - $root-url ~ ($root-url.ends-with('/') ?? '' !! '/') ~ $!fn(|c) + $root-url ~ ($root-url.ends-with('/') ?? '' !! '/') ~ ($!prefix ?? "$!prefix/" !! '') ~ $!fn(|c) } } -sub route-signature-to-sub(Signature $s) is export { - RouteSignatureURLHolder.new(fn => signature-to-sub($s)) +sub route-signature-to-sub(Str $prefix, Signature $s) is export { + RouteSignatureURLHolder.new(:$prefix, fn => signature-to-sub($s)) } sub signature-to-sub(Signature $s) { diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index 0a91bf84..22de70f4 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -77,6 +77,8 @@ module Cro::HTTP::Router { has Str $.id is required; } + my $link-plugin = router-plugin-register('link'); + #| A C that consumes HTTP requests and produces HTTP #| responses by routing them according to the routing specification set #| up using the C subroutine other routines. This class itself is @@ -148,6 +150,7 @@ module Cro::HTTP::Router { has Hash[Array, Cro::HTTP::Router::PluginKey] $.plugin-config; has Hash[Array, Cro::HTTP::Router::PluginKey] $.flattened-plugin-config; has Bool $.from-include; + has Str $.url-prefix is rw = ''; method copy-adding(:@prefix, :@body-parsers!, :@body-serializers!, :@before-matched!, :@after-matched!, :@around!, Hash[Array, Cro::HTTP::Router::PluginKey] :$plugin-config, :$name-prefix, :$from-include!) { @@ -161,7 +164,7 @@ module Cro::HTTP::Router { :around[flat @!around, @around], :$!plugin-config, :flattened-plugin-config(merge-plugin-config($plugin-config, $!flattened-plugin-config // $!plugin-config)), - :$from-include + :$from-include, :$!url-prefix } sub merge-plugin-config($outer, $inner) { @@ -303,11 +306,12 @@ module Cro::HTTP::Router { has $!path-matcher; has @!handlers-to-add; # Closures to defer adding, so they get all the middleware has Array %!plugin-config{Cro::HTTP::Router::PluginKey}; - has %.urls; method consumes() { Cro::HTTP::Request } method produces() { Cro::HTTP::Response } + method urls() { router-plugin-get-innermost-configs($link-plugin) } + method transformer(Supply:D $requests) { supply { whenever $requests -> $request { @@ -322,7 +326,8 @@ module Cro::HTTP::Router { with $routing-outcome { my ($handler-idx, $args) = .ast; my $handler := @!handlers[$handler-idx]; - my %*URLS = %!urls; + my $*CRO-ROUTER-ROUTE-HANDLER = $handler; + my %*URLS = $handler ~~ DelegateHandler ?? %() !! router-plugin-get-configs($link-plugin); whenever $handler.invoke($request, $args) -> $response { emit $response; QUIT { @@ -432,8 +437,18 @@ module Cro::HTTP::Router { } for @!includes -> (:@prefix, :$includee, :$name-prefix) { for $includee.handlers() { - @!handlers.push: .copy-adding(:@prefix, :@!body-parsers, :@!body-serializers, + $_.url-prefix = @prefix.join('/') ~ ($_.url-prefix ?? '/' ~ $_.url-prefix !! ''); + my $outer-handler = .copy-adding(:@prefix, :@!body-parsers, :@!body-serializers, :@!before-matched, :@!after-matched, :@!around, :%!plugin-config, :$name-prefix, :from-include); + if $outer-handler.name { + my $link-config = $outer-handler.get-innermost-plugin-configs($link-plugin); + # Url of included route can have a prefix which we did not know about + # at the generation stage, so overwrite it now when we have all the data we need + $link-config[0] = + %( |$link-config[0], + $outer-handler.name => route-signature-to-sub($_.url-prefix, $_.signature)); + } + @!handlers.push: $outer-handler; } } self!generate-route-matcher(); @@ -446,13 +461,14 @@ module Cro::HTTP::Router { $prefix ~= '.' if $prefix; for @.handlers -> $handler { if $handler ~~ RouteHandler && $handler.name.defined { - my $key = $handler.from-include ?? $handler.name !! $prefix ~ $handler.name; + my $key = $prefix ~ $handler.name; next if $handler.from-include and not $key.contains('.'); die X::Cro::HTTP::Router::DuplicateLinkName.new(:$key) if %urls{$key}:exists; - %urls{$key} = route-signature-to-sub($handler.signature); + my $url-prefix = $handler.url-prefix; + %urls{$key} = route-signature-to-sub($url-prefix, $handler.signature); } } - %.urls = %urls + router-plugin-add-config($link-plugin, (url-storage => %urls)); } method !generate-route-matcher(--> Nil) { @@ -810,7 +826,7 @@ module Cro::HTTP::Router { } for %includees.kv -> $prefix, $routes { if $routes ~~ RouteSet { - $*CRO-ROUTE-SET.add-include([$prefix], $routes); + $*CRO-ROUTE-SET.add-include([$prefix], $routes, name-prefix => $routes.name); } else { die "Can only use 'include' with `route` block, not a $routes.^name()"; @@ -1363,7 +1379,7 @@ module Cro::HTTP::Router { } sub make-link($route-name, *@args, *%args) is export { - my $maker = %*URLS{$route-name} // Nil; + my $maker = %*URLS{$route-name} // Nil; with $maker { return $_(|@args, |%args); } else { diff --git a/t/http-router-named-urls.t b/t/http-router-named-urls.t index 8ea469be..cf401a67 100644 --- a/t/http-router-named-urls.t +++ b/t/http-router-named-urls.t @@ -3,38 +3,43 @@ use Cro::HTTP::Request; use Cro::HTTP::Router; use Test; -{ - my $app = route { - get -> {}; - } - is-deeply $app.urls, %(), "No named urls"; +sub test-route-urls($app) { + my $source = Supplier.new; + my $responses = $app.transformer($source.Supply).Channel; + $source.emit(Cro::HTTP::Request.new(:method, :target)); + $responses.receive; } -{ - my $app = route :name, { - get -> {}; - } - is-deeply $app.urls, %(), "No named urls with a prefix"; +test-route-urls route { + get -> { + is-deeply %*URLS, %(), "No named urls"; + }; +} + +test-route-urls route :name, { + get -> { + is-deeply %*URLS, %(), "No named urls with a prefix"; + }; } { my $*CRO-ROOT-URL = 'https://foobar.com'; - my $app = route { - get :name, -> {}; + test-route-urls route { + get :name, -> { + is-deeply %*URLS.keys, ('home',), "A named url with no prefix"; + is %*URLS(), '/'; + is %*URLS.relative, ''; + is %*URLS.absolute, '/'; + is %*URLS.url, 'https://foobar.com/'; + }; } - is-deeply $app.urls.keys, ('home',), "A named url with no prefix"; - is $app.urls(), '/'; - is $app.urls.relative, ''; - is $app.urls.absolute, '/'; - is $app.urls.url, 'https://foobar.com/'; } -{ - my $app = route :name
, { - get :name, -> {}; - } - is-deeply $app.urls.keys, ('main.home',), "A named url with a prefix"; - is $app.urls(), '/'; +test-route-urls route :name
, { + get :name, -> { + is-deeply %*URLS.keys, ('main.home',), "A named url with a prefix"; + is %*URLS(), '/'; + }; } throws-like { @@ -74,55 +79,58 @@ throws-like { # is-deeply $app.urls.keys, ('main.home',), "A named url in an include with a prefix"; #} -{ - my $app = route { - get :name, -> 'hello', $name { }; +test-route-urls route { + get -> { + is %*URLS('world'), '/hello/world'; + throws-like { %*URLS() }, Exception, message => "Not enough arguments"; + throws-like { %*URLS('a', 'b') }, Exception, message => "Extraneous arguments"; } - is $app.urls('world'), '/hello/world'; - throws-like { $app.urls() }, Exception, message => "Not enough arguments"; - throws-like { $app.urls('a', 'b') }, Exception, message => "Extraneous arguments"; + + get :name, -> 'hello', $name {}; } -{ - my $app = route { - get :name, -> :$a, :$b { }; - } - is $app.urls(:a(1), :b(2)), '/?a=1&b=2'; - is $app.urls(:a(1)), '/?a=1'; - is $app.urls(:b(2)), '/?b=2'; - throws-like { $app.urls(1) }, Exception, message => "Extraneous arguments"; - throws-like { $app.urls(:c(3)) }, Exception, message => "Extraneous named arguments: c."; - throws-like { $app.urls(:a(1), :c(3)) }, Exception, message => "Extraneous named arguments: c."; +test-route-urls route { + get :name, -> :$a, :$b { + is %*URLS(:a(1), :b(2)), '/?a=1&b=2'; + is %*URLS(:a(1)), '/?a=1'; + is %*URLS(:b(2)), '/?b=2'; + throws-like { %*URLS(1) }, Exception, message => "Extraneous arguments"; + throws-like { %*URLS(:c(3)) }, Exception, message => "Extraneous named arguments: c."; + throws-like { %*URLS(:a(1), :c(3)) }, Exception, message => "Extraneous named arguments: c."; + }; } -{ - my $app = route { - get :name, -> :$a!, :$b! { }; +test-route-urls route { + get -> { + is %*URLS(:a(1), :b(2)), '/?a=1&b=2'; + throws-like { %*URLS(:a(1)) }, Exception, message => "Missing named arguments: b."; + throws-like { %*URLS(:b(2)) }, Exception, message => "Missing named arguments: a."; + throws-like { %*URLS(1) }, Exception, message => "Extraneous arguments"; + throws-like { %*URLS(:c(3)) }, Exception, message => "Missing named arguments: a, b. Extraneous named arguments: c."; + throws-like { %*URLS(:a(1), :c(3)) }, Exception, message => "Missing named arguments: b. Extraneous named arguments: c."; } - is $app.urls(:a(1), :b(2)), '/?a=1&b=2'; - throws-like { $app.urls(:a(1)) }, Exception, message => "Missing named arguments: b."; - throws-like { $app.urls(:b(2)) }, Exception, message => "Missing named arguments: a."; - throws-like { $app.urls(1) }, Exception, message => "Extraneous arguments"; - throws-like { $app.urls(:c(3)) }, Exception, message => "Missing named arguments: a, b. Extraneous named arguments: c."; - throws-like { $app.urls(:a(1), :c(3)) }, Exception, message => "Missing named arguments: b. Extraneous named arguments: c."; + + get :name, -> :$a!, :$b! {}; } -{ - my $app = route { - get :name, -> 'css', +a { }; +test-route-urls route { + get -> { + is %*URLS(), '/css'; + is %*URLS('x', 'y', 'z'), '/css/x/y/z'; } - is $app.urls(), '/css'; - is $app.urls('x', 'y', 'z'), '/css/x/y/z'; + + get :name, -> 'css', +a { }; } -{ - my $app = route { - get :name, -> *@a, *%b { }; +test-route-urls route { + get -> { + is %*URLS(), '/', 'Splat with no args at all'; + is %*URLS('x', 'y', 'z'), '/x/y/z', 'Splat with no named args'; + is %*URLS(:a(1), :b(2), :c(3)), '/?a=1&b=2&c=3', 'Splat with no pos args'; + is %*URLS('x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; } - is $app.urls(), '/', 'Splat with no args at all'; - is $app.urls('x', 'y', 'z'), '/x/y/z', 'Splat with no named args'; - is $app.urls(:a(1), :b(2), :c(3)), '/?a=1&b=2&c=3', 'Splat with no pos args'; - is $app.urls('x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; + + get :name, -> *@a, *%b { }; } { @@ -178,4 +186,24 @@ throws-like { } }, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: foo.home"; -done-testing; \ No newline at end of file +{ + test-route-urls route { + get -> { + is-deeply %*URLS.keys.sort, .sort, + 'Named include names are recognized properly'; + is %*URLS(42), '/included/bar/42'; + } + + get :name, -> 'bar', Int $foo {} + + include route :name, { + get :name, -> 'bar', Int $foo {} + } + + include included => route :name, { + get :name, -> 'bar', Int $foo {} + } + } +} + +done-testing; From 9274dd53f584f7390a7ccc9a6a0bda010f5a26cd Mon Sep 17 00:00:00 2001 From: Altai-man Date: Mon, 23 Aug 2021 20:03:33 +0200 Subject: [PATCH 05/11] Remove obsolete rewritten names --- lib/Cro/HTTP/Router.pm6 | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index 22de70f4..704e9adb 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -447,6 +447,7 @@ module Cro::HTTP::Router { $link-config[0] = %( |$link-config[0], $outer-handler.name => route-signature-to-sub($_.url-prefix, $_.signature)); + $link-config[0]{$_.name}:delete unless $_.name eq $outer-handler.name; } @!handlers.push: $outer-handler; } From 43902e077ea85127333c4a995c9f508459b4d18a Mon Sep 17 00:00:00 2001 From: Altai-man Date: Wed, 25 Aug 2021 15:24:27 +0300 Subject: [PATCH 06/11] WIP --- lib/Cro/HTTP/Router.pm6 | 50 ++++++++++++++--------- t/http-router-named-urls.t | 81 +++++++++++++++----------------------- 2 files changed, 63 insertions(+), 68 deletions(-) diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index 704e9adb..b97144d5 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -77,7 +77,11 @@ module Cro::HTTP::Router { has Str $.id is required; } - my $link-plugin = router-plugin-register('link'); + our $link-plugin is export(:link) = router-plugin-register('link'); + + class RouteBlockLinks { + has %.link-generators; + } #| A C that consumes HTTP requests and produces HTTP #| responses by routing them according to the routing specification set @@ -143,6 +147,8 @@ module Cro::HTTP::Router { } } + + my class RouteHandler does Handler { has Str $.method; has Str $.name; @@ -310,8 +316,6 @@ module Cro::HTTP::Router { method consumes() { Cro::HTTP::Request } method produces() { Cro::HTTP::Response } - method urls() { router-plugin-get-innermost-configs($link-plugin) } - method transformer(Supply:D $requests) { supply { whenever $requests -> $request { @@ -326,8 +330,8 @@ module Cro::HTTP::Router { with $routing-outcome { my ($handler-idx, $args) = .ast; my $handler := @!handlers[$handler-idx]; - my $*CRO-ROUTER-ROUTE-HANDLER = $handler; - my %*URLS = $handler ~~ DelegateHandler ?? %() !! router-plugin-get-configs($link-plugin); + # my $*CRO-ROUTER-ROUTE-HANDLER = $handler; + # my %*URLS = $handler ~~ DelegateHandler ?? %() !! router-plugin-get-configs($link-plugin); whenever $handler.invoke($request, $args) -> $response { emit $response; QUIT { @@ -435,25 +439,32 @@ module Cro::HTTP::Router { .body-parsers = @!body-parsers; .body-serializers = @!body-serializers; } + self!generate-urls(); + my %urls; for @!includes -> (:@prefix, :$includee, :$name-prefix) { for $includee.handlers() { + my $key = ($name-prefix ?? $name-prefix ~ '.' !! '') ~ ($_.name // ''); + if $name-prefix && $key && (%urls{$key}:exists) { + die X::Cro::HTTP::Router::DuplicateLinkName.new(:$key); + } + %urls{$key} = True; $_.url-prefix = @prefix.join('/') ~ ($_.url-prefix ?? '/' ~ $_.url-prefix !! ''); my $outer-handler = .copy-adding(:@prefix, :@!body-parsers, :@!body-serializers, :@!before-matched, :@!after-matched, :@!around, :%!plugin-config, :$name-prefix, :from-include); if $outer-handler.name { - my $link-config = $outer-handler.get-innermost-plugin-configs($link-plugin); + my $link-config = $outer-handler.get-innermost-plugin-configs($link-plugin)[0]; # Url of included route can have a prefix which we did not know about # at the generation stage, so overwrite it now when we have all the data we need - $link-config[0] = - %( |$link-config[0], - $outer-handler.name => route-signature-to-sub($_.url-prefix, $_.signature)); - $link-config[0]{$_.name}:delete unless $_.name eq $outer-handler.name; + $link-config.link-generators = + %( |$link-config.link-generators, + $outer-handler.name => route-signature-to-sub($_.url-prefix, $_.signature), + $_.name => route-signature-to-sub($_.url-prefix, $_.signature) + ); } @!handlers.push: $outer-handler; } } self!generate-route-matcher(); - self!generate-urls(); } method !generate-urls() { @@ -469,7 +480,7 @@ module Cro::HTTP::Router { %urls{$key} = route-signature-to-sub($url-prefix, $handler.signature); } } - router-plugin-add-config($link-plugin, (url-storage => %urls)); + router-plugin-add-config($link-plugin, RouteBlockLinks.new(link-generators => %urls)); } method !generate-route-matcher(--> Nil) { @@ -1380,13 +1391,16 @@ module Cro::HTTP::Router { } sub make-link($route-name, *@args, *%args) is export { - my $maker = %*URLS{$route-name} // Nil; - with $maker { - return $_(|@args, |%args); - } else { - warn "Called the make-link subroutine with $route-name but no such route defined"; - ""; + my $maker = router-plugin-get-configs($link-plugin); + my @options; + for @$maker -> $links { + with $links.link-generators{$route-name} { + return $_(|@args, |%args); + } + @options.push: |$links.link-generators.keys; } + warn "Called the make-link subroutine with $route-name but no such route defined, options are: @options.join('/')"; + ""; } #| Register a router plugin. The provided ID is for debugging purposes. diff --git a/t/http-router-named-urls.t b/t/http-router-named-urls.t index cf401a67..5ffc6125 100644 --- a/t/http-router-named-urls.t +++ b/t/http-router-named-urls.t @@ -1,5 +1,7 @@ use Cro; use Cro::HTTP::Request; +use Cro::HTTP::Router :link; +use Cro::HTTP::Router :plugin; use Cro::HTTP::Router; use Test; @@ -12,13 +14,13 @@ sub test-route-urls($app) { test-route-urls route { get -> { - is-deeply %*URLS, %(), "No named urls"; + is-deeply router-plugin-get-innermost-configs($link-plugin)[0].link-generators, %(), "No named urls"; }; } test-route-urls route :name, { get -> { - is-deeply %*URLS, %(), "No named urls with a prefix"; + is-deeply router-plugin-get-innermost-configs($link-plugin)[0].link-generators, %(), "No named urls with a prefix"; }; } @@ -26,19 +28,17 @@ test-route-urls route :name, { my $*CRO-ROOT-URL = 'https://foobar.com'; test-route-urls route { get :name, -> { - is-deeply %*URLS.keys, ('home',), "A named url with no prefix"; - is %*URLS(), '/'; - is %*URLS.relative, ''; - is %*URLS.absolute, '/'; - is %*URLS.url, 'https://foobar.com/'; + is make-link('home'), '/', 'Basic call of a generator is correct'; + is make-link('home').relative, '', 'Relative URL is correct'; + is make-link('home').absolute, '/', 'Absolute URL is correct'; + is make-link('home').url, 'https://foobar.com/', 'CRO-ROOT-URL was used'; }; } } test-route-urls route :name
, { get :name, -> { - is-deeply %*URLS.keys, ('main.home',), "A named url with a prefix"; - is %*URLS(), '/'; + is make-link('main.home'), '/', 'Basic call of a generator by a short name is correct'; }; } @@ -81,9 +81,10 @@ throws-like { test-route-urls route { get -> { - is %*URLS('world'), '/hello/world'; - throws-like { %*URLS() }, Exception, message => "Not enough arguments"; - throws-like { %*URLS('a', 'b') }, Exception, message => "Extraneous arguments"; + say make-link('hello', 'world'); + is make-link('hello', 'world'), '/hello/world', 'URL is generated correctly'; + throws-like { make-link('hello') }, Exception, message => "Not enough arguments"; + throws-like { make-link('hello', 'a', 'b') }, Exception, message => "Extraneous arguments"; } get :name, -> 'hello', $name {}; @@ -91,23 +92,23 @@ test-route-urls route { test-route-urls route { get :name, -> :$a, :$b { - is %*URLS(:a(1), :b(2)), '/?a=1&b=2'; - is %*URLS(:a(1)), '/?a=1'; - is %*URLS(:b(2)), '/?b=2'; - throws-like { %*URLS(1) }, Exception, message => "Extraneous arguments"; - throws-like { %*URLS(:c(3)) }, Exception, message => "Extraneous named arguments: c."; - throws-like { %*URLS(:a(1), :c(3)) }, Exception, message => "Extraneous named arguments: c."; + is make-link('hello', :a(1), :b(2)), '/?a=1&b=2'; + is make-link('hello', :a(1)), '/?a=1'; + is make-link('hello', :b(2)), '/?b=2'; + throws-like { make-link('hello', 1) }, Exception, message => "Extraneous arguments"; + throws-like { make-link('hello', :c(3)) }, Exception, message => "Extraneous named arguments: c."; + throws-like { make-link('hello', :a(1), :c(3)) }, Exception, message => "Extraneous named arguments: c."; }; } test-route-urls route { get -> { - is %*URLS(:a(1), :b(2)), '/?a=1&b=2'; - throws-like { %*URLS(:a(1)) }, Exception, message => "Missing named arguments: b."; - throws-like { %*URLS(:b(2)) }, Exception, message => "Missing named arguments: a."; - throws-like { %*URLS(1) }, Exception, message => "Extraneous arguments"; - throws-like { %*URLS(:c(3)) }, Exception, message => "Missing named arguments: a, b. Extraneous named arguments: c."; - throws-like { %*URLS(:a(1), :c(3)) }, Exception, message => "Missing named arguments: b. Extraneous named arguments: c."; + is make-link('hello', :a(1), :b(2)), '/?a=1&b=2'; + throws-like { make-link('hello', :a(1)) }, Exception, message => "Missing named arguments: b."; + throws-like { make-link('hello', :b(2)) }, Exception, message => "Missing named arguments: a."; + throws-like { make-link('hello', 1) }, Exception, message => "Extraneous arguments"; + throws-like { make-link('hello', :c(3)) }, Exception, message => "Missing named arguments: a, b. Extraneous named arguments: c."; + throws-like { make-link('hello', :a(1), :c(3)) }, Exception, message => "Missing named arguments: b. Extraneous named arguments: c."; } get :name, -> :$a!, :$b! {}; @@ -115,8 +116,8 @@ test-route-urls route { test-route-urls route { get -> { - is %*URLS(), '/css'; - is %*URLS('x', 'y', 'z'), '/css/x/y/z'; + is make-link('css'), '/css'; + is make-link('css', 'x', 'y', 'z'), '/css/x/y/z'; } get :name, -> 'css', +a { }; @@ -124,10 +125,10 @@ test-route-urls route { test-route-urls route { get -> { - is %*URLS(), '/', 'Splat with no args at all'; - is %*URLS('x', 'y', 'z'), '/x/y/z', 'Splat with no named args'; - is %*URLS(:a(1), :b(2), :c(3)), '/?a=1&b=2&c=3', 'Splat with no pos args'; - is %*URLS('x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; + is make-link('css'), '/', 'Splat with no args at all'; + is make-link('css', 'x', 'y', 'z'), '/x/y/z', 'Splat with no named args'; + is make-link('css', :a(1), :b(2), :c(3)), '/?a=1&b=2&c=3', 'Splat with no pos args'; + is make-link('css', 'x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; } get :name, -> *@a, *%b { }; @@ -186,24 +187,4 @@ throws-like { } }, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: foo.home"; -{ - test-route-urls route { - get -> { - is-deeply %*URLS.keys.sort, .sort, - 'Named include names are recognized properly'; - is %*URLS(42), '/included/bar/42'; - } - - get :name, -> 'bar', Int $foo {} - - include route :name, { - get :name, -> 'bar', Int $foo {} - } - - include included => route :name, { - get :name, -> 'bar', Int $foo {} - } - } -} - done-testing; From 40a14fb9fcc328d00cea3dc6a644e530c484ee21 Mon Sep 17 00:00:00 2001 From: Altai-man Date: Fri, 27 Aug 2021 17:19:28 +0200 Subject: [PATCH 07/11] Code cleanup (debug leftover, missing comments) --- lib/Cro/HTTP/Router.pm6 | 4 ++-- t/http-router-named-urls.t | 26 +------------------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index b97144d5..62583cc3 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -330,8 +330,6 @@ module Cro::HTTP::Router { with $routing-outcome { my ($handler-idx, $args) = .ast; my $handler := @!handlers[$handler-idx]; - # my $*CRO-ROUTER-ROUTE-HANDLER = $handler; - # my %*URLS = $handler ~~ DelegateHandler ?? %() !! router-plugin-get-configs($link-plugin); whenever $handler.invoke($request, $args) -> $response { emit $response; QUIT { @@ -444,6 +442,8 @@ module Cro::HTTP::Router { for @!includes -> (:@prefix, :$includee, :$name-prefix) { for $includee.handlers() { my $key = ($name-prefix ?? $name-prefix ~ '.' !! '') ~ ($_.name // ''); + # When checking all included routes in the outer route block for conflicting, + # we omit anonymous ones (if $name-prefix...) if $name-prefix && $key && (%urls{$key}:exists) { die X::Cro::HTTP::Router::DuplicateLinkName.new(:$key); } diff --git a/t/http-router-named-urls.t b/t/http-router-named-urls.t index 5ffc6125..0cee017b 100644 --- a/t/http-router-named-urls.t +++ b/t/http-router-named-urls.t @@ -38,7 +38,7 @@ test-route-urls route :name, { test-route-urls route :name
, { get :name, -> { - is make-link('main.home'), '/', 'Basic call of a generator by a short name is correct'; + is make-link('main.home'), '/', 'Basic call of a generator by a qualified name is correct'; }; } @@ -56,32 +56,8 @@ throws-like { } }, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: main.home"; -# XXX the test intention is bogus? -# We basically have two sorts of cases: -# * A route is not from include for sure, so we append its possible name to possible names of routes -# * A route is from include, so we entrust it to settle the name for itself (because the addressing happens from each individual route bottom-top way -# For the code below, it assumes we should squash the hierarchy when the include is anonymous, but this is forbidden because we explicitly allow: -# my $app = route { -# include route { -# get :name, -> {} -# } -# include route { -# get :name, -> {} -# } -# } -# to exist, implying we do not peek into "anonymous" includes -#{ -# my $app = route :name
, { -# include route { -# get :name, -> {}; -# } -# } -# is-deeply $app.urls.keys, ('main.home',), "A named url in an include with a prefix"; -#} - test-route-urls route { get -> { - say make-link('hello', 'world'); is make-link('hello', 'world'), '/hello/world', 'URL is generated correctly'; throws-like { make-link('hello') }, Exception, message => "Not enough arguments"; throws-like { make-link('hello', 'a', 'b') }, Exception, message => "Extraneous arguments"; From dca10479b06c85adc9be440a06817b41400105f0 Mon Sep 17 00:00:00 2001 From: Altai-man Date: Fri, 27 Aug 2021 17:41:57 +0200 Subject: [PATCH 08/11] Rework API a bit and do some minor cleanup --- lib/Cro/HTTP/Router.pm6 | 22 ++++++++++++--- t/http-router-named-urls.t | 56 +++++++++++++++----------------------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index 62583cc3..b540a161 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -1390,17 +1390,31 @@ module Cro::HTTP::Router { } } - sub make-link($route-name, *@args, *%args) is export { + sub rel-link($route-name, *@params, *%params) is export { + with get-link($route-name, 'rel-link') { + return $_.relative(|@params, |%params); + } + ""; + } + + sub abs-link($route-name, *@params, *%params) is export { + with get-link($route-name, 'abs-link') { + return $_.absolute(|@params, |%params); + } + ""; + } + + my sub get-link($route-name, $sub-name) { my $maker = router-plugin-get-configs($link-plugin); my @options; for @$maker -> $links { with $links.link-generators{$route-name} { - return $_(|@args, |%args); + return $_; } @options.push: |$links.link-generators.keys; } - warn "Called the make-link subroutine with $route-name but no such route defined, options are: @options.join('/')"; - ""; + warn "Called the $sub-name subroutine with $route-name but no such route defined, options are: @options.join('/')"; + Nil; } #| Register a router plugin. The provided ID is for debugging purposes. diff --git a/t/http-router-named-urls.t b/t/http-router-named-urls.t index 0cee017b..9a2fd71a 100644 --- a/t/http-router-named-urls.t +++ b/t/http-router-named-urls.t @@ -24,21 +24,9 @@ test-route-urls route :name, { }; } -{ - my $*CRO-ROOT-URL = 'https://foobar.com'; - test-route-urls route { - get :name, -> { - is make-link('home'), '/', 'Basic call of a generator is correct'; - is make-link('home').relative, '', 'Relative URL is correct'; - is make-link('home').absolute, '/', 'Absolute URL is correct'; - is make-link('home').url, 'https://foobar.com/', 'CRO-ROOT-URL was used'; - }; - } -} - test-route-urls route :name
, { get :name, -> { - is make-link('main.home'), '/', 'Basic call of a generator by a qualified name is correct'; + is abs-link('main.home'), '/', 'Basic call of a generator by a qualified name is correct'; }; } @@ -58,9 +46,9 @@ throws-like { test-route-urls route { get -> { - is make-link('hello', 'world'), '/hello/world', 'URL is generated correctly'; - throws-like { make-link('hello') }, Exception, message => "Not enough arguments"; - throws-like { make-link('hello', 'a', 'b') }, Exception, message => "Extraneous arguments"; + is abs-link('hello', 'world'), '/hello/world', 'URL is generated correctly'; + throws-like { abs-link('hello') }, Exception, message => "Not enough arguments"; + throws-like { abs-link('hello', 'a', 'b') }, Exception, message => "Extraneous arguments"; } get :name, -> 'hello', $name {}; @@ -68,23 +56,23 @@ test-route-urls route { test-route-urls route { get :name, -> :$a, :$b { - is make-link('hello', :a(1), :b(2)), '/?a=1&b=2'; - is make-link('hello', :a(1)), '/?a=1'; - is make-link('hello', :b(2)), '/?b=2'; - throws-like { make-link('hello', 1) }, Exception, message => "Extraneous arguments"; - throws-like { make-link('hello', :c(3)) }, Exception, message => "Extraneous named arguments: c."; - throws-like { make-link('hello', :a(1), :c(3)) }, Exception, message => "Extraneous named arguments: c."; + is abs-link('hello', :a(1), :b(2)), '/?a=1&b=2'; + is abs-link('hello', :a(1)), '/?a=1'; + is abs-link('hello', :b(2)), '/?b=2'; + throws-like { abs-link('hello', 1) }, Exception, message => "Extraneous arguments"; + throws-like { abs-link('hello', :c(3)) }, Exception, message => "Extraneous named arguments: c."; + throws-like { abs-link('hello', :a(1), :c(3)) }, Exception, message => "Extraneous named arguments: c."; }; } test-route-urls route { get -> { - is make-link('hello', :a(1), :b(2)), '/?a=1&b=2'; - throws-like { make-link('hello', :a(1)) }, Exception, message => "Missing named arguments: b."; - throws-like { make-link('hello', :b(2)) }, Exception, message => "Missing named arguments: a."; - throws-like { make-link('hello', 1) }, Exception, message => "Extraneous arguments"; - throws-like { make-link('hello', :c(3)) }, Exception, message => "Missing named arguments: a, b. Extraneous named arguments: c."; - throws-like { make-link('hello', :a(1), :c(3)) }, Exception, message => "Missing named arguments: b. Extraneous named arguments: c."; + is abs-link('hello', :a(1), :b(2)), '/?a=1&b=2'; + throws-like { abs-link('hello', :a(1)) }, Exception, message => "Missing named arguments: b."; + throws-like { abs-link('hello', :b(2)) }, Exception, message => "Missing named arguments: a."; + throws-like { abs-link('hello', 1) }, Exception, message => "Extraneous arguments"; + throws-like { abs-link('hello', :c(3)) }, Exception, message => "Missing named arguments: a, b. Extraneous named arguments: c."; + throws-like { abs-link('hello', :a(1), :c(3)) }, Exception, message => "Missing named arguments: b. Extraneous named arguments: c."; } get :name, -> :$a!, :$b! {}; @@ -92,8 +80,8 @@ test-route-urls route { test-route-urls route { get -> { - is make-link('css'), '/css'; - is make-link('css', 'x', 'y', 'z'), '/css/x/y/z'; + is abs-link('css'), '/css'; + is abs-link('css', 'x', 'y', 'z'), '/css/x/y/z'; } get :name, -> 'css', +a { }; @@ -101,10 +89,10 @@ test-route-urls route { test-route-urls route { get -> { - is make-link('css'), '/', 'Splat with no args at all'; - is make-link('css', 'x', 'y', 'z'), '/x/y/z', 'Splat with no named args'; - is make-link('css', :a(1), :b(2), :c(3)), '/?a=1&b=2&c=3', 'Splat with no pos args'; - is make-link('css', 'x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; + is abs-link('css'), '/', 'Splat with no args at all'; + is abs-link('css', 'x', 'y', 'z'), '/x/y/z', 'Splat with no named args'; + is abs-link('css', :a(1), :b(2), :c(3)), '/?a=1&b=2&c=3', 'Splat with no pos args'; + is abs-link('css', 'x', 'y', 'z', :a(1), :b(2), :où('Ÿ')), '/x/y/z?a=1&b=2&o%C3%B9=%C5%B8', 'Splat with both types of args'; } get :name, -> *@a, *%b { }; From 9f47a63cfd98228594d1f550c8349a51d87bcfd7 Mon Sep 17 00:00:00 2001 From: Jonathan Worthington Date: Wed, 10 Nov 2021 16:55:57 +0100 Subject: [PATCH 09/11] Tweak link generator API and module name --- META6.json | 4 ++-- lib/Cro/HTTP/Router.pm6 | 17 ++++++++------ .../LinkGenerator.pm6} | 22 +++++++++---------- 3 files changed, 23 insertions(+), 20 deletions(-) rename lib/Cro/HTTP/{RouteSignatureToSub.pm6 => Router/LinkGenerator.pm6} (89%) diff --git a/META6.json b/META6.json index 6fad8d91..f34a4400 100644 --- a/META6.json +++ b/META6.json @@ -63,8 +63,8 @@ "Cro::HTTP::ResponseParser": "lib/Cro/HTTP/ResponseParser.pm6", "Cro::HTTP::ResponseSerializer": "lib/Cro/HTTP/ResponseSerializer.pm6", "Cro::HTTP::ReverseProxy": "lib/Cro/HTTP/ReverseProxy.pm6", - "Cro::HTTP::RouteSignatureToSub": "lib/Cro/HTTP/RouteSignatureToSub.pm6", "Cro::HTTP::Router": "lib/Cro/HTTP/Router.pm6", + "Cro::HTTP::Router::LinkGenerator": "lib/Cro/HTTP/Router/LinkGenerator.pm6", "Cro::HTTP::Server": "lib/Cro/HTTP/Server.pm6", "Cro::HTTP::Session::IdGenerator": "lib/Cro/HTTP/Session/IdGenerator.pm6", "Cro::HTTP::Session::InMemory": "lib/Cro/HTTP/Session/InMemory.pm6", @@ -81,4 +81,4 @@ "Server" ], "source-url": "https://github.com/croservices/cro-http.git" -} +} \ No newline at end of file diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index b540a161..575c247c 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -12,7 +12,7 @@ use Cro::HTTP::Request; use Cro::HTTP::Response; use Cro::UnhandledErrorReporter; use IO::Path::ChildSecure; -use Cro::HTTP::RouteSignatureToSub; +use Cro::HTTP::Router::LinkGenerator; class X::Cro::HTTP::Router::OnlyInRouteBlock is Exception { has Str $.what is required; @@ -455,11 +455,12 @@ module Cro::HTTP::Router { my $link-config = $outer-handler.get-innermost-plugin-configs($link-plugin)[0]; # Url of included route can have a prefix which we did not know about # at the generation stage, so overwrite it now when we have all the data we need - $link-config.link-generators = - %( |$link-config.link-generators, - $outer-handler.name => route-signature-to-sub($_.url-prefix, $_.signature), - $_.name => route-signature-to-sub($_.url-prefix, $_.signature) - ); + my $generator = Cro::HTP::Router::LinkGenerator.new(prefix => .url-prefix, signature => .signature); + $link-config.link-generators = %( + |$link-config.link-generators, + $outer-handler.name => $generator, + .name => $generator + ); } @!handlers.push: $outer-handler; } @@ -477,7 +478,9 @@ module Cro::HTTP::Router { next if $handler.from-include and not $key.contains('.'); die X::Cro::HTTP::Router::DuplicateLinkName.new(:$key) if %urls{$key}:exists; my $url-prefix = $handler.url-prefix; - %urls{$key} = route-signature-to-sub($url-prefix, $handler.signature); + %urls{$key} = Cro::HTP::Router::LinkGenerator.new: + prefix => $url-prefix, + signature => $handler.signature; } } router-plugin-add-config($link-plugin, RouteBlockLinks.new(link-generators => %urls)); diff --git a/lib/Cro/HTTP/RouteSignatureToSub.pm6 b/lib/Cro/HTTP/Router/LinkGenerator.pm6 similarity index 89% rename from lib/Cro/HTTP/RouteSignatureToSub.pm6 rename to lib/Cro/HTTP/Router/LinkGenerator.pm6 index fb84db12..3f1b416a 100644 --- a/lib/Cro/HTTP/RouteSignatureToSub.pm6 +++ b/lib/Cro/HTTP/Router/LinkGenerator.pm6 @@ -1,23 +1,23 @@ -unit module Cro::HTTP::RouteSignatureToSub; use Cro::Uri :encode-percents; -class RouteSignatureURLHolder { - has Callable $.fn is required; - has Str $.prefix; +class Cro::HTP::Router::LinkGenerator { + has Str $.prefix is required; + has Signature $.signature is required; + has Callable $!generator; + + submethod TWEAK(--> Nil) { + $!generator = signature-to-sub($!signature) + } method CALL-ME(|c) { self.absolute(|c) } - method relative(|c) { $!fn(|c) } - method absolute(|c) { '/' ~ ($!prefix ?? $!prefix ~ '/' !! '') ~ $!fn(|c) } + method relative(|c) { $!generator(|c) } + method absolute(|c) { '/' ~ ($!prefix ?? $!prefix ~ '/' !! '') ~ $!generator(|c) } method url(|c) { my $root-url = $*CRO-ROOT-URL or die 'No CRO-ROOT-URL configured'; - $root-url ~ ($root-url.ends-with('/') ?? '' !! '/') ~ ($!prefix ?? "$!prefix/" !! '') ~ $!fn(|c) + $root-url ~ ($root-url.ends-with('/') ?? '' !! '/') ~ ($!prefix ?? "$!prefix/" !! '') ~ $!generator(|c) } } -sub route-signature-to-sub(Str $prefix, Signature $s) is export { - RouteSignatureURLHolder.new(:$prefix, fn => signature-to-sub($s)) -} - sub signature-to-sub(Signature $s) { sub extract-static-part(Parameter $p) { if $p.constraint_list == 1 && $p.constraint_list[0] ~~ Str { From af011e2cb3c43379e0444df1e0e2c7e5c763155e Mon Sep 17 00:00:00 2001 From: Altai-man Date: Wed, 10 Nov 2021 18:31:19 +0100 Subject: [PATCH 10/11] A couple of tweaks for link generator --- META6.json | 1 + lib/Cro/HTTP/Router.pm6 | 31 ++++++++++++--------------- lib/Cro/HTTP/Router/LinkGenerator.pm6 | 9 +++++++- lib/Cro/HTTP/Router/Roles.pm6 | 6 ++++++ t/http-router-named-urls.t | 13 +++++++++++ 5 files changed, 42 insertions(+), 18 deletions(-) create mode 100644 lib/Cro/HTTP/Router/Roles.pm6 diff --git a/META6.json b/META6.json index f34a4400..ec2a1956 100644 --- a/META6.json +++ b/META6.json @@ -65,6 +65,7 @@ "Cro::HTTP::ReverseProxy": "lib/Cro/HTTP/ReverseProxy.pm6", "Cro::HTTP::Router": "lib/Cro/HTTP/Router.pm6", "Cro::HTTP::Router::LinkGenerator": "lib/Cro/HTTP/Router/LinkGenerator.pm6", + "Cro::HTTP::Router::Roles": "lib/Cro/HTTP/Router/Roles.pm6", "Cro::HTTP::Server": "lib/Cro/HTTP/Server.pm6", "Cro::HTTP::Session::IdGenerator": "lib/Cro/HTTP/Session/IdGenerator.pm6", "Cro::HTTP::Session::InMemory": "lib/Cro/HTTP/Session/InMemory.pm6", diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index 575c247c..8697025d 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -10,6 +10,7 @@ use Cro::HTTP::MimeTypes; use Cro::HTTP::PushPromise; use Cro::HTTP::Request; use Cro::HTTP::Response; +use Cro::HTTP::Router::Roles; use Cro::UnhandledErrorReporter; use IO::Path::ChildSecure; use Cro::HTTP::Router::LinkGenerator; @@ -52,22 +53,18 @@ class X::Cro::HTTP::Router::ConfusedCapture is Exception { } } -module Cro::HTTP::Router { - role Query {} +package Cro::HTTP::Router { multi trait_mod:(Parameter:D $param, :$query! --> Nil) is export { - $param does Query; + $param does Cro::HTTP::Router::Query; } - role Header {} multi trait_mod:(Parameter:D $param, :$header! --> Nil) is export { - $param does Header; + $param does Cro::HTTP::Router::Header; } - role Cookie {} multi trait_mod:(Parameter:D $param, :$cookie! --> Nil) is export { - $param does Cookie; + $param does Cro::HTTP::Router::Cookie; } - role Auth {} multi trait_mod:(Parameter:D $param, :$auth! --> Nil) is export { - $param does Auth; + $param does Cro::HTTP::Router::Auth; } #| Router plugins register themselves using the C @@ -352,7 +349,7 @@ module Cro::HTTP::Router { $status = 400; last; } - elsif $param ~~ Auth || $param.type ~~ Cro::HTTP::Auth { + elsif $param ~~ Cro::HTTP::Router::Auth || $param.type ~~ Cro::HTTP::Auth { $status = 401; last; } @@ -527,7 +524,7 @@ module Cro::HTTP::Router { # and it and compile the check. my $have-auth-param = False; with @positional[0] -> $param { - if $param ~~ Auth || $param.type ~~ Cro::HTTP::Auth { + if $param ~~ Cro::HTTP::Router::Auth || $param.type ~~ Cro::HTTP::Auth { @positional.shift; $have-auth-param = True; $need-sig-bind = True; @@ -633,11 +630,11 @@ module Cro::HTTP::Router { for @named -> $param { my $target-name = $param.slurpy ?? $param.name !! $param.named_names[0]; my ($exists, $lookup) = do given $param { - when Cookie { + when Cro::HTTP::Router::Cookie { '$req.has-cookie(Q[' ~ $target-name ~ '])', '$req.cookie-value(Q[' ~ $target-name ~ '])' } - when Header { + when Cro::HTTP::Router::Header { '$req.has-header(Q[' ~ $target-name ~ '])', '$req.header(Q[' ~ $target-name ~ '])' } @@ -662,10 +659,10 @@ module Cro::HTTP::Router { } elsif $type =:= Positional { given $param { - when Header { + when Cro::HTTP::Router::Header { push @make-tasks, '%unpacks{Q[' ~ $target-name ~ ']} = $req.headers'; } - when Cookie { + when Cro::HTTP::Router::Cookie { die "Cookies cannot be extracted to List. Maybe you want '%' instead of '@'"; } default { @@ -675,10 +672,10 @@ module Cro::HTTP::Router { } elsif $type =:= Associative { given $param { - when Cookie { + when Cro::HTTP::Router::Cookie { push @make-tasks, '%unpacks{Q[' ~ $target-name ~ ']} = $req.cookie-hash'; } - when Header { + when Cro::HTTP::Router::Header { push @make-tasks, 'my %result;' ~ '$req.headers.map({ %result{$_.name} = $_.value });' diff --git a/lib/Cro/HTTP/Router/LinkGenerator.pm6 b/lib/Cro/HTTP/Router/LinkGenerator.pm6 index 3f1b416a..ef086eab 100644 --- a/lib/Cro/HTTP/Router/LinkGenerator.pm6 +++ b/lib/Cro/HTTP/Router/LinkGenerator.pm6 @@ -1,4 +1,5 @@ use Cro::Uri :encode-percents; +use Cro::HTTP::Router::Roles; class Cro::HTP::Router::LinkGenerator { has Str $.prefix is required; @@ -34,6 +35,9 @@ sub signature-to-sub(Signature $s) { my @default; my $min-args = 0; for $s.params.kv -> $i, $param { + next if $param ~~ Cro::HTTP::Router::Header; + next if $param ~~ Cro::HTTP::Router::Cookie; + if $param.positional { with extract-static-part $param -> $part { @path-parts[$i] = $part; @@ -71,7 +75,10 @@ sub signature-to-sub(Signature $s) { my @available-default = @default; for @fn-parts -> $i { if @args { - @result[$i] = @args.shift + @result[$i] = @args.shift; + if @result[$i] ~~ Str { + @result[$i] = encode-percents(@result[$i]); + } } elsif @available-default { @result[$i] = @available-default.shift } diff --git a/lib/Cro/HTTP/Router/Roles.pm6 b/lib/Cro/HTTP/Router/Roles.pm6 new file mode 100644 index 00000000..00e28dd7 --- /dev/null +++ b/lib/Cro/HTTP/Router/Roles.pm6 @@ -0,0 +1,6 @@ +package Cro::HTTP::Router { + role Query {} + role Header {} + role Cookie {} + role Auth {} +} diff --git a/t/http-router-named-urls.t b/t/http-router-named-urls.t index 9a2fd71a..341bb358 100644 --- a/t/http-router-named-urls.t +++ b/t/http-router-named-urls.t @@ -12,6 +12,19 @@ sub test-route-urls($app) { $responses.receive; } +test-route-urls route { + get -> { + is abs-link('qs', 'tools', query => 'abc?!'), '/search/tools?query=abc%3F%21', 'Escaped named param'; + is abs-link('segs', 42, 'foo bar.jpg'), '/product/42/docs/foo%20bar.jpg', 'Escaped positional'; + is abs-link('noqs'), '/baz', 'Non-path related parameters were not counted'; + }; + + get :name, -> 'foo', 'bar' { } + get :name, -> 'product', $id, 'docs', $file { } + get :name, -> 'search', $category, :$query {} + get :name, -> 'baz', :$foo! is cookie, :$bar! is header {} +} + test-route-urls route { get -> { is-deeply router-plugin-get-innermost-configs($link-plugin)[0].link-generators, %(), "No named urls"; From c30fcbec31032e4c89a803ce7c7b1378781f343e Mon Sep 17 00:00:00 2001 From: Altai-man Date: Wed, 10 Nov 2021 18:46:14 +0100 Subject: [PATCH 11/11] Better joiner for a warning --- lib/Cro/HTTP/Router.pm6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cro/HTTP/Router.pm6 b/lib/Cro/HTTP/Router.pm6 index 8697025d..b0c8939c 100644 --- a/lib/Cro/HTTP/Router.pm6 +++ b/lib/Cro/HTTP/Router.pm6 @@ -1413,7 +1413,7 @@ package Cro::HTTP::Router { } @options.push: |$links.link-generators.keys; } - warn "Called the $sub-name subroutine with $route-name but no such route defined, options are: @options.join('/')"; + warn "Called the $sub-name subroutine with $route-name but no such route defined, options are: @options.join(', ')"; Nil; }