diff --git a/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm b/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm index 4d24a1248..4e11f3ed8 100644 --- a/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm +++ b/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm @@ -684,7 +684,7 @@ sub processGotoExpression { # Parse gotoExpressions one after the other (first one that's true wins) foreach my $line (split /\n/, $expression) { - my $processed = $self->parseGotoExpression($line, $responses); + my $processed = WebGUI::Asset::Wobject::Survey::ResponseJSON->parseGotoExpression($self->session, $line, $responses); next if !$processed; @@ -770,22 +770,22 @@ Replace each "$questionName" with its response value (from the $responses hashre =cut sub parseGotoExpression { - my $self = shift; - my ($expression, $responses) = validate_pos(@_, { type => SCALAR }, { type => HASHREF, default => {} }); + my $class = shift; + my ($session, $expression, $responses) = validate_pos(@_, { isa => 'WebGUI::Session'}, { type => SCALAR }, { type => HASHREF, default => {} }); - $self->session->log->debug("Parsing gotoExpression: $expression"); + $session->log->debug("Parsing gotoExpression: $expression"); my ( $target, $rest ) = $expression =~ /\s* ([^:]+?) \s* : \s* (.*)/x; - $self->session->log->debug("Parsed as Target: [$target], Expression: [$rest]"); + $session->log->debug("Parsed as Target: [$target], Expression: [$rest]"); if ( !defined $target ) { - $self->session->log->warn('Target undefined'); + $session->log->warn('Target undefined'); return; } if ( !defined $rest || $rest eq q{} ) { - $self->session->log->warn('Expression undefined'); + $session->log->warn('Expression undefined'); return; } @@ -794,7 +794,7 @@ sub parseGotoExpression { $rest =~ s/\$$questionName/$response/g; } - $self->session->log->debug("Processed as: $rest"); + $session->log->debug("Processed as: $rest"); return { target => $target, diff --git a/lib/WebGUI/Asset/Wobject/Survey/SurveyJSON.pm b/lib/WebGUI/Asset/Wobject/Survey/SurveyJSON.pm index 13f125a48..404faef6a 100644 --- a/lib/WebGUI/Asset/Wobject/Survey/SurveyJSON.pm +++ b/lib/WebGUI/Asset/Wobject/Survey/SurveyJSON.pm @@ -1209,63 +1209,72 @@ sub validateSurvey{ #set up valid goto targets my $gotoTargets = $self->getGotoTargets(); my $goodTargets; - for my $g(@$gotoTargets){ $$goodTargets{$g} = 1; } + my $duplicateTargets; + for my $g (@{$gotoTargets}) { + $goodTargets->{$g}++; + $duplicateTargets->{$g}++ if $goodTargets->{$g} > 1; + } #step through each section validating it. my $sections = $self->sections(); + for(my $s = 0; $s <= $#$sections; $s++){ + my $sNum = $s + 1; my $section = $self->section([$s]); if(! $self->validateGoto($section,$goodTargets)){ - push(@messages,"Section $s does not have a valid GOTO target."); + push @messages,"Section $sNum does not have a valid Jump target."; + } + if(! $self->validateGotoInfiniteLoop($section)){ + push(@messages,"Section $sNum jumps to itself."); } if(! $self->validateGotoExpression($section,$goodTargets)){ - push(@messages,"Section $s does not have a valid GOTO Expression target."); + push(@messages,"Section $sNum does not have a valid Jump Expression."); } - if(! $self->validateInfLoop($section)){ - push(@messages,"Section $s jumps to itself."); - } - if(! $self->validateExpressionSyntax($section)){ - push(@messages,"Section $s does not appear to have valid GOTO Expression syntax."); + if (my $var = $section->{variable}) { + if (my $count = $duplicateTargets->{$var}) { + push @messages, "Section $sNum variable name $var is re-used in $count other place(s)."; + } } #step through each question validating it. my $questions = $self->questions([$s]); for(my $q = 0; $q <= $#$questions; $q++){ + my $qNum = $q + 1; my $question = $self->question([$s,$q]); if(! $self->validateGoto($question,$goodTargets)){ - push(@messages,"Section $s Question $q does not have a valid GOTO target."); + push(@messages,"Section $sNum Question $qNum does not have a valid Jump target."); + } + if(! $self->validateGotoInfiniteLoop($question)){ + push(@messages,"Section $sNum Question $qNum jumps to itself."); } if(! $self->validateGotoExpression($question,$goodTargets)){ - push(@messages,"Section $s Question $q does not have a valid GOTO Expression target."); - } - if(! $self->validateExpressionSyntax($question)){ - push(@messages,"Section $s Question $q does not appear to have valid GOTO Expression syntax."); + push(@messages,"Section $sNum Question $qNum does not have a valid Jump Expression."); } if($#{$question->{answers}} < 0){ - push(@messages,"Section $s Question $q does not have any answers."); + push(@messages,"Section $sNum Question $qNum does not have any answers."); } if(! $question->{text} =~ /\w/){ - push(@messages,"Section $s Question $q does not have any text."); + push(@messages,"Section $sNum Question $qNum does not have any text."); } - if(! $self->validateInfLoop($question)){ - push(@messages,"Section $s Question $q jumps to itself."); + if (my $var = $question->{variable}) { + if (my $count = $duplicateTargets->{$var}) { + push @messages, "Section $sNum Question $qNum variable name $var is re-used in $count other place(s)."; + } } #step through each answer validating it. my $answers = $self->answers([$s,$q]); for(my $a = 0; $a <= $#$answers; $a++){ + my $aNum = $a + 1; my $answer = $self->answer([$s,$q,$a]); if(! $self->validateGoto($answer,$goodTargets)){ - push(@messages,"Section $s Question $q Answer $a does not have a valid GOTO target."); + push(@messages,"Section $sNum Question $qNum Answer $aNum does not have a valid Jump target."); } - if(! $self->validateExpressionSyntax($answer)){ - push(@messages,"Section $s Question $q Answer $a does not appear to have valid GOTO Expression syntax."); + if(! $self->validateGotoInfiniteLoop($answer)){ + push(@messages,"Section $sNum Question $qNum Answer $aNum jumps to itself."); } if(! $self->validateGotoExpression($answer,$goodTargets)){ - push(@messages,"Section $s Question $q Answer $a does not have a valid GOTO Expression target."); - } - if(! $self->validateInfLoop($answer)){ - push(@messages,"Section $s Question $q Answer $a jumps to itself."); + push(@messages,"Section $sNum Question $qNum Answer $aNum does not have a valid Jump Expression."); } } } @@ -1274,17 +1283,15 @@ sub validateSurvey{ return \@messages; } -sub validateExpressionSyntax{ +sub validateGoto{ my $self = shift; my $object = shift; - if($object->{gotoExpression} =~ /\w/){ - my ( $target, $rest ) = $object->{gotoExpression} =~ /\s* ([^:]+?) \s* : \s* (.*)/x; - return 0 if(! defined $target or ! defined $rest); - } + my $goodTargets = shift; + return 0 if($object->{goto} =~ /\w/ && ! exists($goodTargets->{$object->{goto}})); return 1; } -sub validateInfLoop{ +sub validateGotoInfiniteLoop{ my $self = shift; my $object = shift; return 0 if($object->{goto} =~ /\w/ and $object->{goto} eq $object->{variable}); @@ -1294,20 +1301,10 @@ sub validateInfLoop{ sub validateGotoExpression{ my $self = shift; my $object = shift; - my $goodTargets = shift; - if($object->{gotoExpression} =~ /\w/ and $object->{gotoExpression} =~ /\s*?(\w*)/){ - my $tar = $1; - return 0 if(! exists $goodTargets->{$1}); - } - return 1; -} - -sub validateGoto{ - my $self = shift; - my $object = shift; - my $goodTargets = shift; - return 0 if($object->{goto} =~ /\w/ && ! exists($goodTargets->{$object->{goto}})); - return 1; + return 1 unless $object->{gotoExpression} =~ /\w/; + + # The best we can do is return true/false on whether the gotoExpression parses + return WebGUI::Asset::Wobject::Survey::ResponseJSON->parseGotoExpression($self->session, $object->{gotoExpression}); } =head2 section ($address) diff --git a/t/Asset/Wobject/Survey/ResponseJSON.t b/t/Asset/Wobject/Survey/ResponseJSON.t index 9df8616bf..6ca0996da 100644 --- a/t/Asset/Wobject/Survey/ResponseJSON.t +++ b/t/Asset/Wobject/Survey/ResponseJSON.t @@ -326,28 +326,29 @@ is($rJSON->lastResponse(), 5, 'goto: finds first if there are duplicates'); # parseGotoExpression # #################################################### -throws_ok { $rJSON->parseGotoExpression() } 'WebGUI::Error::InvalidParam', 'processGotoExpression takes exception to empty arguments'; -is($rJSON->parseGotoExpression(q{}), +my $c = 'WebGUI::Asset::Wobject::Survey::ResponseJSON'; +throws_ok { $c->parseGotoExpression($session, ) } 'WebGUI::Error::InvalidParam', 'processGotoExpression takes exception to empty arguments'; +is($c->parseGotoExpression($session, q{}), undef, '.. and undef with empty expression'); -is($rJSON->parseGotoExpression('blah-dee-blah-blah'), +is($c->parseGotoExpression($session, 'blah-dee-blah-blah'), undef, '.. and undef with duff expression'); -is($rJSON->parseGotoExpression(':'), +is($c->parseGotoExpression($session, ':'), undef, '.. and undef with missing target'); -is($rJSON->parseGotoExpression('t1:'), +is($c->parseGotoExpression($session, 't1:'), undef, '.. and undef with missing expression'); -cmp_deeply($rJSON->parseGotoExpression('t1: 1'), +cmp_deeply($c->parseGotoExpression($session, 't1: 1'), { target => 't1', expression => '1'}, 'works for simple numeric expression'); -cmp_deeply($rJSON->parseGotoExpression('t1: 1 - 23 + 456 * (78 / 9.0)'), +cmp_deeply($c->parseGotoExpression($session, 't1: 1 - 23 + 456 * (78 / 9.0)'), { target => 't1', expression => '1 - 23 + 456 * (78 / 9.0)'}, 'works for expression using all algebraic tokens'); -cmp_deeply($rJSON->parseGotoExpression('t1: 1 != 3 <= 4 >= 5'), +cmp_deeply($c->parseGotoExpression($session, 't1: 1 != 3 <= 4 >= 5'), { target => 't1', expression => '1 != 3 <= 4 >= 5'}, q{..works with other ops too}); -cmp_deeply($rJSON->parseGotoExpression('t1: $q1 + $q2 * $q3 - 4', { q1 => 11, q2 => 22, q3 => 33}), +cmp_deeply($c->parseGotoExpression($session, 't1: $q1 + $q2 * $q3 - 4', { q1 => 11, q2 => 22, q3 => 33}), { target => 't1', expression => '11 + 22 * 33 - 4'}, 'substitues q for value'); -cmp_deeply($rJSON->parseGotoExpression('t1: $a silly var name * 10 + $another var name', { 'a silly var name' => 345, 'another var name' => 456}), +cmp_deeply($c->parseGotoExpression($session, 't1: $a silly var name * 10 + $another var name', { 'a silly var name' => 345, 'another var name' => 456}), { target => 't1', expression => '345 * 10 + 456'}, '..it even works for vars with spaces in their names'); -cmp_deeply($rJSON->parseGotoExpression('t1: ($A < 4) and ($B < 4) or ($B > 6) && 1 || 1', { A => 2, B => 3}), +cmp_deeply($c->parseGotoExpression($session, 't1: ($A < 4) and ($B < 4) or ($B > 6) && 1 || 1', { A => 2, B => 3}), { target => 't1', expression => '(2 < 4) and (3 < 4) or (3 > 6) && 1 || 1'}, 'Boolean expressions ok'); -cmp_deeply($rJSON->parseGotoExpression('t1: $a = 1; $a++; $a > 1'), +cmp_deeply($c->parseGotoExpression($session, 't1: $a = 1; $a++; $a > 1'), { target => 't1', expression => '$a = 1; $a++; $a > 1'}, 'Assignment and compound statements ok too'); ####################################################