From 804dbcfd26cd93bf7f2fa789545bdff2778fb155 Mon Sep 17 00:00:00 2001
From: Patrick Donelan <pat@patspam.com>
Date: Thu, 5 Nov 2009 22:09:23 -0500
Subject: [PATCH] One-line fix for Survey ExpressionEngine bug that took over a
 day to find.

One of the file-scoped lexicals in ExpressionEngine wasn't being initialised,
which meant that tagged data was being cached across repeated engine runs
(including, would you believe, across modperl page requests).

The fix includes tests.
---
 docs/changelog/7.x.x.txt                      |  1 +
 .../Asset/Wobject/Survey/ExpressionEngine.pm  |  7 +++--
 t/Asset/Wobject/Survey/ExpressionEngine.t     | 30 +++++++++++++++++--
 3 files changed, 34 insertions(+), 4 deletions(-)

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<run> 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<ExpressionEngine::run>
+    {
+        # 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' );
+    }
 }
 
 #----------------------------------------------------------------------------