error callbacks, more structure for parse/format methods

10 files changed, 181 insertions, 28 deletions

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..9788afa
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,6 @@
+blib/
+*.sw?
+Makefile
+Makefile.old
+MYMETA.yml
+pm_to_blib
diff --git a/BatchPayment/Batch.pm b/BatchPayment/Batch.pm
index eb6716e..d31baae 100644
--- a/BatchPayment/Batch.pm
+++ b/BatchPayment/Batch.pm
@@ -12,6 +12,9 @@ to a bank (or returned from a bank) as a batch.
 
 =over 4
 
+=item incoming - Flag for one-way batches.  The processor must set
+this if the batch was originated by the gateway.
+
 =item batch_id - Batch identifier.  The format is processor-specific
 but usually must be a positive integer, if it's used at all.
 
@@ -30,8 +33,14 @@ included in the batch.
 
 package Business::BatchPayment::Batch;
 
+use strict;
 use Moose;
 
+has incoming => (
+  is => 'rw',
+  isa => 'Bool',
+);
+
 has batch_id => (
   is => 'rw',
   isa => 'Str',
diff --git a/BatchPayment/Debug.pm b/BatchPayment/Debug.pm
index 71a9e6d..1d8c332 100644
--- a/BatchPayment/Debug.pm
+++ b/BatchPayment/Debug.pm
@@ -8,6 +8,7 @@ globally.
 
 =cut
 
+use strict;
 use Moose::Role;
 
 has 'debug' => (
diff --git a/BatchPayment/Item.pm b/BatchPayment/Item.pm
index 6e1e8e5..c929cf3 100644
--- a/BatchPayment/Item.pm
+++ b/BatchPayment/Item.pm
@@ -1,5 +1,6 @@
 package Business::BatchPayment::Item;
 
+use strict;
 use Moose;
 use Moose::Util::TypeConstraints;
 use MooseX::UndefTolerant;
@@ -68,10 +69,10 @@ has amount => (
 
 =item tid
 
-transaction identifier.  Requests must provide this.  It's 
-a token of some kind to be passed to the gateway and used to identify the 
-reply.  For now it's required to be an integer.  An invoice number would
-be a good choice.
+transaction identifier.  Requests must provide this.  It's a token of 
+some kind to be passed to the gateway and used to identify the reply.  
+For now it's required to be an integer.  An invoice number would be 
+a good choice.
 
 =cut
 
@@ -87,8 +88,7 @@ has tid => ( is  => 'rw', isa => 'Int' );
 
 =item customer_id
 
-A customer number or other identifier, for the merchant's
-use.
+A customer number or other identifier, for the merchant's use.
 
 =item first_name
 
@@ -160,8 +160,8 @@ Bank's routing code.
 
 =item account_type
 
-Can be 'personal checking', 'personal savings'
-'business checking', or 'business savings'.
+Can be 'personal checking', 'personal savings', 'business checking', 
+or 'business savings'.
 
 =cut
 
@@ -249,9 +249,8 @@ account again.
 
 =item error_message
 
-The message returned by the gateway.  This may 
-contain a value even if the payment was successful (use C<approved> 
-to determine that.)
+The message returned by the gateway.  This may contain a value even 
+if the payment was successful (use C<approved> to determine that.)
 
 =back
 
diff --git a/BatchPayment/Processor.pm b/BatchPayment/Processor.pm
index 26f5ba0..2093132 100644
--- a/BatchPayment/Processor.pm
+++ b/BatchPayment/Processor.pm
@@ -41,22 +41,38 @@ This statement would be processed as a one-way batch.
 
 =head1 ATTRIBUTES
 
-Most attributes for B::BP::Processor objects are defined by the module.
+Most attributes for Processor objects are defined by the module.
 
 =over 4
 
-=item transport - See L<Business::BatchPayment::Transport>.  This must 
-be set before calling submit() or receive().  Some modules will set it 
-themselves; others require a transport to be supplied.  Check for the 
-existence of a 'default_transport' method.
+=item transport
 
-=item debug - Debug level.  This may be interpreted in various ways by
-the module.
+See L<Business::BatchPayment::Transport>.  This must be set before calling
+submit() or receive().  Some modules will set it themselves; others require a
+transport to be supplied.  Check for the existence of a 'default_transport'
+method.
 
-=item test_mode - Communicate with a test server instead of the production
-gateway.  Not all processors support this.
-C<$processor->does('Business::BatchPayment::TestMode')> should tell whether
-it's supported.
+=item debug
+
+Debug level.  This may be interpreted in various ways by the module.
+
+=item test_mode
+
+Communicate with a test server instead of the production gateway.  Not all
+processors support this.  Test for the L<Business::BatchPayment::TestMode>
+role to determine if it's supported.
+
+=item on_format_error
+
+Callback to handle errors when formatting items.  Arguments are the Processor
+object, the Item object, and the error thrown by C<format_item>.  The callback
+can die to stop submitting the batch.
+
+=item on_parse_error
+
+Callback to handle errors when parsing items.  Arguments are the Processor
+object, the Item object, and the error thrown by C<parse_item>.  The callback
+can die to stop receiving the batch.
 
 =back
 
@@ -68,7 +84,7 @@ it's supported.
 
 =item output FILE
 
-If either of these is passed when constructing a Processor object, the 
+If either of these is passed when constructing a Processor object, the
 transport will be replaced with a File transport with those parameters.
 Specifying only 'input' will direct 'output' to /dev/null, and vice versa.
 
@@ -90,17 +106,57 @@ gateway, parse them, and return a list of L<Business::BatchPayment::Batch>
 objects.  The items in these batches will have, at minimum, the 'approved' 
 field and either the 'tid' or 'amount' field set.
 
+=item format_request BATCH
+
+Default method to serialize BATCH for submission.  Returns the formatted
+text as a string.  By default, this calls C<format_header>, then 
+C<format_item> on each Item in the batch, then C<format_trailer>, 
+joining the output with no delimiters.  Override this if your processor
+needs something different.
+
+=item format_header BATCH
+
+=item format_trailer BATCH
+
+Optional methods to produce the header and trailer sections of the 
+formatted batch.  By default these are empty strings.
+
+=item format_item ITEM, BATCH
+
+Required method (if using the default C<format_request>) to produce
+the per-item part of the formatted batch.  By default this throws 
+a fatal error.
+
+=item parse_response DATA
+
+Default method to deserialize a received batch.  Takes the string received
+from the gateway, returns a L<Business::BatchPayment::Batch>.  By default,
+calls C<parse_batch_id> on the entire batch, then splits DATA into lines 
+and calls C<parse_item> on each line.
+
+=item parse_batch_id DATA
+
+Optional method to obtain the batch identifier from the received file.
+By default this returns nothing.
+
+=item parse_item LINE
+
+Required method to parse a line from the received file.  Should 
+return zero or more L<Business::BatchPayment::Item>s.
+
+=cut
+
 =back
 
 =cut
 
 package Business::BatchPayment::Processor;
 
+use strict;
+use Try::Tiny;
 use Moose::Role;
 with 'Business::BatchPayment::Debug';
 
-requires 'format_request', 'parse_response';
-
 has 'transport' => (
   is      => 'rw',
   does    => 'Business::BatchPayment::Transport',
@@ -109,6 +165,11 @@ has 'transport' => (
   builder => 'default_transport',
 );
 
+sub default_transport {
+  my $self = shift;
+  die blessed($self). " requires a transport or input/output files\n";
+}
+
 around BUILDARGS => sub {
   my ($orig, $class, %args) = @_;
   %args = %{ $class->$orig(%args) }; #process as usual
@@ -122,6 +183,11 @@ around BUILDARGS => sub {
   \%args;
 };
 
+# override this if your processor produces one-way batches
+sub incoming { 0 };
+
+#top-level interface
+
 sub submit {
   my $self = shift;
   my $batch = shift;
@@ -138,4 +204,71 @@ sub receive {
   map { $self->parse_response($_) } @responses;
 }
 
+# next level down
+
+sub format_request {
+  my $self = shift;
+  my $batch = shift;
+  my $output = $self->format_header($batch);
+  foreach my $item ($batch->elements) {
+    try {
+      $output .= $self->format_item($item, $batch);
+    } catch {
+      $self->format_error($self, $item, $_);
+    }
+  }
+  $output .= $self->format_trailer($batch);
+  return $output;
+}
+
+sub parse_response {
+  my $self = shift;
+  my $input = shift;
+  my $batch = Business::BatchPayment->create(Batch =>
+    incoming => $self->incoming,
+    batch_id => $self->parse_batch_id($input)
+  );
+  while ( $input =~ s/(.*)\n//m ) {
+    my $row = $1;
+    try {
+      $batch->push( $self->parse_item($row) );
+    } catch {
+      $self->parse_error($row, $_);
+    };
+  }
+  $batch;
+}
+
+# nuts and bolts
+
+sub format_header  { '' };
+sub format_trailer { '' };
+sub format_item { die "format_item unimplemented\n" }
+
+sub parse_batch_id { '' };
+sub parse_item { die "parse_item unimplemented\n" }
+
+sub default_on_error { #re-throw it
+  my ($self, $item, $error) = @_;
+  die $error;
+};
+
+has 'on_format_error' => (
+  traits  => ['Code'],
+  is      => 'rw',
+  handles => { format_error => 'execute_method' },
+  default => sub { \&default_on_error },
+);
+
+has 'on_parse_error' => (
+  traits  => ['Code'],
+  is      => 'rw',
+  handles => { parse_error => 'execute_method' },
+  default => sub { \&default_on_error },
+);
+
+# No error callbacks for other parts of this.  The per-item case 
+# is special in that it might make sense to continue with the 
+# other items.
+
 1;
diff --git a/BatchPayment/TestMode.pm b/BatchPayment/TestMode.pm
index 551a9aa..54b4d13 100644
--- a/BatchPayment/TestMode.pm
+++ b/BatchPayment/TestMode.pm
@@ -7,6 +7,7 @@ requests will not be submitted to a live account if test_mode is true.
 
 =cut
 
+use strict;
 use Moose::Role;
 
 has 'test_mode' => (
diff --git a/BatchPayment/Transport.pm b/BatchPayment/Transport.pm
index 4ac8b71..5cc3c85 100644
--- a/BatchPayment/Transport.pm
+++ b/BatchPayment/Transport.pm
@@ -1,5 +1,6 @@
 package Business::BatchPayment::Transport;
 
+use strict;
 use Moose::Role;
 with 'Business::BatchPayment::Debug';
 
diff --git a/BatchPayment/Transport/File.pm b/BatchPayment/Transport/File.pm
index eaaba49..27bc241 100644
--- a/BatchPayment/Transport/File.pm
+++ b/BatchPayment/Transport/File.pm
@@ -8,8 +8,9 @@ to /dev/null.
 
 =cut
 
-use IO::File;
+use strict;
 use Moose;
+use IO::File;
 with 'Business::BatchPayment::Transport';
 
 has 'input' => (
diff --git a/BatchPayment/Transport/HTTPS.pm b/BatchPayment/Transport/HTTPS.pm
index c40a78f..fdb2c35 100644
--- a/BatchPayment/Transport/HTTPS.pm
+++ b/BatchPayment/Transport/HTTPS.pm
@@ -8,8 +8,9 @@ Options are 'server', 'port', 'get_path', 'put_path', optionally
 
 =cut
 
-use Net::HTTPS::Any 0.10;
+use strict;
 use Moose;
+use Net::HTTPS::Any 0.10;
 with 'Business::BatchPayment::Transport';
 
 has [ qw( host port get_path put_path ) ] => ( 
diff --git a/BatchPayment/Transport/SFTP.pm b/BatchPayment/Transport/SFTP.pm
index 08d2fb7..1f86ee3 100644
--- a/BatchPayment/Transport/SFTP.pm
+++ b/BatchPayment/Transport/SFTP.pm
@@ -13,9 +13,10 @@ may find it useful to modify or override that behavior.
 
 =cut
 
+use strict;
+use Moose;
 use Net::SFTP::Foreign;
 use File::Slurp qw(read_file);
-use Moose;
 with 'Business::BatchPayment::Transport';
 
 has [ qw( host login password ) ] => (