Skip to content

Commit

Permalink
MDL-47832 cache: removed data source aggregate functionality
Browse files Browse the repository at this point in the history
Cache data source aggregate functionality was found to be broken
and unused, because of this the decision was made to remove it
rather than fix it.
As it was broken we did not follow typical deprecation methods and
instead the code was removed outright with only structure
remaining and left deprecated.
  • Loading branch information
Sam Hemelryk committed Nov 19, 2014
1 parent 296b602 commit 472b438
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 38 deletions.
8 changes: 4 additions & 4 deletions cache/classes/definition.php
Expand Up @@ -242,7 +242,8 @@ class cache_definition {
protected $datasourcefile = null;

/**
* The data source class aggregate to use. This is a super advanced setting.
* Deprecated - this is completely unused.
* @deprecated since 2.8.2
* @var string
*/
protected $datasourceaggregate = null;
Expand Down Expand Up @@ -324,11 +325,11 @@ class cache_definition {
*
* @param string $id
* @param array $definition
* @param string $datasourceaggregate
* @param string $unused Used to be datasourceaggregate but that was removed and this is now unused.
* @return cache_definition
* @throws coding_exception
*/
public static function load($id, array $definition, $datasourceaggregate = null) {
public static function load($id, array $definition, $unused = null) {
global $CFG;

if (!array_key_exists('mode', $definition)) {
Expand Down Expand Up @@ -520,7 +521,6 @@ public static function load($id, array $definition, $datasourceaggregate = null)
$cachedefinition->overrideclassfile = $overrideclassfile;
$cachedefinition->datasource = $datasource;
$cachedefinition->datasourcefile = $datasourcefile;
$cachedefinition->datasourceaggregate = $datasourceaggregate;
$cachedefinition->staticacceleration = $staticacceleration;
$cachedefinition->staticaccelerationsize = $staticaccelerationsize;
$cachedefinition->ttl = $ttl;
Expand Down
26 changes: 9 additions & 17 deletions cache/classes/factory.php
Expand Up @@ -186,17 +186,17 @@ public function reset_cache_instances() {
* @param string $component
* @param string $area
* @param array $identifiers
* @param string $aggregate
* @param string $unused Used to be data source aggregate however that was removed and this is now unused.
* @return cache_application|cache_session|cache_request
*/
public function create_cache_from_definition($component, $area, array $identifiers = array(), $aggregate = null) {
public function create_cache_from_definition($component, $area, array $identifiers = array(), $unused = null) {
$definitionname = $component.'/'.$area;
if (isset($this->cachesfromdefinitions[$definitionname])) {
$cache = $this->cachesfromdefinitions[$definitionname];
$cache->set_identifiers($identifiers);
return $cache;
}
$definition = $this->create_definition($component, $area, $aggregate);
$definition = $this->create_definition($component, $area);
$definition->set_identifiers($identifiers);
$cache = $this->create_cache($definition, $identifiers);
// Loaders are always held onto to speed up subsequent requests.
Expand Down Expand Up @@ -392,14 +392,13 @@ public function create_config_instance($writer = false) {
* Creates a definition instance or returns the existing one if it has already been created.
* @param string $component
* @param string $area
* @param string $aggregate
* @param string $unused This used to be data source aggregate - however that functionality has been removed and
* this argument is now unused.
* @return cache_definition
* @throws coding_exception If the definition cannot be found.
*/
public function create_definition($component, $area, $aggregate = null) {
public function create_definition($component, $area, $unused = null) {
$id = $component.'/'.$area;
if ($aggregate) {
$id .= '::'.$aggregate;
}
if (!isset($this->definitions[$id])) {
// This is the first time this definition has been requested.
if ($this->is_initialising()) {
Expand All @@ -421,13 +420,6 @@ public function create_definition($component, $area, $aggregate = null) {
// To serve this purpose and avoid errors we are going to make use of an ad-hoc cache rather than
// search for the definition which would possibly cause an infitite loop trying to initialise the cache.
$definition = cache_definition::load_adhoc(cache_store::MODE_REQUEST, $component, $area);
if ($aggregate !== null) {
// If you get here you deserve a warning. We have to use an ad-hoc cache here, so we can't find the definition and therefor
// can't find any information about the datasource or any of its aggregated.
// Best of luck.
debugging('An unknown cache was requested during development with an aggregate that could not be loaded. Ad-hoc cache used instead.', DEBUG_DEVELOPER);
$aggregate = null;
}
} else {
// Either a typo of the developer has just created the definition and is using it for the first time.
$this->reset();
Expand All @@ -440,10 +432,10 @@ public function create_definition($component, $area, $aggregate = null) {
debugging('Cache definitions reparsed causing cache reset in order to locate definition.
You should bump the version number to ensure definitions are reprocessed.', DEBUG_DEVELOPER);
}
$definition = cache_definition::load($id, $definition, $aggregate);
$definition = cache_definition::load($id, $definition);
}
} else {
$definition = cache_definition::load($id, $definition, $aggregate);
$definition = cache_definition::load($id, $definition);
}
}
$this->definitions[$id] = $definition;
Expand Down
8 changes: 0 additions & 8 deletions cache/classes/helper.php
Expand Up @@ -280,13 +280,6 @@ public static function invalidate_by_event($event, array $keys) {
/**
* Purges the cache for a specific definition.
*
* If you need to purge a definition that requires identifiers or an aggregate and you don't
* know the details of those please use cache_helper::purge_stores_used_by_definition instead.
* It is a more aggressive purge and will purge all data within the store, not just the data
* belonging to the given definition.
*
* @todo MDL-36660: Change the signature: $aggregate must be added.
*
* @param string $component
* @param string $area
* @param array $identifiers
Expand All @@ -298,7 +291,6 @@ public static function purge_by_definition($component, $area, array $identifiers
// Initialise, in case of a store.
if ($cache instanceof cache_store) {
$factory = cache_factory::instance();
// TODO MDL-36660: Providing $aggregate is required for purging purposes: $definition->get_id()
$definition = $factory->create_definition($component, $area, null);
$definition->set_identifiers($identifiers);
$cache->initialise($definition);
Expand Down
6 changes: 3 additions & 3 deletions cache/classes/loaders.php
Expand Up @@ -166,12 +166,12 @@ class cache implements cache_loader {
* @param string $component The component for the definition
* @param string $area The area for the definition
* @param array $identifiers Any additional identifiers that should be provided to the definition.
* @param string $aggregate Super advanced feature. More docs later.
* @param string $unused Used to be datasourceaggregate but that was removed and this is now unused.
* @return cache_application|cache_session|cache_store
*/
public static function make($component, $area, array $identifiers = array(), $aggregate = null) {
public static function make($component, $area, array $identifiers = array(), $unused = null) {
$factory = cache_factory::instance();
return $factory->create_cache_from_definition($component, $area, $identifiers, $aggregate);
return $factory->create_cache_from_definition($component, $area, $identifiers);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions cache/disabledlib.php
Expand Up @@ -184,10 +184,10 @@ public static function instance($forcereload = false) {
*
* @param string $component
* @param string $area
* @param string $aggregate Unused.
* @param string $unused Used to be datasourceaggregate but that was removed and this is now unused.
* @return cache_definition
*/
public function create_definition($component, $area, $aggregate = null) {
public function create_definition($component, $area, $unused = null) {
return cache_definition::load_adhoc(cache_store::MODE_REQUEST, $component, $area);
}

Expand All @@ -208,11 +208,11 @@ public function create_cache(cache_definition $definition) {
* @param string $component
* @param string $area
* @param array $identifiers
* @param string $aggregate
* @param string $unused Used to be datasourceaggregate but that was removed and this is now unused.
* @return cache_application|cache_session|cache_request
*/
public function create_cache_from_definition($component, $area, array $identifiers = array(), $aggregate = null) {
$definition = $this->create_definition($component, $area, $aggregate);
public function create_cache_from_definition($component, $area, array $identifiers = array(), $unused = null) {
$definition = $this->create_definition($component, $area);
$cache = $this->create_cache($definition, $identifiers);
return $cache;
}
Expand Down
12 changes: 11 additions & 1 deletion cache/upgrade.txt
@@ -1,6 +1,16 @@
This files describes API changes in /cache/stores/* - cache store plugins.
Information provided here is intended especially for developers.

=== 2.8.2 ===
* Cache data source aggregation functionality has been removed. This functionality was found to be broken and unused.
It was decided that rather than fixing it it should be removed.
As well as the processing code being removed the following API changes have been made.
The following changes have come about because of it:
- cache_definition::$datasourceaggregate is deprecated an unused.
- cache_definition::load Argument 3 (final arg) is now unused.
- cache_factory::create_cache_from_definition Argument 4 (final arg) is now unused.
- cache::make Argument 4 (final arg) is now unused.

=== 2.7 ===
* cache_store::is_ready is no longer abstract, calling cache_store::are_requirements_met by default.

Expand All @@ -22,4 +32,4 @@ Information provided here is intended especially for developers.
* cleanup method renamed to instance_deleted.
It is now called when the store is deleted as all comments suggested anyway.
* instance_created method added.
It is called when the store is created for the very first time.
It is called when the store is created for the very first time.

0 comments on commit 472b438

Please sign in to comment.