diff --git a/src/ActiveRecord.php b/src/ActiveRecord.php index 90318b562..0c6e7fc1b 100644 --- a/src/ActiveRecord.php +++ b/src/ActiveRecord.php @@ -8,7 +8,6 @@ use Yiisoft\Db\Exception\Exception; use Yiisoft\Db\Exception\InvalidArgumentException; use Yiisoft\Db\Exception\InvalidConfigException; -use Yiisoft\Db\Exception\StaleObjectException; use Yiisoft\Db\Schema\TableSchemaInterface; use function array_diff; @@ -114,7 +113,7 @@ public function attributes(): array return $this->getTableSchema()->getColumnNames(); } - public function delete(): false|int + public function delete(): int { if (!$this->isTransactional(self::OP_DELETE)) { return $this->deleteInternal(); @@ -124,11 +123,7 @@ public function delete(): false|int try { $result = $this->deleteInternal(); - if ($result === false) { - $transaction->rollBack(); - } else { - $transaction->commit(); - } + $transaction->commit(); return $result; } catch (Throwable $e) { @@ -346,41 +341,6 @@ public function update(array $attributeNames = null): int } } - /** - * Deletes an ActiveRecord without considering transaction. - * - * Note that it is possible the number of rows deleted is 0, even though the deletion execution is successful. - * - * @throws Exception - * @throws StaleObjectException - * @throws Throwable - * - * @return false|int The number of rows deleted, or `false` if the deletion is unsuccessful for some reason. - */ - protected function deleteInternal(): false|int - { - /** - * We do not check the return value of deleteAll() because it's possible the record is already deleted in the - * database and thus the method will return 0. - */ - $condition = $this->getOldPrimaryKey(true); - $lock = $this->optimisticLock(); - - if ($lock !== null) { - $condition[$lock] = $this->$lock; - } - - $result = $this->deleteAll($condition); - - if ($lock !== null && !$result) { - throw new StaleObjectException('The object being deleted is outdated.'); - } - - $this->setOldAttributes(); - - return $result; - } - /** * Valid column names are table column names or column names prefixed with table name or table alias. * diff --git a/src/ActiveRecordInterface.php b/src/ActiveRecordInterface.php index 6af7b3cd7..03bd11b15 100644 --- a/src/ActiveRecordInterface.php +++ b/src/ActiveRecordInterface.php @@ -34,11 +34,11 @@ public function attributes(): array; * is outdated. * @throws Throwable In case delete failed. * - * @return false|int The number of rows deleted, or `false` if the deletion is unsuccessful for some reason. + * @return int The number of rows deleted. * * Note that it's possible the number of rows deleted is 0, even though the deletion execution is successful. */ - public function delete(): false|int; + public function delete(): int; /** * Deletes rows in the table using the provided conditions. diff --git a/src/BaseActiveRecord.php b/src/BaseActiveRecord.php index 96b73cc01..9807ad6d1 100644 --- a/src/BaseActiveRecord.php +++ b/src/BaseActiveRecord.php @@ -66,29 +66,9 @@ public function __construct( ) { } - public function delete(): false|int + public function delete(): int { - /** - * We do not check the return value of deleteAll() because it's possible the record is already deleted in - * the database and thus the method will return 0 - */ - $condition = $this->getOldPrimaryKey(true); - - $lock = $this->optimisticLock(); - - if ($lock !== null) { - $condition[$lock] = $lock; - } - - $result = $this->deleteAll($condition); - - if ($lock !== null && !$result) { - throw new StaleObjectException('The object being deleted is outdated.'); - } - - $this->oldAttributes = null; - - return $result; + return $this->deleteInternal(); } public function deleteAll(array $condition = [], array $params = []): int @@ -1088,6 +1068,41 @@ protected function createRelationQuery(string $arClass, array $link, bool $multi return $this->instantiateQuery($arClass)->primaryModel($this)->link($link)->multiple($multiple); } + /** + * {@see delete()} + * + * @throws Exception + * @throws StaleObjectException + * @throws Throwable + * + * @return int The number of rows deleted. + */ + protected function deleteInternal(): int + { + /** + * We do not check the return value of deleteAll() because it's possible the record is already deleted in + * the database and thus the method will return 0 + */ + $condition = $this->getOldPrimaryKey(true); + $lock = $this->optimisticLock(); + + if ($lock !== null) { + $condition[$lock] = $this->$lock; + + $result = $this->deleteAll($condition); + + if ($result === 0) { + throw new StaleObjectException('The object being deleted is outdated.'); + } + } else { + $result = $this->deleteAll($condition); + } + + $this->setOldAttributes(); + + return $result; + } + /** * Repopulates this active record with the latest data from a newly fetched instance. * diff --git a/tests/ActiveQueryTest.php b/tests/ActiveQueryTest.php index 3ef924af4..0f7f019de 100644 --- a/tests/ActiveQueryTest.php +++ b/tests/ActiveQueryTest.php @@ -2005,6 +2005,36 @@ public function testOptimisticLock(): void $record->save(); } + public function testOptimisticLockOnDelete(): void + { + $this->checkFixture($this->db, 'document', true); + + $documentQuery = new ActiveQuery(Document::class, $this->db); + $document = $documentQuery->findOne(1); + + $this->assertSame(0, $document->version); + + $document->version = 1; + + $this->expectException(StaleObjectException::class); + $document->delete(); + } + + public function testOptimisticLockAfterDelete(): void + { + $this->checkFixture($this->db, 'document', true); + + $documentQuery = new ActiveQuery(Document::class, $this->db); + $document = $documentQuery->findOne(1); + + $this->assertSame(0, $document->version); + $this->assertSame(1, $document->delete()); + $this->assertTrue($document->getIsNewRecord()); + + $this->expectException(StaleObjectException::class); + $document->delete(); + } + /** * {@see https://github.com/yiisoft/yii2/issues/9006} */