Skip to content

Commit

Permalink
Fixed issue #10017: Make sure rand is only evaluated once each survey…
Browse files Browse the repository at this point in the history
…, using a cache for the function result
  • Loading branch information
olleharstedt committed Feb 2, 2016
1 parent b9260c8 commit a2dc8ab
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 1 deletion.
35 changes: 35 additions & 0 deletions application/helpers/expressions/em_core_helper.php
Expand Up @@ -2070,6 +2070,34 @@ private function RDP_RunFunction($funcNameToken,$params)
$result = NAN;
}
break;
case 'rand':

// Get unique hash for this rand usage, from question id, group id, params and tokens
// TODO: Add user id too?
$flattened_tokens = implode(flattenrec($this->RDP_tokens));
$randHash = hash('md5', $this->sid . $this->questionSeq . $this->groupSeq . $params[0] . $params[1] . $flattened_tokens);

// Create cache if it's not there
if (!isset($_SESSION['randResultCache']))
{
$_SESSION['randResultCache'] = array();
}

// Return cache result if it's there; otherwise, store and return
if (isset($_SESSION['randResultCache'][$randHash]))
{
$result = $_SESSION['randResultCache'][$randHash];
}
else
{
$result = $funcName($params[0], $params[1]);
$_SESSION['randResultCache'][$randHash] = $result;
}

// Cache is cleared in function killSurveySession

break;

default:
$result = $funcName($params[0], $params[1]);
break;
Expand Down Expand Up @@ -2925,4 +2953,11 @@ function exprmgr_unique($args)
}
return true;
}

function flattenrec(array $array) {
$return = array();
array_walk_recursive($array, function($a) use (&$return) { $return[] = $a; });
return $return;
}

?>
3 changes: 2 additions & 1 deletion application/helpers/expressions/em_manager_helper.php
Expand Up @@ -4749,7 +4749,6 @@ static function GetAllVarNamesForQ($qseq,$varname)
/**
* Should be first function called on each page - sets/clears internally needed variables
* @param <type> $allOnOnePage - true if StartProcessingGroup will be called multiple times on this page - does some optimizatinos
* @param <type> $rooturl - if set, this tells LEM to enable hyperlinking of syntax highlighting to ease editing of questions
* @param <boolean> $initializeVars - if true, initializes the replacement variables to enable syntax highlighting on admin pages
*/
static function StartProcessingPage($allOnOnePage=false,$initializeVars=false)
Expand Down Expand Up @@ -5552,6 +5551,7 @@ static function JumpTo($seq,$preview=false,$processPOST=true,$force=false,$chang
return $LEM->lastMoveResult;
break;
case 'group':

// First validate the current group
$LEM->StartProcessingPage();
if ($processPOST)
Expand Down Expand Up @@ -6798,6 +6798,7 @@ function _ValidateQuestion($questionSeq,$force=false)
'valid' => $qvalid,
);
$_SESSION[$LEM->sessid]['relevanceStatus'][$qid] = $qrel;

return $qStatus;
}

Expand Down
1 change: 1 addition & 0 deletions application/helpers/frontend_helper.php
Expand Up @@ -2187,6 +2187,7 @@ function killSurveySession($iSurveyID)
{
// Unset the session
unset($_SESSION['survey_'.$iSurveyID]);
unset($_SESSION['randResultCache']);
// Force EM to refresh
LimeExpressionManager::SetDirtyFlag();
}
Expand Down

8 comments on commit a2dc8ab

@olleharstedt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work when you have more than one rand() in a single expression? Is the hash still unique?

@olleharstedt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, check if cache is cleared when newtest=Y

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

killSurveySession kill all rand value for ALL surveys (and all questions)

@olleharstedt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$SESSION['survey'.$iSurveyID]['randResultCache']

@olleharstedt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sha1 instead of md5

@olleharstedt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serialize instead of flatten

@olleharstedt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12:45:30 - shnoulle: Did you do a quick test : join(rand(1,10),rand(1,10),rand(1,10)) ?
12:45:51 - shnoulle: It's not the same ? alwayss 111 or 777 or 888 ?

@olleharstedt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.