fixed Mojo::Transaction to propagate connection close to Mojo::Message

mojolicious · Sep 3, 2012 · f2c6f9d · f2c6f9d
1 parent 33d47b6
commit f2c6f9d
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 53 deletions.
diff --git a/Changes b/Changes
@@ -1,8 +1,10 @@
 
 3.37  2012-09-03
+  - Added finish method to Mojo::Message.
   - Updated jQuery to version 1.8.1.
   - Improved documentation.
   - Improved tests.
+  - Fixed Mojo::Transaction to propagate connection close to Mojo::Message.
   - Fixed small state bug in Mojo::Transaction.
 
 3.36  2012-08-30

diff --git a/lib/Mojo/Content/MultiPart.pm b/lib/Mojo/Content/MultiPart.pm
@@ -6,9 +6,7 @@ use Mojo::Util 'b64_encode';
 has parts => sub { [] };
 
 sub new {
-  my $self = shift->SUPER::new(@_);
-  $self->on(read => \&_read);
-  return $self;
+  shift->SUPER::new(@_)->tap(sub { $_->on(read => \&_read) });
 }
 
 sub body_contains {

diff --git a/lib/Mojo/Content/Single.pm b/lib/Mojo/Content/Single.pm
@@ -8,9 +8,7 @@ has asset => sub { Mojo::Asset::Memory->new(auto_upgrade => 1) };
 has auto_upgrade => 1;
 
 sub new {
-  my $self = shift->SUPER::new(@_);
-  $self->{read} = $self->on(read => \&_read);
-  return $self;
+  shift->SUPER::new(@_)->tap(sub { $_->{read} = $_->on(read => \&_read) });
 }
 
 sub body_contains { shift->asset->contains(shift) >= 0 }

diff --git a/lib/Mojo/Log.pm b/lib/Mojo/Log.pm
@@ -24,9 +24,7 @@ has 'path';
 my $LEVEL = {debug => 1, info => 2, warn => 3, error => 4, fatal => 5};
 
 sub new {
-  my $self = shift->SUPER::new(@_);
-  $self->on(message => \&_message);
-  return $self;
+  shift->SUPER::new(@_)->tap(sub { $_->on(message => \&_message) });
 }
 
 sub debug { shift->log(debug => @_) }

diff --git a/lib/Mojo/Message.pm b/lib/Mojo/Message.pm
@@ -103,25 +103,25 @@ sub dom {
 }
 
 sub error {
-  my $self = shift;
-
-  # Get
-  unless (@_) {
-    return unless my $err = $self->{error};
-    return wantarray ? @$err : $err->[0];
-  }
+  my ($self, @args) = @_;
 
   # Set
-  $self->{error} = [@_];
-  $self->{state} = 'finished';
+  return $self->tap(sub { $_->{error} = [@args] })->finish if @args;
 
-  return $self;
+  # Get
+  return unless my $err = $self->{error};
+  return wantarray ? @$err : $err->[0];
 }
 
 sub extract_start_line {
   croak 'Method "extract_start_line" not implemented by subclass';
 }
 
+sub finish {
+  my $self = shift->tap(sub { $_->{state} = 'finished' });
+  return $self->{finished}++ ? $self : $self->emit('finish');
+}
+
 sub fix_headers {
   my $self = shift;
 
@@ -141,22 +141,21 @@ sub get_body_chunk {
   my ($self, $offset) = @_;
 
   # Progress
-  $self->emit(progress => 'body', $offset);
+  $self->emit('progress', 'body', $offset);
 
   # Chunk
   my $chunk = $self->content->get_body_chunk($offset);
   return $chunk if !defined $chunk || length $chunk;
 
   # Finish
-  $self->{state} = 'finished';
-  $self->emit('finish');
+  $self->finish;
 
   return $chunk;
 }
 
 sub get_header_chunk {
   my ($self, $offset) = @_;
-  $self->emit(progress => 'headers', $offset);
+  $self->emit('progress', 'headers', $offset);
   return $self->fix_headers->content->get_header_chunk($offset);
 }
 
@@ -224,16 +223,10 @@ sub parse {
   return $self->error('Maximum line size exceeded', 431)
     if $self->headers->is_limit_exceeded;
 
-  # Finished
-  $self->{state} = 'finished' if $self->content->is_finished;
-
   # Progress
   $self->emit('progress');
 
-  # Finished
-  $self->emit('finish') if $self->is_finished;
-
-  return $self;
+  return $self->content->is_finished ? $self->finish : $self;
 }
 
 sub start_line_size { length shift->build_start_line }
@@ -592,6 +585,12 @@ Parser errors and codes.
 
 Extract start line from string. Meant to be overloaded in a subclass.
 
+=head2 C<finish>
+
+  $msg = $msg->finish;
+
+Finish message parser/generator.
+
 =head2 C<fix_headers>
 
   $msg = $msg->fix_headers;
@@ -652,7 +651,7 @@ working.
 
   my $success = $msg->is_finished;
 
-Check if parser is finished.
+Check if message parser/generator is finished.
 
 =head2 C<is_limit_exceeded>
 

diff --git a/lib/Mojo/Transaction.pm b/lib/Mojo/Transaction.pm
@@ -9,7 +9,9 @@ has [qw(kept_alive local_address local_port previous remote_port)];
 has req => sub { Mojo::Message::Request->new };
 has res => sub { Mojo::Message::Response->new };
 
-sub client_close { shift->server_close(@_) }
+sub client_close {
+  shift->tap(sub { $_->res->finish })->server_close(@_);
+}
 
 sub client_read  { croak 'Method "client_read" not implemented by subclass' }
 sub client_write { croak 'Method "client_write" not implemented by subclass' }
@@ -39,13 +41,10 @@ sub is_writing {
 }
 
 sub remote_address {
-  my $self = shift;
+  my ($self, @args) = @_;
 
   # New address
-  if (@_) {
-    $self->{remote_address} = shift;
-    return $self;
-  }
+  return $self->tap(sub { $_->{remote_address} = shift @args }) if @args;
 
   # Reverse proxy
   if ($ENV{MOJO_REVERSE_PROXY}) {

diff --git a/lib/Mojo/Transaction/WebSocket.pm b/lib/Mojo/Transaction/WebSocket.pm
@@ -107,10 +107,7 @@ sub client_write { shift->server_write(@_) }
 sub connection { shift->handshake->connection }
 
 sub finish {
-  my $self = shift;
-  $self->send([1, 0, 0, 0, CLOSE, '']);
-  $self->{finished} = 1;
-  return $self;
+  shift->send([1, 0, 0, 0, CLOSE, ''])->tap(sub { $_->{finished} = 1 });
 }
 
 sub is_websocket {1}

diff --git a/t/mojo/request.t b/t/mojo/request.t
@@ -2,7 +2,7 @@ use Mojo::Base -strict;
 
 use utf8;
 
-use Test::More tests => 847;
+use Test::More tests => 848;
 
 use File::Spec::Functions 'catfile';
 use File::Temp 'tempdir';
@@ -13,9 +13,12 @@ use Mojo::Message::Request;
 
 # Parse HTTP 1.1 message with huge "Cookie" header exceeding all limits
 my $req = Mojo::Message::Request->new;
+my $finished;
+$req->on(finish => sub { $finished = shift->is_finished });
 $req->parse("GET / HTTP/1.1\x0d\x0a");
 $req->parse('Cookie: ' . ('a=b; ' x (1024 * 1024)) . "\x0d\x0a");
 $req->parse("Content-Length: 0\x0d\x0a\x0d\x0a");
+ok $finished, 'finish event has been emitted';
 ok $req->is_finished, 'request is finished';
 is $req->error,       'Maximum message size exceeded', 'right error';
 is $req->method,      'GET', 'right method';
@@ -443,8 +446,8 @@ is $req->headers->content_type,   'text/plain', 'right "Content-Type" value';
 is $req->headers->content_length, 27,           'right "Content-Length" value';
 
 # Parse full HTTP 1.0 request with zero chunk
-$req = Mojo::Message::Request->new;
-my $finished;
+$req      = Mojo::Message::Request->new;
+$finished = undef;
 $req->on(finish => sub { $finished = shift->is_finished });
 $req->parse('GET /foo/bar/baz.html?fo');
 $req->parse("o=13#23 HTTP/1.0\x0d\x0aContent");

diff --git a/t/mojo/user_agent.t b/t/mojo/user_agent.t
@@ -6,7 +6,7 @@ BEGIN {
   $ENV{MOJO_REACTOR} = 'Mojo::Reactor::Poll';
 }
 
-use Test::More tests => 112;
+use Test::More tests => 118;
 
 use Mojo::IOLoop;
 use Mojo::UserAgent;
@@ -189,26 +189,43 @@ is $tx->res->code, 200,      'right status';
 is $tx->res->body, 'works!', 'right content';
 
 # GET / (events)
-my $finished;
+my ($finished_tx, $finished_res);
 $tx = $ua->build_tx(GET => '/');
 ok !$tx->is_finished, 'transaction is not finished';
 $ua->once(
   start => sub {
     my ($self, $tx) = @_;
-    $tx->on(finish => sub { $finished++ });
+    $tx->on(finish => sub { $finished_tx++ });
+    $tx->res->on(finish => sub { $finished_res++ });
   }
 );
 $tx = $ua->start($tx);
-ok $tx->success,     'successful';
+ok $tx->success, 'successful';
+is $finished_tx,  1, 'finish event has been emitted once';
+is $finished_res, 1, 'finish event has been emitted once';
 ok $tx->is_finished, 'transaction is finished';
-is $finished, 1, 'finish event has been emitted';
-is $tx->res->code, 200,      'right status';
-is $tx->res->body, 'works!', 'right content';
+ok $tx->res->is_finished, 'response is finished';
+is $tx->res->code,        200, 'right status';
+is $tx->res->body,        'works!', 'right content';
 
 # GET /no_length (missing Content-Length header)
-$tx = $ua->get('/no_length');
-ok $tx->success,     'successful';
+($finished_tx, $finished_res) = undef;
+$tx = $ua->build_tx(GET => '/no_length');
+ok !$tx->is_finished, 'transaction is not finished';
+$ua->once(
+  start => sub {
+    my ($self, $tx) = @_;
+    $tx->on(finish => sub { $finished_tx++ });
+    $tx->res->on(finish => sub { $finished_res++ });
+
+  }
+);
+$tx = $ua->start($tx);
+ok $tx->success, 'successful';
+is $finished_tx,  1, 'finish event has been emitted once';
+is $finished_res, 1, 'finish event has been emitted once';
 ok $tx->is_finished, 'transaction is finished';
+ok $tx->res->is_finished, 'response is finished';
 ok !$tx->error, 'no error';
 ok $tx->kept_alive, 'kept connection alive';
 ok !$tx->keep_alive, 'keep connection not alive';