From 93585b82d958fb6e638a963ab50f19e2cd6fc985 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Fri, 15 Dec 2017 11:36:38 +0100 Subject: [PATCH] Bug 19884 - Improve performance of GetItem Add new subs GetMarcBiblios() and EmbedItemsInMarcBiblios() for embedding items in multiple records at once for improved performance. Add new sub GetItems() making it possible to load multiple items at once in the same manner. Also extend GetMarcItem() to also accept an item record as second parameter. This way an extra call to GetItem() can be avoided by passing an item record directly instead of an item number. How to test: 1) Run tests in t/db_dependent/Items.t Sponsored-by: Gothenburg University Library --- C4/Biblio.pm | 224 +++++++++++++++++++--------- C4/Items.pm | 68 ++++++--- Koha/BiblioUtils.pm | 80 +++++++--- Koha/Items.pm | 66 ++++++++ misc/search_tools/rebuild_elastic_search.pl | 2 +- t/db_dependent/Items.t | 84 ++++++++++- 6 files changed, 404 insertions(+), 120 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 808ed8d067..91033fe433 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1108,6 +1108,26 @@ sub GetMarcSubfieldStructureFromKohaField { return wantarray ? @{$mss->{$kohafield}} : $mss->{$kohafield}->[0]; } + +sub GetMarcBiblio { + my ($params) = @_; + + if (not defined $params) { + carp 'GetMarcBiblio called without parameters'; + return; + } + my $biblionumber = $params->{biblionumber}; + + if (not defined $biblionumber) { + carp 'GetMarcBiblio called with undefined biblionumber'; + return; + } + delete $params->{biblionumber}; + $params->{biblionumbers} = [$biblionumber]; + my ($biblio) = GetMarcBiblios($params); + return $biblio; +} + =head2 GetMarcBiblio my $record = GetMarcBiblio({ @@ -1148,57 +1168,68 @@ It only makes sense in combination both embed_items and opac values true. =cut -sub GetMarcBiblio { +sub GetMarcBiblios { my ($params) = @_; if (not defined $params) { - carp 'GetMarcBiblio called without parameters'; + carp 'GetMarcBiblios called without parameters'; return; } - my $biblionumber = $params->{biblionumber}; + my $biblionumbers = $params->{biblionumbers}; my $embeditems = $params->{embed_items} || 0; my $opac = $params->{opac} || 0; my $borcat = $params->{borcat} // q{}; - if (not defined $biblionumber) { - carp 'GetMarcBiblio called with undefined biblionumber'; + ## START HERE ## + + if (not defined $biblionumbers) { + carp 'GetMarcBiblio called with undefined biblionumbers'; return; } - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber=? "); - $sth->execute($biblionumber); - my $row = $sth->fetchrow_hashref; - my $biblioitemnumber = $row->{'biblioitemnumber'}; - my $marcxml = GetXmlBiblio( $biblionumber ); - $marcxml = StripNonXmlChars( $marcxml ); - my $frameworkcode = GetFrameworkCode($biblionumber); - MARC::File::XML->default_record_format( C4::Context->preference('marcflavour') ); - my $record = MARC::Record->new(); + my $in_placeholders = join ', ', (('?') x @{$biblionumbers}); + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT biblionumber, biblioitemnumber FROM biblioitems WHERE biblionumber IN ($in_placeholders)"); + $sth->execute(@{$biblionumbers}); + + my %marc_records; + my ($biblionumber, $biblioitemnumber); + my $marc_flavour = C4::Context->preference('marcflavour'); + while (my $biblio_keys = $sth->fetchrow_arrayref) { + ($biblionumber, $biblioitemnumber) = @{$biblio_keys}; + + my $marcxml = GetXmlBiblio( $biblionumber ); + $marcxml = StripNonXmlChars( $marcxml ); + my $frameworkcode = GetFrameworkCode($biblionumber); + MARC::File::XML->default_record_format( $marc_flavour ); + my $record = MARC::Record->new(); + + if ($marcxml) { + $record = eval { + MARC::Record::new_from_xml( $marcxml, "utf8", $marc_flavour); + }; + if ($@) { warn " problem with :$biblionumber : $@ \n$marcxml"; } + next unless $record; + + C4::Biblio::_koha_marc_update_bib_ids( + $record, + $frameworkcode, + $biblionumber, + $biblioitemnumber + ); + $marc_records{$biblionumber} = $record; + } + } - if ($marcxml) { - $record = eval { - MARC::Record::new_from_xml( $marcxml, "utf8", - C4::Context->preference('marcflavour') ); - }; - if ($@) { warn " problem with :$biblionumber : $@ \n$marcxml"; } - return unless $record; - - C4::Biblio::_koha_marc_update_bib_ids( $record, $frameworkcode, $biblionumber, - $biblioitemnumber ); - C4::Biblio::EmbedItemsInMarcBiblio({ - marc_record => $record, - biblionumber => $biblionumber, + if ($embeditems) { + C4::Biblio::EmbedItemsInMarcBiblios({ + marc_records => \%marc_records, opac => $opac, - borcat => $borcat }) - if ($embeditems); - - return $record; - } - else { - return; + borcat => $borcat + }); } + return wantarray ? values %marc_records : \%marc_records; } =head2 GetXmlBiblio @@ -2790,57 +2821,104 @@ override if necessary. sub EmbedItemsInMarcBiblio { my ($params) = @_; - my ($marc, $biblionumber, $itemnumbers, $opac, $borcat); - $marc = $params->{marc_record}; - if ( !$marc ) { + + if ( !$params->{marc_record} ) { carp 'EmbedItemsInMarcBiblio: No MARC record passed'; return; } - $biblionumber = $params->{biblionumber}; + if ( !$params->{biblionumber} ) { + carp 'EmbedItemsInMarcBiblio: No biblionumber passed'; + return; + } + + $params->{marc_records} = { $params->{biblionumber} => $params->{marc_record} }; + delete $params->{marc_record}; + delete $params->{biblionumber}; + + my ($marc_with_items) = EmbedItemsInMarcBiblios($params); + return $marc_with_items; +} + +sub EmbedItemsInMarcBiblios { + my ($params) = @_; + my ($marc_records, $biblionumber, $itemnumbers, $opac, $borcat); + $marc_records = $params->{marc_records}; + $itemnumbers = $params->{item_numbers}; $opac = $params->{opac}; $borcat = $params->{borcat} // q{}; $itemnumbers = [] unless defined $itemnumbers; - my $frameworkcode = GetFrameworkCode($biblionumber); - _strip_item_fields($marc, $frameworkcode); - # ... and embed the current items my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT itemnumber FROM items WHERE biblionumber = ?"); - $sth->execute($biblionumber); - my ( $itemtag, $itemsubfield ) = GetMarcFromKohaField( "items.itemnumber", $frameworkcode ); - my @item_fields; # Array holding the actual MARC data for items to be included. - my @items; # Array holding items which are both in the list (sitenumbers) - # and on this biblionumber + my @biblionumbers = keys %{$marc_records}; + my $in_placeholders = join ', ', (('?') x @biblionumbers); - # Flag indicating if there is potential hiding. - my $opachiddenitems = $opac - && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); + my $biblio_itemnumbers = $dbh->selectcol_arrayref( + "SELECT itemnumber FROM items WHERE biblionumber IN ($in_placeholders)", + undef, + @biblionumbers + ); + + # Remove invalid item numbers by intersecting with all valid item numbers + # if $itemnumbers argument was provided + # TODO: MAKE SURE THERE IS A TEST FOR THIS + if(@{$itemnumbers}) { + my %h = map { $_ => undef } @{$itemnumbers}; + $itemnumbers = [grep { exists $h{$_} } @{$biblio_itemnumbers}]; + } + else { + $itemnumbers = $biblio_itemnumbers; + } + # If no item numbers then no point in continuing + return unless (@{$itemnumbers}); + + + my $opachiddenitems = $opac + && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); require C4::Items; - while ( my ($itemnumber) = $sth->fetchrow_array ) { - next if @$itemnumbers and not grep { $_ == $itemnumber } @$itemnumbers; - my $i = $opachiddenitems ? C4::Items::GetItem($itemnumber) : undef; - push @items, { itemnumber => $itemnumber, item => $i }; - } - my @items2pass = map { $_->{item} } @items; - my @hiddenitems = - $opachiddenitems - ? C4::Items::GetHiddenItemnumbers({ - items => \@items2pass, - borcat => $borcat }) - : (); - # 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 $items = C4::Items::GetItems($itemnumbers); + # We have now fetched all items, time to partion by biblionumber + my %items_by_biblionumber = (map { ($_, []) } @biblionumbers); + foreach my $item (@{$items}) { + push @{$items_by_biblionumber{$item->{biblionumber}}}, $item; + } + + foreach my $biblionumber (@biblionumbers) { + my $marc_record = $marc_records->{$biblionumber}; + + # Array holding items which are both in itemnumbers + # (if provided) and in biblionumbers itemnumbers + my $items = $items_by_biblionumber{$biblionumber}; + + # Could this be moved lower down and run only when items + # exists for improved performance? Probably.. + my $frameworkcode = GetFrameworkCode($biblionumber); + _strip_item_fields($marc_record, $frameworkcode); + + # Flag indicating if there is potential hiding + if ($opachiddenitems) { + my %hidden_items = map { $_ => undef } C4::Items::GetHiddenItemnumbers({ + items => @{$items}, + borcat => $borcat + }); + # Reduce items to non hidden items + $items = [grep { !(exists $hidden_items{$_->{itemnumber}}) } @{$items}]; + } + + my ($itemtag) = GetMarcFromKohaField("items.itemnumber", $frameworkcode); + my @item_fields; # Array holding the actual MARC data for items to be included. + foreach my $item (@{$items}) { + my $item_marc = C4::Items::GetMarcItem($biblionumber, $item); + push @item_fields, $item_marc->field($itemtag); + } + $marc_record->append_fields(@item_fields); + } + return $marc_records; } =head1 INTERNAL FUNCTIONS @@ -3292,7 +3370,7 @@ sub ModBiblioMarc { # deal with UNIMARC field 100 (encoding) : create it if needed & set encoding to unicode if ( $encoding eq "UNIMARC" ) { - my $defaultlanguage = C4::Context->preference("UNIMARCField100Language"); + my $defaultlanguage = C4::Context->preference("UNIMARCField100Language"); $defaultlanguage = "fre" if (!$defaultlanguage || length($defaultlanguage) != 3); my $string = $record->subfield( 100, "a" ); if ( ($string) && ( length( $record->subfield( 100, "a" ) ) == 36 ) ) { @@ -3302,7 +3380,7 @@ sub ModBiblioMarc { $string = POSIX::strftime( "%Y%m%d", localtime ); $string =~ s/\-//g; $string = sprintf( "%-*s", 35, $string ); - substr ( $string, 22, 3, $defaultlanguage); + substr ( $string, 22, 3, $defaultlanguage); } substr( $string, 25, 3, "y50" ); unless ( $record->subfield( 100, "a" ) ) { diff --git a/C4/Items.pm b/C4/Items.pm index 3b006c686e..d94dd61c3d 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -129,34 +129,60 @@ of C Return item information, for a given itemnumber or barcode. The return value is a hashref mapping item column -names to values. If C<$serial> is true, include serial publication data. +names to values, or undef if item was not found. If C<$serial> is true, include serial publication data. =cut sub GetItem { - my ($itemnumber,$barcode, $serial) = @_; - my $dbh = C4::Context->dbh; + my ($itemnumber, $barcode, $serial) = @_; + my $items = GetItems($itemnumber ? [$itemnumber] : undef, $barcode ? [$barcode] : undef, $serial); + return @{$items} ? $items->[0] : undef; +} - my $item; - if ($itemnumber) { - $item = Koha::Items->find( $itemnumber ); - } else { - $item = Koha::Items->find( { barcode => $barcode } ); - } +=head2 GetItems - return unless ( $item ); + $item = GetItems($itemnumber,$barcode,$serial); - my $data = $item->unblessed(); - $data->{itype} = $item->effective_itemtype(); # set the correct itype +Return list of item information, for given itemnumbers or barcodes. +The return value is an arrayref with hashrefs mapping item column +names to values, or an empty arrayref if no items were found. +If C<$serial> is true, include serial publication data. +=cut + +sub GetItems { + my ($itemnumbers, $barcodes, $serial) = @_; + my $dbh = C4::Context->dbh; + my $items_data = []; + + # Important: If 'item-level_itypes' preference is not set Koha::Item::effective_itemtype() + # will be equal to $item->{itype} and it is possible to fetch the data directly from items table. + # This will have a much smaller footprint then unblessing the item objects fetched through + # find/search. If more calculated attributes are added in the future, this code will need + # to account for that. + if (C4::Context->preference('item-level_itypes')) { + $items_data = Koha::Items->search_unblessed($itemnumbers ? $itemnumbers : { barcode => $barcodes }); + } + else { + my @items = Koha::Items->search( + $itemnumbers ? { itemnumber => { IN => $itemnumbers } } : { barcode => { IN => $barcodes } } + ); + foreach my $item (@items) { + my $data = $item->unblessed(); + # Set the correct itype + $data->{itype} = $item->effective_itemtype(); + push @{$items_data}, $data; + } + } if ($serial) { - my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=?"); - $ssth->execute( $data->{'itemnumber'} ); - ( $data->{'serialseq'}, $data->{'publisheddate'} ) = $ssth->fetchrow_array(); + foreach my $data (@{$items_data}) { + my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=?"); + $ssth->execute( $data->{'itemnumber'} ); + ( $data->{'serialseq'}, $data->{'publisheddate'} ) = $ssth->fetchrow_array(); + } } - - return $data; -} # sub GetItem + return $items_data; +} =head2 CartToShelf @@ -1331,13 +1357,13 @@ sub GetMarcItem { # while the other treats the MARC representation as authoritative # under certain circumstances. - my $itemrecord = GetItem($itemnumber); + my $itemrecord = ref($itemnumber) ? $itemnumber : GetItem($itemnumber); # 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); + + return Item2Marc($itemrecord, $biblionumber); } sub Item2Marc { diff --git a/Koha/BiblioUtils.pm b/Koha/BiblioUtils.pm index 73fc12aa4b..44cd563aed 100644 --- a/Koha/BiblioUtils.pm +++ b/Koha/BiblioUtils.pm @@ -111,29 +111,69 @@ The iterator is a Koha::MetadataIterator object. =cut sub get_all_biblios_iterator { + my ($self, $params) = @_; + my $batched; + my $batch_size; + if ($params) { + $batched //= $params->{batched}; + $batch_size = $params->{batch_size} || 2000; + } + my $database = Koha::Database->new(); my $schema = $database->schema(); - my $rs = - $schema->resultset('Biblio')->search( {}, - { columns => [qw/ biblionumber /] } ); - my $next_func = sub { - # Warn and skip bad records, otherwise we break the loop - while (1) { - my $row = $rs->next(); - return if !$row; - my $marc = C4::Biblio::GetMarcBiblio({ - biblionumber => $row->biblionumber, - embed_items => 1 }); - my $next = eval { - __PACKAGE__->new($marc, $row->biblionumber); - }; - if ($@) { - warn "Something went wrong reading record for biblio $row->biblionumber: $@\n"; - next; + my $rs = $schema->resultset('Biblio')->search( + {}, + { columns => [qw/ biblionumber /] } + ); + + my $next_func; + if ($batched) { + my @marc_records_batch; + $next_func = sub { + unless (@marc_records_batch) { + #Batch empty, nead to buffer up more records + my $limit = $batch_size; + my @biblionumbers; + while ($limit--) { + my $row = $rs->next(); + last if !$row; + push @biblionumbers, $row->biblionumber; + } + if (@biblionumbers) { + my $marc_records = C4::Biblio::GetMarcBiblios({ + biblionumbers => \@biblionumbers, + embed_items => 1 + }); + while (my ($biblionumber, $marc_record) = each %{$marc_records}) { + my $next = __PACKAGE__->new($marc_record, $biblionumber); + push @marc_records_batch, $next; + } + } } - return $next; - } - }; + return pop @marc_records_batch; + }; + } + else { + $next_func = sub { + # Warn and skip bad records, otherwise we break the loop + while (1) { + my $row = $rs->next(); + return if !$row; + my $marc = C4::Biblio::GetMarcBiblio({ + biblionumber => $row->biblionumber, + embed_items => 1 + }); + my $next = eval { + __PACKAGE__->new($marc, $row->biblionumber); + }; + if ($@) { + warn "Something went wrong reading record for biblio $row->biblionumber: $@\n"; + next; + } + return $next; + } + }; + } return Koha::MetadataIterator->new($next_func); } diff --git a/Koha/Items.pm b/Koha/Items.pm index 7b86c5a5e0..29b46949de 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -53,6 +53,72 @@ sub object_class { return 'Koha::Item'; } +=head2 search_unblessed + + $items = Koha::Items->search_unblessed($conditions); + +In scalar context, returns an array reference with hashes containing item data, in list context returns an array. +Calling search_unblessed should produce the same result as calling Koha::Items->search and iterating +though the result unblessing each item. In cases where you only need the item data using this optimized +method is much faster. The arguments accepted are a subset and approximation of those supported by +Koha::items->search, with only the "barcode" and "itemnumber" attributes supported as conditions. +If called with a non-hash conditions argument a search will be performed as if "itemnumber" was the +provided condition. Only one condition is accepted and if more are provided the method will die with +an error. + +=cut + +sub search_unblessed { + my ($self, $conditions) = @_; + my $items = []; + my $field_name; + if (ref($conditions) eq 'HASH') { + my %valid_condition_fields = ( + 'barcode' => undef, + 'itemnumber' => undef, + ); + my @fields = keys %{$conditions}; + if (@fields == 1) { + foreach my $field (@fields) { + die("Invalid condition: \"$field\"") unless exists $valid_condition_fields{$field}; + } + } + elsif(@fields > 1) { + die("Multiple conditions are not supported"); + } + else { + die("No conditions provided"); + } + } + else { + $conditions = { + 'itemnumber' => $conditions, + }; + } + # Only accepts one condition + my ($field, $values) = (%{$conditions}); + + if (ref($values) eq 'ARRAY' && @{$values} == 1) { + ($values) = @{$values}; + } + if (!ref($values)) { + my $item = C4::Context->dbh->selectrow_hashref(qq{SELECT * FROM items WHERE $field = ?}, undef, $values); + push @{$items}, $item; + } + elsif (ref($values) eq 'ARRAY') { + my $in_placeholders = join ', ', (('?') x @{$values}); + my $sth = C4::Context->dbh->prepare(qq{SELECT * FROM items WHERE $field IN($in_placeholders)}); + $sth->execute(@{$values}); + while (my $item = $sth->fetchrow_hashref) { + push @{$items}, $item; + } + } + else { + die("Invalid value for field: \"$field\""); + } + return wantarray ? @{$items} : $items; +} + =head1 AUTHOR Kyle M Hall diff --git a/misc/search_tools/rebuild_elastic_search.pl b/misc/search_tools/rebuild_elastic_search.pl index caa9a3d870..bde30e4453 100755 --- a/misc/search_tools/rebuild_elastic_search.pl +++ b/misc/search_tools/rebuild_elastic_search.pl @@ -131,7 +131,7 @@ if ($index_biblios) { return ($r, Koha::BiblioUtils->get_from_biblionumber($r, item_data => 1 )); }; } else { - my $records = Koha::BiblioUtils->get_all_biblios_iterator(); + my $records = Koha::BiblioUtils->get_all_biblios_iterator({ batched => 1 }); $next = sub { $records->next(); } diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index bc7aaa9feb..387e750f33 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -42,7 +42,9 @@ my $location = 'My Location'; subtest 'General Add, Get and Del tests' => sub { - plan tests => 16; + plan tests => 25; + + my $item_barcode = 123; $schema->storage->txn_begin; @@ -59,13 +61,21 @@ subtest 'General Add, Get and Del tests' => sub { my ($bibnum, $bibitemnum) = get_biblio(); # Add an item. - my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, itype => $itemtype->{itemtype} } , $bibnum); + my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + location => $location, + itype => $itemtype->{itemtype}, + barcode => $item_barcode + }, $bibnum + ); cmp_ok($item_bibnum, '==', $bibnum, "New item is linked to correct biblionumber."); cmp_ok($item_bibitemnum, '==', $bibitemnum, "New item is linked to correct biblioitemnumber."); # Get item. my $getitem = GetItem($itemnumber); cmp_ok($getitem->{'itemnumber'}, '==', $itemnumber, "Retrieved item has correct itemnumber."); + cmp_ok($getitem->{'barcode'}, '==', $item_barcode, "Retrieved item has correct barcode."); cmp_ok($getitem->{'biblioitemnumber'}, '==', $item_bibitemnum, "Retrieved item has correct biblioitemnumber."); is( $getitem->{location}, $location, "The location should not have been modified" ); is( $getitem->{permanent_location}, $location, "The permanent_location should have been set to the location value" ); @@ -75,18 +85,77 @@ subtest 'General Add, Get and Del tests' => sub { my $dbh = C4::Context->dbh; local $dbh->{RaiseError} = 1; ModItem({}, $bibnum, $itemnumber); + $getitem = GetItem(undef, $item_barcode); + cmp_ok($getitem->{'itemnumber'}, '==', $itemnumber, "Item retrieved by barcode has correct itemnumber."); # Modify item; setting barcode. - ModItem({ barcode => '987654321' }, $bibnum, $itemnumber); + $item_barcode = '987654321'; + ModItem({ barcode => $item_barcode }, $bibnum, $itemnumber); my $moditem = GetItem($itemnumber); - cmp_ok($moditem->{'barcode'}, '==', '987654321', 'Modified item barcode successfully to: '.$moditem->{'barcode'} . '.'); + cmp_ok($moditem->{'barcode'}, '==', $item_barcode, 'Modified item barcode successfully to: ' . $moditem->{'barcode'} . '.'); + + # Update some more properties to get larger coverage + my $item_properties = { + 'dateaccessioned' => '2012-12-12', + 'datelastborrowed' => '2013-12-12', + 'datelastseen' => '2013-12-12', + 'notforloan' => '0', + 'damaged' => '0', + 'itemlost' => '0', + 'itemcallnumber' => '1234', + 'restricted' => '0', + }; + + # Clone item propreties since ModItem modifies this argument + # causing later tests to fail + ModItem({%{$item_properties}}, $bibnum, $itemnumber); + + # Search unblessed tests + my $search_conditions = { + 'itemnumber' => $itemnumber, + }; + my ($item_object) = Koha::Items->search($search_conditions); + my $item_object_data = $item_object->unblessed; + cmp_ok($item_object_data->{'itemnumber'}, '==', $itemnumber, "Item object retrieved by Koha::Items->search using \"itemnumber\" condition has correct itemnumber."); + + # Intersect with updated properties + my $updated_item_properties = { map { ($_ => $item_object_data->{$_}) } keys %{$item_properties} }; + + is_deeply($updated_item_properties, $item_properties, "Updated item properties have correct values."); + + my ($item_data) = Koha::Items->search_unblessed($search_conditions); + cmp_ok( + $item_data->{'itemnumber'}, + '==', + $itemnumber, + "Item data retrieved by Koha::Items->search_unblessed using \"itemnumber\" condition has correct itemnumber." + ); + + ($item_data) = Koha::Items->search_unblessed({ 'barcode' => $item_barcode }); + cmp_ok( + $item_data->{'itemnumber'}, + '==', + $itemnumber, + "Item data retrieved by Koha::Items->search_unblessed using \"barcode\" condition has correct itemnumber." + ); + + is_deeply($item_object_data, $item_data, "Item data retrieved by unblessing item object is identical to item data from Koha::Items->search_unblessed."); # Delete item. DelItem({ biblionumber => $bibnum, itemnumber => $itemnumber }); my $getdeleted = GetItem($itemnumber); is($getdeleted->{'itemnumber'}, undef, "Item deleted as expected."); - ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, permanent_location => 'my permanent location', itype => $itemtype->{itemtype} } , $bibnum); + ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + location => $location, + permanent_location => 'my permanent location', + itype => $itemtype->{itemtype}, + barcode => $item_barcode + }, + $bibnum + ); $getitem = GetItem($itemnumber); is( $getitem->{location}, $location, "The location should not have been modified" ); is( $getitem->{permanent_location}, 'my permanent location', "The permanent_location should not have modified" ); @@ -104,6 +173,11 @@ subtest 'General Add, Get and Del tests' => sub { t::lib::Mocks::mock_preference('item-level_itypes', '1'); $getitem = GetItem($itemnumber); is( $getitem->{itype}, $itemtype->{itemtype}, "Itemtype set correctly when using item-level_itypes" ); + # Good to test this since different code for retrieving items is run in GetItems() depending on item-level_itypes preference + is( $getitem->{itemnumber}, $itemnumber, "Item successfully retrieved by itemnumber when using item-level_itypes" ); + $getitem = GetItem(undef, $item_barcode); + is( $getitem->{itemnumber}, $itemnumber, "Item successfully retrieved by barcode when using item-level_itypes" ); + t::lib::Mocks::mock_preference('item-level_itypes', '0'); $getitem = GetItem($itemnumber); is( $getitem->{itype}, undef, "Itemtype set correctly when not using item-level_itypes" ); -- 2.11.0