Attachment 10240 Details for Bug 7613 – Followup patch.

[patch] Followup patch.

0001-Bug-7613-OCLC-Connexion-web-service-and-desktop-clie.patch (text/plain), 10.93 KB, created by D Ruth Holloway on 2012-06-11 19:40:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: D Ruth Holloway

Created: 2012-06-11 19:40:36 UTC

Size: 10.93 KB

patch

obsolete

>From 731093c9964bb92313b81e1acdd28990734a3f81 Mon Sep 17 00:00:00 2001
>From: D Ruth Bavousett <ruth@bywatersolutions.com>
>Date: Mon, 11 Jun 2012 12:13:01 -0700
>Subject: [PATCH] Bug 7613: OCLC Connexion web service and desktop client, followup patch
>
>Prior patches to this bug had lots of comments like "I don't have a way to test this, so..."
>
>In the OCLC Connexion web, when you choose the option to export to MARC, it'll *send* it, and
>say, "Record Exported," but the web client does nothing whatever to confirm that the record
>actually landed in Koha.  That's a flaw in their software, but can be easily checked by
>looking in Koha to see if an import batch got created.  The desktop client is a little
>smarter about this, but needed much more testing, also.
>
>With this patch, both the client and web will actually work.  With a config file and set up as
>previously described, The record will be staged and/or imported, and the desktop client returns
>a useful message about what happened, *and* the staff client URL to the record.
>
>Oodles of gobs of bunches of thanks to Virginia Military Institute, for loaning me their OCLC
>authorization credentials so this could be tested, as well as for great suggestions of cosmetic
>improvements to the mechanism and output.
>---
> misc/bin/connexion_import_daemon.pl |   83 +++++++++++++++++++++++++++--------
> svc/import_bib                      |   32 ++++++++-----
> 2 files changed, 85 insertions(+), 30 deletions(-)
>
>diff --git a/misc/bin/connexion_import_daemon.pl b/misc/bin/connexion_import_daemon.pl
>index 9d32f8a..7e26d8e 100755
>--- a/misc/bin/connexion_import_daemon.pl
>+++ b/misc/bin/connexion_import_daemon.pl
>@@ -87,12 +87,16 @@ use IO::Socket::INET;
> use IO::Select;
> use POSIX;
> use HTTP::Status qw(:constants);
>+use strict;
>+use warnings;
> 
> use LWP::UserAgent;
> use XML::Simple;
>+use MARC::Record;
>+use MARC::File::XML;
> 
> use constant CLIENT_READ_TIMEOUT     => 5;
>-use constant CLIENT_READ_BUFFER_SIZE => 4 * 1024;
>+use constant CLIENT_READ_BUFFER_SIZE => 100000;
> use constant AUTH_URI       => "/cgi-bin/koha/mainpage.pl";
> use constant IMPORT_SVC_URI => "/cgi-bin/koha/svc/import_bib";
> 
>@@ -222,7 +226,7 @@ sub _ua {
> sub read_request {
>     my ( $self, $io ) = @_;
> 
>-    my ($in, @in, $timeout);
>+    my ($in, @in_arr, $timeout, $bad_marc);
>     my $select = IO::Select->new($io) ;
>     while ( "FOREVER" ) {
>         if ( $select->can_read(CLIENT_READ_TIMEOUT) ){
>@@ -231,22 +235,27 @@ sub read_request {
> 
>             # XXX ignore after NULL
>             if ( $in =~ m/^(.*)\000/so ) { # null received, EOT
>-                push @in, $1;
>+                push @in_arr, $1;
>                 last;
>             }
>-            push @in, $in;
>+            push @in_arr, $in;
>         }
>         else {
>-            $timeout = 1;
>             last;
>         }
>     }
> 
>-    $in = join '', @in;
>+    $in = join '', @in_arr;
> 
>+    $in =~ m/(.)$/;
>+    my $lastchar = $1;
>     my ($xml, $user, $password, $local_user);
>     my $data = $in; # copy for diagmostic purposes
>-    while ( my $first = substr( $data, 0, 1 ) ) {
>+    while () {
>+        my $first = substr( $data, 0, 1 );
>+        if (!defined $first) {
>+           last;
>+        }
>         $first eq 'U' && do {
>             ($user, $data) = _trim_identifier($data);
>             next;
>@@ -256,26 +265,36 @@ sub read_request {
>             next;
>         };
>         $first eq 'P' && do {
>-            ($password,, $data) = _trim_identifier($data);
>+            ($password, $data) = _trim_identifier($data);
>             next;
>         };
>         $first eq ' ' && do {
>             $data = substr( $data, 1 ); # trim
>             next;
>         };
>-        $first eq '<' && do {
>-            $xml = $data;
>+        $data =~ m/^[0-9]/ && do {
>+            # What we have here might be a MARC record...
>+            my $marc_record;
>+            eval { $marc_record = MARC::Record->new_from_usmarc($data); };
>+            if ($@) {
>+                $bad_marc = 1;
>+            } 
>+            else {
>+               $xml = $marc_record->as_xml();
>+            }
>             last;
>         };
>-
>         last; # unexpected input
>     }
> 
>     my @details;
>     push @details, "Timeout" if $timeout;
>+    push @details, "Bad MARC" if $bad_marc;
>     push @details, "User: $user" if $user;
>     push @details, "Password: " . ( $self->{debug} ? $password : ("x" x length($password)) ) if $password;
>     push @details, "Local user: $local_user" if $local_user;
>+    push @details, "XML: $xml" if $xml;
>+    push @details, "Remaining data: $data" if ($data && !$xml);
>     unless ($xml) {
>         $self->log("Invalid request", $in, @details);
>         return;
>@@ -287,9 +306,14 @@ sub read_request {
> }
> 
> sub _trim_identifier {
>-    my ($a, $len) = unpack "cc", substr( $_[0], 0, 2 );
>-
>-    return ( substr( $_[0], 2, $len ), substr( $_[0], 2 + $len ) );
>+    #my ($a, $len) = unpack "cc", substr( $_[0], 0, 2 );
>+    my $len=ord(substr ($_[0], 1, 1)) - 64;
>+    if ($len <0) {  #length is numeric, and thus comes from the web client, not the desktop client.
>+       $_[0] =~ m/.(\d+)/;
>+       $len = $1;
>+       return ( substr( $_[0], length($len)+1 , $len ), substr( $_[0], length($len) + 1 + $len ) );
>+    }
>+    return ( substr( $_[0], 2 , $len ), substr( $_[0], 2 + $len ) );
> }
> 
> sub handle_request {
>@@ -309,13 +333,28 @@ sub handle_request {
>     }
> 
>     my $base_url = $self->{koha};
>-    my $resp = $ua->post( $base_url.IMPORT_SVC_URI, $self->{params}, 'Content-Type' => 'text/plain', Content => $data );
>+    my $resp = $ua->post( $base_url.IMPORT_SVC_URI,  
>+                              {'nomatch_action' => $self->{params}->{nomatch_action},
>+                               'overlay_action' => $self->{params}->{overlay_action}, 
>+                               'match'          => $self->{params}->{match},
>+                               'import_mode'    => $self->{params}->{import_mode},
>+                               'framework'      => $self->{params}->{framework},
>+                               'item_action'    => $self->{params}->{item_action},
>+                               'xml'            => $data});
>+
>     my $status = $resp->code;
>     if ($status == HTTP_UNAUTHORIZED || $status == HTTP_FORBIDDEN) {
>         my $user = $self->{user};
>         my $password = $self->{password};
>         $resp = $ua->post( $base_url.AUTH_URI, { userid => $user, password => $password } );
>-        $resp = $ua->post( $base_url.IMPORT_SVC_URI, $self->{params}, 'Content-Type' => 'text/plain', Content => $data )
>+        $resp = $ua->post( $base_url.IMPORT_SVC_URI,
>+                              {'nomatch_action' => $self->{params}->{nomatch_action},
>+                               'overlay_action' => $self->{params}->{overlay_action}, 
>+                               'match'          => $self->{params}->{match},
>+                               'import_mode'    => $self->{params}->{import_mode},
>+                               'framework'      => $self->{params}->{framework},
>+                               'item_action'    => $self->{params}->{item_action},
>+                               'xml'            => $data})
>           if $resp->is_success;
>     }
>     unless ($resp->is_success) {
>@@ -323,11 +362,14 @@ sub handle_request {
>         return $self->error_response("Unsuccessful request");
>     }
> 
>-    my ($koha_status, $bib, $batch_id, $error);
>+    my ($koha_status, $bib, $overlay, $batch_id, $error, $url);
>     if ( my $r = eval { XMLin($resp->content) } ) {
>         $koha_status = $r->{status};
>         $batch_id    = $r->{import_batch_id};
>         $error       = $r->{error};
>+        $bib         = $r->{biblionumber};
>+        $overlay     = $r->{match_status};
>+        $url         = $r->{url};
>     }
>     else {
>         $koha_status = "error";
>@@ -336,7 +378,12 @@ sub handle_request {
>     }
> 
>     if ($koha_status eq "ok") {
>-        return $self->response( sprintf( "Success. Import batch id: %s", $batch_id ) );
>+        my $response_string = sprintf( "Success.  Batch number %s - biblio record number %s", 
>+                                        $batch_id,$bib);
>+        $response_string .= $overlay eq 'no_match' ? ' added to Koha.' : ' overlaid by import.';
>+        $response_string .= "\n\n$url";
>+        
>+        return $self->response( $response_string );
>     }
> 
>     return $self->error_response( sprintf( "%s.  Please contact administrator.", $error ) );
>diff --git a/svc/import_bib b/svc/import_bib
>index 5e7e493..5b95017 100755
>--- a/svc/import_bib
>+++ b/svc/import_bib
>@@ -41,10 +41,10 @@ unless ($status eq "ok") {
> 
> my $xml;
> if ($query->request_method eq "POST") {
>-    $xml = $query->param('POSTDATA');
>+    $xml = $query->param('xml');
> }
> if ($xml) {
>-    my %params = map { $_ => $query->url_param($_) } $query->url_param;
>+    my %params = map { $_ => $query->param($_) } $query->param;
>     my $result = import_bib($xml, \%params );
>     print $query->header(-type => 'text/xml');
>     print XMLout($result, NoAttr => 1, RootName => 'response', XMLDecl => 1);
>@@ -62,7 +62,7 @@ sub import_bib {
>     my $import_mode = delete $params->{import_mode} || '';
>     my $framework   = delete $params->{framework}   || '';
> 
>-    if (my $matcher_code = delete $params->{matcher}) {
>+    if (my $matcher_code = delete $params->{match}) {
>         $params->{matcher_id} = C4::Matcher::GetMatcherId($matcher_code);
>     }
> 
>@@ -83,19 +83,27 @@ sub import_bib {
> 
>     my $import_record_id = AddBiblioToBatch($batch_id, 0, $marc_record, "utf8", int(rand(99999)));
>     my @import_items_ids = AddItemsToImportBiblio($batch_id, $import_record_id, $marc_record, 'UPDATE COUNTS');
>-    my $marcxml = GetImportRecordMarcXML($import_record_id);
>-    unless ($marcxml) {
>-        $result->{'status'} = "failed";
>-        $result->{'error'} = "database write error";
>-        return $result;
>-    }
>-    $marcxml =~ s/<\?xml.*?\?>//i;
>+    
>+    my $matcher = C4::Matcher->new($params->{record_type} || 'biblio');
>+    $matcher = C4::Matcher->fetch($params->{matcher_id});
>+    my $number_of_matches =  BatchFindBibDuplicates($batch_id, $matcher);
> 
>     # XXX we are ignoring the result of this;
>     BatchCommitBibRecords($batch_id, $framework) if lc($import_mode) eq 'direct';
> 
>+    my $dbh = C4::Context->dbh();
>+    my $sth = $dbh->prepare("SELECT matched_biblionumber FROM import_biblios WHERE import_record_id =?");
>+    $sth->execute($import_record_id);
>+    my $biblionumber=$sth->fetchrow_arrayref->[0] || '';
>+    $sth = $dbh->prepare("SELECT overlay_status FROM import_records WHERE import_record_id =?");
>+    $sth->execute($import_record_id);
>+    my $match_status = $sth->fetchrow_arrayref->[0] || 'no_match';
>+    my $url = 'http://'. C4::Context->preference('staffClientBaseURL') .'/cgi-bin/koha/catalogue/detail.pl?biblionumber='. $biblionumber;
>+
>     $result->{'status'} = "ok";
>-    $result->{'import_batch_id'} =  $batch_id;
>-    $result->{'marcxml'} =  $marcxml;
>+    $result->{'import_batch_id'} = $batch_id;
>+    $result->{'match_status'} = $match_status;
>+    $result->{'biblionumber'} = $biblionumber;
>+    $result->{'url'} = $url;
>     return $result;
> }
>-- 
>1.7.2.5
>

Actions: View | Diff | Splinter Review

Attachments on bug 7613: 8194 | 8256 | 8268 | 8269 | 8270 | 8328 | 8523 | 8527 | 8550 | 8678 | 8691 | 8896 | 10240