Attachment 121858 Details for Bug 28479 – Bug 28479: Use primary keys to check object existence in TestBuilder

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

Bug-28479-Use-primary-keys-to-check-object-existen.patch (text/plain), 3.53 KB, created by Nick Clemens (kidclamp) on 2021-06-11 15:47:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2021-06-11 15:47:22 UTC

Size: 3.53 KB

patch

obsolete

>From db94d5a6609a2a2cd8e2756d98f424830823f7eb Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
>Date: Fri, 28 May 2021 13:33:24 +0300
>Subject: [PATCH] Bug 28479: Use primary keys to check object existence in
> TestBuilder
>
>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>
>---
> t/db_dependent/TestBuilder.t | 23 ++++++++++++++++++++++-
> t/lib/TestBuilder.pm         |  9 ++++++++-
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
>diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t
>index 7d07469196..f97626c06b 100755
>--- a/t/db_dependent/TestBuilder.t
>+++ b/t/db_dependent/TestBuilder.t
>@@ -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;
>+};
>diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm
>index a39be70866..d8897b8145 100644
>--- a/t/lib/TestBuilder.pm
>+++ b/t/lib/TestBuilder.pm
>@@ -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 });
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 28479: 121473 | 121501 | 121858