diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 68aed4767..1e2cbd114 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -11,6 +11,7 @@ - Set a minimum package weight of 0.1 oz for the USPS driver. - Handle per package errors in USPS response data. - fixed #11207: Shop Email receipts going out to everyone. + - fixed tag-caching bug in Survey ExpressionEngine 7.8.3 - Rewrote Spectre's workflow queues to prevent it from "forgetting" about some workflows. diff --git a/lib/WebGUI/Asset/Wobject/Survey/ExpressionEngine.pm b/lib/WebGUI/Asset/Wobject/Survey/ExpressionEngine.pm index 17abd5ed5..3afa244d0 100644 --- a/lib/WebGUI/Asset/Wobject/Survey/ExpressionEngine.pm +++ b/lib/WebGUI/Asset/Wobject/Survey/ExpressionEngine.pm @@ -24,8 +24,10 @@ use WebGUI::Asset; use WebGUI::Asset::Wobject::Survey; Params::Validate::validation_options( on_fail => sub { WebGUI::Error::InvalidParam->throw( error => shift ) } ); -# We need these as semi-globals so that utility subs (which are shared with the safe compartment) +# We need these as file-scoped lexicals so that our utility subs (which are shared with the safe compartment) # can access them. +# N.B. If you add any new ones, make sure you initialize them in L otherwise they will be cached across +# unrelated engine runs, which leads to bugs that are hairy to track down my $session; my $values; my $scores; @@ -431,7 +433,7 @@ sub run { my ( $s, $expression, $opts ) = validate_pos( @_, { isa => 'WebGUI::Session' }, { type => SCALAR }, { type => HASHREF, default => {} } ); - # Init package globals + # Initialize all file-scoped lexicals that our Safe utility subs have access to $session = $s; $values = $opts->{values} || {}; $scores = $opts->{scores} || {}; @@ -439,6 +441,7 @@ sub run { $validate = $opts->{validate}; $validTargets = $opts->{validTargets}; $tags = $opts->{tags} || {}; + $otherInstances = {}; if ( !$session->config->get('enableSurveyExpressionEngine') ) { $session->log->debug('enableSurveyExpressionEngine config option disabled, skipping'); diff --git a/t/Asset/Wobject/Survey/ExpressionEngine.t b/t/Asset/Wobject/Survey/ExpressionEngine.t index 89ffc2eaa..8acc0db17 100644 --- a/t/Asset/Wobject/Survey/ExpressionEngine.t +++ b/t/Asset/Wobject/Survey/ExpressionEngine.t @@ -22,7 +22,7 @@ my $session = WebGUI::Test->session; #---------------------------------------------------------------------------- # Tests -my $tests = 58; +my $tests = 60; plan tests => $tests + 1; #---------------------------------------------------------------------------- @@ -236,8 +236,34 @@ SKIP: { { jump => 'target', tags => {} }, 'external score resolves ok too' ); cmp_deeply( $e->run( $session, qq{jump {scoreX('$url', ext_s0) == 200} target}, {userId => $user->userId} ), { jump => 'target', tags => {} }, 'external score section totals work too' ); - cmp_deeply( $e->run( $session, qq{jump {taggedX('$url', ext_tag, 1) == 199} target}, {userId => $user->userId} ), + cmp_deeply( $e->run( $session, qq{jump {taggedX('$url', ext_tag) == 199} target}, {userId => $user->userId} ), { jump => 'target', tags => {} }, 'external tag lookups work too' ); + + # Test for nasty bugs caused by file-scoped lexicals not being properly initialised in L + { + # Create a second test user + my $survey2 = WebGUI::Asset::Wobject::Survey->new($session, $survey->getId); + my $user2 = WebGUI::User->new( $session, 'new' ); + WebGUI::Test->usersToDelete($user2); + $session->user({userId => $user2->userId}); + my $responseId2 = $survey2->responseId( { userId => $user2->userId } ); + my $rJSON2 = $survey2->responseJSON(undef, $responseId2); + $rJSON2->recordResponses({ + '0-0-0' => 'My ext_s0q0a0 answer', + '0-1-0' => 'My ext_s0q1a0 answer', + }); + $rJSON2->processExpression(q{ tag(ext_tag, 299) }); + # Remember to persist our changes.. + $survey2->persistSurveyJSON(); + $survey2->persistResponseJSON(); + $survey2->surveyEnd; + + cmp_deeply( $e->run( $session, qq{jump {taggedX('$url', ext_tag) == 299} target}, {userId => $user2->userId} ), + { jump => 'target', tags => {} }, 'external tag not cached' ); + + cmp_deeply( $e->run( $session, qq{jump {taggedX('$url', ext_tag) == 199} target}, {userId => $user->userId} ), + { jump => 'target', tags => {} }, 'first external tag lookups still works' ); + } } #----------------------------------------------------------------------------