Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

assert that the cache does not silently fail #1061

Merged
merged 2 commits into from

4 participants

Mark S. Mark Story ADmad José Lorenzo Rodríguez
Mark S.
Collaborator

could also be just a triggered notice/error, though...
but silently doing nothing is a little bit problematic. especially when using it for sessions you only realize it when not being able to login.

Mark Story
Owner

I think this is pretty reasonable. I have some worries that it will cause exceptions where there used to be none, but all of the new exceptions are covering failures cases that probably shouldn't have been quiet in the past.

ADmad
Collaborator

:+1:

José Lorenzo Rodríguez
Owner

I've run into this a couple times, makes sense to add it and seems safe enough for me. People should be informed when the app is not going to cache

José Lorenzo Rodríguez
Owner

This is just missing a test case :(

Mark S.
Collaborator

I will add that then.

Mark Story markstory merged commit 0b43a5c into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 7, 2013
  1. Mark S.
Commits on Jan 8, 2013
  1. Mark S.

    add test case

    dereuromark authored
This page is out of date. Refresh to see the latest.
Showing with 18 additions and 7 deletions.
  1. +7 −7 lib/Cake/Cache/Cache.php
  2. +11 −0 lib/Cake/Test/Case/Cache/CacheTest.php
14 lib/Cake/Cache/Cache.php
View
@@ -154,20 +154,20 @@ protected static function _buildEngine($name) {
$cacheClass = $class . 'Engine';
App::uses($cacheClass, $plugin . 'Cache/Engine');
if (!class_exists($cacheClass)) {
- return false;
+ throw new CacheException(__d('cake_dev', 'Cache engine %s is not available.', $name));
}
$cacheClass = $class . 'Engine';
if (!is_subclass_of($cacheClass, 'CacheEngine')) {
throw new CacheException(__d('cake_dev', 'Cache engines must use CacheEngine as a base class.'));
}
self::$_engines[$name] = new $cacheClass();
- if (self::$_engines[$name]->init($config)) {
- if (self::$_engines[$name]->settings['probability'] && time() % self::$_engines[$name]->settings['probability'] === 0) {
- self::$_engines[$name]->gc();
- }
- return true;
+ if (!self::$_engines[$name]->init($config)) {
+ throw new CacheException(__d('cake_dev', 'Cache engine %s is not properly configured.', $name));
}
- return false;
+ if (self::$_engines[$name]->settings['probability'] && time() % self::$_engines[$name]->settings['probability'] === 0) {
+ self::$_engines[$name]->gc();
+ }
+ return true;
}
/**
11 lib/Cake/Test/Case/Cache/CacheTest.php
View
@@ -65,6 +65,17 @@ public function testConfig() {
}
/**
+ * testConfigInvalidEngine method
+ *
+ * @expectedException CacheException
+ * @return void
+ */
+ public function testConfigInvalidEngine() {
+ $settings = array('engine' => 'Imaginary');
+ Cache::config('imaginary', $settings);
+ }
+
+/**
* Check that no fatal errors are issued doing normal things when Cache.disable is true.
*
* @return void
Something went wrong with that request. Please try again.