EZP-29096: Do not store empty draft values in DB (#1417)

* EZP-29096: Do not store empty draft values in DB
ezsystems · Mar 11, 2019 · 1adb33d · 1adb33d
1 parent 8608700
commit 1adb33d
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 36 deletions.
diff --git a/kernel/classes/datatypes/ezuser/ezuser.php b/kernel/classes/datatypes/ezuser/ezuser.php
@@ -225,15 +225,24 @@ static function create( $contentObjectID )
         return new eZUser( $row );
     }
 
-    function store( $fieldFilters = null )
+    /**
+     * Only stores the entry if it has a Login value
+     *
+     * @param mixed|null $fieldFilters
+     */
+    public function store( $fieldFilters = null )
     {
         $this->Email = trim( $this->Email );
         $userID = $this->attribute( 'contentobject_id' );
         // Clear memory cache
         unset( $GLOBALS['eZUserObject_' . $userID] );
         $GLOBALS['eZUserObject_' . $userID] = $this;
         self::purgeUserCacheByUserId( $userID );
-        parent::store( $fieldFilters );
+
+        if ( $this->Login )
+        {
+            parent::store( $fieldFilters );
+        }
     }
 
     function originalPassword()
@@ -307,6 +316,11 @@ function setInformation( $id, $login, $email, $password, $passwordConfirm = fals
         }
     }
 
+    /**
+     * @param integer $id
+     * @param bool $asObject
+     * @return eZUser|null
+     */
     static function fetch( $id, $asObject = true )
     {
         if ( !$id )

diff --git a/kernel/classes/datatypes/ezuser/ezusertype.php b/kernel/classes/datatypes/ezuser/ezusertype.php
@@ -258,10 +258,22 @@ function storeObjectAttribute( $contentObjectAttribute )
             }
 
             // saving information in the object attribute data_text field to simulate a draft
-            $contentObjectAttribute->setAttribute( 'data_text', $this->serializeDraft( $user ) );
+            // only if the object version is a draft
+            if (
+                $user->Login &&
+                $contentObjectAttribute->attribute( 'object_version' )->attribute( 'status' ) == eZContentObjectVersion::STATUS_DRAFT
+            )
+            {
+                $contentObjectAttribute->setAttribute( 'data_text', $this->serializeDraft( $user ) );
+            }
         }
     }
 
+    /**
+     * @param $contentObjectAttribute
+     * @param eZContentObject $contentObject
+     * @param $publishedNodes
+     */
     function onPublish( $contentObjectAttribute, $contentObject, $publishedNodes )
     {
         /** @var eZContentObjectAttribute $contentObjectAttribute */
@@ -274,10 +286,13 @@ function onPublish( $contentObjectAttribute, $contentObject, $publishedNodes )
         if ( !empty( $serializedDraft ) )
         {
             $user = $this->updateUserDraft( $user, $serializedDraft );
-        }
+            $user->store();
+            $contentObjectAttribute->setContent( $user );
 
-        $user->store();
-        $contentObjectAttribute->setContent( $user );
+            // Clear draft info
+            $contentObjectAttribute->setAttribute( 'data_text', '' );
+            $contentObjectAttribute->store();
+        }
     }
 
     /**
@@ -320,10 +335,13 @@ private function updateUserDraft( eZUser $user, $serializedDraft )
     {
         $draft = $this->unserializeDraft( $serializedDraft );
 
-        $user->setAttribute( 'login', $draft->login );
-        $user->setAttribute( 'password_hash', $draft->password_hash );
-        $user->setAttribute( 'email', $draft->email );
-        $user->setAttribute( 'password_hash_type', $draft->password_hash_type );
+        if ( $draft )
+        {
+            $user->setAttribute( 'login', $draft->login );
+            $user->setAttribute( 'password_hash', $draft->password_hash );
+            $user->setAttribute( 'email', $draft->email );
+            $user->setAttribute( 'password_hash_type', $draft->password_hash_type );
+        }
 
         return $user;
     }
@@ -360,11 +378,10 @@ function objectAttributeContent( $contentObjectAttribute )
             $GLOBALS['eZUserObject_' . $userID] = eZUser::fetch( $userID );
         }
 
-        /** @var eZUser $user */
         $user = eZUser::fetch( $userID );
         eZDebugSetting::writeDebug( 'kernel-user', $user, 'user' );
 
-        // Looking for a "draft" and loading it's content
+        //Looking for a "draft" and loading its content
         $serializedDraft = $contentObjectAttribute->attribute( 'data_text' );
 
         if ( !empty( $serializedDraft ) )

diff --git a/tests/tests/kernel/datatypes/ezuser/ezusertype_regression.php b/tests/tests/kernel/datatypes/ezuser/ezusertype_regression.php
@@ -38,11 +38,16 @@ public function setUp()
             'parent_node_id' => $ini->variable( 'UserSettings', 'DefaultUserPlacement' ),
             'attributes' => array(
                 'first_name' => 'foo',
-                'last_name' => 'bar' ),
+                'last_name' => 'bar',
+                'user_account' => "{$this->userLogin}|{$this->userEmail}|1234|md5_password|0",
+            ),
         );
 
         $contentObject = eZContentFunctions::createAndPublishObject( $params );
 
+        // Re-fetch to get the object after it was published
+        $contentObject = eZContentObject::fetch( $contentObject->ID );
+
         if( !$contentObject instanceof eZContentObject )
         {
             die( 'Impossible to create user object' );
@@ -110,8 +115,9 @@ public function testFromStringHandlesDisabledAccount()
         $userAccount = $dataMap['user_account']->fromString(
             join( '|', array( $tmpLogin, $tmpEmail, self::USER_PASSWORD_HASH, self::USER_PASSWORD_HASH_ID, 0 ) )
         );
-        $userAccount->store();
 
+        $dataMap['user_account']->setContent( $userAccount );
+        $dataMap['user_account']->store();
 
         $tempObject = eZContentObject::fetch( $this->userObject->attribute( 'id' ) );
         $dataMap = $tempObject->dataMap();
@@ -151,8 +157,9 @@ public function testFromStringHandlesEnabledAccount()
         $userAccount = $dataMap['user_account']->fromString(
             join( '|', array( $tmpLogin, $tmpEmail, self::USER_PASSWORD_HASH, self::USER_PASSWORD_HASH_ID, 1 ) )
         );
-        $userAccount->store();
 
+        $dataMap['user_account']->setContent( $userAccount );
+        $dataMap['user_account']->store();
 
         $tempObject = eZContentObject::fetch( $this->userObject->attribute( 'id' ) );
         $dataMap = $tempObject->dataMap();
@@ -188,11 +195,13 @@ public function testUpdatePasswordUpdatesSerializedData()
     {
         $userId = $this->userObject->attribute('id');
 
-        $passwordHash = $this->getSerializedPasswordHash($this->userObject);
+        $user = ezuser::fetch( $userId );
+        $passwordHash = $user->PasswordHash;
 
         eZUserOperationCollection::password($userId, 'newpassword');
 
-        $updatedPasswordHash = $this->getSerializedPasswordHash( eZContentObject::fetch($userId) );
+        $user = ezuser::fetch( $userId );
+        $updatedPasswordHash = $user->PasswordHash;
 
         self::assertNotEquals(
             $passwordHash,
@@ -201,25 +210,6 @@ public function testUpdatePasswordUpdatesSerializedData()
         );
     }
 
-    /**
-     * @param \eZContentObject $userObject
-     *
-     * @return string The serialized password hash, or null if none is set
-     */
-    private function getSerializedPasswordHash(eZContentObject $userObject)
-    {
-        $dataMap = $userObject->dataMap();
-
-        $userAccountAttributeText = $dataMap['user_account']->attribute('data_text');
-
-        // empty on initial version
-        if (!empty($userAccountAttributeText)) {
-            return json_decode($userAccountAttributeText)->password_hash;
-        }
-
-        return null;
-    }
-
     /**
      * Enables the current user
      */