diff --git a/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm b/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm index b6d187c6b..a18f49ed1 100644 --- a/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm +++ b/lib/WebGUI/Asset/Wobject/Survey/ResponseJSON.pm @@ -649,58 +649,30 @@ indicates that we should branch. The gotoExpression. A gotoExpression is a string representing a list of expressions (one per line) of the form: + target: expression target: expression ... This subroutine iterates through the list, processing each line and, all things being well, evaluates the expression. The first expression to evaluate to true triggers a -call to goto($target). +call to L<"processGoto">. -The expression is a simple subset of the formula language used in spreadsheet programs -such as Excel, OpenOffice, Google Docs etc.. +The expression should be valid perl. Any section/question variables that you refer to +should be written as $var, as if your perl code had access to that variable. In reality, +those variables don't exist - they're substituted in via L<"parseGotoExpression"> and +then the expression is evaluated in a safe compartment. Here is an example using section variables S1 and S2 as jump targets and question variables Q1-3 in the expression. It jumps to S1 if the user's answer to Q1 has a value of 3, jumps to S2 if Q2 + Q3 < 10, and otherwise doesn't branch at all (the default). -S1: Q1 = 3 -S2: Q2 + Q3 < 10 -Arguments are evaluated as follows: + S1: $Q1 == 3 + S2: $Q2 + $Q3 < 10 -Numeric arguments evaluate as numbers - -=over 4 - -=item * No support for strings (and hence no string matching) - -=item * Question variable names (e.g. Q1) evaluate to the numeric value associated with -user's answer to that question, or undefined if the user has not answered that question - -=back - -Binary comparisons operators: = != < <= >= > - -=over 4 - -=item * return boolean values based on perl's equivalent numeric comparison operators - -=back - -Simple math operators: + - * / - -=over 4 - -=item * return numeric values - -=back - -Later we may add Boolean operators: AND( x; y; z; ... ), OR( x; y; z; ... ), NOT( x ), with args separated by -semicolons (presumably because spreadsheet formulas use commas to indicate cell ranges) - -Later still you may be able to say AVG(section1) or SUM(section3) and have those functions automatically -compute their result over the set of all questions in the given section. -But for now those things can be done manually using the limited subset defined. +You can do advanced branching by creating your own variables within the expression, for +example, to branch when the average of 3 questions is greater than 5: + S1: $avg = ($Q1 + $Q2 + $Q3) / 3; $avg > 5 =cut @@ -793,12 +765,7 @@ Uses the following simple strategy: First, parse the expression as: target: expression -Replace each questionName with its response value (from the $responses hashref) - -Massage the expression into valid perl - -Check that only valid tokens remain. This last step ensures that any invalid questionNames in -the expression generate an error because our list of valid tokens doesn't include a-z +Replace each "$questionName" with its response value (from the $responses hashref) =cut @@ -808,9 +775,6 @@ sub parseGotoExpression { $self->session->log->debug("Parsing gotoExpression: $expression"); - # Valid gotoExpression tokens are.. - my $tokens = qr{\s|[-0-9=!<>+*/.()&|:?]}; - my ( $target, $rest ) = $expression =~ /\s* ([^:]+?) \s* : \s* (.*)/x; $self->session->log->debug("Parsed as Target: [$target], Expression: [$rest]"); @@ -825,21 +789,9 @@ sub parseGotoExpression { return; } - # convert 'and' and 'or' to '&&' and '||' - $rest =~ s/\band\b/&&/ig; - $rest =~ s/\bor\b/||/ig; - - # Replace each questionName with its response value + # Replace each "$questionName" with its response value while ( my ( $questionName, $response ) = each %{$responses} ) { - $rest =~ s/$questionName/$response/g; - } - - # convert '=' to '==' but don't touch '!=', '<=' or '>=' - $rest =~ s/(?])=(?!=)/==/g; - - if ( $rest !~ /^$tokens+$/ ) { - $self->session->log->warn("Contains invalid tokens: $rest"); - return; + $rest =~ s/\$$questionName/$response/g; } $self->session->log->debug("Processed as: $rest"); diff --git a/t/Asset/Wobject/Survey/ResponseJSON.t b/t/Asset/Wobject/Survey/ResponseJSON.t index a9a25ebfe..9df8616bf 100644 --- a/t/Asset/Wobject/Survey/ResponseJSON.t +++ b/t/Asset/Wobject/Survey/ResponseJSON.t @@ -22,7 +22,7 @@ my $session = WebGUI::Test->session; #---------------------------------------------------------------------------- # Tests -my $tests = 87; +my $tests = 90; plan tests => $tests + 1; #---------------------------------------------------------------------------- @@ -339,25 +339,20 @@ cmp_deeply($rJSON->parseGotoExpression('t1: 1'), { target => 't1', expression => '1'}, 'works for simple numeric expression'); cmp_deeply($rJSON->parseGotoExpression('t1: 1 - 23 + 456 * (78 / 9.0)'), { target => 't1', expression => '1 - 23 + 456 * (78 / 9.0)'}, 'works for expression using all algebraic tokens'); -is($rJSON->parseGotoExpression('t1: 1 + $'), undef, '.. but disallows expression containing non-whitelisted token'); -cmp_deeply($rJSON->parseGotoExpression('t1: 1 = 3'), - { target => 't1', expression => '1 == 3'}, 'converts single = to =='); cmp_deeply($rJSON->parseGotoExpression('t1: 1 != 3 <= 4 >= 5'), - { target => 't1', expression => '1 != 3 <= 4 >= 5'}, q{..but doesn't mess with other ops containing =}); -cmp_deeply($rJSON->parseGotoExpression('t1: q1 + q2 * q3 - 4', { q1 => 11, q2 => 22, q3 => 33}), + { 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}), { 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($rJSON->parseGotoExpression('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'); -is($rJSON->parseGotoExpression('t1: qX + 3', { q1 => '7'}), - undef, q{..but doesn't like invalid var names}); -cmp_deeply($rJSON->parseGotoExpression('t1: (A < 4) AND (B < 4)', { A => 2, B => 3}), - { target => 't1', expression => '(2 < 4) && (3 < 4)'}, 'Boolean AND'); -cmp_deeply($rJSON->parseGotoExpression('t1: (A < 4) OR (B < 4)', { A => 2, B => 3}), - { target => 't1', expression => '(2 < 4) || (3 < 4)'}, 'Boolean OR'); +cmp_deeply($rJSON->parseGotoExpression('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'), + { target => 't1', expression => '$a = 1; $a++; $a > 1'}, 'Assignment and compound statements ok too'); #################################################### # -# gotoExpression +# processGotoExpression # #################################################### @@ -373,34 +368,40 @@ $rJSON->recordResponses({ '1-0-0comment' => 'Section 1, question 0, answer 0 comment', }); is($rJSON->processGotoExpression('blah-dee-blah-blah'), undef, 'invalid gotoExpression is false'); -ok($rJSON->processGotoExpression('s0: s1q0 = 3'), '3 == 3 is true'); -ok(!$rJSON->processGotoExpression('s0: s1q0 = 4'), '3 == 4 is false'); -ok($rJSON->processGotoExpression('s0: s1q0 != 2'), '3 != 2 is true'); -ok(!$rJSON->processGotoExpression('s0: s1q0 != 3'), '3 != 3 is false'); -ok($rJSON->processGotoExpression('s0: s1q0 > 2'), '3 > 2 is true'); -ok($rJSON->processGotoExpression('s0: s1q0 < 4'), '3 < 2 is true'); -ok(!$rJSON->processGotoExpression('s0: s1q0 >= 4'), '3 >= 4 is false'); -ok(!$rJSON->processGotoExpression('s0: s1q0 <= 2'), '3 <= 2 is false'); -ok(!$rJSON->processGotoExpression('s0: s1q0 < 2 or s1q0 < 1'), '3 < 2 || 3 < 1 is false'); -ok($rJSON->processGotoExpression('s0: s1q0 < 2 or s1q0 < 5'), '3 < 2 || 3 < 5 is true'); -ok(!$rJSON->processGotoExpression('s0: s1q0 = 4 and 1 = 1'), '3 == 4 && 1 == 1 is false'); -ok($rJSON->processGotoExpression('s0: s1q0 = 3 and 1 = 1'), '3 == 3 && 1 == 1 is true'); -ok(!$rJSON->processGotoExpression('s0: (s1q0 > 1 ? 10 : 11) = 11'), '(3 > 1 ? 10 : 11) == 11 is false'); -ok($rJSON->processGotoExpression('s0: (s1q0 > 1 ? 10 : 11) = 10'), '(3 > 1 ? 10 : 11) == 10 is true'); +ok($rJSON->processGotoExpression('s0: $s1q0 == 3'), '3 == 3 is true'); +ok(!$rJSON->processGotoExpression('s0: $s1q0 == 4'), '3 == 4 is false'); +ok($rJSON->processGotoExpression('s0: $s1q0 != 2'), '3 != 2 is true'); +ok(!$rJSON->processGotoExpression('s0: $s1q0 != 3'), '3 != 3 is false'); +ok($rJSON->processGotoExpression('s0: $s1q0 > 2'), '3 > 2 is true'); +ok($rJSON->processGotoExpression('s0: $s1q0 < 4'), '3 < 2 is true'); +ok(!$rJSON->processGotoExpression('s0: $s1q0 >= 4'), '3 >= 4 is false'); +ok(!$rJSON->processGotoExpression('s0: $s1q0 <= 2'), '3 <= 2 is false'); +ok(!$rJSON->processGotoExpression('s0: $s1q0 < 2 or $s1q0 < 1'), '3 < 2 || 3 < 1 is false'); +ok($rJSON->processGotoExpression('s0: $s1q0 < 2 or $s1q0 < 5'), '3 < 2 || 3 < 5 is true'); +ok(!$rJSON->processGotoExpression('s0: $s1q0 == 4 and 1 == 1'), '3 == 4 && 1 == 1 is false'); +ok($rJSON->processGotoExpression('s0: $s1q0 == 3 and 1 == 1'), '3 == 3 && 1 == 1 is true'); +ok(!$rJSON->processGotoExpression('s0: ($s1q0 > 1 ? 10 : 11) == 11'), '(3 > 1 ? 10 : 11) == 11 is false'); +ok($rJSON->processGotoExpression('s0: ($s1q0 > 1 ? 10 : 11) == 10'), '(3 > 1 ? 10 : 11) == 10 is true'); +ok($rJSON->processGotoExpression('s0: $a=1; $a++; $a++; $a *= 2; $a == 6'), 'Assignment and compound statements ok'); +ok(!$rJSON->processGotoExpression('s0: $a=1; $a++; $a++; $a *= 2; $a == 7'), '..negative ones too'); +ok($rJSON->processGotoExpression('s0: @a = (1..10); $a[0] == 1 && @a == 10'), 'arrays work too'); +ok($rJSON->processGotoExpression('s0: if ($s1q0 == 3) { 1 } else { 0 }'), 'if statements work'); +ok(!$rJSON->processGotoExpression('s0: if (time) { 1 } else { 1 }'), 'time and other things not allowed'); +ok($rJSON->processGotoExpression('s0: $q2 = 5; $avg = ($s1q0 + $q2) / 2; $avg == 4'), 'look ma, averages!'); -cmp_deeply($rJSON->processGotoExpression(<<"END_EXPRESSION"), {target => 's2', expression => '3 == 3'}, 'first true expression wins'); -s0: s1q0 <= 2 -s2: s1q0 = 3 +cmp_deeply($rJSON->processGotoExpression(<<'END_EXPRESSION'), {target => 's2', expression => '3 == 3'}, 'first true expression wins'); +s0: $s1q0 <= 2 +s2: $s1q0 == 3 END_EXPRESSION -ok(!$rJSON->processGotoExpression(<<"END_EXPRESSION"), 'but multiple false expressions still false'); -s0: s1q0 <= 2 -s2: s1q0 = 345 +ok(!$rJSON->processGotoExpression(<<'END_EXPRESSION'), 'but multiple false expressions still false'); +s0: $s1q0 <= 2 +s2: $s1q0 == 345 END_EXPRESSION -$rJSON->processGotoExpression('s0: s1q0 = 3'); +$rJSON->processGotoExpression('s0: $s1q0 == 3'); is($rJSON->lastResponse(), -1, '.. lastResponse changed to -1 due to processGoto(s0)'); -$rJSON->processGotoExpression('s2: s1q0 = 3'); +$rJSON->processGotoExpression('s2: $s1q0 == 3'); is($rJSON->lastResponse(), 4, '.. lastResponse changed to 4 due to processGoto(s2)'); $rJSON->responses({});