Bugzilla – Attachment 76475 Details for
Bug 21006
GetMarcBiblio is slow for records with many items
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21006: Add GetMarcItems method and refactor GetMarcBiblio
Bug-21006-Add-GetMarcItems-method-and-refactor-Get.patch (text/plain), 16.80 KB, created by
Nick Clemens
on 2018-06-27 02:59:53 UTC
(
hide
)
Description:
Bug 21006: Add GetMarcItems method and refactor GetMarcBiblio
Filename:
MIME Type:
Creator:
Nick Clemens
Created:
2018-06-27 02:59:53 UTC
Size:
16.80 KB
patch
obsolete
>From b6eb10dc88694fdc4c00a01f1d294a634d93b549 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 27 Jun 2018 00:19:00 +0000 >Subject: [PATCH] Bug 21006: Add GetMarcItems method and refactor GetMarcBiblio > >--- > C4/Biblio.pm | 9 ++-- > C4/Items.pm | 154 +++++++++++++++++++++++++++++++++++------------------------ > 2 files changed, 94 insertions(+), 69 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index 0c5a917..8e55254 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -1194,6 +1194,7 @@ sub GetMarcBiblio { > > C4::Biblio::_koha_marc_update_bib_ids( $record, $frameworkcode, $biblionumber, > $biblioitemnumber ); >+ > C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, undef, $opac ) > if ($embeditems); > >@@ -2816,12 +2817,8 @@ sub EmbedItemsInMarcBiblio { > : (); > # Convert to a hash for quick searching > my %hiddenitems = map { $_ => 1 } @hiddenitems; >- foreach my $itemnumber ( map { $_->{itemnumber} } @items ) { >- next if $hiddenitems{$itemnumber}; >- my $item_marc = C4::Items::GetMarcItem( $biblionumber, $itemnumber ); >- push @item_fields, $item_marc->field($itemtag); >- } >- $marc->append_fields(@item_fields); >+ my $item_fields = C4::Items::GetMarcItems( $biblionumber, \@hiddenitems, $itemtag ); >+ $marc->append_fields(@$item_fields); > } > > =head1 INTERNAL FUNCTIONS >diff --git a/C4/Items.pm b/C4/Items.pm >index bf464c3..8cfa14b 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -65,9 +65,9 @@ BEGIN { > ModDateLastSeen > ModItemTransfer > DelItem >- >+ > CheckItemPreSave >- >+ > GetItemsForInventory > GetItemsByBiblioitemnumber > GetItemsInfo >@@ -102,7 +102,7 @@ C4::Items - item management functions > > =head1 DESCRIPTION > >-This module contains an API for manipulating item >+This module contains an API for manipulating item > records in Koha, and is used by cataloguing, circulation, > acquisitions, and serials management. > >@@ -116,7 +116,7 @@ modification transaction must keep the items table > and the MARC XML in sync at all times. > > Consequently, all code that creates, modifies, or deletes >-item records B<must> use an appropriate function from >+item records B<must> use an appropriate function from > C<C4::Items>. If no existing function is suitable, it is > better to add one to C<C4::Items> than to use add > one-off SQL statements to add or modify items. >@@ -222,7 +222,7 @@ sub ShelfToCart { > > =head2 AddItemFromMarc > >- my ($biblionumber, $biblioitemnumber, $itemnumber) >+ my ($biblionumber, $biblioitemnumber, $itemnumber) > = AddItemFromMarc($source_item_marc, $biblionumber); > > Given a MARC::Record object containing an embedded item >@@ -237,7 +237,7 @@ sub AddItemFromMarc { > # parse item hash from MARC > my $frameworkcode = C4::Biblio::GetFrameworkCode( $biblionumber ); > my ($itemtag,$itemsubfield)=C4::Biblio::GetMarcFromKohaField("items.itemnumber",$frameworkcode); >- >+ > my $localitemmarc=MARC::Record->new; > $localitemmarc->append_fields($source_item_marc->field($itemtag)); > my $item = &TransformMarcToKoha( $localitemmarc, $frameworkcode ,'items'); >@@ -247,7 +247,7 @@ sub AddItemFromMarc { > > =head2 AddItem > >- my ($biblionumber, $biblioitemnumber, $itemnumber) >+ my ($biblionumber, $biblioitemnumber, $itemnumber) > = AddItem($item, $biblionumber[, $dbh, $frameworkcode, $unlinked_item_subfields]); > > Given a hash containing item column names as keys, >@@ -261,7 +261,7 @@ The final optional parameter, C<$unlinked_item_subfields>, contains > an arrayref containing subfields present in the original MARC > representation of the item (e.g., from the item editor) that are > not mapped to C<items> columns directly but should instead >-be stored in C<items.more_subfields_xml> and included in >+be stored in C<items.more_subfields_xml> and included in > the biblio items tag for display and indexing. > > =cut >@@ -309,7 +309,7 @@ sub AddItem { > > =head2 AddItemBatchFromMarc > >- ($itemnumber_ref, $error_ref) = AddItemBatchFromMarc($record, >+ ($itemnumber_ref, $error_ref) = AddItemBatchFromMarc($record, > $biblionumber, $biblioitemnumber, $frameworkcode); > > Efficiently create item records from a MARC biblio record with >@@ -374,7 +374,7 @@ sub AddItemBatchFromMarc { > # and there is no TransformMarcFieldToKoha > my $temp_item_marc = MARC::Record->new(); > $temp_item_marc->append_fields($item_field); >- >+ > # add biblionumber and biblioitemnumber > my $item = TransformMarcToKoha( $temp_item_marc, $frameworkcode, 'items' ); > my $unlinked_item_subfields = _get_unlinked_item_subfields($temp_item_marc, $frameworkcode); >@@ -397,7 +397,7 @@ sub AddItemBatchFromMarc { > push @itemnumbers, $itemnumber; # FIXME not checking error > $item->{'itemnumber'} = $itemnumber; > >- logaction("CATALOGUING", "ADD", $itemnumber, "item") if C4::Context->preference("CataloguingLog"); >+ logaction("CATALOGUING", "ADD", $itemnumber, "item") if C4::Context->preference("CataloguingLog"); > > my $new_item_marc = _marc_from_item_hash($item, $frameworkcode, $unlinked_item_subfields); > $item_field->replace_with($new_item_marc->field($itemtag)); >@@ -420,7 +420,7 @@ sub AddItemBatchFromMarc { > > This function updates an item record based on a supplied > C<MARC::Record> object containing an embedded item field. >-This API is meant for the use of C<additem.pl>; for >+This API is meant for the use of C<additem.pl>; for > other purposes, C<ModItem> should be used. > > This function uses the hash %default_values_for_mod_from_marc, >@@ -539,11 +539,11 @@ C<$unlinked_item_subfields> contains an arrayref containing > subfields present in the original MARC > representation of the item (e.g., from the item editor) that are > not mapped to C<items> columns directly but should instead >-be stored in C<items.more_subfields_xml> and included in >+be stored in C<items.more_subfields_xml> and included in > the biblio items tag for display and indexing. > > If one of the changed columns is used to calculate >-the derived value of a column such as C<items.cn_sort>, >+the derived value of a column such as C<items.cn_sort>, > this routine will perform the necessary calculation > and set the value. > >@@ -771,7 +771,7 @@ sub CheckItemPreSave { > > =head1 EXPORTED SPECIAL ACCESSOR FUNCTIONS > >-The following functions provide various ways of >+The following functions provide various ways of > getting an item record, a set of item records, or > lists of authorized values for certain item fields. > >@@ -947,9 +947,9 @@ sub GetItemsByBiblioitemnumber { > my $sth = $dbh->prepare("SELECT * FROM items WHERE items.biblioitemnumber = ?") || die $dbh->errstr; > # Get all items attached to a biblioitem > my $i = 0; >- my @results; >+ my @results; > $sth->execute($bibitem) || die $sth->errstr; >- while ( my $data = $sth->fetchrow_hashref ) { >+ while ( my $data = $sth->fetchrow_hashref ) { > # Foreach item, get circulation information > my $sth2 = $dbh->prepare( "SELECT * FROM issues,borrowers > WHERE itemnumber = ? >@@ -964,9 +964,9 @@ sub GetItemsByBiblioitemnumber { > } > else { > # set date_due to blank, so in the template we check itemlost, and withdrawn >- $data->{'date_due'} = ''; >- } # else >- # Find the last 3 people who borrowed this item. >+ $data->{'date_due'} = ''; >+ } # else >+ # Find the last 3 people who borrowed this item. > my $query2 = "SELECT * FROM old_issues, borrowers WHERE itemnumber = ? > AND old_issues.borrowernumber = borrowers.borrowernumber > ORDER BY returndate desc,timestamp desc LIMIT 3"; >@@ -980,8 +980,8 @@ sub GetItemsByBiblioitemnumber { > $i2++; > } > push(@results,$data); >- } >- return (\@results); >+ } >+ return (\@results); > } > > =head2 GetItemsInfo >@@ -1165,7 +1165,7 @@ The intranet description for the Shelving Location as set in authorised_values ' > > =item C<$data-E<gt>{location_opac}> > >-The OPAC description for the Shelving Location as set in authorised_values 'LOC'. Falls back to intranet description if no OPAC >+The OPAC description for the Shelving Location as set in authorised_values 'LOC'. Falls back to intranet description if no OPAC > description is set. > > =item C<$data-E<gt>{itemcallnumber}> >@@ -1177,7 +1177,7 @@ Item's itemcallnumber > Item's call number normalized for sorting > > =back >- >+ > =cut > > sub GetItemsLocationInfo { >@@ -1185,10 +1185,10 @@ sub GetItemsLocationInfo { > my @results; > > my $dbh = C4::Context->dbh; >- my $query = "SELECT a.branchname as homebranch, b.branchname as holdingbranch, >+ my $query = "SELECT a.branchname as homebranch, b.branchname as holdingbranch, > location, itemcallnumber, cn_sort > FROM items, branches as a, branches as b >- WHERE homebranch = a.branchcode AND holdingbranch = b.branchcode >+ WHERE homebranch = a.branchcode AND holdingbranch = b.branchcode > AND biblionumber = ? > ORDER BY cn_sort ASC"; > my $sth = $dbh->prepare($query); >@@ -1243,7 +1243,7 @@ sub GetHostItemsInfo { > > =head2 GetLastAcquisitions > >- my $lastacq = GetLastAcquisitions({'branches' => ('branch1','branch2'), >+ my $lastacq = GetLastAcquisitions({'branches' => ('branch1','branch2'), > 'itemtypes' => ('BK','BD')}, 10); > > =cut >@@ -1252,43 +1252,43 @@ sub GetLastAcquisitions { > my ($data,$max) = @_; > > my $itemtype = C4::Context->preference('item-level_itypes') ? 'itype' : 'itemtype'; >- >+ > my $number_of_branches = @{$data->{branches}}; > my $number_of_itemtypes = @{$data->{itemtypes}}; >- >- >- my @where = ('WHERE 1 '); >+ >+ >+ my @where = ('WHERE 1 '); > $number_of_branches and push @where >- , 'AND holdingbranch IN (' >+ , 'AND holdingbranch IN (' > , join(',', ('?') x $number_of_branches ) > , ')' > ; >- >+ > $number_of_itemtypes and push @where >- , "AND $itemtype IN (" >+ , "AND $itemtype IN (" > , join(',', ('?') x $number_of_itemtypes ) > , ')' > ; > > my $query = "SELECT biblio.biblionumber as biblionumber, title, dateaccessioned >- FROM items RIGHT JOIN biblio ON (items.biblionumber=biblio.biblionumber) >+ FROM items RIGHT JOIN biblio ON (items.biblionumber=biblio.biblionumber) > RIGHT JOIN biblioitems ON (items.biblioitemnumber=biblioitems.biblioitemnumber) > @where >- GROUP BY biblio.biblionumber >+ GROUP BY biblio.biblionumber > ORDER BY dateaccessioned DESC LIMIT $max"; > > my $dbh = C4::Context->dbh; > my $sth = $dbh->prepare($query); >- >+ > $sth->execute((@{$data->{branches}}, @{$data->{itemtypes}})); >- >+ > my @results; > while( my $row = $sth->fetchrow_hashref){ >- push @results, {date => $row->{dateaccessioned} >+ push @results, {date => $row->{dateaccessioned} > , biblionumber => $row->{biblionumber} > , title => $row->{title}}; > } >- >+ > return @results; > } > >@@ -1492,18 +1492,46 @@ sub GetMarcItem { > # Tack on 'items.' prefix to column names so that C4::Biblio::TransformKohaToMarc will work. > # Also, don't emit a subfield if the underlying field is blank. > >- >+ > return Item2Marc($itemrecord,$biblionumber); > > } >+ >+=head2 GetMarcItems >+ >+ my @items_marc = GetMarcItems($biblionumber, \@hiddenitemnumbers); >+ >+Returns array of MARC::Records of the items for the biblio passed in parameter. >+ >+=cut >+ >+sub GetMarcItems { >+ my ( $biblionumber, $hiddenitems, $itemtag ) = @_; >+ my $params; >+ $params->{biblionumber} = $biblionumber; >+ $params->{itemnumber} = {'not in' => @$hiddenitems} if defined $hiddenitems && scalar @$hiddenitems; >+ my $items = Koha::Items->search($params ); >+ return unless $items; >+ >+ # Tack on 'items.' prefix to column names so that C4::Biblio::TransformKohaToMarc will work. >+ # Also, don't emit a subfield if the underlying field is blank. >+ >+ my $framework = Koha::Biblios->find( $biblionumber)->frameworkcode(); >+ my @marc_items; >+ while ( my $this_item = $items->next() ){ >+ push @marc_items, Item2Marc($this_item->unblessed,$biblionumber,$framework)->field($itemtag); >+ } >+ return \@marc_items; >+} >+ > sub Item2Marc { >- my ($itemrecord,$biblionumber)=@_; >- my $mungeditem = { >- map { >- defined($itemrecord->{$_}) && $itemrecord->{$_} ne '' ? ("items.$_" => $itemrecord->{$_}) : () >- } keys %{ $itemrecord } >+ my ($itemrecord,$biblionumber, $frameworkcode)=@_; >+ my $mungeditem = { >+ map { >+ defined($itemrecord->{$_}) && $itemrecord->{$_} ne '' ? ("items.$_" => $itemrecord->{$_}) : () >+ } keys %{ $itemrecord } > }; >- my $framework = C4::Biblio::GetFrameworkCode( $biblionumber ); >+ my $framework = $frameworkcode // C4::Biblio::GetFrameworkCode( $biblionumber ); > my $itemmarc = C4::Biblio::TransformKohaToMarc( > $mungeditem, { no_split => 1}, > ); >@@ -1553,7 +1581,7 @@ my %derived_columns = ( > } > ); > >-=head2 _set_derived_columns_for_add >+=head2 _set_derived_columns_for_add > > _set_derived_column_for_add($item); > >@@ -1577,7 +1605,7 @@ sub _set_derived_columns_for_add { > } > } > >-=head2 _set_derived_columns_for_mod >+=head2 _set_derived_columns_for_mod > > _set_derived_column_for_mod($item); > >@@ -1680,12 +1708,12 @@ row specified by C<$itemnumber>. > sub _get_single_item_column { > my $column = shift; > my $itemnumber = shift; >- >+ > my $dbh = C4::Context->dbh; > my $sth = $dbh->prepare("SELECT $column FROM items WHERE itemnumber = ?"); > $sth->execute($itemnumber); > my ($value) = $sth->fetchrow(); >- return $value; >+ return $value; > } > > =head2 _calc_items_cn_sort >@@ -1703,7 +1731,7 @@ sub _calc_items_cn_sort { > $item->{'items.cn_sort'} = GetClassSort($source_values->{'items.cn_source'}, $source_values->{'itemcallnumber'}, ""); > } > >-=head2 _set_defaults_for_add >+=head2 _set_defaults_for_add > > _set_defaults_for_add($item_hash); > >@@ -1714,7 +1742,7 @@ columns: > > =over 2 > >-=item * >+=item * > > C<items.dateaccessioned> > >@@ -1754,7 +1782,7 @@ Perform the actual insert into the C<items> table. > > sub _koha_new_item { > my ( $item, $barcode ) = @_; >- my $dbh=C4::Context->dbh; >+ my $dbh=C4::Context->dbh; > my $error; > $item->{permanent_location} //= $item->{location}; > _mod_item_dates( $item ); >@@ -1885,7 +1913,7 @@ sub MoveItemFromBiblio { > if ($return == 1) { > ModZebra( $tobiblio, "specialUpdate", "biblioserver" ); > ModZebra( $frombiblio, "specialUpdate", "biblioserver" ); >- # Checking if the item we want to move is in an order >+ # Checking if the item we want to move is in an order > require C4::Acquisition; > my $order = C4::Acquisition::GetOrderFromItemnumber($itemnumber); > if ($order) { >@@ -2016,7 +2044,7 @@ routine accepts a hashref specifying the columns to update. > > sub _koha_modify_item { > my ( $item ) = @_; >- my $dbh=C4::Context->dbh; >+ my $dbh=C4::Context->dbh; > my $error; > > my $query = "UPDATE items SET "; >@@ -2126,12 +2154,12 @@ sub _marc_from_item_hash { > if (@_) { > $unlinked_item_subfields = shift; > } >- >+ > # Tack on 'items.' prefix to column names so lookup from MARC frameworks will work > # Also, don't emit a subfield if the underlying field is blank. >- my $mungeditem = { map { (defined($item->{$_}) and $item->{$_} ne '') ? >- (/^items\./ ? ($_ => $item->{$_}) : ("items.$_" => $item->{$_})) >- : () } keys %{ $item } }; >+ my $mungeditem = { map { (defined($item->{$_}) and $item->{$_} ne '') ? >+ (/^items\./ ? ($_ => $item->{$_}) : ("items.$_" => $item->{$_})) >+ : () } keys %{ $item } }; > > my $item_marc = MARC::Record->new(); > foreach my $item_field ( keys %{$mungeditem} ) { >@@ -2175,7 +2203,7 @@ sub _repack_item_errors { > $repacked_error->{'error_code'} = $error_code; > $repacked_error->{'error_information'} = $error_ref->{$error_code}; > push @repacked_errors, $repacked_error; >- } >+ } > > return @repacked_errors; > } >@@ -2225,7 +2253,7 @@ sub _get_unlinked_subfields_xml { > # use of tag 999 is arbitrary, and doesn't need to match the item tag > # used in the framework > $marc->append_fields(MARC::Field->new('999', ' ', ' ', @$unlinked_item_subfields)); >- $marc->encoding("UTF-8"); >+ $marc->encoding("UTF-8"); > $xml = $marc->as_xml("USMARC"); > } > >@@ -2257,7 +2285,7 @@ sub _parse_unlinked_item_subfields_from_xml { > > $count= &GetAnalyticsCount($itemnumber) > >-counts Usage of itemnumber in Analytical bibliorecords. >+counts Usage of itemnumber in Analytical bibliorecords. > > =cut > >-- >2.1.4
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 21006
:
76475
|
76490
|
76659
|
76660
|
76661