MDL-46031 output: better handling of once-per-page items

In several places we only want to output a certain bit of HTML (e.g. the
contents on the mod chooser) once per page.

This used to be done with a static variable in the function, which
meant the logic was once per PHP-script execution, not once-per-page.
Now there is a new bit of API in page_requirements_manager to handle
this properly.

diff --git a/course/renderer.php b/course/renderer.php
index 8450ceb..8780286 100644 (file)
--- a/course/renderer.php
+++ b/course/renderer.php
@@ -69,9 +69,13 @@ class core_course_renderer extends plugin_renderer_base {
      */
     protected function add_modchoosertoggle() {
         global $CFG;
-        static $modchoosertoggleadded = false;
-        // Add the module chooser toggle to the course page
-        if ($modchoosertoggleadded || $this->page->state > moodle_page::STATE_PRINTING_HEADER ||
+
+        // Only needs to be done once per page.
+        if (!$this->page->requires->should_create_one_time_item_now('core_course_modchoosertoggle')) {
+            return;
+        }
+
+        if ($this->page->state > moodle_page::STATE_PRINTING_HEADER ||
                 $this->page->course->id == SITEID ||
                 !$this->page->user_is_editing() ||
                 !($context = context_course::instance($this->page->course->id)) ||
@@ -79,11 +83,11 @@ class core_course_renderer extends plugin_renderer_base {
                 !course_ajax_enabled($this->page->course) ||
                 !($coursenode = $this->page->settingsnav->find('courseadmin', navigation_node::TYPE_COURSE)) ||
                 !($turneditingnode = $coursenode->get('turneditingonoff'))) {
-            // too late or we are on site page or we could not find the adjacent nodes in course settings menu
-            // or we are not allowed to edit
+            // Too late, or we are on site page, or we could not find the
+            // adjacent nodes in course settings menu, or we are not allowed to edit.
             return;
         }
-        $modchoosertoggleadded = true;
+
         if ($this->page->url->compare(new moodle_url('/course/view.php'), URL_MATCH_BASE)) {
             // We are on the course page, retain the current page params e.g. section.
             $modchoosertoggleurl = clone($this->page->url);
@@ -168,11 +172,9 @@ class core_course_renderer extends plugin_renderer_base {
      * @return string The composed HTML for the module
      */
     public function course_modchooser($modules, $course) {
-        static $isdisplayed = false;
-        if ($isdisplayed) {
+        if (!$this->page->requires->should_create_one_time_item_now('core_course_modchooser')) {
             return '';
         }
-        $isdisplayed = true;
 
         // Add the module chooser
         $this->page->requires->yui_module('moodle-course-modchooser',
@@ -1492,14 +1494,14 @@ class core_course_renderer extends plugin_renderer_base {
      * Make sure that javascript file for AJAX expanding of courses and categories content is included
      */
     protected function coursecat_include_js() {
-        global $CFG;
-        static $jsloaded = false;
-        if (!$jsloaded) {
-            // We must only load this module once.
-            $this->page->requires->yui_module('moodle-course-categoryexpander',
-                    'Y.Moodle.course.categoryexpander.init');
-            $jsloaded = true;
+        if (!$this->page->requires->should_create_one_time_item_now('core_course_categoryexpanderjsinit')) {
+            return;
         }
+
+        // We must only load this module once.
+        $this->page->requires->set_required_html_output('core_course_categoryexpanderjsinit');
+        $this->page->requires->yui_module('moodle-course-categoryexpander',
+                'Y.Moodle.course.categoryexpander.init');
     }
 
     /**

diff --git a/files/renderer.php b/files/renderer.php
index f4e5ffa..7623b14 100644 (file)
--- a/files/renderer.php
+++ b/files/renderer.php
@@ -102,7 +102,6 @@ class core_files_renderer extends plugin_renderer_base {
      * @return string HTML fragment
      */
     public function render_form_filemanager($fm) {
-        static $filemanagertemplateloaded;
         $html = $this->fm_print_generallayout($fm);
         $module = array(
             'name'=>'form_filemanager',
@@ -117,8 +116,7 @@ class core_files_renderer extends plugin_renderer_base {
                 array('confirmrenamefile', 'repository'), array('newfolder', 'repository'), array('edit', 'moodle')
             )
         );
-        if (empty($filemanagertemplateloaded)) {
-            $filemanagertemplateloaded = true;
+        if ($this->page->requires->should_create_one_time_item_now('core_file_managertemplate')) {
             $this->page->requires->js_init_call('M.form_filemanager.set_templates',
                     array($this->filemanager_js_templates()), true, $module);
         }

diff --git a/lib/outputrequirementslib.php b/lib/outputrequirementslib.php
index 01700af..0442053 100644 (file)
--- a/lib/outputrequirementslib.php
+++ b/lib/outputrequirementslib.php
@@ -115,6 +115,13 @@ class page_requirements_manager {
      */
     protected $extramodules = array();
 
+    /**
+     * @var array trackes the names of bits of HTML that are only required once
+     * per page. See {@link has_one_time_item_been_created()},
+     * {@link set_one_time_item_created()} and {@link should_create_one_time_item_now()}.
+     */
+    protected $onetimeitemsoutput = array();
+
     /**
      * @var bool Flag indicated head stuff already printed
      */
@@ -1492,6 +1499,68 @@ class page_requirements_manager {
     public function is_top_of_body_done() {
         return $this->topofbodydone;
     }
+
+    /**
+     * Should we generate a bit of content HTML that is only required once  on
+     * this page (e.g. the contents of the modchooser), now? Basically, we call
+     * {@link has_one_time_item_been_created()}, and if the thing has not already
+     * been output, we return true to tell the caller to generate it, and also
+     * call {@link set_one_time_item_created()} to record the fact that it is
+     * about to be generated.
+     *
+     * That is, a typical usage pattern (in a renderer method) is:
+     * <pre>
+     * if (!$this->page->requires->should_create_one_time_item_now($thing)) {
+     *     return '';
+     * }
+     * // Else generate it.
+     * </pre>
+     *
+     * @param string $thing identifier for the bit of content. Should be of the form
+     *      frankenstyle_things, e.g. core_course_modchooser.
+     * @return bool if true, the caller should generate that bit of output now, otherwise don't.
+     */
+    public function should_create_one_time_item_now($thing) {
+        if ($this->has_one_time_item_been_created($thing)) {
+            return false;
+        }
+
+        $this->set_one_time_item_created($thing);
+        return true;
+    }
+
+    /**
+     * Has a particular bit of HTML that is only required once  on this page
+     * (e.g. the contents of the modchooser) already been generated?
+     *
+     * Normally, you can use the {@link should_create_one_time_item_now()} helper
+     * method rather than calling this method directly.
+     *
+     * @param string $thing identifier for the bit of content. Should be of the form
+     *      frankenstyle_things, e.g. core_course_modchooser.
+     * @return bool whether that bit of output has been created.
+     */
+    public function has_one_time_item_been_created($thing) {
+        return isset($this->onetimeitemsoutput[$thing]);
+    }
+
+    /**
+     * Indicate that a particular bit of HTML that is only required once on this
+     * page (e.g. the contents of the modchooser) has been generated (or is about to be)?
+     *
+     * Normally, you can use the {@link should_create_one_time_item_now()} helper
+     * method rather than calling this method directly.
+     *
+     * @param string $thing identifier for the bit of content. Should be of the form
+     *      frankenstyle_things, e.g. core_course_modchooser.
+     */
+    public function set_one_time_item_created($thing) {
+        if ($this->has_one_time_item_been_created($thing)) {
+            throw new coding_exception($thing . ' is only supposed to be ouput ' .
+                    'once per page, but it seems to be being output again.');
+        }
+        return $this->onetimeitemsoutput[$thing] = true;
+    }
 }
 
 /**

diff --git a/lib/tests/outputrequirementslib_test.php b/lib/tests/outputrequirementslib_test.php
index 863315b..c37ba55 100644 (file)
--- a/lib/tests/outputrequirementslib_test.php
+++ b/lib/tests/outputrequirementslib_test.php
@@ -36,14 +36,31 @@ class core_outputrequirementslib_testcase extends advanced_testcase {
         $page = new moodle_page();
         $page->requires->string_for_js('course', 'moodle', 1);
         $page->requires->string_for_js('course', 'moodle', 1);
-        try {
-            $page->requires->string_for_js('course', 'moodle', 2);
-            $this->fail('Exception expected when the same string with different $a requested');
-        } catch (Exception $e) {
-            $this->assertInstanceOf('coding_exception', $e);
-        }
+        $this->setExpectedException('coding_exception');
+        $page->requires->string_for_js('course', 'moodle', 2);
 
         // Note: we can not switch languages in phpunit yet,
         //       it would be nice to test that the strings are actually fetched in the footer.
     }
+
+    public function test_one_time_output_normal_case() {
+        $page = new moodle_page();
+        $this->assertTrue($page->requires->should_create_one_time_item_now('test_item'));
+        $this->assertFalse($page->requires->should_create_one_time_item_now('test_item'));
+    }
+
+    public function test_one_time_output_repeat_output_throws() {
+        $page = new moodle_page();
+        $page->requires->set_one_time_item_created('test_item');
+        $this->setExpectedException('coding_exception');
+        $page->requires->set_one_time_item_created('test_item');
+    }
+
+    public function test_one_time_output_different_pages_independent() {
+        $firstpage = new moodle_page();
+        $secondpage = new moodle_page();
+        $this->assertTrue($firstpage->requires->should_create_one_time_item_now('test_item'));
+        $this->assertTrue($secondpage->requires->should_create_one_time_item_now('test_item'));
+    }
+
 }