Check out the LimeSurvey source code on GitHub!

templatereplace() - how many levels of recursive substitution do we need?

5 years 7 months ago #63367 by TMSWhite
... and do we really need to call it and other replacements functions so many times per page?

This thread is for making the discussion about user issue #5268 easier to discuss, especially as it pertains to the Code Ignitor branch.

templatereplace() substitutes the majority of the {TOKEN:xxx}, {INSERTANS:xxx}, and other {xxxxx} variables. Some of the replacement values include other {xxxx} values - such as {THEREAREXQUESTIONS} which will generate "There are {NUMBEROFQUESTIONS} questions in this survey" if there is more than one question. Other examples of this include {GROUPDESCRIPTION}, {QUESTION}, etc. - any place where the user might put {INSERTANS:xxxx} or {TOKEN:xxxx}

The old-style templatereplace() did all of the substitutions via sequential str_replace() functions that were tightly coupled with decision logic to determine what the replacement values should be. It had many "hidden" order dependencies within it such that if you tried to add a new replacement type and didn't know where to put it in the sequence of str_replace() calls, you could get unexpected results.

replacements.php is moving us to pre-identify all of the needed replacement variables and values, and then run them all in a simple for loop. After a couple of hiccups, we discovered that we need to do at least two levels of recursive substitution. According to my regression tests, as long as people don't try to put things like {THEREAREXQUESTIONS} directly into an {INSERTANS:xxx} or {TOKEN:xxxx} value, two levels of recursive substitution is enough. Can we think of any cases where we might want or need even deeper recursive substitution?

The second question, which may also be applicable to the Code Ignitor branch, is whether we really need to call templatereplace() (and similar functions) so many times per page; and whether we can cache any of the substitution mappings.

templatereplace() is called from 161 places in the code. Internally, it calls insertAnsReplace() and tokenReplace()
dTexts::run() is called from 24 places (it only calls insertAnsReplace() and tokenReplace())
Replacefields() is called from 26 places
PassThruReplace()is called from 8 places
str_replace() itself is called about 80 times for replacements that should be managed within replacemets.php:
  1. common_functions.php::getQuotaInformation() - 4 times (for {SAVEDID}, {SID}, {LANG}, {TOKEN})
  2. admin/dataentry.php - 3 times (for {QUESTION}, {ANSWER})
  3. admin/conditionshandling.php - 1 time (for {QID})
  4. group.php - 6 times (for {SAVEDID}, {SID}, {LANG}, {TOKEN}, {QUESTION_ESSENTIALS}, {QUESTION_CLASS})
  5. index.php - 4 times (for {PERC}, {TOTAL})
  7. qanda.php::retrieveAnswers() - many times for ({QUESTION_SGQA},{TIME},{QUESTION_START})
  8. qanda.php::answer_replace() - called internally - should be replaced with insertAnsReplace()
  9. question.php - 6 times (for {SAVEDID}, {SID}, {LANG}, {TOKEN}, {QUESTION_ESSENTIALS}, {QUESTION_CLASS})
  10. survey.php - 6 times (for {SAVEDID}, {SID}, {LANG}, {TOKEN}, {QUESTION_ESSENTIALS}, {QUESTION_CLASS})
  12. admin/scripts/kcfinder/core/uploader.php - has own copy of RaplaceFields()
  13. admin/usercontrol.php - 2 times ({NAME}, {EMAIL})

Trying to keep track of which modules are calling which can be mind-numbing, and error prone.

Long term, can we think about refactoring this into static and dynamic substitutions?
(1) Some should be essentially global for the survey
(2) Others, like {QUESTION}, {ANSWER}, {QID}, {QUESTION_ESSENTIALS} could be called from a loop for each Question being processed. Rather than trying to do the final replacement value, we could map them to other replacements values - so {QUESTION} might map to {INSERTQUEST:xxxxx} where xxxxx is the SGQA code. Final substitution of those values could be done in a later step.
(3) Others like the email macros iterate over the Tokens table to output user names, etc. - those should not need recursive replacement.
(4) What other variable substitutions are used within groups?

If this sort of approach could work, it might simplify CodeIgnitor branch processing. Advantages include:
  • templatereplace() could cache all global replacement values instead of re-setting them multiple times per page
  • LimeSurvey could optionally show templates that are rendered all except for the final step of inserting QUESTION, ANSWER, etc. values - this might make template development and debugging easier
  • ExpressionManager could do all replacements in a single pass. This would have much better performance, and be a single point of debugging
  • We could easily add deeper recursive substitution and have it all done in one place (for easier validation and debugging)

Is such refactoring already part of the Code Ignitor planning process?


Please Log in to join the conversation.

5 years 7 months ago #63507 by jelo

TMSWhite wrote: According to my regression tests, as long as people don't try to put things like {THEREAREXQUESTIONS} directly into an {INSERTANS:xxx} or {TOKEN:xxxx} value, two levels of recursive substitution is enough.

If you would allow such things would be a third recursive run be enough? Or would it depend on which kind of data will be inserted.

I am not aware of the new roadmap ( refactoring the codebase of ls1 with knowhow of ls2) but if the price in running time is not that high and another recurive run would allow the flexibility (even if I currently see no practical value in putting a var inside the INSERTANS placeholder) I would choose three recursive runs.

I have to admit that still don't have enough knowledge of the codebase since I was waiting for LS2 for release. I now need to think about a new face-saver ;-)

I am really impressed by your posts and your motivation. I am afraid that I am too slow to keep up with your posts. I hope that you have lot of patience with us. I am still chewing the framework change and the restart of LS2 in shape of LS1+CodeIgniter. Since you're used a surveypackage based on java a broader discussion about how deep and broad the refactoring will take place would be interesting to bring LS on a new level.

Please Log in to join the conversation.

Imprint                   Privacy policy         General Terms & Conditions         Revocation information and revocation form