Skip to content

Commit

Permalink
Fixed a bug where hooks would only fire in a global context if the "e…
Browse files Browse the repository at this point in the history 
…nabled by default on projects" setting was checked. Global hooks for enabled modules should always fire, regardless of this setting.
  • Loading branch information
@mmcev106
mmcev106 committed Dec 8, 2016
1 parent 9d40015 commit 5578bd2
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 47 deletions.
93 changes: 67 additions & 26 deletions classes/ExternalModules.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ class ExternalModules
private static $instanceCache = array();
private static $idsByPrefix;

private static $enabledVersions;
private static $globallyEnabledPrefixes;
private static $globallyEnabledVersions;
private static $projectEnabledDefaults;
private static $projectEnabledOverrides;
private static $enabledInstancesByPID = array();

Expand Down Expand Up @@ -455,7 +455,7 @@ static function callHook($name, $arguments)
}
}

$modules = self::getEnabledModulesForProject($pid);
$modules = self::getEnabledModuleInstances($pid);
foreach($modules as $instance){
$methodName = "hook_$name";

Expand Down Expand Up @@ -526,20 +526,24 @@ private static function getMainClassName($prefix)
return $className;
}

private static function getEnabledModulesForProject($pid)
// Accepts a project id as the first parameter.
// If the project id is null, all globally enabled module instances are returned.
// Otherwise, only instances enabled for the current project id are returned.
private static function getEnabledModuleInstances($pid)
{
$instances = @self::$enabledInstancesByPID[$pid];
if(!isset($instances)){
$enabledPrefixes = self::getEnabledModulePrefixesForProject($pid);
if($pid == null){
// Cache globally enabled module instances. Yes, the caching will still work even though the key ($pid) is null.
$prefixes = self::getGloballyEnabledVersions();
}
else{
$prefixes = self::getEnabledModuleVersionsForProject($pid);
}

$instances = array();
foreach(array_keys($enabledPrefixes) as $prefix){
$version = @self::$enabledVersions[$prefix];

// Check the version to make sure the module is not globally disabled.
if(isset($version)){
$instances[] = self::getModuleInstance($prefix, $version);
}
foreach($prefixes as $prefix=>$version){
$instances[] = self::getModuleInstance($prefix, $version);
}

self::$enabledInstancesByPID[$pid] = $instances;
Expand All @@ -548,16 +552,41 @@ private static function getEnabledModulesForProject($pid)
return $instances;
}

private static function getEnabledModulePrefixesForProject($pid)
private static function getGloballyEnabledVersions()
{
if(!isset(self::$globallyEnabledVersions)){
self::cacheAllEnableData();
}

return self::$globallyEnabledVersions;
}

private static function getProjectEnabledDefaults()
{
if(!isset(self::$projectEnabledDefaults)){
self::cacheAllEnableData();
}

return self::$projectEnabledDefaults;
}

private static function getProjectEnabledOverrides()
{
if(!isset(self::$enabledVersions)){
if(!isset(self::$projectEnabledOverrides)){
self::cacheAllEnableData();
}

$enabledPrefixes = self::$globallyEnabledPrefixes;
$projectPrefixes = @self::$projectEnabledOverrides[$pid];
if(isset($projectPrefixes)){
foreach($projectPrefixes as $prefix => $value){
return self::$projectEnabledOverrides;
}

private static function getEnabledModuleVersionsForProject($pid)
{
$projectEnabledOverrides = self::getProjectEnabledOverrides();

$enabledPrefixes = self::getProjectEnabledDefaults();
$overrides = @$projectEnabledOverrides[$pid];
if(isset($overrides)){
foreach($overrides as $prefix => $value){
if($value == 1){
$enabledPrefixes[$prefix] = true;
}
Expand All @@ -567,31 +596,43 @@ private static function getEnabledModulePrefixesForProject($pid)
}
}

return $enabledPrefixes;
$globallyEnabledVersions = self::getGloballyEnabledVersions();

$enabledVersions = array();
foreach(array_keys($enabledPrefixes) as $prefix){
$version = @$globallyEnabledVersions[$prefix];

// Check the version to make sure the module is not globally disabled.
if(isset($version)){
$enabledVersions[$prefix] = $version;
}
}

return $enabledVersions;
}

private static function cacheAllEnableData()
{
$result = self::getSettings(null, null, array(self::KEY_VERSION, self::KEY_ENABLED));

$enabledVersions = array();
$globallyEnabledVersions = array();
$projectEnabledOverrides = array();
$globallyEnabledPrefixes = array();
$projectEnabledDefaults = array();
while($row = db_fetch_assoc($result)){
$pid = $row['project_id'];
$prefix = $row['directory_prefix'];
$key = $row['key'];
$value = $row['value'];

if($key == self::KEY_VERSION){
$enabledVersions[$prefix] = $value;
$globallyEnabledVersions[$prefix] = $value;
}
else if($key == self::KEY_ENABLED){
if(isset($pid)){
$projectEnabledOverrides[$pid][$prefix] = $value;
}
else if($value == 1) {
$globallyEnabledPrefixes[$prefix] = true;
$projectEnabledDefaults[$prefix] = true;
}
}
else{
Expand All @@ -600,8 +641,8 @@ private static function cacheAllEnableData()
}

// Overwrite any previously cached results
self::$enabledVersions = $enabledVersions;
self::$globallyEnabledPrefixes = $globallyEnabledPrefixes;
self::$globallyEnabledVersions = $globallyEnabledVersions;
self::$projectEnabledDefaults = $projectEnabledDefaults;
self::$projectEnabledOverrides = $projectEnabledOverrides;
self::$enabledInstancesByPID = array();
}
Expand Down Expand Up @@ -661,7 +702,7 @@ private function getLinks($pid = null){

$links = array();

$modules = self::getEnabledModulesForProject($pid);
$modules = self::getEnabledModuleInstances($pid);
foreach($modules as $instance){
$config = $instance->getConfig();

Expand Down
14 changes: 14 additions & 0 deletions tests/BaseTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,24 @@ abstract class BaseTest extends TestCase
{
protected $backupGlobals = FALSE;

protected function setUp(){
self::cleanupSettings();
}

protected function tearDown()
{
self::cleanupSettings();
}

private function cleanupSettings()
{
$this->removeGlobalSetting();
$this->removeProjectSetting();

$m = self::getInstance();
$m->removeGlobalSetting(ExternalModules::KEY_VERSION, TEST_SETTING_PID);
$m->removeGlobalSetting(ExternalModules::KEY_ENABLED, TEST_SETTING_PID);
$m->removeProjectSetting(ExternalModules::KEY_ENABLED, TEST_SETTING_PID);
}

protected function setGlobalSetting($value)
Expand Down
47 changes: 26 additions & 21 deletions tests/ExternalModulesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,76 +110,81 @@ function testCacheAllEnableData()
$m->setGlobalSetting(ExternalModules::KEY_VERSION, $version);

self::callPrivateMethod('cacheAllEnableData');
$this->assertEquals($version, self::getPrivateVariable('enabledVersions')[TEST_MODULE_PREFIX]);
$this->assertEquals($version, self::callPrivateMethod('getGloballyEnabledVersions')[TEST_MODULE_PREFIX]);

$m->removeGlobalSetting(ExternalModules::KEY_VERSION);

// the other values set by cacheAllEnableData() are tested via testGetEnabledModulePrefixesForProject()
// the other values set by cacheAllEnableData() are tested via testgetEnabledModuleVersionsForProject()
}

function testGetEnabledModulePrefixesForProject_multiplePrefixes()
function testgetEnabledModuleVersionsForProject_multiplePrefixesAndVersions()
{
$prefix1 = TEST_MODULE_PREFIX . '-1';
$prefix2 = TEST_MODULE_PREFIX . '-2';

ExternalModules::setGlobalSetting($prefix1, ExternalModules::KEY_VERSION, TEST_MODULE_VERSION);
ExternalModules::setGlobalSetting($prefix2, ExternalModules::KEY_VERSION, TEST_MODULE_VERSION);
ExternalModules::setGlobalSetting($prefix1, ExternalModules::KEY_ENABLED, true);
ExternalModules::setGlobalSetting($prefix2, ExternalModules::KEY_ENABLED, true);

$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNotNull($prefixes[$prefix1]);
$this->assertNotNull($prefixes[$prefix2]);

ExternalModules::removeGlobalSetting($prefix2, ExternalModules::KEY_ENABLED);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
ExternalModules::removeGlobalSetting($prefix2, ExternalModules::KEY_VERSION);
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNotNull($prefixes[$prefix1]);
$this->assertNull($prefixes[$prefix2]);


ExternalModules::removeGlobalSetting($prefix1, ExternalModules::KEY_ENABLED);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNull($prefixes[$prefix1]);

ExternalModules::removeGlobalSetting($prefix1, ExternalModules::KEY_VERSION);
ExternalModules::removeGlobalSetting($prefix2, ExternalModules::KEY_ENABLED);
}

function testGetEnabledModulePrefixesForProject_overrides()
function testgetEnabledModuleVersionsForProject_overrides()
{
$m = self::getInstance();
$m->removeProjectSetting(ExternalModules::KEY_ENABLED, TEST_SETTING_PID);

$m->setGlobalSetting(ExternalModules::KEY_VERSION, TEST_MODULE_VERSION);
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNull($prefixes[TEST_MODULE_PREFIX]);

$m->setGlobalSetting(ExternalModules::KEY_ENABLED, true);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNotNull($prefixes[TEST_MODULE_PREFIX]);

$m->setProjectSetting(ExternalModules::KEY_ENABLED, true, TEST_SETTING_PID);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNotNull($prefixes[TEST_MODULE_PREFIX]);

$m->setProjectSetting(ExternalModules::KEY_ENABLED, false, TEST_SETTING_PID);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNull($prefixes[TEST_MODULE_PREFIX]);

$m->removeProjectSetting(ExternalModules::KEY_ENABLED, TEST_SETTING_PID);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNotNull($prefixes[TEST_MODULE_PREFIX]);

$m->setGlobalSetting(ExternalModules::KEY_ENABLED, false);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNull($prefixes[TEST_MODULE_PREFIX]);

$m->setProjectSetting(ExternalModules::KEY_ENABLED, true, TEST_SETTING_PID);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNotNull($prefixes[TEST_MODULE_PREFIX]);

$m->setProjectSetting(ExternalModules::KEY_ENABLED, false, TEST_SETTING_PID);
$prefixes = self::getEnabledModulePrefixesForProjectIgnoreCache();
$prefixes = self::getEnabledModuleVersionsForProjectIgnoreCache();
$this->assertNull($prefixes[TEST_MODULE_PREFIX]);

$this->removeGlobalSetting(ExternalModules::KEY_ENABLED);
}

private function getEnabledModulePrefixesForProjectIgnoreCache()
private function getEnabledModuleVersionsForProjectIgnoreCache()
{
self::callPrivateMethod('cacheAllEnableData'); // Call this every time to clear/reset the cache.
return self::callPrivateMethod('getEnabledModulePrefixesForProject', TEST_SETTING_PID);
return self::callPrivateMethod('getEnabledModuleVersionsForProject', TEST_SETTING_PID);
}

private function callPrivateMethod($methodName)
Expand Down

0 comments on commit 5578bd2

Please sign in to comment.