Browse Source

Bug 14057: Inventory is painfully slow

This patch is a rough start. I believe it runs exponentially
faster, but its equality to the previous version needs to be
tested before I clean it up to acceptable standards.

Nested hashes of hashes was being a debugging nightmare.

Moved the SQL select to C4::Koha.

Changed the GetItemsForInventory to have a hashref parameter.
Added interface, in case there is a need for 'opac' vs. 'staff'.

Added t/db_dependent/Items/GetItemsForInventory.t
Added t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
3.20.x
Mark Tompsett 7 years ago
committed by Tomas Cohen Arazi
parent
commit
4e0468e8c2
  1. 41
      C4/Items.pm
  2. 45
      C4/Koha.pm
  3. 156
      t/db_dependent/Items/GetItemsForInventory.t
  4. 54
      t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t
  5. 30
      tools/inventory.pl

41
C4/Items.pm

@ -1024,7 +1024,20 @@ sub GetLostItems {
=head2 GetItemsForInventory
($itemlist, $iTotalRecords) = GetItemsForInventory($minlocation, $maxlocation, $location, $itemtype, $ignoreissued, $datelastseen, $branchcode, $offset, $size, $statushash);
($itemlist, $iTotalRecords) = GetItemsForInventory( {
minlocation => $minlocation,
maxlocation => $maxlocation,
location => $location,
itemtype => $itemtype,
ignoreissued => $ignoreissued,
datelastseen => $datelastseen,
branchcode => $branchcode,
branch => $branch,
offset => $offset,
size => $size,
stautshash => $statushash
interface => $interface,
} );
Retrieve a list of title/authors/barcode/callnumber, for biblio inventory.
@ -1042,7 +1055,20 @@ $iTotalRecords is the number of rows that would have been returned without the $
=cut
sub GetItemsForInventory {
my ( $minlocation, $maxlocation,$location, $itemtype, $ignoreissued, $datelastseen, $branchcode, $branch, $offset, $size, $statushash ) = @_;
my ( $parameters ) = @_;
my $minlocation = $parameters->{'minlocation'} // '';
my $maxlocation = $parameters->{'maxlocation'} // '';
my $location = $parameters->{'location'} // '';
my $itemtype = $parameters->{'itemtype'} // '';
my $ignoreissued = $parameters->{'ignoreissued'} // '';
my $datelastseen = $parameters->{'datelastseen'} // '';
my $branchcode = $parameters->{'branchcode'} // '';
my $branch = $parameters->{'branch'} // '';
my $offset = $parameters->{'offset'} // '';
my $size = $parameters->{'size'} // '';
my $statushash = $parameters->{'statushash'} // '';
my $interface = $parameters->{'interface'} // '';
my $dbh = C4::Context->dbh;
my ( @bind_params, @where_strings );
@ -1121,16 +1147,15 @@ sub GetItemsForInventory {
$sth->execute( @bind_params );
my ($iTotalRecords) = $sth->fetchrow_array();
my $avmapping = C4::Koha::GetKohaAuthorisedValuesMapping( {
interface => $interface
} );
foreach my $row (@$tmpresults) {
# Auth values
foreach (keys %$row) {
# If the koha field is mapped to a marc field
my ($f, $sf) = GetMarcFromKohaField("items.$_", $row->{'frameworkcode'});
if ($f and $sf) {
# We replace the code with it's description
my $authvals = C4::Koha::GetKohaAuthorisedValuesFromField($f, $sf, $row->{'frameworkcode'});
$row->{$_} = $authvals->{$row->{$_}} if defined $authvals->{$row->{$_}};
if (defined($avmapping->{"items.$_,".$row->{'frameworkcode'}.",".$row->{$_}})) {
$row->{$_} = $avmapping->{"items.$_,".$row->{'frameworkcode'}.",".$row->{$_}};
}
}
push @results, $row;

45
C4/Koha.pm

@ -62,6 +62,7 @@ BEGIN {
&IsAuthorisedValueCategory
&GetKohaAuthorisedValues
&GetKohaAuthorisedValuesFromField
&GetKohaAuthorisedValuesMapping
&GetKohaAuthorisedValueLib
&GetAuthorisedValueByCode
&GetKohaImageurlFromAuthorisedValues
@ -1327,6 +1328,50 @@ sub GetKohaAuthorisedValuesFromField {
}
}
=head2 GetKohaAuthorisedValuesMapping
Takes a hash as a parameter. The interface key indicates the
description to use in the mapping.
Returns hashref of:
"{kohafield},{frameworkcode},{authorised_value}" => "{description}"
for all the kohafields, frameworkcodes, and authorised values.
Returns undef if nothing is found.
=cut
sub GetKohaAuthorisedValuesMapping {
my ($parameter) = @_;
my $interface = $parameter->{'interface'} // '';
my $query_mapping = q{
SELECT TA.kohafield,TA.authorised_value AS category,
TA.frameworkcode,TB.authorised_value,
IF(TB.lib_opac>'',TB.lib_opac,TB.lib) AS OPAC,
TB.lib AS Intranet,TB.lib_opac
FROM marc_subfield_structure AS TA JOIN
authorised_values as TB ON
TA.authorised_value=TB.category
WHERE TA.kohafield>'' AND TA.authorised_value>'';
};
my $dbh = C4::Context->dbh;
my $sth = $dbh->prepare($query_mapping);
$sth->execute();
my $avmapping;
if ($interface eq 'opac') {
while (my $row = $sth->fetchrow_hashref) {
$avmapping->{$row->{kohafield}.",".$row->{frameworkcode}.",".$row->{authorised_value}} = $row->{OPAC};
}
}
else {
while (my $row = $sth->fetchrow_hashref) {
$avmapping->{$row->{kohafield}.",".$row->{frameworkcode}.",".$row->{authorised_value}} = $row->{Intranet};
}
}
return $avmapping;
}
=head2 xml_escape
my $escaped_string = C4::Koha::xml_escape($string);

156
t/db_dependent/Items/GetItemsForInventory.t

@ -0,0 +1,156 @@
#!/usr/bin/perl
#
# This file is part of Koha.
#
# Copyright (c) 2015 Mark Tompsett
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Test::More tests => 6;
$| = 1;
BEGIN {
use_ok('C4::Context');
use_ok('C4::Items');
use_ok('C4::Biblio');
use_ok('C4::Koha');
}
can_ok('C4::Items','GetItemsForInventory');
my $dbh = C4::Context->dbh;
$dbh->{AutoCommit} = 0;
$dbh->{RaiseError} = 1;
diag("This could take a while for large datasets...");
my ($oldResults, $oldCount) = OldWay($dbh);
my ($newResults, $newCount) = GetItemsForInventory( { interface => 'staff' } );
is_deeply($newResults,$oldResults,"Inventory results unchanged.");
$dbh->rollback;
sub OldWay {
my ($tdbh) = @_;
my $ldbh = $tdbh;
my $minlocation = '';
my $maxlocation = '';
my $location = '';
my $itemtype = '';
my $ignoreissued = '';
my $datelastseen = '';
my $branchcode = '';
my $branch = '';
my $offset = '';
my $size = '';
my $statushash = '';
my $interface = '';
my ( @bind_params, @where_strings );
my $select_columns = q{
SELECT items.itemnumber, barcode, itemcallnumber, title, author, biblio.biblionumber, biblio.frameworkcode, datelastseen, homebranch, location, notforloan, damaged, itemlost, withdrawn, stocknumber
};
my $select_count = q{SELECT COUNT(*)};
my $query = q{
FROM items
LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber
LEFT JOIN biblioitems on items.biblionumber = biblioitems.biblionumber
};
if ($statushash){
for my $authvfield (keys %$statushash){
if ( scalar @{$statushash->{$authvfield}} > 0 ){
my $joinedvals = join ',', @{$statushash->{$authvfield}};
push @where_strings, "$authvfield in (" . $joinedvals . ")";
}
}
}
if ($minlocation) {
push @where_strings, 'itemcallnumber >= ?';
push @bind_params, $minlocation;
}
if ($maxlocation) {
push @where_strings, 'itemcallnumber <= ?';
push @bind_params, $maxlocation;
}
if ($datelastseen) {
$datelastseen = format_date_in_iso($datelastseen);
push @where_strings, '(datelastseen < ? OR datelastseen IS NULL)';
push @bind_params, $datelastseen;
}
if ( $location ) {
push @where_strings, 'items.location = ?';
push @bind_params, $location;
}
if ( $branchcode ) {
if($branch eq "homebranch"){
push @where_strings, 'items.homebranch = ?';
}else{
push @where_strings, 'items.holdingbranch = ?';
}
push @bind_params, $branchcode;
}
if ( $itemtype ) {
push @where_strings, 'biblioitems.itemtype = ?';
push @bind_params, $itemtype;
}
if ( $ignoreissued) {
$query .= "LEFT JOIN issues ON items.itemnumber = issues.itemnumber ";
push @where_strings, 'issues.date_due IS NULL';
}
if ( @where_strings ) {
$query .= 'WHERE ';
$query .= join ' AND ', @where_strings;
}
$query .= ' ORDER BY items.cn_sort, itemcallnumber, title';
my $count_query = $select_count . $query;
$query .= " LIMIT $offset, $size" if ($offset and $size);
$query = $select_columns . $query;
my $sth = $ldbh->prepare($query);
$sth->execute( @bind_params );
my @results = ();
my $tmpresults = $sth->fetchall_arrayref({});
$sth = $ldbh->prepare( $count_query );
$sth->execute( @bind_params );
my ($iTotalRecords) = $sth->fetchrow_array();
foreach my $row (@$tmpresults) {
# Auth values
foreach my $field (sort keys %$row) {
# If the koha field is mapped to a marc field
my ($f, $sf) = C4::Biblio::GetMarcFromKohaField("items.$field", $row->{'frameworkcode'});
if (defined($f) and defined($sf)) {
# We replace the code with it's description
my $authvals = C4::Koha::GetKohaAuthorisedValuesFromField($f, $sf, $row->{'frameworkcode'});
$row->{$field} = $authvals->{$row->{$field}} if defined $authvals && defined $row->{$field} && defined $authvals->{$row->{$field}};
}
}
push @results, $row;
}
return (\@results, $iTotalRecords);
}

54
t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t

@ -0,0 +1,54 @@
#!/usr/bin/perl
#
# This file is part of Koha.
#
# Copyright (c) 2015 Mark Tompsett
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use C4::Context;
use Test::More tests => 8;
BEGIN {
use_ok('C4::Context');
use_ok('C4::Koha');
}
can_ok('C4::Koha','GetKohaAuthorisedValuesMapping');
my $avMappingOPAC;
my $avMappingStaff;
my $avMappingUndef1;
my $avMappingUndef2;
my $avMappingUndef3;
SKIP: {
my $dbh = C4::Context->dbh;
my $count = $dbh->selectrow_arrayref("SELECT COUNT(*) FROM marc_subfield_structure WHERE kohafield LIKE 'item%';");
skip "Lacking item mappings in marc_subfield_structure",5 unless ($count && $count->[0]>0);
$count = $dbh->selectrow_arrayref("SELECT COUNT(*) FROM authorised_values;");
skip "Lacking authorised_values",5 unless ($count && $count->[0]>0);
$avMappingOPAC = GetKohaAuthorisedValuesMapping( { interface => 'opac' });
$avMappingStaff = GetKohaAuthorisedValuesMapping( { interface => 'staff' });
$avMappingUndef1 = GetKohaAuthorisedValuesMapping( { interface => undef });
$avMappingUndef2 = GetKohaAuthorisedValuesMapping( { } );
$avMappingUndef3 = GetKohaAuthorisedValuesMapping();
is_deeply($avMappingUndef1,$avMappingStaff,"Undefined interface = Staff test 1");
is_deeply($avMappingUndef2,$avMappingStaff,"Undefined interface = Staff test 2");
is_deeply($avMappingUndef3,$avMappingStaff,"Undefined interface = Staff test 3");
isnt($avMappingOPAC ,undef,"OPAC has a mapping");
isnt($avMappingStaff,undef,"Staff has a mapping");
}

30
tools/inventory.pl

@ -234,10 +234,36 @@ if ( $markseen or $op ) {
# We use datelastseen only when comparing the results to the barcode file.
my $paramdatelastseen = ($compareinv2barcd) ? $datelastseen : '';
($inventorylist, $totalrecords) = GetItemsForInventory($minlocation, $maxlocation, $location, $itemtype, $ignoreissued, $paramdatelastseen, $branchcode, $branch, 0, undef, $staton);
($inventorylist, $totalrecords) = GetItemsForInventory( {
minlocation => $minlocation,
maxlocation => $maxlocation,
location => $location,
itemtype => $itemtype,
ignoreissued => $ignoreissued,
datelastseen => $paramdatelastseen,
branchcode => $branchcode,
branch => $branch,
offset => 0,
size => undef,
statushash => $staton,
interface => 'staff',
} );
# For the items that may be marked as "wrong place", we only check the location (callnumbers, location and branch)
($wrongplacelist, $totalrecords) = GetItemsForInventory($minlocation, $maxlocation, $location, undef, undef, undef, $branchcode, $branch, 0, undef, undef);
($wrongplacelist, $totalrecords) = GetItemsForInventory( {
minlocation => $minlocation,
maxlocation => $maxlocation,
location => $location,
itemtype => undef,
ignoreissued => undef,
datelastseen => undef,
branchcode => $branchcode,
branch => $branch,
offset => 0,
size => undef,
statushash => undef,
interface => 'staff',
} );
}

Loading…
Cancel
Save