Clear Module Settings Cache after deleting settings from database (#6190)

CHANGELOG-DEV.md

@@ -7,6 +7,7 @@ HumHub Changelog
 - Fix #6192: Where Group::getAdminGroupId() would sometimes return int, sometimes string
 - Enh #6260: Improve migration class
 - Fix #6199: Module manager Add types to properties
+- Fix #6189: Module settings survive deactivation in cache
 - Enh #6236: Logging: Show log entries from migrations with category migration
 - Fix #6216: Spaces icon in admin menu
 - Fix #6229: Bug on saving forms: Zend OPcache API is restricted by "restrict_api"

protected/humhub/components/SettingActiveRecord.php

@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * @link      https://www.humhub.org/
+ * @copyright Copyright (c) 2023 HumHub GmbH & Co. KG
+ * @license   https://www.humhub.com/licences
+ */
+
+namespace humhub\components;
+
+use Yii;
+use yii\db\ActiveRecord;
+
+/**
+ * BaseSetting
+ *
+ * @since  1.13.2
+ * @author Martin Rüegg <martin.rueegg@metaworx.ch>
+ */
+abstract class SettingActiveRecord extends ActiveRecord
+{
+    /**
+     * @const array List of fields to be used to generate the cache key
+     */
+    protected const CACHE_KEY_FIELDS = ['module_id'];
+
+    /**
+     * @const string Used as the formatting pattern for sprintf when generating the cache key
+     */
+    protected const CACHE_KEY_FORMAT = 'settings-%s';
+
+    public static function deleteAll($condition = null, $params = [])
+    {
+        // get a grouped list of cache entries that are going to be deleted, grouped by static::CACHE_KEY_FIELDS
+        $containers = self::find()
+            ->where($condition, $params)
+            ->groupBy(static::CACHE_KEY_FIELDS)
+            ->select(static::CACHE_KEY_FIELDS)
+            ->all();
+
+        // going through that list, deleting the respective cache
+        array_walk($containers, static function (ActiveRecord $rec) {
+            $key = static::getCacheKey(...array_values($rec->toArray()));
+            Yii::$app->cache->delete($key);
+        });
+
+        // proceed to delete the records from the database
+        return parent::deleteAll($condition, $params);
+    }
+
+    /**
+     * @param string $moduleId  Name of the module to create the cache key for
+     * @param mixed  ...$values Additional arguments, if required by the static::CACHE_KEY_FORMAT
+     *
+     * @return string The key used for cache operation
+     */
+    public static function getCacheKey(string $moduleId, ...$values): string
+    {
+        return sprintf(static::CACHE_KEY_FORMAT, $moduleId, ...$values);
+    }
+}

protected/humhub/components/SettingsManager.php

@@ -105,7 +105,7 @@ class SettingsManager extends BaseSettingsManager
     /**
      * @inheritdoc
      */
-    public function get($name, $default = null)
+    public function get(string $name, $default = null)
     {
         if ($this->isFixed($name)) {
             return Yii::$app->params['fixed-settings'][$this->moduleId][$name];

protected/humhub/libs/BaseSettingsManager.php

@@ -8,6 +8,7 @@
 
 namespace humhub\libs;
 
+use humhub\components\SettingActiveRecord;
 use Yii;
 use yii\base\Component;
 use yii\base\Exception;
@@ -24,21 +25,20 @@ use yii\helpers\Json;
  */
 abstract class BaseSettingsManager extends Component
 {
-
     /**
-     * @var string module id this settings manager belongs to.
+     * @var string|null module id this settings manager belongs to.
      */
-    public $moduleId = null;
+    public ?string $moduleId = null;
 
     /**
      * @var array|null of loaded settings
      */
-    protected $_loaded = null;
+    protected ?array $_loaded = null;
 
     /**
      * @var string settings model class name
      */
-    public $modelClass = 'humhub\models\Setting';
+    public string $modelClass = 'humhub\models\Setting';
 
     /**
      * @inheritdoc
@@ -94,11 +94,10 @@ abstract class BaseSettingsManager extends Component
     /**
      * Can be used to set object/arrays as a serialized values.
      *
-     *
      * @param string $name
      * @param mixed $value array or object
      */
-    public function setSerialized($name, $value)
+    public function setSerialized(string $name, $value)
     {
         $this->set($name, Json::encode($value));
     }
@@ -109,7 +108,7 @@ abstract class BaseSettingsManager extends Component
      * @param string $name
      * @param mixed $default the setting value or null when not exists
      */
-    public function getSerialized($name, $default = null)
+    public function getSerialized(string $name, $default = null)
     {
         $value = $this->get($name, $default);
         if (is_string($value)) {
@@ -126,20 +125,22 @@ abstract class BaseSettingsManager extends Component
      * Returns value of setting
      *
      * @param string $name the name of setting
+     *
      * @return string|null the setting value or null when not exists
      */
-    public function get($name, $default = null)
+    public function get(string $name, $default = null)
     {
-        return isset($this->_loaded[$name]) ? $this->_loaded[$name] : $default;
+        return $this->_loaded[$name] ?? $default;
     }
 
     /**
      * Returns the value of setting without any caching
      *
      * @param string $name the name of setting
+     *
      * @return string the setting value or null when not exists
      */
-    public function getUncached($name, $default = null)
+    public function getUncached(string $name, $default = null): ?string
     {
         $record = $this->find()->andWhere(['name' => $name])->one();
         return ($record !== null) ? $record->value : $default;
@@ -150,7 +151,7 @@ abstract class BaseSettingsManager extends Component
      *
      * @param string $name
      */
-    public function delete($name)
+    public function delete(string $name)
     {
         $record = $this->find()->andWhere(['name' => $name])->one();
         if ($record !== null) {
@@ -166,8 +167,6 @@ abstract class BaseSettingsManager extends Component
         if (isset($this->_loaded[$name])) {
             unset($this->_loaded[$name]);
         }
-
-        $this->invalidateCache();
     }
 
     /**
@@ -180,7 +179,7 @@ abstract class BaseSettingsManager extends Component
             $this->_loaded = [];
             $settings = &$this->_loaded;
 
-            array_map(function ($record) use (&$settings) {
+            array_map(static function ($record) use (&$settings) {
                 $settings[$record->name] = $record->value;
             }, $this->find()->all());
 
@@ -212,9 +211,11 @@ abstract class BaseSettingsManager extends Component
      *
      * @return string the cache key
      */
-    protected function getCacheKey()
+    protected function getCacheKey(): string
     {
-        return 'settings-' . $this->moduleId;
+        /** @var SettingActiveRecord $modelClass */
+        $modelClass = $this->modelClass;
+        return $modelClass::getCacheKey($this->moduleId);
     }
 
     /**
@@ -222,7 +223,7 @@ abstract class BaseSettingsManager extends Component
      */
     protected function createRecord()
     {
-        $model = new $this->modelClass;
+        $model = new $this->modelClass();
         $model->module_id = $this->moduleId;
 
         return $model;

protected/humhub/models/Setting.php

@@ -8,8 +8,8 @@
 
 namespace humhub\models;
 
+use humhub\components\SettingActiveRecord;
 use Yii;
-use yii\db\ActiveRecord;
 use yii\base\Exception;
 
 /**
@@ -20,7 +20,7 @@ use yii\base\Exception;
  * @property string $value
  * @property string $module_id
  */
-class Setting extends ActiveRecord
+class Setting extends SettingActiveRecord
 {
 
     /**

protected/humhub/modules/content/components/ContentContainerSettingsManager.php

@@ -8,8 +8,9 @@
 
 namespace humhub\modules\content\components;
 
-use Yii;
+use humhub\components\SettingActiveRecord;
 use humhub\libs\BaseSettingsManager;
+use Yii;
 
 /**
  * ContentContainerSettingManager
@@ -23,18 +24,18 @@ class ContentContainerSettingsManager extends BaseSettingsManager
     /**
      * @inheritdoc
      */
-    public $modelClass = 'humhub\modules\content\models\ContentContainerSetting';
+    public string $modelClass = 'humhub\modules\content\models\ContentContainerSetting';
 
     /**
      * @var ContentContainerActiveRecord the content container this settings manager belongs to
      */
     public $contentContainer;
-    
+
     /**
      * Returns the setting value of this container for the given setting $name.
      * If there is not container specific setting, this function will search for a global setting or
      * return default or null if there is also no global setting.
-     * 
+     *
      * @param string $name
      * @param string $default
      * @return boolean
@@ -45,12 +46,12 @@ class ContentContainerSettingsManager extends BaseSettingsManager
         return ($result !== null) ? $result
             : Yii::$app->getModule($this->moduleId)->settings->get($name, $default);
     }
-    
+
     /**
      * Returns the setting value of this container for the given setting $name.
      * If there is not container specific setting, this function will search for a global setting or
      * return default or null if there is also no global setting.
-     * 
+     *
      * @param string $name
      * @param string $default
      * @return boolean
@@ -83,9 +84,11 @@ class ContentContainerSettingsManager extends BaseSettingsManager
     /**
      * @inheritdoc
      */
-    protected function getCacheKey()
+    protected function getCacheKey(): string
     {
-        return parent::getCacheKey() . '-' . $this->contentContainer->contentcontainer_id;
+        /** @var \humhub\components\SettingActiveRecord $modelClass */
+        $modelClass = $this->modelClass;
+        return $modelClass::getCacheKey($this->moduleId, $this->contentContainer->contentcontainer_id);
     }
 
 }

protected/humhub/modules/content/models/ContentContainerSetting.php

@@ -8,6 +8,8 @@
 
 namespace humhub\modules\content\models;
 
+use humhub\components\SettingActiveRecord;
+
 /**
  * This is the model class for table "contentcontainer_setting".
  *
@@ -19,9 +21,15 @@ namespace humhub\modules\content\models;
  * @property ContentContainer $contentcontainer
  * @since 1.1
  */
-class ContentContainerSetting extends \yii\db\ActiveRecord
+class ContentContainerSetting extends SettingActiveRecord
 {
 
+    /** @inheritdoc */
+    protected const CACHE_KEY_FORMAT = 'settings-%s-%d';
+
+    /** @inheritdoc */
+    protected const CACHE_KEY_FIELDS = ['module_id', 'contentcontainer_id'];
+
     /**
      * @inheritdoc
      */