Merge pull request #864 from guillaumelecerf/dont_save_invalid_usercache

Fix EZP-22181: eZUser: do not generate cache content for invalid user
ezsystems · Jan 28, 2014 · f3dc566 · f3dc566
2 parents 556cf75 + 90fdaee
commit f3dc566
Show file tree

Hide file tree

Showing 3 changed files with 130 additions and 43 deletions.
diff --git a/kernel/classes/datatypes/ezuser/ezuser.php b/kernel/classes/datatypes/ezuser/ezuser.php
@@ -1304,8 +1304,7 @@ static protected function getUserCacheByUserId( $userId )
 
         if ( $userCache === null || $userCache instanceof eZClusterFileFailure )
         {
-            $userCache = self::generateUserCacheForFile( null, $userId );
-            $userCache = $userCache['content'];
+            $userCache = self::generateUserCacheContent( $userId );
         }
 
         return $userCache;
@@ -1341,56 +1340,96 @@ static function retrieveUserCacheFromFile( $filePath, $mtime, $userId )
     }
 
     /**
-     * Callback which generates user cache for user
+     * Generate cache content or fallback to an empty array (for BC)
+     *
+     * @since 5.3
+     * @param int $userId
+     * @return array
+     */
+    private static function generateUserCacheContent( $userId )
+    {
+        $cacheData = self::generateUserCacheData( $userId );
+
+        if ( $cacheData === null )
+        {
+            $cacheData = array( 'info' => array(),
+                                'groups' => array(),
+                                'roles' => array(),
+                                'role_limitations' => array(),
+                                'access_array' => array(),
+                                'discount_rules' => array() );
+        }
+
+        return $cacheData;
+    }
+
+    /**
+     * Generate cache content
+     *
+     * @since 5.3
+     * @param int $userId
+     * @return array|null
+     */
+    private static function generateUserCacheData( $userId )
+    {
+        $user = eZUser::fetch( $userId );
+        if ( !$user instanceof eZUser )
+        {
+            return null;
+        }
+
+        // user info (session: eZUserInfoCache)
+        $data['info'][$userId] = array( 'contentobject_id'   => $user->attribute( 'contentobject_id' ),
+                                        'login'              => $user->attribute( 'login' ),
+                                        'email'              => $user->attribute( 'email' ),
+                                        'is_enabled'         => $user->isEnabled( false ),
+                                        'password_hash'      => $user->attribute( 'password_hash' ),
+                                        'password_hash_type' => $user->attribute( 'password_hash_type' ) );
+
+        // user groups list (session: eZUserGroupsCache)
+        $groups = $user->generateGroupIdList();
+        $data['groups'] = $groups;
+
+        // role list (session: eZRoleIDList)
+        $groups[] = $userId;
+        $data['roles'] = eZRole::fetchIDListByUser( $groups );
+
+        // role limitation list (session: eZRoleLimitationValueList)
+        $limitList = $user->limitList( false );
+        foreach ( $limitList as $limit )
+        {
+            $data['role_limitations'][] = $limit['limit_value'];
+        }
+
+        // access array (session: AccessArray)
+        $data['access_array'] = $user->generateAccessArray();
+
+        // discount rules (session: eZUserDiscountRules<userId>)
+        $data['discount_rules'] = eZUserDiscountRule::generateIDListByUserID( $userId );
+
+        return $data;
+    }
+
+    /**
+     * Callback which generates user cache for user, returns null on invalid user
      *
      * @internal
      * @since 4.4
      * @see eZUser::getUserCacheByUserId()
      */
     static function generateUserCacheForFile( $filePath, $userId )
     {
-        $data = array( 'info' => array(),
-                       'groups' => array(),
-                       'roles' => array(),
-                       'role_limitations' => array(),
-                       'access_array' => array(),
-                       'discount_rules' => array() );
-        $user = eZUser::fetch( $userId );
-        if ( $user instanceOf eZUser )
-        {
-            // user info (session: eZUserInfoCache)
-            $data['info'][$userId] = array( 'contentobject_id'   => $user->attribute( 'contentobject_id' ),
-                                            'login'              => $user->attribute( 'login' ),
-                                            'email'              => $user->attribute( 'email' ),
-                                            'is_enabled'         => $user->isEnabled( false ),
-                                            'password_hash'      => $user->attribute( 'password_hash' ),
-                                            'password_hash_type' => $user->attribute( 'password_hash_type' ) );
-
-            // user groups list (session: eZUserGroupsCache)
-            $groups = $user->generateGroupIdList();
-            $data['groups'] = $groups;
-
-            // role list (session: eZRoleIDList)
-            $groups[] = $userId;
-            $data['roles'] = eZRole::fetchIDListByUser( $groups );
-
-            // role limitation list (session: eZRoleLimitationValueList)
-            $limitList = $user->limitList( false );
-            foreach ( $limitList as $limit )
-            {
-                $data['role_limitations'][] = $limit['limit_value'];
-            }
-
-            // access array (session: AccessArray)
-            $data['access_array'] = $user->generateAccessArray();
+        $cacheData = self::generateUserCacheData( $userId );
 
-            // discount rules (session: eZUserDiscountRules<userId>)
-            $data['discount_rules'] = eZUserDiscountRule::generateIDListByUserID( $userId );
+        if ( $cacheData !== null )
+        {
+            $cacheData = array( 'content'  => $cacheData,
+                                'scope'    => 'user-info-cache',
+                                'datatype' => 'php',
+                                'store'    => true );
         }
-        return array( 'content'  => $data,
-                      'scope'    => 'user-info-cache',
-                      'datatype' => 'php',
-                      'store'    => true );
+
+        return $cacheData;
     }
 
     /*!

diff --git a/tests/tests/kernel/datatypes/ezuser/ezusercache_test.php b/tests/tests/kernel/datatypes/ezuser/ezusercache_test.php
@@ -0,0 +1,47 @@
+<?php
+/**
+ * File containing the eZUserCacheTest class.
+ *
+ * @copyright Copyright (C) 1999-2013 eZ Systems AS. All rights reserved.
+ * @license http://www.gnu.org/licenses/gpl-2.0.txt GNU General Public License v2
+ * @version //autogentag//
+ * @package tests
+ */
+
+class eZUserCacheTest extends ezpDatabaseTestCase
+{
+    private $userId;
+
+    public function __construct()
+    {
+        parent::__construct();
+        $this->setName( "eZUser Cache Unit Tests" );
+    }
+
+    public function setUp()
+    {
+        $this->userId = 12345;
+        parent::setUp();
+    }
+
+    public function tearDown()
+    {
+        eZUser::purgeUserCacheByUserId( $this->userId );
+        eZDir::cleanupEmptyDirectories( eZUser::getCacheDir( $this->userId ) );
+        parent::tearDown();
+    }
+
+    /**
+     * eZUser: do not generate cache content for invalid user
+     *
+     * @link http://issues.ez.no/22181
+     */
+    public function testNullUserCache()
+    {
+        eZUser::instance( $this->userId );
+
+        $cacheFilePath = eZUser::getCacheDir( $this->userId ) . "/user-data-{$this->userId}.cache.php";
+        $this->assertFalse( file_exists( $cacheFilePath ) );
+    }
+}
+?>
diff --git a/tests/tests/kernel/datatypes/suite.php b/tests/tests/kernel/datatypes/suite.php
@@ -19,6 +19,7 @@ public function __construct()
         $this->addTestSuite( 'eZStringTypeTest' );
         $this->addTestSuite( 'eZCountryTypeTest' );
         $this->addTestSuite( 'eZUserTest' );
+        $this->addTestSuite( 'eZUserCacheTest' );
 //        $this->addTestSuite( 'eZLDAPUserTest' );
         $this->addTestSuite( 'eZTextFileUserTest' );
         $this->addTestSuite( 'eZEmailTypeTest' );