From: Peter Rabbitson <ribasushi@cpan.org>
Date: Wed, 17 Sep 2014 16:58:29 +0000 (+0200)
Subject: Revert e9f71ab2 - it ends up breaking a declared API
X-Git-Tag: v0.082800~35
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FDBIx-Class.git;a=commitdiff_plain;h=8494142cea239b72298004f762cf500f71650533

Revert e9f71ab2 - it ends up breaking a declared API

I should have thought of this earlier - a debug object is not necessarily
isa(IO::Handle) (the documentation never mentioned this). And indeed: the
tests of DBIx::Class::QueryLog do exactly this - they use an object with
just a print() method and absolutely nothing else.

So instead of disabling the sticky autoflush (which really is only a visible
change on STDERR dup) document this behavior and move on.
---

diff --git a/lib/DBIx/Class/Storage/Statistics.pm b/lib/DBIx/Class/Storage/Statistics.pm
index 4894cf7..5768db6 100644
--- a/lib/DBIx/Class/Storage/Statistics.pm
+++ b/lib/DBIx/Class/Storage/Statistics.pm
@@ -15,7 +15,6 @@ BEGIN {
 
 extends 'DBIx::Class';
 use DBIx::Class::_Util qw(sigwarn_silencer qsub);
-use IO::Handle ();
 use namespace::clean;
 
 =head1 NAME
@@ -44,12 +43,13 @@ Returns a new L<DBIx::Class::Storage::Statistics> object.
 
 Sets or retrieves the filehandle used for trace/debug output.  This should
 be an L<IO::Handle> compatible object (only the
-L<< printflush|IO::Handle/$io->printflush_(_ARGS_) >> method is used). By
+L<< print|IO::Handle/METHODS >> method is used). By
 default it is initially set to STDERR - although see discussion of the
 L<DBIC_TRACE|DBIx::Class::Storage/DBIC_TRACE> environment variable.
 
-Invoked as a getter it will lazily open a filehandle for you if one is not
-already set.
+Invoked as a getter it will lazily open a filehandle and set it to
+L<< autoflush|perlvar/HANDLE->autoflush( EXPR ) >> (if one is not
+already set).
 
 =cut
 
@@ -85,6 +85,8 @@ sub _build_debugfh {
     $_[0]->_defaulted_to_stderr(1);
   }
 
+  $fh->autoflush(1);
+
   $fh;
 }
 
@@ -109,7 +111,7 @@ sub print {
   local $SIG{__WARN__} = sigwarn_silencer(qr/^Wide character in print/)
     if $self->_defaulted_to_stderr;
 
-  $fh->printflush($msg);
+  $fh->print($msg);
 }
 
 =head2 silence
diff --git a/t/storage/debug.t b/t/storage/debug.t
index f28d4b5..e023fff 100644
--- a/t/storage/debug.t
+++ b/t/storage/debug.t
@@ -24,6 +24,7 @@ $schema->storage->debugobj(DBIx::Class::Storage::Statistics->new);
 
 ok ( $schema->storage->debug(1), 'debug' );
 $schema->storage->debugfh($lfn->openw);
+$schema->storage->debugfh->autoflush(1);
 $schema->resultset('CD')->count;
 
 my @loglines = $lfn->slurp;
@@ -94,7 +95,6 @@ die "How did that fail... $exception"
 
 is_deeply(\@warnings, [], 'No warnings with unicode on STDERR');
 
-
 # test debugcb and debugobj protocol
 {
   my $rs = $schema->resultset('CD')->search( {
@@ -136,4 +136,40 @@ is_deeply(\@warnings, [], 'No warnings with unicode on STDERR');
   is_deeply ( $do->{_traced_bind}, \@bind_trace );
 }
 
+# recreate test as seen in DBIx::Class::QueryLog
+# the rationale is that if someone uses a non-IO::Handle object
+# on CPAN, many are *bound* to use one on darkpan. Thus this
+# test to ensure there is no future silent breakage
+{
+  my $output = "";
+
+  {
+    package DBICTest::_Printable;
+
+    sub print {
+      my ($self, @args) = @_;
+      $output .= join('', @args);
+    }
+  }
+
+  $schema->storage->debugobj(undef);
+  $schema->storage->debug(1);
+  $schema->storage->debugfh( bless {}, "DBICTest::_Printable" );
+  $schema->storage->txn_do( sub { $schema->resultset('Artist')->count } );
+
+  like (
+    $output,
+    qr/
+      \A
+      ^ \QBEGIN WORK\E \s*?
+      ^ \QSELECT COUNT( * ) FROM artist me:\E \s*?
+      ^ \QCOMMIT\E \s*?
+      \z
+    /xm
+  );
+
+  $schema->storage->debug(0);
+  $schema->storage->debugfh(undef);
+}
+
 done_testing;