Skip to content

Commit c919c16

Browse files
Altai-manAltai-man
committed
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.
1 parent f7802c4 commit c919c16

File tree

2 files changed

+104
-25
lines changed

2 files changed

+104
-25
lines changed

‎lib/Cro/HTTP/Router.pm6

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,19 +135,21 @@ module Cro::HTTP::Router {
135135
has &.implementation;
136136
has Hash[Array, Cro::HTTP::Router::PluginKey] $.plugin-config;
137137
has Hash[Array, Cro::HTTP::Router::PluginKey] $.flattened-plugin-config;
138+
has Bool $.from-include;
138139

139140
method copy-adding(:@prefix, :@body-parsers!, :@body-serializers!, :@before-matched!, :@after-matched!, :@around!,
140-
Hash[Array, Cro::HTTP::Router::PluginKey] :$plugin-config, :$name-prefix) {
141+
Hash[Array, Cro::HTTP::Router::PluginKey] :$plugin-config, :$name-prefix, :$from-include!) {
141142
self.bless:
142-
:$!method, :&!implementation, |(name => ($name-prefix // '') ~ $!name with $!name),
143+
:$!method, :&!implementation, |(name => ($name-prefix ?? "$name-prefix." !! '') ~ $!name with $!name),
143144
:prefix[flat @prefix, @!prefix],
144145
:body-parsers[flat @!body-parsers, @body-parsers],
145146
:body-serializers[flat @!body-serializers, @body-serializers],
146147
:before-matched[flat @before-matched, @!before-matched],
147148
:after-matched[flat @!after-matched, @after-matched],
148149
:around[flat @!around, @around],
149150
:$!plugin-config,
150-
:flattened-plugin-config(merge-plugin-config($plugin-config, $!flattened-plugin-config // $!plugin-config))
151+
:flattened-plugin-config(merge-plugin-config($plugin-config, $!flattened-plugin-config // $!plugin-config)),
152+
:$from-include
151153
}
152154

153155
sub merge-plugin-config($outer, $inner) {
@@ -301,6 +303,7 @@ module Cro::HTTP::Router {
301303
with $routing-outcome {
302304
my ($handler-idx, $args) = .ast;
303305
my $handler := @!handlers[$handler-idx];
306+
my %*URLS = %!urls;
304307
whenever $handler.invoke($request, $args) -> $response {
305308
emit $response;
306309
QUIT {
@@ -347,7 +350,7 @@ module Cro::HTTP::Router {
347350
method add-handler(Str $method, &implementation, Str :$name --> Nil) {
348351
@!handlers-to-add.push: {
349352
@!handlers.push(RouteHandler.new(:$method, :&implementation, :@!before-matched, :@!after-matched,
350-
:@!around, :%!plugin-config, :$name));
353+
:@!around, :%!plugin-config, :$name, :!from-include));
351354
}
352355
}
353356

@@ -411,7 +414,7 @@ module Cro::HTTP::Router {
411414
for @!includes -> (:@prefix, :$includee, :$name-prefix) {
412415
for $includee.handlers() {
413416
@!handlers.push: .copy-adding(:@prefix, :@!body-parsers, :@!body-serializers,
414-
:@!before-matched, :@!after-matched, :@!around, :%!plugin-config, :$name-prefix);
417+
:@!before-matched, :@!after-matched, :@!around, :%!plugin-config, :$name-prefix, :from-include);
415418
}
416419
}
417420
self!generate-route-matcher();
@@ -421,9 +424,11 @@ module Cro::HTTP::Router {
421424
method !generate-urls() {
422425
my %urls;
423426
my $prefix = $.name // "";
427+
$prefix ~= '.' if $prefix;
424428
for @.handlers -> $handler {
425429
if $handler ~~ RouteHandler && $handler.name.defined {
426-
my $key = $prefix ~ $handler.name;
430+
my $key = $handler.from-include ?? $handler.name !! $prefix ~ $handler.name;
431+
next if $handler.from-include and not $key.contains('.');
427432
die X::Cro::HTTP::Router::DuplicateLinkName.new(:$key) if %urls{$key}:exists;
428433
%urls{$key} = route-signature-to-sub($handler.signature);
429434
}
@@ -576,7 +581,7 @@ module Cro::HTTP::Router {
576581

577582
# Turned nameds into unpacks.
578583
for @named -> $param {
579-
my $target-name = $param.named_names[0];
584+
my $target-name = $param.slurpy ?? $param.name !! $param.named_names[0];
580585
my ($exists, $lookup) = do given $param {
581586
when Cookie {
582587
'$req.has-cookie(Q[' ~ $target-name ~ '])',
@@ -756,17 +761,17 @@ module Cro::HTTP::Router {
756761
sub include(*@includees, *%includees --> Nil) is export {
757762
for @includees {
758763
when RouteSet {
759-
$*CRO-ROUTE-SET.add-include([], $_);
764+
$*CRO-ROUTE-SET.add-include([], $_, name-prefix => $_.name);
760765
}
761766
when Pair {
762767
my ($prefix, $routes) = .kv;
763768
if $routes ~~ RouteSet {
764769
given $prefix {
765770
when Str {
766-
$*CRO-ROUTE-SET.add-include([$prefix], $routes);
771+
$*CRO-ROUTE-SET.add-include([$prefix], $routes, name-prefix => $routes.name);
767772
}
768773
when Iterable {
769-
$*CRO-ROUTE-SET.add-include($prefix, $routes);
774+
$*CRO-ROUTE-SET.add-include($prefix, $routes, name-prefix => $routes.name);
770775
}
771776
default {
772777
die "An 'include' prefix may be a Str or Iterable, but not " ~ .^name;
@@ -1321,6 +1326,16 @@ module Cro::HTTP::Router {
13211326
}
13221327
}
13231328

1329+
sub make-link($route-name, *@args, *%args) is export {
1330+
my $maker = %*URLS{$route-name} // Nil;
1331+
with $maker {
1332+
return $_(|@args, |%args);
1333+
} else {
1334+
warn "Called the make-link subroutine with $route-name but no such route defined";
1335+
"";
1336+
}
1337+
}
1338+
13241339
#| Register a router plugin. The provided ID is for debugging purposes.
13251340
#| Returns a plugin key object which can be used for further interactions
13261341
#| with the router plugin infrastructure.

‎t/http-router-named-urls.t

Lines changed: 79 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ use Test;
3030
}
3131

3232
{
33-
my $app = route :name<main->, {
33+
my $app = route :name<main>, {
3434
get :name<home>, -> {};
3535
}
36-
is-deeply $app.urls.keys, ('main-home',), "A named url with a prefix";
37-
is $app.urls<main-home>(), '/';
36+
is-deeply $app.urls.keys, ('main.home',), "A named url with a prefix";
37+
is $app.urls<main.home>(), '/';
3838
}
3939

4040
throws-like {
@@ -45,20 +45,34 @@ throws-like {
4545
}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: home";
4646

4747
throws-like {
48-
route :name<main->, {
48+
route :name<main>, {
4949
get :name<home>, -> {};
5050
get :name<home>, -> {};
5151
}
52-
}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: main-home";
52+
}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: main.home";
5353

54-
{
55-
my $app = route :name<main->, {
56-
include route {
57-
get :name<home>, -> {};
58-
}
59-
}
60-
is-deeply $app.urls.keys, ('main-home',), "A named url in an include with a prefix";
61-
}
54+
# XXX the test intention is bogus?
55+
# We basically have two sorts of cases:
56+
# * A route is not from include for sure, so we append its possible name to possible names of routes
57+
# * 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
58+
# For the code below, it assumes we should squash the hierarchy when the include is anonymous, but this is forbidden because we explicitly allow:
59+
# my $app = route {
60+
# include route {
61+
# get :name<home>, -> {}
62+
# }
63+
# include route {
64+
# get :name<home>, -> {}
65+
# }
66+
# }
67+
# to exist, implying we do not peek into "anonymous" includes
68+
#{
69+
# my $app = route :name<main>, {
70+
# include route {
71+
# get :name<home>, -> {};
72+
# }
73+
# }
74+
# is-deeply $app.urls.keys, ('main.home',), "A named url in an include with a prefix";
75+
#}
6276

6377
{
6478
my $app = route {
@@ -111,7 +125,57 @@ throws-like {
111125
is $app.urls<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';
112126
}
113127

114-
# TODO before/after
115-
# TODO include should not trigger name conflicts
128+
{
129+
lives-ok {
130+
my $app = route {
131+
include route {
132+
get :name<home>, -> {};
133+
}
134+
include route {
135+
get :name<home>, -> {};
136+
}
137+
}
138+
}, 'Conflict check is per-route block 1';
139+
}
140+
141+
{
142+
lives-ok {
143+
my $app = route {
144+
get :name<simple>, -> {};
145+
include route {
146+
get :name<home>, -> {};
147+
get :name<homeA>, -> {};
148+
}
149+
include route :name<secondRoute>, {
150+
get :name<home>, -> {};
151+
get :name<homeB>, -> {};
152+
}
153+
}
154+
}, 'Conflict check is per-route block 2';
155+
}
156+
157+
{
158+
lives-ok {
159+
my $app = route {
160+
include route :name<foo>, {
161+
get :name<home>, -> {};
162+
}
163+
include route :name<bar>, {
164+
get :name<home>, -> {};
165+
}
166+
}
167+
}, 'Conflict check is by route name';
168+
}
169+
170+
throws-like {
171+
my $app = route {
172+
include route :name<foo>, {
173+
get :name<home>, -> {};
174+
}
175+
include route :name<foo>, {
176+
get :name<home>, -> {};
177+
}
178+
}
179+
}, X::Cro::HTTP::Router::DuplicateLinkName, message => "Conflicting link name: foo.home";
116180

117181
done-testing;

0 commit comments

Comments
 (0)