Bugzilla – Attachment 124087 Details for
Bug 28371
Improve performance of XSLTParse4Display
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28371: Passpreviously fetched branches and itemtypes through and fetch all needed AV at once
Bug-28371-Passpreviously-fetched-branches-and-item.patch (text/plain), 9.21 KB, created by
Nick Clemens
on 2021-08-25 15:35:27 UTC
(
hide
)
Description:
Bug 28371: Passpreviously fetched branches and itemtypes through and fetch all needed AV at once
Filename:
MIME Type:
Creator:
Nick Clemens
Created:
2021-08-25 15:35:27 UTC
Size:
9.21 KB
patch
obsolete
>From 9746276aeeec2d6ebe031060c32508f5aefc189a Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 25 Aug 2021 15:26:23 +0000 >Subject: [PATCH] Bug 28371: Passpreviously fetched branches and itemtypes > through and fetch all needed AV at once > >This patch updates the searchResuls code to pass through the pre-constructed branches and itemtype lookups >to XSLTParse4Display to avoid repeating this > >It also updates getAuthorisedValues4MARCSubfields to fetch the values for mapped subfields and pass >then through to transforMarc4XSLT > >Note that we currently blank invalid branches and itemtypes - I presrve this, we should open another bug >if we want to change this behaviour > >Changes are covered by tests > >To test: >1 - Perform searches in OPAC and staff client that return many records >2 - Use the 'Network' tab on the browser console (opened with F12 usually) to see the time taken >3 - Note the speed before the patch >4 - Apply patch >5 - restart all the things >6 - Note improvement in speed > >**Note: The improvement is more drastic the more items per record, try adding large numbers of items to your search results to test** >--- > C4/Search.pm | 4 +++- > C4/XSLT.pm | 58 ++++++++++++++++++++++++++++++++++------------------ > 2 files changed, 41 insertions(+), 21 deletions(-) > >diff --git a/C4/Search.pm b/C4/Search.pm >index 72c81ec06c..7307ce3d70 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -1970,7 +1970,9 @@ sub searchResults { > ), > fix_amps => 1, > hidden_items => \@hiddenitems, >- xslt_variables => $xslt_variables >+ xslt_variables => $xslt_variables, >+ branches => \%branches, >+ itemtypes => \%itemtypes > } > ); > } >diff --git a/C4/XSLT.pm b/C4/XSLT.pm >index 2de14b89a5..38126096ec 100644 >--- a/C4/XSLT.pm >+++ b/C4/XSLT.pm >@@ -65,7 +65,7 @@ Is only used in this module currently. > =cut > > sub transformMARCXML4XSLT { >- my ($biblionumber, $record) = @_; >+ my ($biblionumber, $record, $branches, $itemtypes, $av) = @_; > my $frameworkcode = GetFrameworkCode($biblionumber) || ''; > my $tagslib = &GetMarcStructure(1, $frameworkcode, { unsafe => 1 }); > my @fields; >@@ -75,26 +75,39 @@ sub transformMARCXML4XSLT { > }; > if ($@) { warn "PROBLEM WITH RECORD"; next; } > my $marcflavour = C4::Context->preference('marcflavour'); >- my $av = getAuthorisedValues4MARCSubfields($frameworkcode); > foreach my $tag ( keys %$av ) { > foreach my $field ( $record->field( $tag ) ) { >- if ( $av->{ $tag } ) { >+ if ( defined $av->{ $tag } ) { > my @new_subfields = (); >+ my $updated = 0; > for my $subfield ( $field->subfields() ) { > my ( $letter, $value ) = @$subfield; >- # Replace the field value with the authorised value *except* for MARC21/NORMARC field 942$n (suppression in opac) >- if ( !( $tag eq '942' && $subfield->[0] eq 'n' ) || $marcflavour eq 'UNIMARC' ) { >- $value = GetAuthorisedValueDesc( $tag, $letter, $value, '', $tagslib ) >- if $av->{ $tag }->{ $letter }; >+ if( defined $av->{ $tag }->{ $letter } ){ >+ my $category = $av->{$tag}->{$letter}->{category}; >+ # Replace the field value with the authorised value *except* for MARC21/NORMARC field 942$n (suppression in opac) >+ if( $category eq 'branches' ){ >+ $value = defined $branches->{$value} ? $branches->{$value}: q{} ; >+ $updated = 1; >+ } >+ elsif( $category eq 'itemtypes' ){ >+ $value = defined $itemtypes->{$value}->{translated_description} ? $itemtypes->{$value}->{translated_description} : q{}; >+ $updated = 1; >+ } >+ elsif ( !( $tag eq '942' && $subfield->[0] eq 'n' ) || $marcflavour eq 'UNIMARC' ) { >+ if ( defined $av->{$tag}->{$letter}->{$category}->{$value}->{lib} ) { >+ $value = $av->{$tag}->{$letter}->{$category}->{$value}->{lib} || $value; >+ $updated = 1; >+ } >+ } > } > push( @new_subfields, $letter, $value ); >- } >+ } > $field ->replace_with( MARC::Field->new( > $tag, > $field->indicator(1), > $field->indicator(2), > @new_subfields >- ) ); >+ ) ) if $updated; > } > } > } >@@ -112,15 +125,17 @@ sub getAuthorisedValues4MARCSubfields { > my ($frameworkcode) = @_; > unless ( $authval_per_framework{ $frameworkcode } ) { > my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare("SELECT DISTINCT tagfield, tagsubfield >+ my $sth = $dbh->prepare("SELECT DISTINCT tagfield, tagsubfield, authorised_value > FROM marc_subfield_structure > WHERE authorised_value IS NOT NULL > AND authorised_value!='' > AND frameworkcode=?"); > $sth->execute( $frameworkcode ); > my $av = { }; >- while ( my ( $tag, $letter ) = $sth->fetchrow() ) { >- $av->{ $tag }->{ $letter } = 1; >+ while ( my ( $tag, $letter, $category ) = $sth->fetchrow() ) { >+ my $authorised_values = { map { $_->{authorised_value} => $_ } @{ Koha::AuthorisedValues->search({ category => $category })->unblessed } }; >+ $av->{ $tag }->{ $letter }->{category} = $category; >+ $av->{ $tag }->{ $letter }->{$category} = $authorised_values unless ( $category eq 'itemtypes' || $category eq 'branches' ); > } > $authval_per_framework{ $frameworkcode } = $av; > } >@@ -252,16 +267,22 @@ sub XSLTParse4Display { > my $hidden_items = $params->{hidden_items} || []; > my $variables = $params->{xslt_variables}; > my $items_rs = $params->{items_rs}; >+ my $branches = $params->{branches}; >+ my $itemtypes = $params->{itemtypes}; >+ >+ $branches = { map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' }) } unless $branches; >+ $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } } unless $itemtypes; > > my $xslfilename = get_xsl_filename( $xslsyspref); > > # grab the XML, run it through our stylesheet, push it out to the browser >- my $record = transformMARCXML4XSLT($biblionumber, $orig_record); >+ my $authorised_values = getAuthorisedValues4MARCSubfields(""); >+ my $record = transformMARCXML4XSLT($biblionumber, $orig_record, $branches, $itemtypes, $authorised_values ); > my $itemsxml; > if ( $xslsyspref eq "OPACXSLTDetailsDisplay" || $xslsyspref eq "XSLTDetailsDisplay" || $xslsyspref eq "XSLTResultsDisplay" ) { > $itemsxml = ""; #We don't use XSLT for items display on these pages > } else { >- $itemsxml = buildKohaItemsNamespace($biblionumber, $hidden_items, $items_rs); >+ $itemsxml = buildKohaItemsNamespace($biblionumber, $hidden_items, $items_rs, $branches, $itemtypes); > } > my $xmlrecord = $record->as_xml(C4::Context->preference('marcflavour')); > >@@ -319,7 +340,7 @@ Is only used in this module currently. > =cut > > sub buildKohaItemsNamespace { >- my ($biblionumber, $hidden_items, $items_rs) = @_; >+ my ($biblionumber, $hidden_items, $items_rs, $branches, $itemtypes) = @_; > > $hidden_items ||= []; > >@@ -339,9 +360,6 @@ sub buildKohaItemsNamespace { > my $ccodes = > { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => "", kohafield => 'items.ccode' } ) }; > >- my %branches = map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' }); >- >- my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search->unblessed } }; > my $xml = ''; > my %descs = map { $_->{authorised_value} => $_ } Koha::AuthorisedValues->get_descriptions_by_koha_field( { kohafield => 'items.notforloan' } ); > my $ref_status = C4::Context->preference('Reference_NFL_Statuses') || '1|2'; >@@ -389,8 +407,8 @@ sub buildKohaItemsNamespace { > else { > $status = "available"; > } >- my $homebranch = xml_escape($branches{$item->homebranch}); >- my $holdingbranch = xml_escape($branches{$item->holdingbranch}); >+ my $homebranch = xml_escape($branches->{$item->homebranch}); >+ my $holdingbranch = xml_escape($branches->{$item->holdingbranch}); > my $location = xml_escape($item->location && exists $shelflocations->{$item->location} ? $shelflocations->{$item->location} : $item->location); > my $ccode = xml_escape($item->ccode && exists $ccodes->{$item->ccode} ? $ccodes->{$item->ccode} : $item->ccode); > my $itemcallnumber = xml_escape($item->itemcallnumber); >-- >2.20.1
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 28371
:
121111
|
124086
|
124087
|
127082
|
127083
|
127090
|
127091
|
132801
|
132802
|
132943
|
132944
|
133719
|
133720
|
133872
|
135373
|
135381