Bugzilla – Attachment 13807 Details for
Bug 9183
C4::Search should only have one ZOOM event loop
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9183: Refactor ZOOM event loop
Bug-9183-Refactor-ZOOM-event-loop.patch (text/plain), 22.45 KB, created by
Jared Camins-Esakov
on 2012-12-01 04:09:02 UTC
(
hide
)
Description:
Bug 9183: Refactor ZOOM event loop
Filename:
MIME Type:
Creator:
Jared Camins-Esakov
Created:
2012-12-01 04:09:02 UTC
Size:
22.45 KB
patch
obsolete
>From 7de23e6abdb6f5cc210256b5842beb2ea4e9d555 Mon Sep 17 00:00:00 2001 >From: Jared Camins-Esakov <jcamins@cpbibliography.com> >Date: Fri, 30 Nov 2012 22:45:48 -0500 >Subject: [PATCH] Bug 9183: Refactor ZOOM event loop > >Prior to this patch, there were three identical ZOOM event loops in >C4::Search. This is wasteful, and goes against all good programming >practice. This patch refactors the ZOOM event loops into a separate >subroutine which is called by SimpleSearch, searchResults, and >GetDistinctValues call. > >The new routine, _ZOOM_event_loop process the ZOOM event loop and, >once it has been fully processed, passes control to a closure provided >by the calling routine for processing the results, and destroys the >result sets. > >To test (after applying patch): >1) Do a regular bibliographic search that should return results. >2) Do a search in the Cataloging module that should return results. >3) If you get results from both searches, the patch works. >--- > C4/Search.pm | 363 +++++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 219 insertions(+), 144 deletions(-) > >diff --git a/C4/Search.pm b/C4/Search.pm >index 1e850d9..50f7d2b 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -254,28 +254,29 @@ sub SimpleSearch { > return ( $error, undef, undef ); > } > } >- while ( ( my $i = ZOOM::event( \@zconns ) ) != 0 ) { >- my $event = $zconns[ $i - 1 ]->last_event(); >- if ( $event == ZOOM::Event::ZEND ) { > >- my $first_record = defined( $offset ) ? $offset+1 : 1; >+ _ZOOM_event_loop( >+ \@zconns, >+ \@tmpresults, >+ sub { >+ my ($i, $size) = @_; >+ my $first_record = defined($offset) ? $offset + 1 : 1; > my $hits = $tmpresults[ $i - 1 ]->size(); > $total_hits += $hits; > my $last_record = $hits; > if ( defined $max_results && $offset + $max_results < $hits ) { >- $last_record = $offset + $max_results; >+ $last_record = $offset + $max_results; > } > >- for my $j ( $first_record..$last_record ) { >- my $record = $tmpresults[ $i - 1 ]->record( $j-1 )->raw(); # 0 indexed >+ for my $j ( $first_record .. $last_record ) { >+ my $record = >+ $tmpresults[ $i - 1 ]->record( $j - 1 )->raw() >+ ; # 0 indexed > push @{$results}, $record; > } > } >- } >+ ); > >- foreach my $result (@tmpresults) { >- $result->destroy(); >- } > foreach my $zoom_query (@zoom_queries) { > $zoom_query->destroy(); > } >@@ -410,12 +411,11 @@ sub getRecords { > } # finished looping through servers > > # The big moment: asynchronously retrieve results from all servers >- while ( ( my $i = ZOOM::event( \@zconns ) ) != 0 ) { >- my $ev = $zconns[ $i - 1 ]->last_event(); >- if ( $ev == ZOOM::Event::ZEND ) { >- next unless $results[ $i - 1 ]; >- my $size = $results[ $i - 1 ]->size(); >- if ( $size > 0 ) { >+ _ZOOM_event_loop( >+ \@zconns, >+ \@results, >+ sub { >+ my ( $i, $size ) = @_; > my $results_hash; > > # loop through the results >@@ -444,16 +444,26 @@ sub getRecords { > my $tmpauthor; > > # the minimal record in author/title (depending on MARC flavour) >- if (C4::Context->preference("marcflavour") eq "UNIMARC") { >- $tmptitle = MARC::Field->new('200',' ',' ', a => $term, f => $occ); >+ if ( C4::Context->preference("marcflavour") eq >+ "UNIMARC" ) >+ { >+ $tmptitle = MARC::Field->new( >+ '200', ' ', ' ', >+ a => $term, >+ f => $occ >+ ); > $tmprecord->append_fields($tmptitle); >- } else { >- $tmptitle = MARC::Field->new('245',' ',' ', a => $term,); >- $tmpauthor = MARC::Field->new('100',' ',' ', a => $occ,); >+ } >+ else { >+ $tmptitle = >+ MARC::Field->new( '245', ' ', ' ', a => $term, ); >+ $tmpauthor = >+ MARC::Field->new( '100', ' ', ' ', a => $occ, ); > $tmprecord->append_fields($tmptitle); > $tmprecord->append_fields($tmpauthor); > } >- $results_hash->{'RECORDS'}[$j] = $tmprecord->as_usmarc(); >+ $results_hash->{'RECORDS'}[$j] = >+ $tmprecord->as_usmarc(); > } > > # not an index scan >@@ -467,143 +477,179 @@ sub getRecords { > } > $results_hashref->{ $servers[ $i - 1 ] } = $results_hash; > >- # Fill the facets while we're looping, but only for the biblioserver and not for a scan >+# Fill the facets while we're looping, but only for the biblioserver and not for a scan > if ( !$scan && $servers[ $i - 1 ] =~ /biblioserver/ ) { > >- my $jmax = $size>$facets_maxrecs? $facets_maxrecs: $size; >- for my $facet ( @$facets ) { >- for ( my $j = 0 ; $j < $jmax ; $j++ ) { >- my $render_record = $results[ $i - 1 ]->record($j)->render(); >+ my $jmax = >+ $size > $facets_maxrecs ? $facets_maxrecs : $size; >+ for my $facet (@$facets) { >+ for ( my $j = 0 ; $j < $jmax ; $j++ ) { >+ my $render_record = >+ $results[ $i - 1 ]->record($j)->render(); > my @used_datas = (); >- foreach my $tag ( @{$facet->{tags}} ) { >+ foreach my $tag ( @{ $facet->{tags} } ) { >+ > # avoid first line >- my $tag_num = substr($tag, 0, 3); >- my $letters = substr($tag, 3); >- my $field_pattern = '\n' . $tag_num . ' ([^z][^\n]+)'; >- $field_pattern = '\n' . $tag_num . ' ([^\n]+)' if (int($tag_num) < 10); >- my @field_tokens = ( $render_record =~ /$field_pattern/g ) ; >+ my $tag_num = substr( $tag, 0, 3 ); >+ my $letters = substr( $tag, 3 ); >+ my $field_pattern = >+ '\n' . $tag_num . ' ([^z][^\n]+)'; >+ $field_pattern = '\n' . $tag_num . ' ([^\n]+)' >+ if ( int($tag_num) < 10 ); >+ my @field_tokens = >+ ( $render_record =~ /$field_pattern/g ); > foreach my $field_token (@field_tokens) { >- my @subf = ( $field_token =~ /\$([a-zA-Z0-9]) ([^\$]+)/g ); >+ my @subf = ( $field_token =~ >+ /\$([a-zA-Z0-9]) ([^\$]+)/g ); > my @values; >- for (my $i = 0; $i < @subf; $i += 2) { >+ for ( my $i = 0 ; $i < @subf ; $i += 2 ) { > if ( $letters =~ $subf[$i] ) { >- my $value = $subf[$i+1]; >- $value =~ s/^ *//; >- $value =~ s/ *$//; >- push @values, $value; >+ my $value = $subf[ $i + 1 ]; >+ $value =~ s/^ *//; >+ $value =~ s/ *$//; >+ push @values, $value; > } > } >- my $data = join($facet->{sep}, @values); >+ my $data = join( $facet->{sep}, @values ); > unless ( $data ~~ @used_datas ) { >- $facets_counter->{ $facet->{idx} }->{$data}++; >+ $facets_counter->{ $facet->{idx} } >+ ->{$data}++; > push @used_datas, $data; > } >- } # fields >- } # field codes >- } # records >- $facets_info->{ $facet->{idx} }->{label_value} = $facet->{label}; >- $facets_info->{ $facet->{idx} }->{expanded} = $facet->{expanded}; >- } # facets >+ } # fields >+ } # field codes >+ } # records >+ $facets_info->{ $facet->{idx} }->{label_value} = >+ $facet->{label}; >+ $facets_info->{ $facet->{idx} }->{expanded} = >+ $facet->{expanded}; >+ } # facets > } >- } > >- # warn "connection ", $i-1, ": $size hits"; >- # warn $results[$i-1]->record(0)->render() if $size > 0; >+ # warn "connection ", $i-1, ": $size hits"; >+ # warn $results[$i-1]->record(0)->render() if $size > 0; > >- # BUILD FACETS >- if ( $servers[ $i - 1 ] =~ /biblioserver/ ) { >- for my $link_value ( >- sort { $facets_counter->{$b} <=> $facets_counter->{$a} } >- keys %$facets_counter ) >- { >- my $expandable; >- my $number_of_facets; >- my @this_facets_array; >- for my $one_facet ( >- sort { >- $facets_counter->{$link_value}->{$b} >- <=> $facets_counter->{$link_value}->{$a} >- } keys %{ $facets_counter->{$link_value} } >+ # BUILD FACETS >+ if ( $servers[ $i - 1 ] =~ /biblioserver/ ) { >+ for my $link_value ( >+ sort { $facets_counter->{$b} <=> $facets_counter->{$a} } >+ keys %$facets_counter > ) > { >- $number_of_facets++; >- if ( ( $number_of_facets < 6 ) >- || ( $expanded_facet eq $link_value ) >- || ( $facets_info->{$link_value}->{'expanded'} ) ) >+ my $expandable; >+ my $number_of_facets; >+ my @this_facets_array; >+ for my $one_facet ( >+ sort { >+ $facets_counter->{$link_value} >+ ->{$b} <=> $facets_counter->{$link_value} >+ ->{$a} >+ } keys %{ $facets_counter->{$link_value} } >+ ) > { >+ $number_of_facets++; >+ if ( ( $number_of_facets < 6 ) >+ || ( $expanded_facet eq $link_value ) >+ || ( $facets_info->{$link_value}->{'expanded'} ) >+ ) >+ { >+ >+# Sanitize the link value : parenthesis, question and exclamation mark will cause errors with CCL >+ my $facet_link_value = $one_facet; >+ $facet_link_value =~ s/[()!?¡¿Ø]/ /g; >+ >+ # fix the length that will display in the label, >+ my $facet_label_value = $one_facet; >+ my $facet_max_length = C4::Context->preference( >+ 'FacetLabelTruncationLength') >+ || 20; >+ $facet_label_value = >+ substr( $one_facet, 0, $facet_max_length ) >+ . "..." >+ if length($facet_label_value) > >+ $facet_max_length; > >- # Sanitize the link value : parenthesis, question and exclamation mark will cause errors with CCL >- my $facet_link_value = $one_facet; >- $facet_link_value =~ s/[()!?¡¿Ø]/ /g; >+ # if it's a branch, label by the name, not the code, >+ if ( $link_value =~ /branch/ ) { >+ if ( defined $branches >+ && ref($branches) eq "HASH" >+ && defined $branches->{$one_facet} >+ && ref( $branches->{$one_facet} ) eq >+ "HASH" ) >+ { >+ $facet_label_value = >+ $branches->{$one_facet} >+ ->{'branchname'}; >+ } >+ else { >+ $facet_label_value = "*"; >+ } >+ } > >- # fix the length that will display in the label, >- my $facet_label_value = $one_facet; >- my $facet_max_length = >- C4::Context->preference('FacetLabelTruncationLength') || 20; >- $facet_label_value = >- substr( $one_facet, 0, $facet_max_length ) . "..." >- if length($facet_label_value) > $facet_max_length; >+ # if it's a itemtype, label by the name, not the code, >+ if ( $link_value =~ /itype/ ) { >+ if ( defined $itemtypes >+ && ref($itemtypes) eq "HASH" >+ && defined $itemtypes->{$one_facet} >+ && ref( $itemtypes->{$one_facet} ) eq >+ "HASH" ) >+ { >+ $facet_label_value = >+ $itemtypes->{$one_facet} >+ ->{'description'}; >+ } >+ } > >- # if it's a branch, label by the name, not the code, >- if ( $link_value =~ /branch/ ) { >- if (defined $branches >- && ref($branches) eq "HASH" >- && defined $branches->{$one_facet} >- && ref ($branches->{$one_facet}) eq "HASH") >- { >- $facet_label_value = >- $branches->{$one_facet}->{'branchname'}; >- } >- else { >- $facet_label_value = "*"; >- } >- } >- # if it's a itemtype, label by the name, not the code, >- if ( $link_value =~ /itype/ ) { >- if (defined $itemtypes >- && ref($itemtypes) eq "HASH" >- && defined $itemtypes->{$one_facet} >- && ref ($itemtypes->{$one_facet}) eq "HASH") >- { >+ # also, if it's a location code, use the name instead of the code >+ if ( $link_value =~ /location/ ) { > $facet_label_value = >- $itemtypes->{$one_facet}->{'description'}; >+ GetKohaAuthorisedValueLib( 'LOC', >+ $one_facet, $opac ); > } >- } > >- # also, if it's a location code, use the name instead of the code >- if ( $link_value =~ /location/ ) { >- $facet_label_value = GetKohaAuthorisedValueLib('LOC', $one_facet, $opac); >+ # but we're down with the whole label being in the link's title. >+ push @this_facets_array, >+ { >+ facet_count => >+ $facets_counter->{$link_value} >+ ->{$one_facet}, >+ facet_label_value => $facet_label_value, >+ facet_title_value => $one_facet, >+ facet_link_value => $facet_link_value, >+ type_link_value => $link_value, >+ } >+ if ($facet_label_value); > } >- >- # but we're down with the whole label being in the link's title. >- push @this_facets_array, { >- facet_count => $facets_counter->{$link_value}->{$one_facet}, >- facet_label_value => $facet_label_value, >- facet_title_value => $one_facet, >- facet_link_value => $facet_link_value, >- type_link_value => $link_value, >- } if ( $facet_label_value ); > } >- } > >- # handle expanded option >- unless ( $facets_info->{$link_value}->{'expanded'} ) { >- $expandable = 1 >- if ( ( $number_of_facets > 6 ) >- && ( $expanded_facet ne $link_value ) ); >+ # handle expanded option >+ unless ( $facets_info->{$link_value}->{'expanded'} ) { >+ $expandable = 1 >+ if ( ( $number_of_facets > 6 ) >+ && ( $expanded_facet ne $link_value ) ); >+ } >+ push @facets_loop, >+ { >+ type_link_value => $link_value, >+ type_id => $link_value . "_id", >+ "type_label_" >+ . $facets_info->{$link_value}->{'label_value'} => >+ 1, >+ facets => \@this_facets_array, >+ expandable => $expandable, >+ expand => $link_value, >+ } >+ unless ( >+ ( >+ $facets_info->{$link_value}->{'label_value'} =~ >+ /Libraries/ >+ ) >+ and ( C4::Context->preference('singleBranchMode') ) >+ ); > } >- push @facets_loop, { >- type_link_value => $link_value, >- type_id => $link_value . "_id", >- "type_label_" . $facets_info->{$link_value}->{'label_value'} => 1, >- facets => \@this_facets_array, >- expandable => $expandable, >- expand => $link_value, >- } unless ( ($facets_info->{$link_value}->{'label_value'} =~ /Libraries/) and (C4::Context->preference('singleBranchMode')) ); > } > } >- } >- } >+ ); > return ( undef, $results_hashref, \@facets_loop ); > } > >@@ -2816,24 +2862,53 @@ sub GetDistinctValues { > } > # The big moment: asynchronously retrieve results from all servers > my @elements; >- while ( ( my $i = ZOOM::event( \@zconns ) ) != 0 ) { >- my $ev = $zconns[ $i - 1 ]->last_event(); >- if ( $ev == ZOOM::Event::ZEND ) { >- next unless $results[ $i - 1 ]; >- my $size = $results[ $i - 1 ]->size(); >- if ( $size > 0 ) { >- for (my $j=0;$j<$size;$j++){ >- my %hashscan; >- @hashscan{qw(value cnt)}=$results[ $i - 1 ]->display_term($j); >- push @elements, \%hashscan; >- } >- } >- } >- } >+ _ZOOM_event_loop( >+ \@zconns, >+ \@results, >+ sub { >+ my ( $i, $size ) = @_; >+ for ( my $j = 0 ; $j < $size ; $j++ ) { >+ my %hashscan; >+ @hashscan{qw(value cnt)} = >+ $results[ $i - 1 ]->display_term($j); >+ push @elements, \%hashscan; >+ } >+ } >+ ); > return \@elements; > } > } > >+=head2 _ZOOM_event_loop >+ >+ _ZOOM_event_loop(\@zconns, \@results, sub { >+ my ( $i, $size ) = @_; >+ .... >+ } ); >+ >+Processes a ZOOM event loop and passes control to a closure for >+processing the results, and destroying the resultsets. >+ >+=cut >+ >+sub _ZOOM_event_loop { >+ my ($zconns, $results, $callback) = @_; >+ while ( ( my $i = ZOOM::event( $zconns ) ) != 0 ) { >+ my $ev = $zconns->[ $i - 1 ]->last_event(); >+ if ( $ev == ZOOM::Event::ZEND ) { >+ next unless $results->[ $i - 1 ]; >+ my $size = $results->[ $i - 1 ]->size(); >+ if ( $size > 0 ) { >+ $callback->($i, $size); >+ } >+ } >+ } >+ >+ foreach my $result (@$results) { >+ $result->destroy(); >+ } >+} >+ > > END { } # module clean-up code here (global destructor) > >-- >1.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9183
:
13807
|
13924
|
14269