Enforce conditions to return a class object on evaluate

A strict type checking on the return value of the evaluate
call will prevent accidential decission errors due to bugs
in the condition classes.
The interface contract for conditions now enforces that an
instance of Workflow::Condition::IsTrue/::IsFalse is returned.
Any other value will cause the workflow to croak with an error

lib/Workflow/Condition.pm

@@ -5,6 +5,7 @@ use strict;
 use parent qw( Workflow::Base );
 use v5.14.0;
 use Carp qw(croak);
+use Data::Dumper;
 use Log::Any qw( $log );
 use Workflow::Exception qw( workflow_error );
 
@@ -61,14 +62,51 @@ sub evaluate_condition {
         $condition = $wf->_factory()
             ->get_condition( $orig_condition, $wf->type );
         $log->debug( "Evaluating condition '$orig_condition'" );
-        my $return_value = $condition->evaluate($wf);
+
+        my $return_value;
+        my $result = $condition->evaluate($wf);
+        if (ref $result eq 'Workflow::Condition::IsTrue') {
+            $log->info( "Got true result with '$result' on '$orig_condition'");
+            $return_value = 1;
+        } elsif (ref $result eq 'Workflow::Condition::IsFalse') {
+            $log->info( "Got false result with '$result' on '$orig_condition'");
+            $return_value = 0;
+        } else {
+            $log->fatal( "Evaluate on '$orig_condition' did not return a valid result object" );
+            $log->trace( 'Eval result was ' . Dumper $result );
+            croak "Evaluate on '$orig_condition' did not return a valid result object";
+        }
+
         $wf->{'_condition_result_cache'}->{$orig_condition} = $return_value;
 
         return $return_value;
     }
 }
 
+package Workflow::Condition::Result;
+use parent qw( Class::Accessor );
+
+use overload '""' => 'to_string';
+
+__PACKAGE__->mk_accessors('message');
+
+sub new {
+    my ( $class, @params ) = @_;
+    my $self = bless { }, $class;
+    $self->message( shift @params ) if (@params);
+    return $self;
+}
+
+sub to_string {
+    my $self = shift;
+    return $self->message() || '<no message>';
+}
+
+package Workflow::Condition::IsTrue;
+use base 'Workflow::Condition::Result';
 
+package Workflow::Condition::IsFalse;
+use base 'Workflow::Condition::Result';
 
 1;
 

lib/Workflow/Condition/Evaluate.pm

@@ -46,7 +46,9 @@ sub evaluate {
                        ( defined $rv ? $rv : '<undef>' ),
                        "'" );
 
-    return $rv;
+    return $rv ?
+        Workflow::Condition::IsTrue->new() :
+        Workflow::Condition::IsFalse->new();
 }
 
 1;

lib/Workflow/Condition/HasUser.pm

@@ -23,7 +23,9 @@ sub evaluate {
     my $current_user = $wf->context->param($user_key);
     $self->log->debug( "Current user in the context is '$current_user' retrieved ",
         "using parameter key '$user_key'" );
-    return $current_user;
+
+    return Workflow::Condition::IsTrue->new() if($current_user);
+    return Workflow::Condition::IsFalse->new();
 }
 
 1;

lib/Workflow/Condition/LazyAND.pm

@@ -33,15 +33,17 @@ sub evaluate {
 
     my $total = 0;
 
+    return Workflow::Condition::IsFalse->new("No conditions were defined") unless(@{$conditions});
+
     foreach my $cond ( @{$conditions} ) {
         my $result = $self->evaluate_condition( $wf, $cond );
         if ( not $result ) {
-            return $result; # return false
+            return Workflow::Condition::IsFalse->new("Stopped after checking $total conditions");
         }
         $total++;
     }
 
-    return $total; # returns false if no conditions ran: the contract
+    return Workflow::Condition::IsTrue->new("Matched a total of $total conditions");
 }
 
 1;

lib/Workflow/Condition/LazyOR.pm

@@ -37,10 +37,10 @@ sub evaluate {
     foreach my $cond ( @{$conditions} ) {
         my $result = $self->evaluate_condition( $wf, $cond );
         if ($result) {
-            return $result;
+            return Workflow::Condition::IsTrue->new("Got match in " . $cond );
         }
     }
-    return ''; # false
+    return Workflow::Condition::IsFalse->new();
 }
 
 1;

lib/Workflow/Condition/Negated.pm

@@ -21,7 +21,11 @@ sub _init {
 
 sub evaluate {
     my ($self, $wf) = @_;
-    return not $self->evaluate_condition($wf, $self->negated);
+    if ($self->evaluate_condition($wf, $self->negated)) {
+        return Workflow::Condition::IsFalse->new();
+    } else {
+        return Workflow::Condition::IsTrue->new();
+    }
 }
 
 

t/TestApp/Condition/AlwaysTrue.pm

@@ -10,7 +10,8 @@ sub evaluate {
     my ( $self, $wf ) = @_;
     $log->debug( "Trying to execute condition ", ref( $self ) );
     $log->debug( 'Condition met ok' );
-    return 1;
+
+    return Workflow::Condition::IsTrue->new();
 }
 
 1;

t/TestApp/Condition/HasUserType.pm

@@ -15,7 +15,8 @@ sub evaluate {
         return 0;
     }
     $log->debug( 'Condition met ok' );
-    return 1;
+
+    return Workflow::Condition::IsTrue->new();
 }
 
 1;

t/TestCachedApp/Condition/EvenCounts.pm

@@ -17,10 +17,12 @@ sub evaluate {
         #  the same result twice: the first time on 'get_current_actions'
         #  and the second time on 'execute_action'
         $log->debug(__PACKAGE__, '::evaluate(', $count, '): fail');
-        return 0;
+        return Workflow::Condition::IsFalse->new();
     }
     $log->debug(__PACKAGE__, '::evaluate(', $count, '): success');
-    return 1;
+
+    return Workflow::Condition::IsTrue->new();
+
 }
 
 1;