Revision performance improvements

CHANGELOG.md

@@ -138,6 +138,7 @@
 - Added `craft\elements\conditions\users\UserCondition`.
 - Added `craft\elements\conditions\users\UsernameConditionRule`.
 - Added `craft\elements\MatrixBlock::$primaryOwnerId`.
+- Added `craft\elements\MatrixBlock::$saveOwnership`.
 - Added `craft\elements\User::$active`.
 - Added `craft\elements\User::canAssignUserGroups()`.
 - Added `craft\elements\User::getIsCredentialed()`.
@@ -260,9 +261,11 @@
 - Added `craft\services\ElementSources`, which replaces `craft\services\ElementIndexes`.
 - Added `craft\services\Fields::createLayout()`.
 - Added `craft\services\Fs`.
+- Added `craft\services\Gc::hardDeleteElements()`.
 - Added `craft\services\Gql::prepareFieldDefinitions()`.
 - Added `craft\services\ImageTransforms`.
 - Added `craft\services\Matrix::duplicateOwnership()`.
+- Added `craft\services\Matrix::createRevisionBlocks()`.
 - Added `craft\services\ProjectConfig::applyExternalChanges()`.
 - Added `craft\services\ProjectConfig::ASSOC_KEY`.
 - Added `craft\services\ProjectConfig::getDoesExternalConfigExist()`.
@@ -348,6 +351,7 @@
 - Craft now requires MySQL 5.7.8 / MariaDB 10.2.7 / PostgreSQL 10.0 or later.
 - Craft now requires the [Intl](https://php.net/manual/en/book.intl.php) and [BCMath](https://www.php.net/manual/en/book.bc.php) PHP extensions.
 - Improved draft creation/application performance. ([#10577](https://github.com/craftcms/cms/pull/10577))
+- Improved revision creation performance. ([#10589](https://github.com/craftcms/cms/pull/10577))
 - The “What’ New” HUD now displays an icon and label above each announcement, identifying where it came from (Craft CMS or a plugin). ([#9747](https://github.com/craftcms/cms/discussions/9747))
 - The control panel now keeps track of the currently-edited site on a per-tab basis by adding a `site` query string param to all control panel URLs. ([#8920](https://github.com/craftcms/cms/discussions/8920))
 - Users are no longer required to have a username or email.
@@ -363,6 +367,7 @@
 - Images that are not web-safe now are always converted to JPEGs when transforming, if no format was specified.
 - Entry post dates are no longer set automatically until the entry is validated with the `live` scenario. ([#10093](https://github.com/craftcms/cms/pull/10093))
 - Entry queries’ `authorGroup()` param method now accepts an array of `craft\models\UserGroup` objects.
+- Element queries’ `revision` params can now be set to `null` to include normal and revision elements.
 - Relational fields now load elements in the current site rather than the primary site, if the source element isn’t localizable. ([#7048](https://github.com/craftcms/cms/issues/7048))
 - Built-in queue jobs are now always translated for the current user’s language. ([#9745](https://github.com/craftcms/cms/pull/9745))
 - Path options passed to console commands (e.g. `--basePath`) now take precedence over their enivronment variable/PHP constant counterparts.
@@ -528,6 +533,7 @@
 - `craft\services\Assets::EVENT_GET_ASSET_THUMB_URL` has been renamed to `EVENT_DEFINE_THUMB_URL`.
 - `craft\services\Assets::EVENT_GET_ASSET_URL` has been renamed to `EVENT_DEFINE_ASSET_URL`.
 - `craft\services\Assets::EVENT_GET_THUMB_PATH` has been renamed to `EVENT_DEFINE_THUMB_PATH`.
+- `craft\services\Matrix::duplicateBlocks()` now has a `$deleteOtherBlocks` argument.
 - `craft\services\Plugins::doesPluginRequireDatabaseUpdate()` has been renamed to `isPluginUpdatePending()`.
 - `craft\services\ProjectConfig::set()` now returns `true` or `false` depending on whether the project config was modified.
 - `craft\services\Revisions::createRevision()` now returns the ID of the revision, rather than the revision itself.

src/elements/MatrixBlock.php

@@ -208,6 +208,12 @@ public static function gqlTypeNameByContext($context): string
      */
     public bool $deletedWithOwner = false;
 
+    /**
+     * @var bool Whether to save the block’s row in the `matrixblocks_owners` table in [[afterSave()]].
+     * @since 4.0.0
+     */
+    public bool $saveOwnership = true;
+
     /**
      * @var ElementInterface|null The owner element, or false if [[ownerId]] is invalid
      */
@@ -420,13 +426,28 @@ public function getGqlTypeName(): string
     // Events
     // -------------------------------------------------------------------------
 
+    /**
+     * @inheritdoc
+     */
+    public function beforeSave(bool $isNew): bool
+    {
+        if (!$this->primaryOwnerId && !$this->ownerId) {
+            throw new InvalidConfigException('No owner ID assigned to the Matrix block.');
+        }
+
+        return parent::beforeSave($isNew);
+    }
+
     /**
      * @inheritdoc
      * @throws Exception if reasons
      */
     public function afterSave(bool $isNew): void
     {
         if (!$this->propagating) {
+            $this->primaryOwnerId = $this->primaryOwnerId ?? $this->ownerId;
+            $this->ownerId = $this->ownerId ?? $this->primaryOwnerId;
+
             // Get the block record
             if (!$isNew) {
                 $record = MatrixBlockRecord::findOne($this->id);
@@ -439,16 +460,28 @@ public function afterSave(bool $isNew): void
                 $record->id = (int)$this->id;
             }
 
-            $record->fieldId = (int)$this->fieldId;
-            $record->primaryOwnerId = (int)$this->primaryOwnerId;
-            $record->typeId = (int)$this->typeId;
+            $record->fieldId = $this->fieldId;
+            $record->primaryOwnerId = $this->primaryOwnerId ?? $this->ownerId;
+            $record->typeId = $this->typeId;
             $record->save(false);
 
-            Db::upsert(Table::MATRIXBLOCKS_OWNERS, [
-                'blockId' => $this->id,
-                'ownerId' => $this->ownerId,
-                'sortOrder' => $this->sortOrder ?? 0,
-            ]);
+            // ownerId will be null when creating a revision
+            if ($this->saveOwnership) {
+                if ($isNew) {
+                    Db::insert(Table::MATRIXBLOCKS_OWNERS, [
+                        'blockId' => $this->id,
+                        'ownerId' => $this->ownerId,
+                        'sortOrder' => $this->sortOrder ?? 0,
+                    ], false);
+                } else {
+                    Db::update(Table::MATRIXBLOCKS_OWNERS, [
+                        'sortOrder' => $this->sortOrder ?? 0,
+                    ], [
+                        'blockId' => $this->id,
+                        'ownerId' => $this->ownerId,
+                    ], [], false);
+                }
+            }
         }
 
         parent::afterSave($isNew);

src/elements/db/ElementQuery.php

@@ -166,10 +166,10 @@ class ElementQuery extends Query implements ElementQueryInterface
     public bool $savedDraftsOnly = false;
 
     /**
-     * @var bool Whether revision elements should be returned.
+     * @var bool|null Whether revision elements should be returned.
      * @since 3.2.0
      */
-    public bool $revisions = false;
+    public ?bool $revisions = false;
 
     /**
      * @var int|null The ID of the revision to return (from the `revisions` table)
@@ -727,7 +727,7 @@ public function savedDraftsOnly(bool $value = true): self
      * @inheritdoc
      * @uses $revisions
      */
-    public function revisions(bool $value = true): self
+    public function revisions(?bool $value = true): self
     {
         $this->revisions = $value;
         return $this;
@@ -741,7 +741,9 @@ public function revisions(bool $value = true): self
     public function revisionId(?int $value = null): self
     {
         $this->revisionId = $value;
-        $this->revisions = $value !== null;
+        if ($value !== null && $this->revisions === false) {
+            $this->revisions = true;
+        }
         return $this;
     }
 
@@ -759,7 +761,9 @@ public function revisionOf($value): self
         } else {
             throw new InvalidArgumentException('Invalid revisionOf value');
         }
-        $this->revisions = $value !== null;
+        if ($value !== null && $this->revisions === false) {
+            $this->revisions = true;
+        }
         return $this;
     }
 
@@ -777,7 +781,9 @@ public function revisionCreator($value): self
         } else {
             throw new InvalidArgumentException('Invalid revisionCreator value');
         }
-        $this->revisions = $value !== null;
+        if ($value !== null && $this->revisions === false) {
+            $this->revisions = true;
+        }
         return $this;
     }
 
@@ -1814,12 +1820,20 @@ public function createElement(array $row): ElementInterface
             }
         }
 
-        if ($this->revisions) {
-            $behaviors['revision'] = new RevisionBehavior([
-                'creatorId' => ArrayHelper::remove($row, 'revisionCreatorId'),
-                'revisionNum' => ArrayHelper::remove($row, 'revisionNum'),
-                'revisionNotes' => ArrayHelper::remove($row, 'revisionNotes'),
-            ]);
+        if ($this->revisions !== false) {
+            if (!empty($row['revisionId'])) {
+                $behaviors['revision'] = new RevisionBehavior([
+                    'creatorId' => ArrayHelper::remove($row, 'revisionCreatorId'),
+                    'revisionNum' => ArrayHelper::remove($row, 'revisionNum'),
+                    'revisionNotes' => ArrayHelper::remove($row, 'revisionNotes'),
+                ]);
+            } else {
+                unset(
+                    $row['revisionCreatorId'],
+                    $row['revisionNum'],
+                    $row['revisionNotes'],
+                );
+            }
         }
 
         $element = new $class($row);
@@ -2467,16 +2481,17 @@ private function _applyRevisionParams(): void
             $this->subQuery->andWhere($this->_placeholderCondition(['elements.draftId' => null]));
         }
 
-        if ($this->revisions) {
-            $this->subQuery->innerJoin(['revisions' => Table::REVISIONS], '[[revisions.id]] = [[elements.revisionId]]');
-            $this->query
-                ->innerJoin(['revisions' => Table::REVISIONS], '[[revisions.id]] = [[elements.revisionId]]')
-                ->addSelect([
-                    'elements.revisionId',
-                    'revisions.creatorId as revisionCreatorId',
-                    'revisions.num as revisionNum',
-                    'revisions.notes as revisionNotes',
-                ]);
+        if ($this->revisions !== false) {
+            $joinType = $this->revisions === true ? 'INNER JOIN' : 'LEFT JOIN';
+            $this->subQuery->join($joinType, ['revisions' => Table::REVISIONS], '[[revisions.id]] = [[elements.revisionId]]');
+            $this->query->join($joinType, ['revisions' => Table::REVISIONS], '[[revisions.id]] = [[elements.revisionId]]');
+
+            $this->query->addSelect([
+                'elements.revisionId',
+                'revisions.creatorId as revisionCreatorId',
+                'revisions.num as revisionNum',
+                'revisions.notes as revisionNotes',
+            ]);
 
             if ($this->revisionId) {
                 $this->subQuery->andWhere(['elements.revisionId' => $this->revisionId]);

src/elements/db/ElementQueryInterface.php

@@ -286,11 +286,11 @@ public function savedDraftsOnly(bool $value = true): self;
      *     ->one();
      * ```
      *
-     * @param bool $value The property value (defaults to true)
+     * @param bool|null $value The property value (defaults to true)
      * @return self self reference
      * @since 3.2.0
      */
-    public function revisions(bool $value = true): self;
+    public function revisions(?bool $value = true): self;
 
     /**
      * Narrows the query results based on the {elements}’ revision’s ID (from the `revisions` table).

src/fields/Matrix.php

@@ -525,6 +525,13 @@ private function _populateQuery(MatrixBlockQuery $query, ?ElementInterface $elem
             if ($query->id === false) {
                 $query->id = null;
             }
+
+            // If the owner is a revision, allow revision blocks to be returned as well
+            if ($element->getIsRevision()) {
+                $query
+                    ->revisions(null)
+                    ->trashed(null);
+            }
         } else {
             $query->id = false;
         }
@@ -1023,6 +1030,8 @@ public function afterElementPropagate(ElementInterface $element, bool $isNew): v
             // If this is a draft, just duplicate the relations
             if ($element->getIsDraft()) {
                 $matrixService->duplicateOwnership($this, $element->duplicateOf, $element);
+            } else if ($element->getIsRevision()) {
+                $matrixService->createRevisionBlocks($this, $element->duplicateOf, $element);
             } else {
                 $matrixService->duplicateBlocks($this, $element->duplicateOf, $element, true, !$isNew);
             }

src/services/Gc.php

@@ -8,6 +8,7 @@
 namespace craft\services;
 
 use Craft;
+use craft\base\BlockElementInterface;
 use craft\config\GeneralConfig;
 use craft\db\Connection;
 use craft\db\Query;
@@ -95,8 +96,10 @@ public function run(bool $force = false): void
         $this->_deleteStaleSessions();
         $this->_deleteStaleAnnouncements();
 
+        // elements should always go first
+        $this->hardDeleteElements();
+
         $this->hardDelete([
-            Table::ELEMENTS, // elements should always go first
             Table::CATEGORYGROUPS,
             Table::ENTRYTYPES,
             Table::FIELDGROUPS,
@@ -166,6 +169,72 @@ public function hardDeleteVolumes(): void
         Volume::deleteAll(['id' => $volumeIds]);
     }
 
+    /**
+     * Hard-deletes eligible elements.
+     *
+     * Any soft-deleted block elements which have revisions will be skipped, as their revisions may still be needed by the owner element.
+     *
+     * @since 4.0.0
+     */
+    public function hardDeleteElements(): void
+    {
+        if (!$this->_shouldHardDelete()) {
+            return;
+        }
+
+        $normalElementTypes = [];
+        $blockElementTypes = [];
+
+        foreach (Craft::$app->getElements()->getAllElementTypes() as $elementType) {
+            if (is_subclass_of($elementType, BlockElementInterface::class)) {
+                $blockElementTypes[] = $elementType;
+            } else {
+                $normalElementTypes[] = $elementType;
+            }
+        }
+
+        if ($normalElementTypes) {
+            Db::delete(Table::ELEMENTS, [
+                'and',
+                $this->_hardDeleteCondition(),
+                ['type' => $normalElementTypes],
+            ]);
+        }
+
+        if ($blockElementTypes) {
+            // Only hard-delete block elements that don't have any revisions
+            $elementsTable = Table::ELEMENTS;
+            $revisionsTable = Table::REVISIONS;
+            $params = [];
+            $conditionSql = $this->db->getQueryBuilder()->buildCondition([
+                'and',
+                $this->_hardDeleteCondition('e'),
+                [
+                    'e.type' => $blockElementTypes,
+                    'r.id' => null,
+                ]
+            ], $params);
+
+            if ($this->db->getIsMysql()) {
+                $sql = <<<SQL
+DELETE [[e]].* FROM $elementsTable [[e]]
+LEFT JOIN $revisionsTable [[r]] ON [[r.canonicalId]] = [[e.id]]
+WHERE $conditionSql
+SQL;
+            } else {
+                $sql = <<<SQL
+DELETE FROM $elementsTable
+USING $elementsTable [[e]]
+LEFT JOIN $revisionsTable [[r]] ON [[r.canonicalId]] = [[e.id]]
+WHERE
+  $elementsTable.[[id]] = [[e.id]] AND $conditionSql
+SQL;
+            }
+
+            $this->db->createCommand($sql, $params)->execute();
+        }
+    }
+
     /**
      * Hard-deletes any rows in the given table(s), that are due for it.
      *
@@ -292,9 +361,10 @@ private function _deleteOrphanedDraftsAndRevisions(): void
     /**
      * @return array
      */
-    private function _hardDeleteCondition(): array
+    private function _hardDeleteCondition(?string $tableAlias = null): array
     {
-        $condition = ['not', ['dateDeleted' => null]];
+        $tableAlias = $tableAlias ? "$tableAlias." : '';
+        $condition = ['not', ["{$tableAlias}dateDeleted" => null]];
 
         if (!$this->deleteAllTrashed) {
             $expire = DateTimeHelper::currentUTCDateTime();
@@ -303,9 +373,10 @@ private function _hardDeleteCondition(): array
             $condition = [
                 'and',
                 $condition,
-                ['<', 'dateDeleted', Db::prepareDateForDb($pastTime)],
+                ['<', "{$tableAlias}dateDeleted", Db::prepareDateForDb($pastTime)],
             ];
         }
+
         return $condition;
     }
 }