From 5890035d60682437d2be33d608c162f0dd0bf595 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 16 Mar 2026 13:07:20 +0000 Subject: [PATCH] MDL-88176 customfield: handle static singleton pattern in base class. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace duplicate implementation in all handler implementations. Ensure that it is consistently reset during PHPUnit/Behat tests. Co-authored-by: Yerai Rodríguez --- .upgradenotes/MDL-88176-2026031613182551.yml | 8 ++++ .../classes/customfield/cohort_handler.php | 30 ------------ .../classes/customfield/content_handler.php | 30 ------------ .../classes/customfield/course_handler.php | 48 ++----------------- .../classes/customfield/shared_handler.php | 30 ------------ public/customfield/classes/handler.php | 39 +++++++++------ .../classes/customfield/group_handler.php | 30 ------------ .../classes/customfield/grouping_handler.php | 30 ------------ public/lib/behat/classes/util.php | 1 + .../lib/classes/test/phpunit/phpunit_util.php | 19 +------- .../classes/customfield/question_handler.php | 32 ------------- .../classes/customfield/report_handler.php | 30 ------------ 12 files changed, 40 insertions(+), 287 deletions(-) create mode 100644 .upgradenotes/MDL-88176-2026031613182551.yml diff --git a/.upgradenotes/MDL-88176-2026031613182551.yml b/.upgradenotes/MDL-88176-2026031613182551.yml new file mode 100644 index 00000000000..9ee65adfa11 --- /dev/null +++ b/.upgradenotes/MDL-88176-2026031613182551.yml @@ -0,0 +1,8 @@ +issueNumber: MDL-88176 +notes: + core_customfield: + - message: >- + The base `\core_customfield\handler` class now implements static + caching/reset itself, so all implementations of the same from extending + handler classes should be removed + type: changed diff --git a/public/cohort/classes/customfield/cohort_handler.php b/public/cohort/classes/customfield/cohort_handler.php index 99d8545ec5e..26b7bdbb0b1 100644 --- a/public/cohort/classes/customfield/cohort_handler.php +++ b/public/cohort/classes/customfield/cohort_handler.php @@ -27,36 +27,6 @@ use core_customfield\field_controller; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class cohort_handler extends handler { - - /** - * @var cohort_handler - */ - static protected $singleton; - - /** - * Returns a singleton. - * - * @param int $itemid - * @return \core_customfield\handler - */ - public static function create(int $itemid = 0): handler { - if (static::$singleton === null) { - self::$singleton = new static(0); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new \coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. * diff --git a/public/contentbank/classes/customfield/content_handler.php b/public/contentbank/classes/customfield/content_handler.php index 076230ce29d..2ebaf843172 100644 --- a/public/contentbank/classes/customfield/content_handler.php +++ b/public/contentbank/classes/customfield/content_handler.php @@ -27,12 +27,6 @@ use core_customfield\field_controller; * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class content_handler extends \core_customfield\handler { - - /** - * @var content_handler - */ - static protected $singleton; - /** * @var \context */ @@ -43,30 +37,6 @@ class content_handler extends \core_customfield\handler { /** @var int Field is not displayed in the content bank edit page */ const NOTVISIBLE = 0; - /** - * Returns a singleton - * - * @param int $itemid - * @return \core_contentbank\customfield\content_handler - */ - public static function create(int $itemid = 0): \core_contentbank\customfield\content_handler { - if (static::$singleton === null) { - self::$singleton = new static(0); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new \coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. * diff --git a/public/course/classes/customfield/course_handler.php b/public/course/classes/customfield/course_handler.php index 42e42d86705..7d7616293ae 100644 --- a/public/course/classes/customfield/course_handler.php +++ b/public/course/classes/customfield/course_handler.php @@ -14,6 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +namespace core_course\customfield; + +use core_customfield\field_controller; + /** * Course handler for custom fields * @@ -21,27 +25,7 @@ * @copyright 2018 David Matamoros * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -namespace core_course\customfield; - -defined('MOODLE_INTERNAL') || die; - -use core_customfield\field_controller; - -/** - * Course handler for custom fields - * - * @package core_course - * @copyright 2018 David Matamoros - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ class course_handler extends \core_customfield\handler { - - /** - * @var course_handler - */ - static protected $singleton; - /** * @var \context */ @@ -54,30 +38,6 @@ class course_handler extends \core_customfield\handler { /** @var int Field is not displayed in the course listing */ const NOTVISIBLE = 0; - /** - * Returns a singleton - * - * @param int $itemid - * @return \core_course\customfield\course_handler - */ - public static function create(int $itemid = 0): \core_customfield\handler { - if (static::$singleton === null) { - self::$singleton = new static(0); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new \coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. * diff --git a/public/customfield/classes/customfield/shared_handler.php b/public/customfield/classes/customfield/shared_handler.php index f4c13a18e95..4cb9e214c6a 100644 --- a/public/customfield/classes/customfield/shared_handler.php +++ b/public/customfield/classes/customfield/shared_handler.php @@ -20,7 +20,6 @@ namespace core_customfield\customfield; use core\context; use core\context\system; -use core\exception\coding_exception; use core\url; use core_customfield\field_controller; @@ -32,35 +31,6 @@ use core_customfield\field_controller; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class shared_handler extends \core_customfield\handler { - /** - * @var shared_handler|null - */ - protected static ?shared_handler $singleton = null; - - /** - * Returns a singleton - * - * @param int $itemid - * @return self - */ - public static function create(int $itemid = 0): self { - if (static::$singleton === null) { - self::$singleton = new static($itemid); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. * diff --git a/public/customfield/classes/handler.php b/public/customfield/classes/handler.php index c5c6fab9ff7..94bb31d0ebf 100644 --- a/public/customfield/classes/handler.php +++ b/public/customfield/classes/handler.php @@ -14,24 +14,15 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * The abstract custom fields handler - * - * @package core_customfield - * @copyright 2018 David Matamoros - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core_customfield; use backup_nested_element; +use core\exception\coding_exception; use core_customfield\output\field_data; use stdClass; -defined('MOODLE_INTERNAL') || die; - /** - * Base class for custom fields handlers + * The abstract custom fields handler * * This handler provides callbacks for field configuration form and also allows to add the fields to the instance editing form * @@ -46,11 +37,13 @@ defined('MOODLE_INTERNAL') || die; * - \core_customfield\api::get_field($fieldid) * - \core_customfield\api::get_category($categoryid) * - * @package core_customfield + * @package core_customfield * @copyright 2018 David Matamoros * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ abstract class handler { + /** @var handler[] $instances */ + private static $instances = []; /** * The component this handler handles @@ -95,13 +88,31 @@ abstract class handler { /** * Returns an instance of the handler * - * Some areas may choose to use singleton/caching here + * We statically cache the list of instances during request lifecycle, to allow this method to be called + * repeatedly without potential performance problems * * @param int $itemid * @return static */ public static function create(int $itemid = 0): handler { - return new static($itemid); + $instancekey = static::class . ':' . $itemid; + if (!array_key_exists($instancekey, static::$instances)) { + static::$instances[$instancekey] = new static($itemid); + } + return static::$instances[$instancekey]; + } + + /** + * Run reset code after tests to reset the instance cache + * + * @throws coding_exception If called outside of test infrastructure + */ + public static function reset_caches(): void { + if (PHPUNIT_TEST || defined('BEHAT_TEST')) { + static::$instances = []; + } else { + throw new coding_exception('This feature is only intended for use in tests'); + } } /** diff --git a/public/group/classes/customfield/group_handler.php b/public/group/classes/customfield/group_handler.php index ffa47249cd9..cbde105d1b0 100644 --- a/public/group/classes/customfield/group_handler.php +++ b/public/group/classes/customfield/group_handler.php @@ -34,36 +34,6 @@ use restore_task; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class group_handler extends handler { - - /** - * @var group_handler - */ - static protected $singleton; - - /** - * Returns a singleton. - * - * @param int $itemid - * @return \core_customfield\handler - */ - public static function create(int $itemid = 0): handler { - if (static::$singleton === null) { - self::$singleton = new static(0); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new \coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. * diff --git a/public/group/classes/customfield/grouping_handler.php b/public/group/classes/customfield/grouping_handler.php index 1bd951bb3f0..9a95813ebcf 100644 --- a/public/group/classes/customfield/grouping_handler.php +++ b/public/group/classes/customfield/grouping_handler.php @@ -34,36 +34,6 @@ use restore_task; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class grouping_handler extends handler { - - /** - * @var grouping_handler - */ - static protected $singleton; - - /** - * Returns a singleton. - * - * @param int $itemid - * @return \core_customfield\handler - */ - public static function create(int $itemid = 0): handler { - if (static::$singleton === null) { - self::$singleton = new static(0); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new \coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. * diff --git a/public/lib/behat/classes/util.php b/public/lib/behat/classes/util.php index c8708eddbc9..348510123cf 100644 --- a/public/lib/behat/classes/util.php +++ b/public/lib/behat/classes/util.php @@ -422,6 +422,7 @@ class behat_util extends \core\test\testing_util { filter_manager::reset_caches(); + \core_customfield\handler::reset_caches(); \core_reportbuilder\manager::reset_caches(); // Reset course and module caches. diff --git a/public/lib/classes/test/phpunit/phpunit_util.php b/public/lib/classes/test/phpunit/phpunit_util.php index 651c2bad181..bab1060fb34 100644 --- a/public/lib/classes/test/phpunit/phpunit_util.php +++ b/public/lib/classes/test/phpunit/phpunit_util.php @@ -251,27 +251,12 @@ class phpunit_util extends \core\test\testing_util { if (class_exists(\core\update\checker::class)) { \core\update\checker::reset_caches(true); } - if (class_exists(\core_course\customfield\course_handler::class)) { - \core_course\customfield\course_handler::reset_caches(); + if (class_exists(\core_customfield\handler::class)) { + \core_customfield\handler::reset_caches(); } if (class_exists(\core_reportbuilder\manager::class)) { \core_reportbuilder\manager::reset_caches(); } - if (class_exists(\core_cohort\customfield\cohort_handler::class)) { - \core_cohort\customfield\cohort_handler::reset_caches(); - } - if (class_exists(\core_group\customfield\group_handler::class)) { - \core_group\customfield\group_handler::reset_caches(); - } - if (class_exists(\core_group\customfield\grouping_handler::class)) { - \core_group\customfield\grouping_handler::reset_caches(); - } - if (class_exists(\core_reportbuilder\customfield\report_handler::class)) { - \core_reportbuilder\customfield\report_handler::reset_caches(); - } - if (class_exists(\core_customfield\customfield\shared_handler::class)) { - \core_customfield\customfield\shared_handler::reset_caches(); - } // Clear static cache within restore. if (class_exists(\restore_section_structure_step::class)) { diff --git a/public/question/bank/customfields/classes/customfield/question_handler.php b/public/question/bank/customfields/classes/customfield/question_handler.php index 785bd790882..86892009b80 100644 --- a/public/question/bank/customfields/classes/customfield/question_handler.php +++ b/public/question/bank/customfields/classes/customfield/question_handler.php @@ -29,12 +29,6 @@ use core_customfield\output\field_data; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class question_handler extends \core_customfield\handler { - - /** - * @var question_handler - */ - static protected $singleton; - /** * @var \context */ @@ -47,32 +41,6 @@ class question_handler extends \core_customfield\handler { /** @var int Field is not displayed in the question display and question preview */ const NOTVISIBLE = 0; - /** - * Creates the custom field handler and returns a singleton. - * Itemid is always zero as the custom fields are the same - * for every question across the system. - * - * @param int $itemid Always zero. - * @return \qbank_customfields\customfield\question_handler - */ - public static function create(int $itemid = 0): \core_customfield\handler { - if (static::$singleton === null) { - self::$singleton = new static(0); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new \coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. * diff --git a/public/reportbuilder/classes/customfield/report_handler.php b/public/reportbuilder/classes/customfield/report_handler.php index 37b9933aba4..5c756824742 100644 --- a/public/reportbuilder/classes/customfield/report_handler.php +++ b/public/reportbuilder/classes/customfield/report_handler.php @@ -20,7 +20,6 @@ namespace core_reportbuilder\customfield; use core\context; use core\context\system; -use core\exception\coding_exception; use core\url; use core_customfield\field_controller; use core_reportbuilder\local\models\report; @@ -35,35 +34,6 @@ use core_reportbuilder\permission; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class report_handler extends \core_customfield\handler { - /** - * @var report_handler|null - */ - protected static ?report_handler $singleton = null; - - /** - * Returns a singleton - * - * @param int $itemid - * @return self - */ - public static function create(int $itemid = 0): self { - if (static::$singleton === null) { - self::$singleton = new static($itemid); - } - return self::$singleton; - } - - /** - * Run reset code after unit tests to reset the singleton usage. - */ - public static function reset_caches(): void { - if (!PHPUNIT_TEST) { - throw new coding_exception('This feature is only intended for use in unit tests'); - } - - static::$singleton = null; - } - /** * The current user can configure custom fields on this component. *