Content negotiation v2

[KES777]

In continue of this

jberger understands me right:

once you've arrived at the resource you want, the representation is determined by content negotiation this is explicitly after routing is complete

As we both were agree yesterday: route matching is not content negotiation.
Therefore $c->stash->{format} after route matching should be empty.
(otherwise other code will think that content negotiation is done). Because of this I just do not merge {format} into stash:

lib/Mojolicious/Routes/Pattern.pm

@@ -26,6 +26,8 @@ sub match_partial {
   $$pathref = ${^POSTMATCH};
   @captures = () if $#+ == 0;
   my $captures = {%{$self->defaults}};
+  # default {format} will be checked while content negotiation. See accepts
+  delete $captures->{format};
   for my $placeholder (@{$self->placeholders}, 'format') {
     last unless @captures;
     my $capture = shift @captures;

lib/Mojolicious/Routes.pm

@@ -30,6 +30,9 @@ sub continue {
   @{$stash->{'mojo.captures'} //= {}}{keys %$field} = values %$field;
   @$stash{keys %$field} = values %$field;

+  # The {format} is negotiation process
+  delete $stash->{format};
+
   my $continue;
   my $last = !$stack->[++$position];
   if (my $cb = $field->{cb}) { $continue = $self->_callback($c, $cb, $last) }

Also I remove this code which was added by commit#300485b22:

@@ -75,11 +75,7 @@ sub _match {
   # Endpoint (or intermediate destination)
   if (($endpoint && $empty) || $r->inline) {
     push @{$self->stack}, {%$captures};
-    if ($endpoint && $empty) {
-      my $format = $captures->{format};
-      if ($format) { $_->{format} = $format for @{$self->stack} }
-      return !!$self->endpoint($r);
-    }
+    return !!$self->endpoint($r) if $endpoint && $empty;
     delete @$captures{qw(app cb)};
   }

Instead of this I have added content negotiation (which sets up $c->stash->{format}) here and here so bridges / under routes can see $c->stash->{format} too:

lib/Mojolicious/Plugin/DefaultHelpers.pm
@@ -107,7 +108,7 @@ sub _development {
   my $renderer = $app->renderer;
   my $options  = {
     exception => $page eq 'exception' ? $e : undef,
-    format    => $stash->{format} || $renderer->default_format,
+    format    => scalar $c->format,
     handler   => undef,
     snapshot  => \%snapshot,
     status    => $page eq 'exception' ? 500 : 404,

https://github.com/KES777/mojo/pull/40/files#diff-ccf996555125c90064b288a0e4c139c5
@@ -93,7 +128,7 @@ sub render {
   };
   my $inline = $options->{inline} = delete $stash->{inline};
   $options->{handler} //= $self->default_handler if defined $inline;
-  $options->{format} = $stash->{format} || $self->default_format;
+  $options->{format} = $self->format( $c ) || $self->default_format;

   # Data
   return delete $stash->{data}, $options->{format} if defined $stash->{data};
@@ -115,7 +150,7 @@ sub render {
     if $stash->{extends} || $stash->{layout};
   while ((my $next = _next($stash)) && !defined $inline) {
     @$options{qw(handler template)} = ($stash->{handler}, $next);
-    $options->{format} = $stash->{format} || $self->default_format;
+    $options->{format} = $self->format( $c ) || $self->default_format;
     if ($self->_render_template($c, \my $tmp, $options)) { $output = $tmp }
     $content->{content} //= $output if $output =~ /\S/;
   }

NOTICE: It would be better to do content negotiation at around_action hook but, unfortunately, default template rendering for actionless controller do not emit around_action hook.

After these few changes bridges/under routes can see detected format and access it as before $c->stash->{format}

After that we can fix accepts. Now it uses captured format instead of "half" negotiated format by route matching

@@ -27,7 +27,8 @@ sub accepts {

   # List representations
   my $req  = $c->req;
-  my $fmt  = $req->param('format') || $c->stash->{format};
+  my $capture =  $c->match->stack->[-1] || {};
+  my $fmt  = $req->param('format') || $capture->{format};
   my @exts = $fmt ? ($fmt) : ();
   push @exts, @{$c->app->types->detect($req->headers->accept)};
   return \@exts unless @_;
@@ -37,6 +38,40 @@ sub accepts {
   return @exts ? undef : shift;
 }

II. Downside of accepts helper method is that it returns list of requested format by user (It really stands for its purpose). Because of that accepts is not usable for content negotiation which should return format. So I implement additional helper with the same name format which allow better fallbacks for content negotiation when html is not the default.

  # /custom                      -> "json"
  # /custom.html                 -> undef
  # /custom.xml                  -> "xml"
  # /custom.txt                  -> undef
  $r->get( '/custom', { format => [qw( json, xml )] );

Also with this helper respond_to became more clear:

sub respond_to {
  my ($self, $args) = (shift, ref $_[0] ? $_[0] : {@_});

  # Detect format
  my $renderer = $self->app->renderer;
  my $format = $self->format( $renderer->default_format, keys %$args );

  # Find target
  my $target = $args->{$format};
  unless( $target ) {
    return $self->rendered(204) unless $target = $args->{any};
  }

  # Dispatch
  ref $target eq 'CODE' ? $target->($self) : $self->render(%$target);

  return $self;
}

III. Also this patch fixing broken Mojolicious tests where it expects wrong data Here is testcase (short snippet from t/mojolicous/exception_lite_app.t)