From: Tyler Riddle <t.riddle@shadowcat.co.uk>
Date: Sat, 10 Nov 2012 01:12:07 +0000 (-0800)
Subject: better method of log forwarding
X-Git-Tag: v0.003001_01~72
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=scpubgit%2FObject-Remote.git;a=commitdiff_plain;h=f048e6dfeaefce08546e91cc9d4bf79492d7ca0d

better method of log forwarding
---

diff --git a/lib/Object/Remote/Logging.pm b/lib/Object/Remote/Logging.pm
index c137b3e..5901b0c 100644
--- a/lib/Object/Remote/Logging.pm
+++ b/lib/Object/Remote/Logging.pm
@@ -143,6 +143,9 @@ sub init_logging_forwarding {
   my ($self, %controller_info) = @_;
   
   router()->_remote_metadata({ connection_id => $controller_info{connection_id} });
+  #TODO having an instance of an object in the remote interpreter causes it to hang
+  #on exit intermitently or leave a zombie laying around frequently - not a bug limited
+  #to log forwarding
   router()->_forward_destination($controller_info{router}) if $ENV{OBJECT_REMOTE_LOG_FORWARDING};
 }
 
diff --git a/lib/Object/Remote/Logging/Router.pm b/lib/Object/Remote/Logging/Router.pm
index 7479dd7..9ab2b77 100644
--- a/lib/Object/Remote/Logging/Router.pm
+++ b/lib/Object/Remote/Logging/Router.pm
@@ -35,8 +35,6 @@ sub _get_loggers {
 
     foreach my $logger ($selector->($package, { %metadata })) {
       next unless defined $logger;
-      #my $method = $logger->can($is_level);
-      #next unless defined $method;
       next unless $logger->$is_level;
       push(@loggers, $logger);
     }
@@ -47,6 +45,8 @@ sub _get_loggers {
   return @loggers; 
 }
 
+#overloadable so a router can invoke a logger
+#in a different way
 sub _invoke_logger {
   my ($self, $logger, $level_name, $content, $metadata) = @_;
   #Invoking the logger like this gets all available data to the
@@ -59,6 +59,22 @@ sub _invoke_logger {
   $logger->$level_name($content, $metadata);
 }
 
+#overloadable so forwarding can have the updated
+#metadata but does not have to wrap get_loggers
+#which has too many drawbacks
+sub _deliver_message {
+  my ($self, $level, $generator, $args, $metadata) = @_;
+  my @loggers = $self->_get_loggers(%$metadata);
+  
+  return unless @loggers > 0;
+  #this is the point where the user provided log message
+  #code block is executed
+  my @content = $generator->(@$args);
+  foreach my $logger (@loggers) {
+    $self->_invoke_logger($logger, $level, \@content, $metadata);
+  }
+}
+
 sub handle_log_request {
   my ($self, $metadata_in, $generator, @args) = @_;
   my %metadata = %{$metadata_in};
@@ -83,9 +99,7 @@ sub handle_log_request {
   $metadata{method} = $caller_info[3];
   $metadata{method} =~ s/^${package}::// if defined $metadata{method};
 
-  foreach my $logger ($self->_get_loggers(%metadata)) {
-    $self->_invoke_logger($logger, $level, [ $generator->(@args) ], \%metadata);
-  }
+  $self->_deliver_message($level, $generator, [ @args ], \%metadata);
 }
 
 sub connect {
diff --git a/lib/Object/Remote/Role/LogForwarder.pm b/lib/Object/Remote/Role/LogForwarder.pm
index a6ba939..d701b72 100644
--- a/lib/Object/Remote/Role/LogForwarder.pm
+++ b/lib/Object/Remote/Role/LogForwarder.pm
@@ -7,29 +7,24 @@ has _forward_destination => ( is => 'rw' );
 has enable_forward => ( is => 'rw', default => sub { 1 } );
 has _forward_stop => ( is => 'ro', required => 1, default => sub { {} } );
 
-around _get_loggers => sub {
-  my ($orig, $self, %metadata) = @_;
-  my $package = $metadata{package};
-  my %clone = %metadata;
+after _deliver_message => sub {
+  my ($self, $level, $generator, $args, $metadata) = @_;
+  my $package = $metadata->{package};
+  my $destination = $self->_forward_destination;
   our $reentrant;
-  
-  return if $reentrant;
-  local($reentrant) = 1; 
-    
-  my @loggers = $orig->($self, %clone);
 
-  if (! $self->enable_forward || $self->_forward_stop->{$package}) {
-    #warn "will not forward log events for '$package'";
-    return @loggers;
+  return unless $self->enable_forward;
+  return unless defined $destination;
+  return if $self->_forward_stop->{$package};
+
+  if (defined $reentrant) {
+    warn "log forwarding went reentrant. bottom: '$reentrant' top: '$package'";
+    return;
   }
   
-  my $forward_to = $self->_forward_destination;
-  
-  if ($forward_to) {
-    push(@loggers, $forward_to->_get_loggers(%clone));
-  }
+  local $reentrant = $package;
   
-  return @loggers;
+  $destination->_deliver_message($level, $generator, $args, $metadata);
 };
 
 sub exclude_forwarding {