Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-36878

course/lib.php's get_module_metadata() function never uses its cache

    Details

      Description

      This is not a functionality bug, but potentially a performance issue. In course/lib.php's get_module_metadata() function a static variable is initialized to cache module information and the information is stored in it, but cache never gets used because the check to see if module information has been stored is incorrect.

      The check current looks like this:

      if (isset($modlist[$modname])) {
          // This module is already cached
          $return[$modname] = $modlist[$course->id][$modname];
          continue;
      }

      It should look like this:

      if (isset($modlist[$course->id][$modname])) {
          // This module is already cached
          $return[$modname] = $modlist[$course->id][$modname];
          continue;
      }

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              dobedobedoh Andrew Nicols added a comment -

              Just to note, that the cache was only ever introduced in Moodle 2.3 when work was done to create the activity chooser. I guess it's never been used

              Show
              dobedobedoh Andrew Nicols added a comment - Just to note, that the cache was only ever introduced in Moodle 2.3 when work was done to create the activity chooser. I guess it's never been used
              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for reporting this and providing a patch. I'll leave it for Sam to triage.

              Show
              salvetore Michael de Raadt added a comment - Thanks for reporting this and providing a patch. I'll leave it for Sam to triage.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks for the report Sam. Increasing the priority as this you've already provided a patch and this is a simple fix.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks for the report Sam. Increasing the priority as this you've already provided a patch and this is a simple fix.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Putting this up for integration review now.

              Show
              samhemelryk Sam Hemelryk added a comment - Putting this up for integration review now.
              Hide
              poltawski Dan Poltawski added a comment -

              I've integrated this now, thanks Sams!

              Show
              poltawski Dan Poltawski added a comment - I've integrated this now, thanks Sams!
              Hide
              poltawski Dan Poltawski added a comment -

              Looking good here, passing. Thanks!

              Show
              poltawski Dan Poltawski added a comment - Looking good here, passing. Thanks!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Y E S !

              Closing as fixed, many thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Y E S ! Closing as fixed, many thanks!

                People

                • Votes:
                  2 Vote for this issue
                  Watchers:
                  4 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    14/Jan/13