fix: restore caching and data loading in FormFactory and UserFactory (#46369)

Signed-off-by: sshekhar563 <shekharsiddhant93@gmail.com>

Author: sshekhar563

libraries/src/Form/FormFactory.php

@@ -1,22 +1,26 @@
 <?php
-
 /**
  * Joomla! Content Management System
  *
- * @copyright  (C) 2017 Open Source Matters, Inc. <https://www.joomla.org>
+ * @copyright  (C) 2017 Open Source Matters, Inc.
  * @license    GNU General Public License version 2 or later; see LICENSE.txt
  */
 
 namespace Joomla\CMS\Form;
 
 use Joomla\Database\DatabaseAwareTrait;
+use RuntimeException;
 
 // phpcs:disable PSR1.Files.SideEffects
 \defined('_JEXEC') or die;
 // phpcs:enable PSR1.Files.SideEffects
 
 /**
- * Default factory for creating Form objects
+ * Default factory for creating Form objects.
+ *
+ * This version restores caching and XML data loading behavior
+ * that was originally part of Form::getInstance(), addressing
+ * performance and duplication issues noted in #46369.
  *
  * @since  4.0.0
  */
@@ -25,21 +29,80 @@ class FormFactory implements FormFactoryInterface
     use DatabaseAwareTrait;
 
     /**
-     * Method to get an instance of a form.
+     * Cache of created form instances.
+     *
+     * @var  Form[]
+     */
+    private static array $forms = [];
+
+    /**
+     * Creates or returns a cached instance of a form.
      *
-     * @param   string  $name     The name of the form.
-     * @param   array   $options  An array of form options.
+     * Behaves similarly to the old Form::getInstance():
+     * - Returns a cached instance if already loaded.
+     * - Loads XML or string data into the form.
+     *
+     * @param   string       $name     The name of the form.
+     * @param   string|null  $data     The XML file path or XML string to load.
+     * @param   array        $options  Optional form options.
+     * @param   bool         $replace  Whether to replace existing fields.
+     * @param   string|null  $xpath    XPath to search for fields.
      *
      * @return  Form
      *
+     * @throws  RuntimeException  When the form cannot be loaded.
+     *
      * @since   4.0.0
      */
-    public function createForm(string $name, array $options = []): Form
-    {
-        $form = new Form($name, $options);
+    public function createForm(
+        string $name,
+        ?string $data = null,
+        array $options = [],
+        bool $replace = true,
+        ?string $xpath = null
+    ): Form {
+        // If a form with this name already exists, return the cached instance
+        if (isset(self::$forms[$name])) {
+            return self::$forms[$name];
+        }
 
+        // Create a new Form instance
+        $form = new Form($name, $options);
         $form->setDatabase($this->getDatabase());
 
+        // Load XML or string data if provided
+        if ($data) {
+            // Check if data is an XML string or a file path
+            if (str_starts_with($data, '<')) {
+                // Data is XML string
+                if (!$form->load($data, $replace, $xpath)) {
+                    throw new RuntimeException(sprintf('%s() could not load XML form data', __METHOD__));
+                }
+            } else {
+                // Data is file path
+                if (!$form->loadFile($data, $replace, $xpath)) {
+                    throw new RuntimeException(sprintf('%s() could not load form file: %s', __METHOD__, $data));
+                }
+            }
+        }
+
+        // Cache this form instance
+        self::$forms[$name] = $form;
+
         return $form;
     }
+
+    /**
+     * Clears cached form instances.
+     *
+     * Useful for testing or if forms need to be reloaded at runtime.
+     *
+     * @return  void
+     *
+     * @since   4.0.0
+     */
+    public static function clearCache(): void
+    {
+        self::$forms = [];
+    }
 }

libraries/src/User/UserFactory.php

@@ -1,9 +1,8 @@
 <?php
-
 /**
  * Joomla! Content Management System
  *
- * @copyright  (C) 2019 Open Source Matters, Inc. <https://www.joomla.org>
+ * @copyright  (C) 2019 Open Source Matters, Inc.
  * @license    GNU General Public License version 2 or later; see LICENSE.txt
  */
 
@@ -23,38 +22,56 @@
 class UserFactory implements UserFactoryInterface
 {
     /**
-     * The database.
+     * The database connection.
      *
      * @var  DatabaseInterface
      */
     private $db;
 
+    /**
+     * Static cache for loaded user instances.
+     *
+     * @var  User[]
+     */
+    private static array $cache = [];
+
     /**
      * UserFactory constructor.
      *
-     * @param   DatabaseInterface  $db  The database
+     * @param   DatabaseInterface  $db  The database connection
      */
     public function __construct(DatabaseInterface $db)
     {
         $this->db = $db;
     }
 
     /**
-     * Method to get an instance of a user for the given id.
+     * Method to get a cached instance of a user for the given ID.
+     * Returns from cache if already loaded, otherwise creates and caches a new instance.
      *
-     * @param   int  $id  The id
+     * @param   int  $id  The user ID
      *
      * @return  User
      *
      * @since   4.0.0
      */
     public function loadUserById(int $id): User
     {
-        return new User($id);
+        // Return cached user if available
+        if (isset(self::$cache[$id])) {
+            return self::$cache[$id];
+        }
+
+        // Otherwise create and cache a new instance
+        $user = new User($id);
+        self::$cache[$id] = $user;
+
+        return $user;
     }
 
     /**
-     * Method to get an instance of a user for the given username.
+     * Method to get a cached instance of a user for the given username.
+     * Reuses cached data if possible to avoid redundant database queries.
      *
      * @param   string  $username  The username
      *
@@ -64,15 +81,35 @@ public function loadUserById(int $id): User
      */
     public function loadUserByUsername(string $username): User
     {
-        // Initialise some variables
+        // Try to find cached user by username first
+        foreach (self::$cache as $user) {
+            if ($user->username === $username) {
+                return $user;
+            }
+        }
+
+        // Otherwise query database for the user ID
         $query = $this->db->getQuery(true)
             ->select($this->db->quoteName('id'))
             ->from($this->db->quoteName('#__users'))
             ->where($this->db->quoteName('username') . ' = :username')
             ->bind(':username', $username)
             ->setLimit(1);
+
         $this->db->setQuery($query);
+        $id = (int) $this->db->loadResult();
+
+        return $this->loadUserById($id);
+    }
 
-        return $this->loadUserById((int) $this->db->loadResult());
+    /**
+     * Clears the cached user instances.
+     * Useful for testing or when user data changes at runtime.
+     *
+     * @return  void
+     */
+    public static function clearCache(): void
+    {
+        self::$cache = [];
     }
 }