Stop caching all module variables in the vars table using the

name=_cache row.  If that overflows, it will cause us to be unable to
load variables, and we can't recover from that.

Instead, use the Cache table.  Bump the gallery module to v40.  Fixes
ticket #1405.

5 files changed, 15 insertions, 55 deletions

diff --git a/modules/gallery/controllers/packager.php b/modules/gallery/controllers/packager.php
index f463d0de..bd51b93c 100644
--- a/modules/gallery/controllers/packager.php
+++ b/modules/gallery/controllers/packager.php
@@ -81,11 +81,6 @@ class Packager_Controller extends Controller {
     Database::instance()->query("TRUNCATE {caches}");
     Database::instance()->query("TRUNCATE {sessions}");
     Database::instance()->query("TRUNCATE {logs}");
-    db::build()
-      ->delete("vars")
-      ->where("module_name", "=", "gallery")
-      ->where("name", "=", "_cache")
-      ->execute();
     db::build()->update("users")
       ->set(array("password" => ""))
       ->where("id", "in", array(1, 2))
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index 83961d6b..3d82bc69 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -309,7 +309,7 @@ class gallery_installer {
     module::set_var("gallery", "show_user_profiles_to", "registered_users");
     module::set_var("gallery", "extra_binary_paths", "/usr/local/bin:/opt/local/bin:/opt/bin");
 
-    module::set_version("gallery", 40);
+    module::set_version("gallery", 41);
   }
 
   static function upgrade($version) {
@@ -637,6 +637,11 @@ class gallery_installer {
       module::set_var("gallery", "extra_binary_paths", "/usr/local/bin:/opt/local/bin:/opt/bin");
       module::set_version("gallery", $version = 40);
     }
+
+    if ($version == 40) {
+      module::clear_var("gallery", "_cache");
+      module::set_version("gallery", $version = 41);
+    }
   }
 
   static function uninstall() {
diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php
index 7863520e..64d0d1d6 100644
--- a/modules/gallery/helpers/module.php
+++ b/modules/gallery/helpers/module.php
@@ -425,48 +425,21 @@ class module_Core {
    * @return the value
    */
   static function get_var($module_name, $name, $default_value=null) {
-    // We cache all vars in gallery._cache so that we can load all vars at once for
-    // performance.
+    // We cache vars so we can load them all at once for performance.
     if (empty(self::$var_cache)) {
-      $row = db::build()
-        ->select("value")
-        ->from("vars")
-        ->where("module_name", "=", "gallery")
-        ->where("name", "=", "_cache")
-        ->execute()
-        ->current();
-      if ($row) {
-        self::$var_cache = unserialize($row->value);
-      } else {
-        // gallery._cache doesn't exist.  Create it now.
+      self::$var_cache = Cache::instance()->get("var_cache");
+      if (empty(self::$var_cache)) {
+        // Cache doesn't exist, create it now.
         foreach (db::build()
                  ->select("module_name", "name", "value")
                  ->from("vars")
                  ->order_by("module_name")
                  ->order_by("name")
                  ->execute() as $row) {
-          if ($row->module_name == "gallery" && $row->name == "_cache") {
-            // This could happen if there's a race condition
-            continue;
-          }
           // Mute the "Creating default object from empty value" warning below
           @self::$var_cache->{$row->module_name}->{$row->name} = $row->value;
         }
-        $cache = ORM::factory("var");
-        $cache->module_name = "gallery";
-        $cache->name = "_cache";
-        $cache->value = serialize(self::$var_cache);
-        try {
-          $cache->save();
-        } catch (Database_Exception $e) {
-          // There's a potential race condition here.  Don't fail if that happens because it's
-          // bound to be transient and not a huge deal, but at least put something in the logs.
-          if (stristr($e->getMessage(), "duplicate entry")) {
-            Kohana_Log::add("error", "Failed to cache vars");
-          } else {
-            throw $e;
-          }
-        }
+        Cache::instance()->set("var_cache", self::$var_cache, array("vars"));
       }
     }
 
@@ -495,11 +468,7 @@ class module_Core {
     $var->value = $value;
     $var->save();
 
-    db::build()
-      ->delete("vars")
-      ->where("module_name", "=", "gallery")
-      ->where("name", "=", "_cache")
-      ->execute();
+    Cache::instance()->delete("var_cache");
     self::$var_cache = null;
  }
 
@@ -524,11 +493,7 @@ class module_Core {
       ->where("name", "=", $name)
       ->execute();
 
-    db::build()
-      ->delete("vars")
-      ->where("module_name", "=", "gallery")
-      ->where("name", "=", "_cache")
-      ->execute();
+    Cache::instance()->delete("var_cache");
     self::$var_cache = null;
   }
 
@@ -546,11 +511,7 @@ class module_Core {
       $var->delete();
     }
 
-    db::build()
-      ->delete("vars")
-      ->where("module_name", "=", "gallery")
-      ->where("name", "=", "_cache")
-      ->execute();
+    Cache::instance()->delete("var_cache");
     self::$var_cache = null;
   }
 
diff --git a/modules/gallery/module.info b/modules/gallery/module.info
index 1155ddf7..2b684e5e 100644
--- a/modules/gallery/module.info
+++ b/modules/gallery/module.info
@@ -1,3 +1,3 @@
 name = "Gallery 3"
 description = "Gallery core application"
-version = 40
+version = 41
diff --git a/modules/gallery/views/admin_advanced_settings.html.php b/modules/gallery/views/admin_advanced_settings.html.php
index 1f7d2f64..edaeecaf 100644
--- a/modules/gallery/views/admin_advanced_settings.html.php
+++ b/modules/gallery/views/admin_advanced_settings.html.php
@@ -17,7 +17,6 @@
         <th> <?= t("Value") ?></th>
       </tr>
       <? foreach ($vars as $var): ?>
-      <? if ($var->module_name == "gallery" && $var->name == "_cache") continue ?>
       <tr class="<?= text::alternate("g-odd", "g-even") ?>">
         <td> <?= $var->module_name ?> </td>
         <td> <?= html::clean($var->name) ?> </td>