Don't replace Controller->match in Routes->match

[D1CED]

• Mojolicious version: v8.57
• Perl version: 5.32
• Operating system: Linux, kernel version 5.8.11

Im split on whether this is to be considered a bug or a feature request, but I'll treat this as a bug for this issue.

Steps to reproduce the behavior

Replace Controller->match with a custom implementation of Mojolicious::Routes::Match early in its life cycle. Via a hook, override of Mojolicous->build_controller or via a new Mojolicous->controller_class.

Expected behavior

I get access to my customized match object in my actions.

Actual behavior

The match object is always a Mojolicious::Routes::Match and not my extended class.

So the 'bug' is located in Routes->match where a new Match object is created even if the controller already has one.

  my $match = Mojolicious::Routes::Match->new(root => $self);
  $c->match($match);

My suggestion would be to do something like

  $c->match->root($self);

instead.

My current workaround uses monkey_patch but I'd prefer the above stated solution.

Thanks Mojo-Team I had lots of fun working with your framework.

[kiwiroy]

As in this minimal example, which renders Mojolicious::Routes::Match?

package MyApp::Match;
use Mojo::Base 'Mojolicious::Routes::Match';

package MyApp::Controller;
use Mojo::Base 'Mojolicious::Controller';
has match => sub { MyApp::Match->new };

package main;
use Mojolicious::Lite -signatures;

get '/' => sub ($c) { $c->render(text => ref($c->match)) };

app->controller_class('MyApp::Controller')->start;

Slightly modifying your suggestion results in rendering MyApp::Match.

diff --git a/lib/Mojolicious/Routes.pm b/lib/Mojolicious/Routes.pm
index f96a0cea2..bc636cf6e 100644
--- a/lib/Mojolicious/Routes.pm
+++ b/lib/Mojolicious/Routes.pm
@@ -81,8 +81,7 @@ sub match {

   # Check cache
   my $ws    = $c->tx->is_websocket ? 1 : 0;
-  my $match = Mojolicious::Routes::Match->new(root => $self);
-  $c->match($match);
+  my $match = $c->match->tap(sub { $_->root($self) });
   my $cache = $self->cache;
   if (my $result = $cache->get("$method:$path:$ws")) {
     return $match->endpoint($result->{endpoint})->stack($result->{stack});

Without the patch above and without monkey_patch, use rebless trick/hack to render "bar"

package MyApp::Match;
use Mojo::Base 'Mojolicious::Routes::Match';
sub foo { return 'bar'; }
package MyApp::Controller;
use Mojo::Base 'Mojolicious::Controller', -signatures;

sub match ($self, @match) {
  return exists $self->{match} ? $self->{match} : ($self->{match} = MyApp::Match->new) if @match == 0;
  $self->{match} = bless $match[0] => 'MyApp::Match';
  $self;
}
package main;
use Mojolicious::Lite -signatures;

get '/' => sub ($c) { $c->render(text => $c->match->foo) };

app->controller_class('MyApp::Controller')->start;

[D1CED]

Hi, thanks for your response.

Your second code example was exactly what I had in mind as a proposed change.

The rebless trick though is not applicable in my case. This is because the match modifying code lives inside a plugin and I am using a separate controller class. Even if I'd replace the default cotroller class to rebless the matcher the replacment happens in the context of the separate controller class which directly inherits from Mojolicious::Controller.

[D1CED]

Here's a testable example:

#!/usr/bin/env perl

package MyApp::Match {
    use Mojo::Base 'Mojolicious::Routes::Match';
    use Mojo::URL;

    sub path_for { {path => Mojo::URL->new('HelloMojo')} }
}

package MyApp::Controller::Example { # this code should not change
    use Mojo::Base 'Mojolicious::Controller', -signatures;

    sub my_action ($self) { $self->render(text => $self->match->path_for('test')->{path}) }
}

package MyApp {
    use Mojo::Base 'Mojolicious', -signatures;

    sub build_controller ($self, @rest) {
        my $c = $self->next::method(@rest);
        # currently match will be replaced when reaching MyApp::Controller::Example :(
        $c->match(MyApp::Match->new(root => $self->routes));
        return $c;
    }
    sub startup ($self) { $self->routes->get('/test')->to('example#my_action') }
}

#MyApp->new->start('daemon')

use Test::More;
use Test::Mojo;

my $t = Test::Mojo->new('MyApp');
$t->get_ok('/test')->status_is(200)->content_is('HelloMojo');

done_testing;

[kraih]

It's one less line of code, you should have tried selling it that way from the start. 😉 Reducing the line count is the easiest way to get changes accepted.