From 96f0c1f28e64f14e2e4b402795e0512f48d5fe74 Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Wed, 23 Jan 2013 17:43:35 -0500 Subject: [PATCH] Acq: style and whitespace cleanups in O::A::Acq::EDI.pm All I could stand before I just couldn't take it anymore. Signed-off-by: Lebbeous Fogle-Weekley --- .../perlmods/lib/OpenILS/Application/Acq/EDI.pm | 154 +++++++++++++-------- 1 file changed, 99 insertions(+), 55 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm index 854c80e905..f26e98c7ac 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm @@ -48,19 +48,6 @@ my %map = ( ); -## Just for debugging stuff: -sub add_a_msg { - my ($self, $conn) = @_; - my $e = new_editor(xact=>1); - my $incoming = Fieldmapper::acq::edi_message->new; - $incoming->edi("This is content"); - $incoming->account(1); - $incoming->remote_file('in/some_file.edi'); - $e->create_acq_edi_message($incoming);; - $e->commit; -} -# __PACKAGE__->register_method( method => 'add_a_msg', api_name => 'open-ils.acq.edi.add_a_msg'); # debugging - __PACKAGE__->register_method( method => 'retrieve', api_name => 'open-ils.acq.edi.retrieve', @@ -93,27 +80,34 @@ sub retrieve_core { foreach my $account (@$set) { my $count = 0; my $server; - $logger->info("EDI check for vendor " . ++$vcount . " of " . scalar(@$set) . ": " . $account->host); - unless ($server = __PACKAGE__->remote_account($account)) { # assignment, not comparison - $logger->err(sprintf "Failed remote account mapping for %s (%s)", $account->host, $account->id); + $logger->info( + "EDI check for vendor " . + ++$vcount . " of " . scalar(@$set) . ": " . $account->host + ); + unless ($server = __PACKAGE__->remote_account($account)) { # assignment + $logger->err( + sprintf "Failed remote account mapping for %s (%s)", + $account->host, $account->id + ); next; }; -# my $rf_starter = './'; # default to current dir + if ($account->in_dir) { if ($account->in_dir =~ /\*+.*\//) { - $logger->err("EDI in_dir has a slash after an asterisk in value: '" . $account->in_dir . "'. Skipping account with indeterminate target dir!"); + $logger->err( + "EDI in_dir has a slash after an asterisk in value: '" . + $account->in_dir . + "'. Skipping account with indeterminate target dir!" + ); next; } -# $rf_starter = $account->in_dir; -# $rf_starter =~ s/((\/)?[^\/]*)\*+[^\/]*$//; # kill up to the first (possible) slash before the asterisk: keep the preceeding static dir -# $rf_starter .= '/' if $rf_starter or $2; # recap the dir, or replace leading "/" if there was one (but don't add if empty) } + my @files = ($server->ls({remote_file => ($account->in_dir || './')})); my @ok_files = grep {$_ !~ /\/\.?\.$/ and $_ ne '0'} @files; $logger->info(sprintf "%s of %s files at %s/%s", scalar(@ok_files), scalar(@files), $account->host, $account->in_dir); - # $server->remote_path(undef); + foreach my $remote_file (@ok_files) { - # my $remote_file = $rf_starter . $_; my $description = sprintf "%s/%s", $account->host, $remote_file; # deduplicate vs. acct/filenames already in DB. @@ -136,37 +130,49 @@ sub retrieve_core { }, { join => {"acqedi" => {}} } ]); - if (scalar(@$hits)) { + + if (@$hits) { $logger->debug("EDI: $remote_file already retrieved. Skipping"); warn "EDI: $remote_file already retrieved. Skipping"; next; } ++$count; - $max and $count > $max and last; - $logger->info(sprintf "%s of %s targets: %s", $count, scalar(@ok_files), $description); - print sprintf "%s of %s targets: %s\n", $count, scalar(@ok_files), $description; + if ($max and $count > $max) { + last; + } + + $logger->info( + sprintf "%s of %s targets: %s", + $count, scalar(@ok_files), $description + ); + printf("%d of %d targets: %s\n", $count, scalar(@ok_files), $description); if ($test) { push @return, "test_$count"; next; } my $content; my $io = IO::Scalar->new(\$content); - unless ( $server->get({remote_file => $remote_file, local_file => $io}) ) { + + unless ( + $server->get({remote_file => $remote_file, local_file => $io}) + ) { $logger->error("(S)FTP get($description) failed"); next; } - my $incoming = __PACKAGE__->process_retrieval($content, $remote_file, $server, $account->id); -# $server->delete(remote_file => $_); # delete remote copies of saved message + + my $incoming = __PACKAGE__->process_retrieval( + $content, $remote_file, $server, $account->id + ); + push @return, @$incoming; } } return \@return; } -# my $msg_ids = OpenILS::Application::Acq::EDI->process_retrieval( -# $file_content, $remote_filename, $server, $account_id, $editor); +# procses_retrieval() returns a reference to a list of acq.edi_message IDs sub process_retrieval { my ($class, $content, $filename, $server, $account_or_id) = @_; $content or return; @@ -234,63 +240,95 @@ sub process_retrieval { # ->send_core # $account is a Fieldmapper object for acq.edi_account row -# $messageset is an arrayref with acq.edi_message.id values +# $message_ids is an arrayref with acq.edi_message.id values # $e is optional editor object sub send_core { my ($class, $account, $message_ids, $e) = @_; # $e is a working editor - ($account and scalar @$message_ids) or return; + return unless $account and @$message_ids; $e ||= new_editor(); $e->xact_begin; my @messageset = map {$e->retrieve_acq_edi_message($_)} @$message_ids; $e->xact_rollback; my $m_count = scalar(@messageset); - (scalar(@$message_ids) == $m_count) or + if (@$message_ids != $m_count) { $logger->warn(scalar(@$message_ids) - $m_count . " bad IDs passed to send_core (ignored)"); + } my $log_str = sprintf "EDI send to edi_account %s (%s)", $account->id, $account->host; $logger->info("$log_str: $m_count message(s)"); - $m_count or return; + return unless $m_count; my $server; my $server_error; - unless ($server = __PACKAGE__->remote_account($account, 1)) { # assignment, not comparison + unless ($server = __PACKAGE__->remote_account($account, 1)) { # assignment $logger->error("Failed remote account connection for $log_str"); $server_error = 1; - }; + } + foreach (@messageset) { $_ or next; # we already warned about bum ids my ($res, $error); if ($server_error) { - $error = "Server error: Failed remote account connection for $log_str"; # already told $logger, this is to update object below + # We already told $logger; this is to update object below + $error = "Server error: Failed remote account connection ". + "for $log_str"; } elsif (! $_->edi) { - $logger->error("Message (id " . $_->id. ") for $log_str has no EDI content"); + $logger->error( + "Message (id " . $_->id. ") for $log_str has no EDI content" + ); $error = "EDI empty!"; - } elsif ($res = $server->put({remote_path => $account->path, content => $_->edi, single_ext => 1})) { + } elsif ( + $res = $server->put({ + remote_path => $account->path, content => $_->edi, + single_ext => 1 + }) + ) { # This is the successful case! $_->remote_file($res); $_->status('complete'); - $_->process_time('NOW'); # For outbound files, sending is the end of processing on the EG side. + $_->process_time('NOW'); + + # For outbound files, sending is the end of processing on + # the EG side. + $logger->info("Sent message (id " . $_->id. ") via $log_str"); } else { - $logger->error("(S)FTP put to $log_str FAILED: " . ($server->error || 'UNKOWNN')); + $logger->error( + "(S)FTP put to $log_str FAILED: " . + ($server->error || 'UNKOWNN') + ); $error = "put FAILED: " . ($server->error || 'UNKOWNN'); } + if ($error) { $_->error($error); $_->error_time('NOW'); } + $logger->info("Calling update_acq_edi_message"); $e->xact_begin; + unless ($e->update_acq_edi_message($_)) { - $logger->error("EDI send_core update_acq_edi_message failed for message object: " . Dumper($_)); - OpenILS::Application::Acq::EDI::Translator->debug_file(Dumper($_ ), '/tmp/update_acq_edi_message.FAIL'); - OpenILS::Application::Acq::EDI::Translator->debug_file(Dumper($_->to_bare_hash), '/tmp/update_acq_edi_message.FAIL.to_bare_hash'); + $logger->error( + "EDI send_core update_acq_edi_message failed " . + "for message object: " . Dumper($_) + ); + + OpenILS::Application::Acq::EDI::Translator->debug_file( + Dumper($_), + '/tmp/update_acq_edi_message.FAIL' + ); + OpenILS::Application::Acq::EDI::Translator->debug_file( + Dumper($_->to_bare_hash), + '/tmp/update_acq_edi_message.FAIL.to_bare_hash' + ); } + # There's always an update, even if we failed. $e->xact_commit; - __PACKAGE__->record_activity($account, $e); # There's always an update, even if we failed. + __PACKAGE__->record_activity($account, $e); } return \@messageset; } @@ -298,17 +336,23 @@ sub send_core { # attempt_translation does not touch the DB, just the object. sub attempt_translation { my ($class, $edi_message, $to_edi) = @_; - my $tran = translator(); - my $ret = $to_edi ? $tran->json2edi($edi_message->jedi) : $tran->edi2json($edi_message->edi); -# $logger->error("json: " . Dumper($json)); # debugging - if (not $ret or (! ref($ret)) or $ret->is_fault) { # RPC::XML::fault on failure + my $ret = $to_edi ? translator->json2edi($edi_message->jedi) : + translator->edi2json($edi_message->edi); + + if (not $ret or (! ref($ret)) or $ret->is_fault) { + # RPC::XML::fault on failure + $edi_message->status('trans_error'); $edi_message->error_time('NOW'); - my $pre = "EDI Translator " . ($to_edi ? 'json2edi' : 'edi2json') . " failed"; + my $pre = "EDI Translator " . + ($to_edi ? 'json2edi' : 'edi2json') . " failed"; + my $message = ref($ret) ? - ("$pre, Error " . $ret->code . ": " . __PACKAGE__->nice_string($ret->string)) : - ("$pre: " . __PACKAGE__->nice_string($ret) ) ; + ("$pre, Error " . $ret->code . ": " . + __PACKAGE__->nice_string($ret->string)) : + ("$pre: " . __PACKAGE__->nice_string($ret)) ; + $edi_message->error($message); $logger->error($message); return; @@ -322,6 +366,7 @@ sub attempt_translation { } else { $edi_message->jedi($ret->value); # translator returns an object } + return $edi_message; } @@ -341,7 +386,6 @@ sub retrieve_vendors { } } ]); -# {"id":{"!=":null},"+acqpro":{"active":"t"}}, {"join":"acqpro", "flesh_fields":{"acqedi":["provider"]},"flesh":1} } # This is the SRF-exposed call, so it does checkauth -- 2.11.0