[Properties] Update properties only with dirty state (#16010)

* [Properties] Update properties only with dirty state - #9720 * Save dependencies irrespective of properties dirty check * Mark dirty on setProperty * remove unused & docs changes
pimcore · Sep 27, 2023 · 2ad37be · 2ad37be
1 parent e86896a
commit 2ad37be
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 74 deletions.
diff --git a/doc/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md
@@ -11,6 +11,7 @@
     - `isMaintenanceModeScheduledForLogin`, `scheduleMaintenanceModeOnLogin`, `unscheduleMaintenanceModeOnLogin` will be removed in Pimcore 12
 - [CoreCacheHandler] Remove redundant cache item tagging with own key
 - [Auth] The tokens for password reset are now stored in the DB and are one time use only (gets expired whenever a new one is generated or when consumed).
+- [Elements] Properties are now only updated in the database with dirty state (when calling `setProperties` or `setProperty`).
 
 ## Pimcore 11.0.7
 - Putting `null` to the `Pimcore\Model\DataObject\Data::setIndex()` method is deprecated now. Only booleans are allowed.

diff --git a/models/Asset.php b/models/Asset.php
@@ -732,16 +732,18 @@ protected function update(array $params = []): void
 
         $this->postPersistData();
 
-        // save properties
-        $this->getProperties();
-        $this->getDao()->deleteAllProperties();
-        foreach ($this->getProperties() as $property) {
-            if (!$property->getInherited()) {
-                $property->setDao(null);
-                $property->setCid($this->getId());
-                $property->setCtype('asset');
-                $property->setCpath($this->getRealFullPath());
-                $property->save();
+        if ($this->isFieldDirty('properties')) {
+            // save properties
+            $properties = $this->getProperties();
+            $this->getDao()->deleteAllProperties();
+            foreach ($properties as $property) {
+                if (!$property->getInherited()) {
+                    $property->setDao(null);
+                    $property->setCid($this->getId());
+                    $property->setCtype('asset');
+                    $property->setCpath($this->getRealFullPath());
+                    $property->save();
+                }
             }
         }
 
@@ -752,7 +754,7 @@ protected function update(array $params = []): void
 
         foreach ($this->resolveDependencies() as $requirement) {
             if ($requirement['id'] == $this->getId() && $requirement['type'] == 'asset') {
-                // dont't add a reference to yourself
+                // don't add a reference to yourself
                 continue;
             } else {
                 $d->addRequirement($requirement['id'], $requirement['type']);
@@ -1567,7 +1569,8 @@ protected function resolveDependencies(): array
                         /** @var DataDefinitionInterface $implementation */
                         $implementation = $loader->build($elementType);
                         $dependencies[] = $implementation->resolveDependencies($elementData, $metaData);
-                    } catch (UnsupportedException $e) {
+                    } catch (UnsupportedException) {
+                        //nothing to log here
                     }
                 }
             }

diff --git a/models/Asset/Dao.php b/models/Asset/Dao.php
@@ -224,15 +224,21 @@ public function updateChildPaths(string $oldPath): array
     /**
      * Get the properties for the object from database and assign it
      *
-     *
+     * @throws \Exception
      */
     public function getProperties(bool $onlyInherited = false): array
     {
         $properties = [];
 
         // collect properties via parent - ids
         $parentIds = $this->getParentIds();
-        $propertiesRaw = $this->db->fetchAllAssociative('SELECT * FROM properties WHERE ((cid IN (' . implode(',', $parentIds) . ") AND inheritable = 1) OR cid = ? )  AND ctype='asset'", [$this->model->getId()]);
+        $propertiesRaw = $this->db->fetchAllAssociative(
+            'SELECT * FROM properties WHERE
+                             (
+                                 (cid IN (' . implode(',', $parentIds) . ") AND inheritable = 1) OR cid = ? )
+                                 AND ctype='asset'",
+            [$this->model->getId()]
+        );
 
         // because this should be faster than mysql
         usort($propertiesRaw, function ($left, $right) {
@@ -261,18 +267,13 @@ public function getProperties(bool $onlyInherited = false): array
                 }
 
                 $properties[$propertyRaw['name']] = $property;
-            } catch (\Exception $e) {
-                Logger::error("can't add property " . $propertyRaw['name'] . ' to asset ' . $this->model->getRealFullPath());
+            } catch (\Exception) {
+                Logger::error(
+                    "can't add property " . $propertyRaw['name'] . ' to asset ' . $this->model->getRealFullPath()
+                );
             }
         }
 
-        // if only inherited then only return it and dont call the setter in the model
-        if ($onlyInherited) {
-            return $properties;
-        }
-
-        $this->model->setProperties($properties);
-
         return $properties;
     }
 

diff --git a/models/DataObject/AbstractObject.php b/models/DataObject/AbstractObject.php
@@ -704,17 +704,19 @@ protected function update(bool $isUpdate = null, array $params = []): void
     {
         $this->updateModificationInfos();
 
-        // save properties
-        $this->getProperties();
-        $this->getDao()->deleteAllProperties();
-
-        foreach ($this->getProperties() as $property) {
-            if (!$property->getInherited()) {
-                $property->setDao(null);
-                $property->setCid($this->getId());
-                $property->setCtype('object');
-                $property->setCpath($this->getRealFullPath());
-                $property->save();
+        if ($this->isFieldDirty('properties')) {
+            // save properties
+            $properties = $this->getProperties();
+            $this->getDao()->deleteAllProperties();
+
+            foreach ($properties as $property) {
+                if (!$property->getInherited()) {
+                    $property->setDao(null);
+                    $property->setCid($this->getId());
+                    $property->setCtype('object');
+                    $property->setCpath($this->getRealFullPath());
+                    $property->save();
+                }
             }
         }
 
@@ -725,7 +727,7 @@ protected function update(bool $isUpdate = null, array $params = []): void
 
         foreach ($this->resolveDependencies() as $requirement) {
             if ($requirement['id'] == $this->getId() && $requirement['type'] === 'object') {
-                // dont't add a reference to yourself
+                // don't add a reference to yourself
                 continue;
             }
 

diff --git a/models/DataObject/AbstractObject/Dao.php b/models/DataObject/AbstractObject/Dao.php
@@ -222,15 +222,21 @@ public function getVersionCountForUpdate(): int
     /**
      * Get the properties for the object from database and assign it
      *
-     *
+     * @throws \Exception
      */
     public function getProperties(bool $onlyInherited = false): array
     {
         $properties = [];
 
         // collect properties via parent - ids
         $parentIds = $this->getParentIds();
-        $propertiesRaw = $this->db->fetchAllAssociative('SELECT name, type, data, cid, inheritable, cpath FROM properties WHERE ((cid IN (' . implode(',', $parentIds) . ") AND inheritable = 1) OR cid = ? ) AND ctype='object'", [$this->model->getId()]);
+        $propertiesRaw = $this->db->fetchAllAssociative(
+            'SELECT name, type, data, cid, inheritable, cpath FROM properties WHERE
+                     (
+                         (cid IN (' . implode(',', $parentIds) . ") AND inheritable = 1)
+                         OR cid = ? ) AND ctype='object'",
+            [$this->model->getId()]
+        );
 
         // because this should be faster than mysql
         usort($propertiesRaw, function ($left, $right) {
@@ -259,18 +265,13 @@ public function getProperties(bool $onlyInherited = false): array
                 }
 
                 $properties[$propertyRaw['name']] = $property;
-            } catch (\Exception $e) {
-                Logger::error("can't add property " . $propertyRaw['name'] . ' to object ' . $this->model->getRealFullPath());
+            } catch (\Exception) {
+                Logger::error(
+                    "can't add property " . $propertyRaw['name'] . ' to object ' . $this->model->getRealFullPath()
+                );
             }
         }
 
-        // if only inherited then only return it and dont call the setter in the model
-        if ($onlyInherited) {
-            return $properties;
-        }
-
-        $this->model->setProperties($properties);
-
         return $properties;
     }
 

diff --git a/models/Document.php b/models/Document.php
@@ -468,16 +468,18 @@ protected function update(array $params = []): void
             $this->setIndex($this->getDao()->getNextIndex());
         }
 
-        // save properties
-        $this->getProperties();
-        $this->getDao()->deleteAllProperties();
-        foreach ($this->getProperties() as $property) {
-            if (!$property->getInherited()) {
-                $property->setDao(null);
-                $property->setCid($this->getId());
-                $property->setCtype('document');
-                $property->setCpath($this->getRealFullPath());
-                $property->save();
+        if ($this->isFieldDirty('properties')) {
+            // save properties
+            $properties = $this->getProperties();
+            $this->getDao()->deleteAllProperties();
+            foreach ($properties as $property) {
+                if (!$property->getInherited()) {
+                    $property->setDao(null);
+                    $property->setCid($this->getId());
+                    $property->setCtype('document');
+                    $property->setCpath($this->getRealFullPath());
+                    $property->save();
+                }
             }
         }
 
@@ -488,7 +490,7 @@ protected function update(array $params = []): void
 
         foreach ($this->resolveDependencies() as $requirement) {
             if ($requirement['id'] == $this->getId() && $requirement['type'] == 'document') {
-                // dont't add a reference to yourself
+                // don't add a reference to yourself
                 continue;
             } else {
                 $d->addRequirement((int)$requirement['id'], $requirement['type']);

diff --git a/models/Document/Dao.php b/models/Document/Dao.php
@@ -250,19 +250,30 @@ public function getVersionCountForUpdate(): int
     }
 
     /**
-     * Returns properties for the object from the database and assigns these.
-     *
+     * Returns properties for the object from the database
      *
+     * @throws \Exception
      */
     public function getProperties(bool $onlyInherited = false, bool $onlyDirect = false): array
     {
         $properties = [];
 
         if ($onlyDirect) {
-            $propertiesRaw = $this->db->fetchAllAssociative("SELECT * FROM properties WHERE cid = ? AND ctype='document'", [$this->model->getId()]);
+            $propertiesRaw =
+                $this->db->fetchAllAssociative(
+                    "SELECT * FROM properties WHERE cid = ? AND ctype='document'",
+                    [$this->model->getId()]
+                );
         } else {
             $parentIds = $this->getParentIds();
-            $propertiesRaw = $this->db->fetchAllAssociative('SELECT * FROM properties WHERE ((cid IN (' . implode(',', $parentIds) . ") AND inheritable = 1) OR cid = ? )  AND ctype='document'", [$this->model->getId()]);
+            $propertiesRaw =
+                $this->db->fetchAllAssociative(
+                    'SELECT * FROM properties WHERE
+                             (
+                                 (cid IN (' . implode(',', $parentIds) . ") AND inheritable = 1) OR cid = ?
+                             ) AND ctype='document'",
+                    [$this->model->getId()]
+                );
         }
 
         // because this should be faster than mysql
@@ -295,18 +306,13 @@ public function getProperties(bool $onlyInherited = false, bool $onlyDirect = fa
                 }
 
                 $properties[$propertyRaw['name']] = $property;
-            } catch (\Exception $e) {
-                Logger::error("can't add property " . $propertyRaw['name'] . ' to document ' . $this->model->getRealFullPath());
+            } catch (\Exception) {
+                Logger::error(
+                    "can't add property " . $propertyRaw['name'] . ' to document ' . $this->model->getRealFullPath()
+                );
             }
         }
 
-        // if only inherited then only return it and dont call the setter in the model
-        if ($onlyInherited || $onlyDirect) {
-            return $properties;
-        }
-
-        $this->model->setProperties($properties);
-
         return $properties;
     }
 

diff --git a/models/Element/AbstractElement.php b/models/Element/AbstractElement.php
@@ -244,22 +244,29 @@ public function getProperties(): array
                 Cache::save($properties, $cacheKey, $cacheTags);
             }
 
-            $this->setProperties($properties);
+            $this->properties = $properties;
         }
 
         return $this->properties;
     }
 
     public function setProperties(?array $properties): static
     {
+        $this->markFieldDirty('properties');
         $this->properties = $properties;
 
         return $this;
     }
 
-    public function setProperty(string $name, string $type, mixed $data, bool $inherited = false, bool $inheritable = false): static
+    public function setProperty(
+        string $name,
+        string $type,
+        mixed $data,
+        bool $inherited = false,
+        bool $inheritable = false
+    ): static
     {
-        $this->getProperties();
+        $properties = $this->getProperties();
 
         $id = $this->getId();
         $property = new Model\Property();
@@ -274,7 +281,9 @@ public function setProperty(string $name, string $type, mixed $data, bool $inher
         $property->setInherited($inherited);
         $property->setInheritable($inheritable);
 
-        $this->properties[$name] = $property;
+        $properties[$name] = $property;
+
+        $this->setProperties($properties);
 
         return $this;
     }