Koha/t/lib/TestBuilder.pm
Marcel de Rooy 99672d1f00 Bug 16155: Composite keys in TestBuilder and more
The number of tests using TestBuilder is gradually growing. Once you
are familiar with its use, you will appreciate it and find yourself
using it when writing new tests. Although it works quite well, some details
still needs some polishing.

While looking at the handling of composite keys in TestBuilder, a number
of related points came up too. This patch finally ended up in setting
the following design goals:

[1] TB should not only look at the first column of a composite FK.
[2] TB should optionally create records for fixed FK values.
[3] TB should create a new record, never change an existing record.
[4] TB should respect auto_increment columns.
[5] Passing a hash for a foreign key should always imply a new record.
[6] Explicitly passing undef for a column should mean NULL.
[7] Add a delete method; it will replace the clear method.
[8] Simplify by removing $default_values, hash key _fk and param only_fk.

The comments below further clarify these points. Note that they refer to
the old behavior (before this patch) unless stated otherwise.

Comments for point 1
====================
Look at:
    $builder->build({
        source => 'UserPermission',
        value => {
            borrowernumber => $borrowerno,
            module_bit => { flag => 'my flag' },
            code => 'will_be_ignored',
        },
    });
Module_bit and code here are a composite FK to permissions.
TB ignores the value for the code column here and creates a record with a
randomized code.

But if we would pass the hash in the second column of this compound key like:
            borrowernumber => $borrowerno,
            module_bit => 10,
            code => { code => 'new_code' },
TB would now crash when passing the hash for code thru to DBIx.

Comments for point 2
====================
Look at:
    $builder->build({
        source => 'UserPermission',
        value => {
            borrowernumber => $borrowerno,
            module_bit => 99,
            code => 'new_super_tool',
        },
    });
TB detects a fixed value for the module_bit, continues and will crash on
DBIx if the foreign keys are not found. In this case it would be friendly
to create a missing linked record.

Comments for point 3
====================
Look at:
    $builder->build({
        source => 'Branch',
        value  => { branchcode => 'CPL' },
    });
If this branch already exists, this call would modify the branch record and
overwrite all columns with randomized values (expected or not). In any case,
it would be safer here to return undef than modifying an existing record.

Comments for point 4
====================
Look at:
    $builder->build({
        source => 'Borrower',
        value  => { borrowernumber => '123456789' },
    });
If this number would exist, we would update (as earlier). But if this
number does not exist, we would create the record. Although that is
technically possible, I would prefer to have TB respect the auto
increment property of this column.

Comments for point 5
====================
Look at:
    $builder->build({
        source => 'Item',
        value  => { homebranch => { branchcode => 'MPL' } },
    });
As discussed under point 3, we should actually not pass primary key values
if we expect to build new records. The same also holds for the deeper
(recursive) calls to build when using hashes like here for homebranch.
In this case again an existing record might be overwritten.

Comments for point 6
====================
Look at:
    $builder->build({
        source => 'Reserve',
        value => { itemnumber => undef },
    });
As you know, a reserve without an itemnumber is a biblio level hold.
Unfortunately, TB did not care about passing undefs until now. So you would
get an item level hold.
In the new situation TB will respect these explicit undefs, as long as the
corresponding foreign key column is nullable of course.

Comments for point 7
====================
This patch will allow you to delete records created by TB:
    my $patron = $builder->build({ source => 'Borrower' });
    $builder->delete({ source => 'Borrower', records => $patron });
Or:
    $builder->delete({ source => 'Borrower', records => [ $patron, ... ] });
For safety, delete requires you to provide all primary key values in the
passed hashref(s).
Deleting all records in a table via clear is no longer supported and can
still be arranged in one statement.

Comments for point 8
====================
Current use of TestBuilder reveals that $default_values and only_fk
are not really needed. The current $default_values should imo not be in the
module anyway; if you want to use it, you could still pass it to TB:
    $builder->build({ ..., value => { %defa, %your_values } });

Only_fk stops at the very last step of saving the top level record while
storing all linked records at the lower levels. Practical use is not
very obvious; it can be easily simulated by one delete statement.

The hash key _fk is now used to store all linked records one or more levels
down. It is used in a few tests to retrieve a value one level down.
Why not retrieve that one value via the database and get rid of the
whole structure?

Implementation
==============
This highlights the main changes:

The $default_value hash (with some hardcoded values for UserPermission)
is removed from the module. It was used by a test in TestBuilder.t and has
been relocated.
The value of $gen_type is returned now by sub _gen_type. (See new.)

The main change in the build method is moving the foreign keys logic to a
new subroutine _create_links. This routine now looks at all columns of a
composite FK. It checks if a linked record exists for passed values, and
it looks at NULL values.

Routine _buildColumnValues is slightly adjusted to allow for passed undef
values. The theoretically endless loop is replaced by three tries. For
composite unique constraints we only check complete sets of values.

Routine _getForeignKeys contains a check to not return the same relation
twice in case of doubled belongs_to relations in the DBIx scheme.

The eval in _storeColumnValues is removed. The autoincrement check in sub
_buildColumnValue got more priority; the handling of foreign keys has been
adjusted and a check for not-nullable columns has been added.

TEST PLAN
=========
This patch only deals with the TestBuilder module itself. In the follow-up
patches TestBuilder.t and a few other unit tests are adjusted.

[1] Do not yet apply this patch. But apply the 'OldBehavior' patch.
    Verify that all tests pass. (They cover the first six design goals.)
[2] Apply this patch. Does TestBuilder still compile (perl -c)?
[3] Run the OldBehavior test again. Do all tests fail now? This means
    that we got rid of all unwanted side-effects in the list of goals.
[4] Run some other tests that use TestBuilder. (See below.)
    Skip the following tests; a follow-up patch deals with them.
    t/db_dependent/Accounts.t
    t/db_dependent/Barcodes.t
    t/db_dependent/Circulation/AnonymiseIssueHistory.t
    t/db_dependent/Circulation/CalcFine.t
    t/db_dependent/Holds.t
    t/db_dependent/Items/MoveItemFromBiblio.t
    t/db_dependent/Koha/BiblioFrameworks.t
    t/db_dependent/Members.t
    t/db_dependent/TestBuilder.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The following tests pass:
    t/db_dependent/Acquisition/OrderUsers.t
    t/db_dependent/Auth/haspermission.t
    t/db_dependent/Barcodes_ValueBuilder.t
    t/db_dependent/Budgets.t
    t/db_dependent/Category.t
    t/db_dependent/Circulation.t
    t/db_dependent/Circulation/GetTopIssues.t
    t/db_dependent/Circulation/IsItemIssued.t
    t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
    t/db_dependent/Circulation/Returns.t
    t/db_dependent/Circulation/TooMany.t
    t/db_dependent/Circulation_dateexpiry.t
    t/db_dependent/Circulation_transfers.t
    t/db_dependent/CourseReserves.t
    t/db_dependent/Creators/Lib.t
    t/db_dependent/DecreaseLoanHighHolds.t
    t/db_dependent/Exporter/Record.t
    t/db_dependent/Holds/LocalHoldsPriority.t
    t/db_dependent/Holds/RevertWaitingStatus.t:
    t/db_dependent/HoldsQueue.t
    t/db_dependent/Holidays.t
    t/db_dependent/Items.t
    t/db_dependent/Items_DelItem.t
    t/db_dependent/Koha/Acquisition/Currencies.t
    t/db_dependent/Koha/Authorities.t
    t/db_dependent/Koha/BiblioFrameworks.t
    t/db_dependent/Koha/Cities.t
    t/db_dependent/Koha/Libraries.t
    t/db_dependent/Koha/Objects.t
    t/db_dependent/Koha/Patron/Categories.t
    t/db_dependent/Koha/Patron/Images.t
    t/db_dependent/Koha/Patron/Messages.t
    t/db_dependent/Koha/Patrons.t
    t/db_dependent/Koha/SMS_Providers.t
    t/db_dependent/Koha_template_plugin_Branches.t
    t/db_dependent/Letters.t
    t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
    t/db_dependent/Members/GetUpcomingMembershipExpires.t
    t/db_dependent/Members_Attributes.t
    t/db_dependent/Patron/Borrower_Debarments.t
    t/db_dependent/Patron/Borrower_Discharge.t
    t/db_dependent/Patron/Borrower_Files.t
    t/db_dependent/Ratings.t
    t/db_dependent/Reports_Guided.t
    t/db_dependent/Reserves/GetReserveFee.t
    t/db_dependent/Review.t
    t/db_dependent/Serials_2.t
    t/db_dependent/ShelfBrowser.t
    t/db_dependent/Virtualshelves.t
    t/db_dependent/api/v1/patrons.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:47:57 +00:00

510 lines
16 KiB
Perl

package t::lib::TestBuilder;
use Modern::Perl;
use Koha::Database;
use String::Random;
sub new {
my ($class) = @_;
my $self = {};
bless( $self, $class );
$self->schema( Koha::Database->new()->schema );
$self->schema->storage->sql_maker->quote_char('`');
$self->{gen_type} = _gen_type();
return $self;
}
sub schema {
my ($self, $schema) = @_;
if( defined( $schema ) ) {
$self->{schema} = $schema;
}
return $self->{schema};
}
# sub clear has been obsoleted; use delete_all from the schema resultset
sub delete {
my ( $self, $params ) = @_;
my $source = $params->{source} || return;
my @recs = ref( $params->{records} ) eq 'ARRAY'?
@{$params->{records}}: ( $params->{records} // () );
# tables without PK are not supported
my @pk = $self->schema->source( $source )->primary_columns;
return if !@pk;
my $rv = 0;
foreach my $rec ( @recs ) {
# delete only works when you supply full primary key values
# $cond does not include searches for undef (not allowed in PK)
my $cond = { map { defined $rec->{$_}? ($_, $rec->{$_}): (); } @pk };
next if keys %$cond < @pk;
$self->schema->resultset( $source )->search( $cond )->delete;
# we clear the pk columns in the supplied hash
# this indirectly signals at least an attempt to delete
map { delete $rec->{$_}; } @pk;
$rv++;
}
return $rv;
}
sub build {
# build returns a hash of column values for a created record, or undef
# build does NOT update a record, or pass back values of an existing record
my ($self, $params) = @_;
my $source = $params->{source} || return;
my $value = $params->{value};
my $col_values = $self->_buildColumnValues({
source => $source,
value => $value,
});
return if !$col_values; # did not meet unique constraints?
# loop thru all fk and create linked records if needed
# fills remaining entries in $col_values
my $foreign_keys = $self->_getForeignKeys( { source => $source } );
for my $fk ( @$foreign_keys ) {
# skip when FK points to itself: e.g. borrowers:guarantorid
next if $fk->{source} eq $source;
my $keys = $fk->{keys};
my $tbl = $fk->{source};
my $res = $self->_create_links( $tbl, $keys, $col_values, $value );
return if !$res; # failed: no need to go further
foreach( keys %$res ) { # save new values
$col_values->{$_} = $res->{$_};
}
}
# store this record and return hashref
return $self->_storeColumnValues({
source => $source,
values => $col_values,
});
}
# ------------------------------------------------------------------------------
# Internal helper routines
sub _create_links {
# returns undef for failure to create linked records
# otherwise returns hashref containing new column values for parent record
my ( $self, $linked_tbl, $keys, $col_values, $value ) = @_;
my $fk_value = {};
my ( $cnt_scalar, $cnt_null ) = ( 0, 0 );
# First, collect all values for creating a linked record (if needed)
foreach my $fk ( @$keys ) {
my ( $col, $destcol ) = ( $fk->{col_name}, $fk->{col_fk_name} );
if( ref( $value->{$col} ) eq 'HASH' ) {
# add all keys from the FK hash
$fk_value = { %{ $value->{$col} }, %$fk_value };
}
if( exists $col_values->{$col} ) {
# add specific value (this does not necessarily exclude some
# values from the hash in the preceding if)
$fk_value->{ $destcol } = $col_values->{ $col };
$cnt_scalar++;
$cnt_null++ if !defined( $col_values->{$col} );
}
}
# If we saw all FK columns, first run the following checks
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 );
}
# create record with a recursive build call
my $row = $self->build({ source => $linked_tbl, value => $fk_value });
return if !$row; # failure
# Finally, only return the new values
my $rv = {};
foreach my $fk ( @$keys ) {
my ( $col, $destcol ) = ( $fk->{col_name}, $fk->{col_fk_name} );
next if exists $col_values->{ $col };
$rv->{ $col } = $row->{ $destcol };
}
return $rv; # success
}
sub _formatSource {
my ($params) = @_;
my $source = $params->{source} || return;
$source =~ s|(\w+)$|$1|;
return $source;
}
sub _buildColumnValues {
my ($self, $params) = @_;
my $source = _formatSource( $params ) || return;
my $original_value = $params->{value};
my $col_values = {};
my @columns = $self->schema->source($source)->columns;
my %unique_constraints = $self->schema->source($source)->unique_constraints();
my $build_value = 3;
# we try max three times if there are unique constraints
BUILD_VALUE: while ( $build_value ) {
# generate random values for all columns
for my $col_name( @columns ) {
my $valref = $self->_buildColumnValue({
source => $source,
column_name => $col_name,
value => $original_value,
});
return if !$valref; # failure
if( @$valref ) { # could be empty
# there will be only one value, but it could be undef
$col_values->{$col_name} = $valref->[0];
}
}
# verify the data would respect each unique constraint
# note that this is INCOMPLETE since not all col_values are filled
CONSTRAINTS: foreach my $constraint (keys %unique_constraints) {
my $condition;
my $constraint_columns = $unique_constraints{$constraint};
# loop through all constraint columns and build the condition
foreach my $constraint_column ( @$constraint_columns ) {
# build the filter
# if one column does not exist or is undef, skip it
# an insert with a null will not trigger the constraint
next CONSTRAINTS
if !exists $col_values->{ $constraint_column } ||
!defined $col_values->{ $constraint_column };
$condition->{ $constraint_column } =
$col_values->{ $constraint_column };
}
my $count = $self->schema
->resultset( $source )
->search( $condition )
->count();
if ( $count > 0 ) {
# no point checking more stuff, exit the loop
$build_value--;
next BUILD_VALUE;
}
}
last; # you passed all tests
}
return $col_values if $build_value > 0;
# if you get here, we have a problem
warn "Violation of unique constraint in $source";
return;
}
sub _getForeignKeys {
# Returns the following arrayref
# [ [ source => name, keys => [ col_name => A, col_fk_name => B ] ], ... ]
# The array gives source name and keys for each FK constraint
my ($self, $params) = @_;
my $source = $self->schema->source( $params->{source} );
my ( @foreign_keys, $check_dupl );
my @relationships = $source->relationships;
for my $rel_name( @relationships ) {
my $rel_info = $source->relationship_info($rel_name);
if( $rel_info->{attrs}->{is_foreign_key_constraint} ) {
$rel_info->{source} =~ s/^.*:://g;
my $rel = { source => $rel_info->{source} };
my @keys;
while( my ($col_fk_name, $col_name) = each(%{$rel_info->{cond}}) ) {
$col_name =~ s|self.(\w+)|$1|;
$col_fk_name =~ s|foreign.(\w+)|$1|;
push @keys, {
col_name => $col_name,
col_fk_name => $col_fk_name,
};
}
# check if the combination table and keys is unique
# so skip double belongs_to relations (as in Biblioitem)
my $tag = $rel->{source}. ':'.
join ',', sort map { $_->{col_name} } @keys;
next if $check_dupl->{$tag};
$check_dupl->{$tag} = 1;
$rel->{keys} = \@keys;
push @foreign_keys, $rel;
}
}
return \@foreign_keys;
}
sub _storeColumnValues {
my ($self, $params) = @_;
my $source = $params->{source};
my $col_values = $params->{values};
my $new_row = $self->schema->resultset( $source )->create( $col_values );
return $new_row? { $new_row->get_columns }: {};
}
sub _buildColumnValue {
# returns an arrayref if all goes well
# an empty arrayref typically means: auto_incr column or fk column
# undef means failure
my ($self, $params) = @_;
my $source = $params->{source};
my $value = $params->{value};
my $col_name = $params->{column_name};
my $col_info = $self->schema->source($source)->column_info($col_name);
my $retvalue = [];
if( $col_info->{is_auto_increment} ) {
if( exists $value->{$col_name} ) {
warn "Value not allowed for auto_incr $col_name in $source";
return;
}
# otherwise: no need to assign a value
} elsif( $col_info->{is_foreign_key} || _should_be_fk($source,$col_name) ) {
if( exists $value->{$col_name} ) {
if( !defined $value->{$col_name} && !$col_info->{is_nullable} ) {
# This explicit undef is not allowed
warn "Null value for $col_name in $source not allowed";
return;
}
if( ref( $value->{$col_name} ) ne 'HASH' ) {
push @$retvalue, $value->{$col_name};
}
# sub build will handle a passed hash value later on
}
} elsif( ref( $value->{$col_name} ) eq 'HASH' ) {
# this is not allowed for a column that is not a FK
warn "Hash not allowed for $col_name in $source";
return;
} elsif( exists $value->{$col_name} ) {
if( !defined $value->{$col_name} && !$col_info->{is_nullable} ) {
# This explicit undef is not allowed
warn "Null value for $col_name in $source not allowed";
return;
}
push @$retvalue, $value->{$col_name};
} else {
my $data_type = $col_info->{data_type};
$data_type =~ s| |_|;
if( my $hdlr = $self->{gen_type}->{$data_type} ) {
push @$retvalue, &$hdlr( $self, { info => $col_info } );
} else {
warn "Unknown type $data_type for $col_name in $source";
return;
}
}
return $retvalue;
}
sub _should_be_fk {
# This sub is only needed for inconsistencies in the schema
# A column is not marked as FK, but a belongs_to relation is defined
my ( $source, $column ) = @_;
my $inconsistencies = {
'Item.biblionumber' => 1, #FIXME: Please remove me when I become FK
};
return $inconsistencies->{ "$source.$column" };
}
sub _gen_type {
return {
tinyint => \&_gen_int,
smallint => \&_gen_int,
mediumint => \&_gen_int,
integer => \&_gen_int,
bigint => \&_gen_int,
float => \&_gen_real,
decimal => \&_gen_real,
double_precision => \&_gen_real,
timestamp => \&_gen_date,
datetime => \&_gen_date,
date => \&_gen_date,
char => \&_gen_text,
varchar => \&_gen_text,
tinytext => \&_gen_text,
text => \&_gen_text,
mediumtext => \&_gen_text,
longtext => \&_gen_text,
set => \&_gen_set_enum,
enum => \&_gen_set_enum,
tinyblob => \&_gen_blob,
mediumblob => \&_gen_blob,
blob => \&_gen_blob,
longblob => \&_gen_blob,
};
};
sub _gen_int {
my ($self, $params) = @_;
my $data_type = $params->{info}->{data_type};
my $max = 1;
if( $data_type eq 'tinyint' ) {
$max = 127;
}
elsif( $data_type eq 'smallint' ) {
$max = 32767;
}
elsif( $data_type eq 'mediumint' ) {
$max = 8388607;
}
elsif( $data_type eq 'integer' ) {
$max = 2147483647;
}
elsif( $data_type eq 'bigint' ) {
$max = 9223372036854775807;
}
return int( rand($max+1) );
}
sub _gen_real {
my ($self, $params) = @_;
my $max = 10 ** 38;
if( defined( $params->{info}->{size} ) ) {
$max = 10 ** ($params->{info}->{size}->[0] - $params->{info}->{size}->[1]);
}
return rand($max) + 1;
}
sub _gen_date {
my ($self, $params) = @_;
return $self->schema->storage->datetime_parser->format_datetime(DateTime->now());
}
sub _gen_text {
my ($self, $params) = @_;
# From perldoc String::Random
# max: specify the maximum number of characters to return for * and other
# regular expression patters that don't return a fixed number of characters
my $regex = '[A-Za-z][A-Za-z0-9_]*';
my $size = $params->{info}{size};
if ( defined $size and $size > 1 ) {
$size--;
} elsif ( defined $size and $size == 1 ) {
$regex = '[A-Za-z]';
}
my $random = String::Random->new( max => $size );
return $random->randregex($regex);
}
sub _gen_set_enum {
my ($self, $params) = @_;
return $params->{info}->{extra}->{list}->[0];
}
sub _gen_blob {
my ($self, $params) = @_;;
return 'b';
}
=head1 NAME
t::lib::TestBuilder.pm - Koha module to create test records
=head1 SYNOPSIS
use t::lib::TestBuilder;
my $builder = t::lib::TestBuilder->new;
# The following call creates a patron, linked to branch CPL.
# Surname is provided, other columns are randomly generated.
# Branch CPL is created if it does not exist.
my $patron = $builder->build({
source => 'Borrower',
value => { surname => 'Jansen', branchcode => 'CPL' },
});
=head1 DESCRIPTION
This module automatically creates database records for you.
If needed, records for foreign keys are created too.
Values will be randomly generated if not passed to TestBuilder.
Note that you should wrap these actions in a transaction yourself.
=head1 METHODS
=head2 new
my $builder = t::lib::TestBuilder->new;
Constructor - Returns the object TestBuilder
=head2 schema
my $schema = $builder->schema;
Getter - Returns the schema of DBIx::Class
=head2 delete
$builder->delete({
source => $source,
records => $patron, # OR: records => [ $patron, ... ],
});
Delete individual records, created by builder.
Returns the number of delete attempts, or undef.
=head2 build
$builder->build({ source => $source_name, value => $value });
Create a test record in the table, represented by $source_name.
The name is required and must conform to the DBIx::Class schema.
Values may be specified by the optional $value hashref. Will be
randomized otherwise.
If needed, TestBuilder creates linked records for foreign keys.
Returns the values of the new record as a hashref, or undef if
the record could not be created.
Note that build also supports recursive hash references inside the
value hash for foreign key columns, like:
value => {
column1 => 'some_value',
fk_col2 => {
columnA => 'another_value',
}
}
The hash for fk_col2 here means: create a linked record with build
where columnA has this value. In case of a composite FK the hashes
are merged.
Realize that passing primary key values to build may result in undef
if a record with that primary key already exists.
=head1 AUTHOR
Yohann Dufour <yohann.dufour@biblibre.com>
Koha Development Team
=head1 COPYRIGHT
Copyright 2014 - Biblibre SARL
=head1 LICENSE
This file is part of Koha.
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>.
=cut
1;