From c5bc51d7d1b6c98e9d897022f91d8e0806cf4524 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@koha-community.org>
Date: Thu, 4 Jun 2015 11:35:15 +0200
Subject: [PATCH] Bug 14256: (follow-up) Check for unique constraint to
 regenerate random data

There were some issues in the previous patch. This patch fixes the
following:
- rename $value with $original_value
- remove $at_least_one_constraint_failed and $values_ok which make the
  code unnecessarily complicated
- the constraints have to be checked only if no original value is passed
- _buildColumnValue created a key to the default value hashref, it broke
  the test:
    last BUILD_VALUE if exists( $default_value->{$source} );

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
---
 t/lib/TestBuilder.pm | 47 +++++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm
index 7814df380f..5d3157a356 100644
--- a/t/lib/TestBuilder.pm
+++ b/t/lib/TestBuilder.pm
@@ -150,42 +150,44 @@ sub _formatSource {
 
 sub _buildColumnValues {
     my ($self, $params) = @_;
-    my $source  = _formatSource( { source => $params->{source} } );
-    my $value   = $params->{value};
+    my $source = _formatSource( { source => $params->{source} } );
+    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 $values_ok = 0;
-    my $at_least_one_constraint_failed;
-
-    while ( not $values_ok ) {
-
-        $at_least_one_constraint_failed = 0;
-        # generate random values
+    my $build_value = 1;
+    BUILD_VALUE: while ( $build_value ) {
+        # generate random values for all columns
         for my $col_name( @columns ) {
             my $col_value = $self->_buildColumnValue({
                 source      => $source,
                 column_name => $col_name,
-                value       => $value,
+                value       => $original_value,
             });
             $col_values->{$col_name} = $col_value if( defined( $col_value ) );
         }
+        $build_value = 0;
 
         # If default values are set, maybe the data exist in the DB
         # But no need to wait for another value
-        last if exists( $default_value->{$source} );
+        # FIXME this can be wrong if a default value is defined for a field
+        # which is not a constraint and that the generated value for the
+        # constraint already exists.
+        last BUILD_VALUE if exists( $default_value->{$source} );
 
-        if ( scalar keys %unique_constraints > 0 ) {
+        # If there is no original value given and unique constraints exist,
+        # check if the generated values do not exist yet.
+        if ( not defined $original_value and scalar keys %unique_constraints > 0 ) {
 
             # verify the data would respect each unique constraint
-            foreach my $constraint (keys %unique_constraints) {
+            CONSTRAINTS: foreach my $constraint (keys %unique_constraints) {
 
                 my $condition;
-                my @constraint_columns = $unique_constraints{$constraint};
+                my $constraint_columns = $unique_constraints{$constraint};
                 # loop through all constraint columns and build the condition
-                foreach my $constraint_column ( @constraint_columns ) {
+                foreach my $constraint_column ( @$constraint_columns ) {
                     # build the filter
                     $condition->{ $constraint_column } =
                             $col_values->{ $constraint_column };
@@ -196,20 +198,11 @@ sub _buildColumnValues {
                                  ->search( $condition )
                                  ->count();
                 if ( $count > 0 ) {
-                    $at_least_one_constraint_failed = 1;
                     # no point checking more stuff, exit the loop
-                    last;
+                    $build_value = 1;
+                    last CONSTRAINTS;
                 }
             }
-
-            if ( $at_least_one_constraint_failed ) {
-                $values_ok = 0;
-            } else {
-                $values_ok = 1;
-            }
-
-        } else {
-            $values_ok = 1;
         }
     }
     return $col_values;
@@ -283,7 +276,7 @@ sub _buildColumnValue {
     if( exists( $value->{$col_name} ) ) {
         $col_value = $value->{$col_name};
     }
-    elsif( exists( $default_value->{$source}->{$col_name} ) ) {
+    elsif( exists $default_value->{$source} and exists $default_value->{$source}->{$col_name} ) {
         $col_value = $default_value->{$source}->{$col_name};
     }
     elsif( not $col_info->{default_value} and not $col_info->{is_auto_increment} and not $col_info->{is_foreign_key} ) {