Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 28479: Use primary keys to check object existence in TestBuilder

Browse source 
The TestBuilder::build_object function used any foreign keys to check
whether an object already exists or not. This brought incorrectly
results of unrelated objects because using any other keys other than
primary keys don't guarantee our results to point to one single
object. For example, as is put here in the unit test, if you created
two items with the same biblionumber and then tried to create a hold
using build_object() we were using the biblionumber to check whether
an item was linked to the hold already. Thus, we were checking whether
a random item was already linked to the hold instead of the one we
wanted either by passing it explicitly to build_object() or the one
build_object() created implicitly. This also resulted in following
warnings when there were more than one match:

DBIx::Class::Storage::DBI::select_single(): Query returned more than
one row.  SQL that returns multiple rows is DEPRECATED for ->find and
->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235

To test:
 $ prove t/db_dependent

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Joonas Kylmälä 2021-05-28 13:33:24 +03:00 committed by Kyle M Hall
parent 839f61cf97
commit 188479881f
2 changed files with 30 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
t/db_dependent/TestBuilder.t
View file

@ -21,7 +21,7 @@ use Modern::Perl;
use utf8;
use Test::More tests => 14;
use Test::More tests => 15;
use Test::Warn;
use File::Basename qw(dirname);
@ -477,3 +477,24 @@ subtest 'build_sample_biblio() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'Existence of object is only checked using primary keys' => sub {
plan tests => 1;
$schema->storage->txn_begin;
my $biblio = $builder->build_sample_biblio();
my $item1 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber });
my $item2 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber });
warnings_are {
$builder->build_object({
class => 'Koha::Holds',
value => {
biblionumber => $biblio->biblionumber
}
});
} [], "No warning about query returning more than one row";
$schema->storage->txn_rollback;
};

9
t/lib/TestBuilder.pm
View file

@ -231,8 +231,15 @@ sub _create_links {
if( $cnt_scalar == @$keys ) {
# if one or more fk cols are null, the FK constraint will not be forced
return {} if $cnt_null > 0;
# does the record exist already?
return {} if $self->schema->resultset($linked_tbl)->find( $fk_value );
my @pks = $self->schema->source( $linked_tbl )->primary_columns;
my %fk_pk_value;
for (@pks) {
$fk_pk_value{$_} = $fk_value->{$_} if defined $fk_value->{$_};
}
return {} if !(keys %fk_pk_value);
return {} if $self->schema->resultset($linked_tbl)->find( \%fk_pk_value );
}
# create record with a recursive build call
my $row = $self->build({ source => $linked_tbl, value => $fk_value });