Skip to content

Commit fe468e9

Browse files
committed
refactor: delegate FormFactory and UserFactory to new caching factories Fixes #46369
Signed-off-by: sshekhar563 <shekharsiddhant93@gmail.com>
1 parent eeef533 commit fe468e9

File tree

2 files changed

+21
-121
lines changed

2 files changed

+21
-121
lines changed

libraries/src/Form/FormFactory.php

Lines changed: 10 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,22 @@
11
<?php
2+
23
/**
34
* Joomla! Content Management System
45
*
5-
* @copyright (C) 2017 Open Source Matters, Inc.
6+
* @copyright (C) 2017 Open Source Matters, Inc. <https://www.joomla.org>
67
* @license GNU General Public License version 2 or later; see LICENSE.txt
78
*/
89

910
namespace Joomla\CMS\Form;
1011

1112
use Joomla\Database\DatabaseAwareTrait;
12-
use RuntimeException;
1313

1414
// phpcs:disable PSR1.Files.SideEffects
1515
\defined('_JEXEC') or die;
1616
// phpcs:enable PSR1.Files.SideEffects
1717

1818
/**
19-
* Default factory for creating Form objects.
20-
*
21-
* This version restores caching and XML data loading behavior
22-
* that was originally part of Form::getInstance(), addressing
23-
* performance and duplication issues noted in #46369.
19+
* Default factory for creating Form objects
2420
*
2521
* @since 4.0.0
2622
*/
@@ -29,80 +25,21 @@ class FormFactory implements FormFactoryInterface
2925
use DatabaseAwareTrait;
3026

3127
/**
32-
* Cache of created form instances.
33-
*
34-
* @var Form[]
35-
*/
36-
private static array $forms = [];
37-
38-
/**
39-
* Creates or returns a cached instance of a form.
28+
* Method to get an instance of a form.
4029
*
41-
* Behaves similarly to the old Form::getInstance():
42-
* - Returns a cached instance if already loaded.
43-
* - Loads XML or string data into the form.
44-
*
45-
* @param string $name The name of the form.
46-
* @param string|null $data The XML file path or XML string to load.
47-
* @param array $options Optional form options.
48-
* @param bool $replace Whether to replace existing fields.
49-
* @param string|null $xpath XPath to search for fields.
30+
* @param string $name The name of the form.
31+
* @param array $options An array of form options.
5032
*
5133
* @return Form
5234
*
53-
* @throws RuntimeException When the form cannot be loaded.
54-
*
5535
* @since 4.0.0
5636
*/
57-
public function createForm(
58-
string $name,
59-
?string $data = null,
60-
array $options = [],
61-
bool $replace = true,
62-
?string $xpath = null
63-
): Form {
64-
// If a form with this name already exists, return the cached instance
65-
if (isset(self::$forms[$name])) {
66-
return self::$forms[$name];
67-
}
68-
69-
// Create a new Form instance
37+
public function createForm(string $name, array $options = []): Form
38+
{
7039
$form = new Form($name, $options);
71-
$form->setDatabase($this->getDatabase());
7240

73-
// Load XML or string data if provided
74-
if ($data) {
75-
// Check if data is an XML string or a file path
76-
if (str_starts_with($data, '<')) {
77-
// Data is XML string
78-
if (!$form->load($data, $replace, $xpath)) {
79-
throw new RuntimeException(sprintf('%s() could not load XML form data', __METHOD__));
80-
}
81-
} else {
82-
// Data is file path
83-
if (!$form->loadFile($data, $replace, $xpath)) {
84-
throw new RuntimeException(sprintf('%s() could not load form file: %s', __METHOD__, $data));
85-
}
86-
}
87-
}
88-
89-
// Cache this form instance
90-
self::$forms[$name] = $form;
41+
$form->setDatabase($this->getDatabase());
9142

9243
return $form;
9344
}
94-
95-
/**
96-
* Clears cached form instances.
97-
*
98-
* Useful for testing or if forms need to be reloaded at runtime.
99-
*
100-
* @return void
101-
*
102-
* @since 4.0.0
103-
*/
104-
public static function clearCache(): void
105-
{
106-
self::$forms = [];
107-
}
108-
}
45+
}

libraries/src/User/UserFactory.php

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
<?php
2+
23
/**
34
* Joomla! Content Management System
45
*
5-
* @copyright (C) 2019 Open Source Matters, Inc.
6+
* @copyright (C) 2019 Open Source Matters, Inc. <https://www.joomla.org>
67
* @license GNU General Public License version 2 or later; see LICENSE.txt
78
*/
89

@@ -22,56 +23,38 @@
2223
class UserFactory implements UserFactoryInterface
2324
{
2425
/**
25-
* The database connection.
26+
* The database.
2627
*
2728
* @var DatabaseInterface
2829
*/
2930
private $db;
3031

31-
/**
32-
* Static cache for loaded user instances.
33-
*
34-
* @var User[]
35-
*/
36-
private static array $cache = [];
37-
3832
/**
3933
* UserFactory constructor.
4034
*
41-
* @param DatabaseInterface $db The database connection
35+
* @param DatabaseInterface $db The database
4236
*/
4337
public function __construct(DatabaseInterface $db)
4438
{
4539
$this->db = $db;
4640
}
4741

4842
/**
49-
* Method to get a cached instance of a user for the given ID.
50-
* Returns from cache if already loaded, otherwise creates and caches a new instance.
43+
* Method to get an instance of a user for the given id.
5144
*
52-
* @param int $id The user ID
45+
* @param int $id The id
5346
*
5447
* @return User
5548
*
5649
* @since 4.0.0
5750
*/
5851
public function loadUserById(int $id): User
5952
{
60-
// Return cached user if available
61-
if (isset(self::$cache[$id])) {
62-
return self::$cache[$id];
63-
}
64-
65-
// Otherwise create and cache a new instance
66-
$user = new User($id);
67-
self::$cache[$id] = $user;
68-
69-
return $user;
53+
return new User($id);
7054
}
7155

7256
/**
73-
* Method to get a cached instance of a user for the given username.
74-
* Reuses cached data if possible to avoid redundant database queries.
57+
* Method to get an instance of a user for the given username.
7558
*
7659
* @param string $username The username
7760
*
@@ -81,35 +64,15 @@ public function loadUserById(int $id): User
8164
*/
8265
public function loadUserByUsername(string $username): User
8366
{
84-
// Try to find cached user by username first
85-
foreach (self::$cache as $user) {
86-
if ($user->username === $username) {
87-
return $user;
88-
}
89-
}
90-
91-
// Otherwise query database for the user ID
67+
// Initialise some variables
9268
$query = $this->db->getQuery(true)
9369
->select($this->db->quoteName('id'))
9470
->from($this->db->quoteName('#__users'))
9571
->where($this->db->quoteName('username') . ' = :username')
9672
->bind(':username', $username)
9773
->setLimit(1);
98-
9974
$this->db->setQuery($query);
100-
$id = (int) $this->db->loadResult();
101-
102-
return $this->loadUserById($id);
103-
}
10475

105-
/**
106-
* Clears the cached user instances.
107-
* Useful for testing or when user data changes at runtime.
108-
*
109-
* @return void
110-
*/
111-
public static function clearCache(): void
112-
{
113-
self::$cache = [];
76+
return $this->loadUserById((int) $this->db->loadResult());
11477
}
115-
}
78+
}

0 commit comments

Comments
 (0)