MDL-34347 remove all problematic global text caching

Instead we will create new MUC caches inside each filter plugin. Please note that all cache filters should work with local caches without the need of strict cache invalidation.
vsiguero · Jan 14, 2014 · 62c8032 · 62c8032
1 parent 12efa52
commit 62c8032
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 137 deletions.
diff --git a/admin/settings/plugins.php b/admin/settings/plugins.php
@@ -142,31 +142,7 @@
     // "filtersettings" settingpage
     $temp = new admin_settingpage('commonfiltersettings', new lang_string('commonfiltersettings', 'admin'));
     if ($ADMIN->fulltree) {
-        $cachetimes = array(
-            604800 => new lang_string('numdays','',7),
-            86400 => new lang_string('numdays','',1),
-            43200 => new lang_string('numhours','',12),
-            10800 => new lang_string('numhours','',3),
-            7200 => new lang_string('numhours','',2),
-            3600 => new lang_string('numhours','',1),
-            2700 => new lang_string('numminutes','',45),
-            1800 => new lang_string('numminutes','',30),
-            900 => new lang_string('numminutes','',15),
-            600 => new lang_string('numminutes','',10),
-            540 => new lang_string('numminutes','',9),
-            480 => new lang_string('numminutes','',8),
-            420 => new lang_string('numminutes','',7),
-            360 => new lang_string('numminutes','',6),
-            300 => new lang_string('numminutes','',5),
-            240 => new lang_string('numminutes','',4),
-            180 => new lang_string('numminutes','',3),
-            120 => new lang_string('numminutes','',2),
-            60 => new lang_string('numminutes','',1),
-            30 => new lang_string('numseconds','',30),
-            0 => new lang_string('no')
-        );
         $items = array();
-        $items[] = new admin_setting_configselect('cachetext', new lang_string('cachetext', 'admin'), new lang_string('configcachetext', 'admin'), 60, $cachetimes);
         $items[] = new admin_setting_configselect('filteruploadedfiles', new lang_string('filteruploadedfiles', 'admin'), new lang_string('configfilteruploadedfiles', 'admin'), 0,
                 array('0' => new lang_string('none'), '1' => new lang_string('allfiles'), '2' => new lang_string('htmlfilesonly')));
         $items[] = new admin_setting_configcheckbox('filtermatchoneperpage', new lang_string('filtermatchoneperpage', 'admin'), new lang_string('configfiltermatchoneperpage', 'admin'), 0);

diff --git a/filter/upgrade.txt b/filter/upgrade.txt
@@ -1,6 +1,12 @@
 This file describes API changes in core filter API and plugins,
 information provided here is intended especially for developers.
 
+=== 2.7 ===
+
+* Finally filter may use $PAGE and $OUTPUT, yay!
+* Old global text caching was removed, each filter is now responsible
+  for own caching.
+
 === 2.6 ===
 
 * filtersettings.php is now deprecated, migrate to standard settings.php

diff --git a/lang/en/admin.php b/lang/en/admin.php
@@ -92,7 +92,6 @@
 $string['bookmarkthispage'] = 'Bookmark this page';
 $string['cachejs'] = 'Cache Javascript';
 $string['cachejs_help'] = 'Javascript caching and compression greatly improves page loading performance. it is strongly recommended for production sites. Developers will probably want to disable this feature.';
-$string['cachetext'] = 'Text cache lifetime';
 $string['calendarexportsalt'] = 'Calendar export salt';
 $string['calendarsettings'] = 'Calendar';
 $string['calendartype'] = 'Calendar type';
@@ -147,7 +146,6 @@
 $string['configautolang'] = 'Detect default language from browser setting, if disabled site default is used.';
 $string['configautologinguests'] = 'Should visitors be logged in as guests automatically when entering courses with guest access?';
 $string['configbloglevel'] = 'This setting allows you to restrict the level to which user blogs can be viewed on this site.  Note that they specify the maximum context of the VIEWER not the poster or the types of blog posts.  Blogs can also be disabled completely if you don\'t want them at all.';
-$string['configcachetext'] = 'For larger sites or sites that use text filters, this setting can really speed things up.  Copies of texts will be retained in their processed form for the time specified here.  Setting this too small may actually slow things down slightly,  but setting it too large may mean texts take too long to refresh (with new links, for example).';
 $string['configcalendarcustomexport'] = 'Enable custom date range export of calendar';
 $string['configcalendarexportsalt'] = 'This random text is used for improving of security of authentication tokens used for exporting of calendars. Please note that all current tokens are invalidated if you change this hash salt.';
 $string['configclamactlikevirus'] = 'Treat files like viruses';

diff --git a/lib/cronlib.php b/lib/cronlib.php
@@ -133,14 +133,6 @@ function cron_run() {
         }
 
 
-        // Delete old cached texts
-        if (!empty($CFG->cachetext)) {   // Defined in config.php
-            $cachelifetime = time() - $CFG->cachetext - 60;  // Add an extra minute to allow for really heavy sites
-            $DB->delete_records_select('cache_text', "timemodified < ?", array($cachelifetime));
-            mtrace(" Deleted old cache_text records");
-        }
-
-
         if (!empty($CFG->usetags)) {
             require_once($CFG->dirroot.'/tag/lib.php');
             tag_cron();

diff --git a/lib/db/install.xml b/lib/db/install.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="lib/db" VERSION="20131009" COMMENT="XMLDB file for core Moodle tables"
+<XMLDB PATH="lib/db" VERSION="20140112" COMMENT="XMLDB file for core Moodle tables"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
 >
@@ -532,21 +532,6 @@
         <INDEX NAME="filter_md5key" UNIQUE="false" FIELDS="filter, md5key"/>
       </INDEXES>
     </TABLE>
-    <TABLE NAME="cache_text" COMMENT="For storing temporary copies of processed texts">
-      <FIELDS>
-        <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
-        <FIELD NAME="md5key" TYPE="char" LENGTH="32" NOTNULL="true" SEQUENCE="false"/>
-        <FIELD NAME="formattedtext" TYPE="text" NOTNULL="true" SEQUENCE="false"/>
-        <FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
-      </FIELDS>
-      <KEYS>
-        <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
-      </KEYS>
-      <INDEXES>
-        <INDEX NAME="md5key" UNIQUE="false" FIELDS="md5key"/>
-        <INDEX NAME="timemodified" UNIQUE="false" FIELDS="timemodified" COMMENT="Mainly to help deletion of expired records from cron"/>
-      </INDEXES>
-    </TABLE>
     <TABLE NAME="log" COMMENT="Every action is logged as far as possible">
       <FIELDS>
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
@@ -2903,5 +2903,21 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2013122400.01);
     }
 
+    if ($oldversion < 2014011000.01) {
+
+        // Define table cache_text to be dropped.
+        $table = new xmldb_table('cache_text');
+
+        // Conditionally launch drop table for cache_text.
+        if ($dbman->table_exists($table)) {
+            $dbman->drop_table($table);
+        }
+
+        unset_config('cachetext');
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2014011000.01);
+    }
+
     return true;
 }
diff --git a/lib/phpunit/bootstrap.php b/lib/phpunit/bootstrap.php
@@ -205,7 +205,6 @@
 ini_set('log_errors', '1');
 
 $CFG->noemailever = true; // better not mail anybody from tests, override temporarily if necessary
-$CFG->cachetext = 0; // disable this very nasty setting
 
 // some ugly hacks
 $CFG->themerev = 1;

diff --git a/lib/weblib.php b/lib/weblib.php
@@ -1097,7 +1097,6 @@ function format_text_menu() {
  */
 function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseiddonotuse = null) {
     global $CFG, $DB, $PAGE;
-    static $croncache = array();
 
     if ($text === '' || is_null($text)) {
         // No need to do any filters and cleaning.
@@ -1166,34 +1165,6 @@ function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseidd
         $filtermanager = new null_filter_manager();
     }
 
-    if (!empty($CFG->cachetext) and empty($options['nocache'])) {
-        $hashstr = $text.'-'.$filtermanager->text_filtering_hash($context).'-'.$context->id.'-'.current_language().'-'.
-                (int)$format.(int)$options['trusted'].(int)$options['noclean'].
-                (int)$options['para'].(int)$options['newlines'];
-
-        $time = time() - $CFG->cachetext;
-        $md5key = md5($hashstr);
-        if (CLI_SCRIPT) {
-            if (isset($croncache[$md5key])) {
-                return $croncache[$md5key];
-            }
-        }
-
-        if ($oldcacheitem = $DB->get_record('cache_text', array('md5key' => $md5key), '*', IGNORE_MULTIPLE)) {
-            if ($oldcacheitem->timemodified >= $time) {
-                if (CLI_SCRIPT) {
-                    if (count($croncache) > 150) {
-                        reset($croncache);
-                        $key = key($croncache);
-                        unset($croncache[$key]);
-                    }
-                    $croncache[$md5key] = $oldcacheitem->formattedtext;
-                }
-                return $oldcacheitem->formattedtext;
-            }
-        }
-    }
-
     switch ($format) {
         case FORMAT_HTML:
             if (!$options['noclean']) {
@@ -1257,65 +1228,15 @@ function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseidd
         }
     }
 
-    // Warn people that we have removed this old mechanism, just in case they
-    // were stupid enough to rely on it.
-    if (isset($CFG->currenttextiscacheable)) {
-        debugging('Once upon a time, Moodle had a truly evil use of global variables ' .
-            'called $CFG->currenttextiscacheable. The good news is that this no ' .
-            'longer exists. The bad news is that you seem to be using a filter that '.
-            'relies on it. Please seek out and destroy that filter code.', DEBUG_DEVELOPER);
-    }
-
     if (!empty($options['overflowdiv'])) {
         $text = html_writer::tag('div', $text, array('class' => 'no-overflow'));
     }
 
-    if (empty($options['nocache']) and !empty($CFG->cachetext)) {
-        if (CLI_SCRIPT) {
-            // Special static cron cache - no need to store it in db if its not already there.
-            if (count($croncache) > 150) {
-                reset($croncache);
-                $key = key($croncache);
-                unset($croncache[$key]);
-            }
-            $croncache[$md5key] = $text;
-            return $text;
-        }
-
-        $newcacheitem = new stdClass();
-        $newcacheitem->md5key = $md5key;
-        $newcacheitem->formattedtext = $text;
-        $newcacheitem->timemodified = time();
-        if ($oldcacheitem) {
-            // See bug 4677 for discussion.
-            $newcacheitem->id = $oldcacheitem->id;
-            try {
-                // Update existing record in the cache table.
-                $DB->update_record('cache_text', $newcacheitem);
-            } catch (dml_exception $e) {
-                // It's unlikely that the cron cache cleaner could have
-                // deleted this entry in the meantime, as it allows
-                // some extra time to cover these cases.
-            }
-        } else {
-            try {
-                // Insert a new record in the cache table.
-                $DB->insert_record('cache_text', $newcacheitem);
-            } catch (dml_exception $e) {
-                // Again, it's possible that another user has caused this
-                // record to be created already in the time that it took
-                // to traverse this function.  That's OK too, as the
-                // call above handles duplicate entries, and eventually
-                // the cron cleaner will delete them.
-            }
-        }
-    }
-
     return $text;
 }
 
 /**
- * Resets all data related to filters, called during upgrade or when filter settings change.
+ * Resets some data related to filters, called during upgrade or when general filter settings change.
  *
  * @param bool $phpunitreset true means called from our PHPUnit integration test reset
  * @return void
@@ -1325,14 +1246,12 @@ function reset_text_filters_cache($phpunitreset = false) {
 
     if ($phpunitreset) {
         // HTMLPurifier does not change, DB is already reset to defaults,
-        // nothing to do here.
+        // nothing to do here, the dataroot was cleared too.
         return;
     }
 
-    $DB->delete_records('cache_text');
-
-    $purifdir = $CFG->cachedir.'/htmlpurifier';
-    remove_dir($purifdir, true);
+    // The purge_all_caches() deals with cachedir and localcachedir purging,
+    // the individual filter caches are invalidated as necessary elsewhere.
 
     // Update $CFG->filterall cache flag.
     if (empty($CFG->stringfilters)) {

diff --git a/version.php b/version.php
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2014011000.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2014011000.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.