From b57fe20c58bf404aa473339cb4fda897deea2561 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Fri, 24 Oct 2025 16:52:31 +0300 Subject: [PATCH 1/2] Remove result from `ActiveRecordInterface` methods: `insert`, `upsert`, `save` --- src/AbstractActiveRecord.php | 21 ++++++-------- src/ActiveRecord.php | 18 ++++-------- src/ActiveRecordInterface.php | 12 ++------ src/Event/AfterInsert.php | 3 +- src/Event/AfterSave.php | 3 +- src/Event/AfterUpsert.php | 3 +- src/Trait/EventsTrait.php | 30 ++++++++----------- tests/ActiveQueryTest.php | 4 +-- tests/ActiveRecordTest.php | 43 +++++++++++++--------------- tests/EventsTraitTest.php | 18 ++++-------- tests/MagicActiveRecordTest.php | 17 +++++------ tests/PrivatePropertiesTraitTest.php | 3 +- 12 files changed, 69 insertions(+), 106 deletions(-) diff --git a/src/AbstractActiveRecord.php b/src/AbstractActiveRecord.php index ae46523d0..825ed39e6 100644 --- a/src/AbstractActiveRecord.php +++ b/src/AbstractActiveRecord.php @@ -82,10 +82,8 @@ abstract protected function propertyValuesInternal(): array; * @throws InvalidArgumentException * @throws InvalidConfigException * @throws Throwable - * - * @return bool Whether the record inserted successfully. */ - abstract protected function insertInternal(array|null $properties = null): bool; + abstract protected function insertInternal(array|null $properties = null): void; /** * Sets the value of the named property. @@ -103,7 +101,7 @@ abstract protected function populateProperty(string $name, mixed $value): void; abstract protected function upsertInternal( array|null $insertProperties = null, array|bool $updateProperties = true, - ): bool; + ): void; public function createQuery(ActiveRecordInterface|string|null $modelClass = null): ActiveQueryInterface { @@ -370,9 +368,9 @@ public function hasOne(ActiveRecordInterface|string $modelClass, array $link): A return $this->createRelationQuery($modelClass, $link, false); } - public function insert(array|null $properties = null): bool + public function insert(array|null $properties = null): void { - return $this->insertInternal($properties); + $this->insertInternal($properties); } public function isChanged(): bool @@ -608,15 +606,14 @@ protected function retrieveRelation(string $name): ActiveRecordInterface|array|n return $this->related[$name] = $query->relatedRecords(); } - public function save(array|null $properties = null): bool + public function save(array|null $properties = null): void { if ($this->isNewRecord()) { - return $this->insert($properties); + $this->insert($properties); + return; } $this->update($properties); - - return true; } public function set(string $propertyName, mixed $value): void @@ -794,9 +791,9 @@ public function updateCounters(array $counters): bool return true; } - public function upsert(array|null $insertProperties = null, array|bool $updateProperties = true): bool + public function upsert(array|null $insertProperties = null, array|bool $updateProperties = true): void { - return $this->upsertInternal($insertProperties, $updateProperties); + $this->upsertInternal($insertProperties, $updateProperties); } public function unlink(string $relationName, ActiveRecordInterface $linkedModel, bool $delete = false): void diff --git a/src/ActiveRecord.php b/src/ActiveRecord.php index 5afc78d07..7e7e8e822 100644 --- a/src/ActiveRecord.php +++ b/src/ActiveRecord.php @@ -159,7 +159,7 @@ protected function propertyValuesInternal(): array return get_object_vars($this); } - protected function insertInternal(array|null $properties = null): bool + protected function insertInternal(array|null $properties = null): void { if (!$this->isNewRecord()) { throw new InvalidCallException('The record is not new and cannot be inserted.'); @@ -168,10 +168,10 @@ protected function insertInternal(array|null $properties = null): bool $values = $this->newPropertyValues($properties); $primaryKeys = $this->db()->createCommand()->insertReturningPks($this->tableName(), $values); - return $this->populateRawValues($primaryKeys, $values); + $this->populateRawValues($primaryKeys, $values); } - protected function upsertInternal(array|null $insertProperties = null, array|bool $updateProperties = true): bool + protected function upsertInternal(array|null $insertProperties = null, array|bool $updateProperties = true): void { if (!$this->isNewRecord()) { throw new InvalidCallException('The record is not new and cannot be inserted.'); @@ -205,7 +205,7 @@ protected function upsertInternal(array|null $insertProperties = null, array|boo $returnedValues = $this->db()->createCommand() ->upsertReturning($this->tableName(), $insertValues, $updateProperties, $returnProperties); - return $this->populateRawValues($returnedValues); + $this->populateRawValues($returnedValues); } protected function populateProperty(string $name, mixed $value): void @@ -214,14 +214,10 @@ protected function populateProperty(string $name, mixed $value): void } /** - * @psalm-param array|false $rawValues + * @psalm-param array $rawValues */ - private function populateRawValues(array|false $rawValues, array $oldValues = []): bool + private function populateRawValues(array $rawValues, array $oldValues = []): void { - if ($rawValues === false) { - return false; - } - $values = $this->phpTypecastValues($rawValues); foreach ($values as $name => $value) { @@ -229,8 +225,6 @@ private function populateRawValues(array|false $rawValues, array $oldValues = [] } $this->assignOldValues(array_merge($oldValues, $values)); - - return true; } /** diff --git a/src/ActiveRecordInterface.php b/src/ActiveRecordInterface.php index 84faa32a4..f4fb38d01 100644 --- a/src/ActiveRecordInterface.php +++ b/src/ActiveRecordInterface.php @@ -244,10 +244,8 @@ public function hasProperty(string $name): bool; * @throws InvalidCallException If the record {@see isNewRecord() is not new}. * @throws InvalidConfigException * @throws Throwable In case insert failed. - * - * @return bool Whether the record is inserted successfully. */ - public function insert(array|null $properties = null): bool; + public function insert(array|null $properties = null): void; /** * Checks if any property returned by {@see propertyNames()} method has changed. @@ -424,10 +422,8 @@ public function resetRelation(string $name): void; * * @param array|null $properties List of property names or name-values pairs that need to be saved. * Defaults to `null`, meaning all changed property values will be saved. - * - * @return bool Whether the saving succeeded (that's no validation errors occurred). */ - public function save(array|null $properties = null): bool; + public function save(array|null $properties = null): void; /** * Sets the named property value. @@ -562,10 +558,8 @@ public function updateAll(array $propertyValues, array|string $condition = [], a * * @throws InvalidConfigException * @throws Throwable In case query failed. - * - * @return bool Whether the record is inserted or updated successfully. */ - public function upsert(array|null $insertProperties = null, array|bool $updateProperties = true): bool; + public function upsert(array|null $insertProperties = null, array|bool $updateProperties = true): void; /** * Destroys the relationship between two records. diff --git a/src/Event/AfterInsert.php b/src/Event/AfterInsert.php index 88368f3eb..2b321bd5d 100644 --- a/src/Event/AfterInsert.php +++ b/src/Event/AfterInsert.php @@ -15,9 +15,8 @@ final class AfterInsert extends AbstractEvent { /** * @param ActiveRecordInterface $model The model that has been inserted. - * @param bool $isSuccessful Whether the insert operation is successful. */ - public function __construct(ActiveRecordInterface $model, public bool &$isSuccessful) + public function __construct(ActiveRecordInterface $model) { parent::__construct($model); } diff --git a/src/Event/AfterSave.php b/src/Event/AfterSave.php index 84e3f09f0..454763f00 100644 --- a/src/Event/AfterSave.php +++ b/src/Event/AfterSave.php @@ -15,9 +15,8 @@ final class AfterSave extends AbstractEvent { /** * @param ActiveRecordInterface $model The model that was saved. - * @param bool $isSuccessful Whether the save operation was successful. */ - public function __construct(ActiveRecordInterface $model, public bool &$isSuccessful) + public function __construct(ActiveRecordInterface $model) { parent::__construct($model); } diff --git a/src/Event/AfterUpsert.php b/src/Event/AfterUpsert.php index 6f342a499..b431bb752 100644 --- a/src/Event/AfterUpsert.php +++ b/src/Event/AfterUpsert.php @@ -15,9 +15,8 @@ final class AfterUpsert extends AbstractEvent { /** * @param ActiveRecordInterface $model The model that was upserted. - * @param bool $isSuccessful Whether the upsert operation was successful. */ - public function __construct(ActiveRecordInterface $model, public bool &$isSuccessful) + public function __construct(ActiveRecordInterface $model) { parent::__construct($model); } diff --git a/src/Trait/EventsTrait.php b/src/Trait/EventsTrait.php index 0684c5c31..80c255928 100644 --- a/src/Trait/EventsTrait.php +++ b/src/Trait/EventsTrait.php @@ -54,20 +54,18 @@ public function delete(): int return $result; } - public function insert(array|null $properties = null): bool + public function insert(array|null $properties = null): void { $eventDispatcher = EventDispatcherProvider::get(static::class); $eventDispatcher->dispatch($event = new BeforeInsert($this, $properties)); if ($event->isDefaultPrevented()) { - return $event->getReturnValue() ?? false; + return; } - $result = parent::insert($properties); + parent::insert($properties); - $eventDispatcher->dispatch(new AfterInsert($this, $result)); - - return $result; + $eventDispatcher->dispatch(new AfterInsert($this)); } public function populateRecord(array|object $data): static @@ -109,20 +107,18 @@ public static function query(ActiveRecordInterface|Closure|null|string $modelCla return $query; } - public function save(array|null $properties = null): bool + public function save(array|null $properties = null): void { $eventDispatcher = EventDispatcherProvider::get(static::class); $eventDispatcher->dispatch($event = new BeforeSave($this, $properties)); if ($event->isDefaultPrevented()) { - return $event->getReturnValue() ?? false; + return; } - $result = parent::save($properties); - - $eventDispatcher->dispatch(new AfterSave($this, $result)); + parent::save($properties); - return $result; + $eventDispatcher->dispatch(new AfterSave($this)); } public function update(array|null $properties = null): int @@ -141,19 +137,17 @@ public function update(array|null $properties = null): int return $result; } - public function upsert(array|null $insertProperties = null, array|bool $updateProperties = true): bool + public function upsert(array|null $insertProperties = null, array|bool $updateProperties = true): void { $eventDispatcher = EventDispatcherProvider::get(static::class); $eventDispatcher->dispatch($event = new BeforeUpsert($this, $insertProperties, $updateProperties)); if ($event->isDefaultPrevented()) { - return $event->getReturnValue() ?? false; + return; } - $result = parent::upsert($insertProperties, $updateProperties); - - $eventDispatcher->dispatch(new AfterUpsert($this, $result)); + parent::upsert($insertProperties, $updateProperties); - return $result; + $eventDispatcher->dispatch(new AfterUpsert($this)); } } diff --git a/tests/ActiveQueryTest.php b/tests/ActiveQueryTest.php index 78eba0323..00f98d064 100644 --- a/tests/ActiveQueryTest.php +++ b/tests/ActiveQueryTest.php @@ -2065,12 +2065,12 @@ public function testOldPropertyAfterInsertAndUpdate(): void ]); $this->assertNull($customer->oldValue('name')); - $this->assertTrue($customer->save()); + $customer->save(); $this->assertSame('Jack', $customer->oldValue('name')); $customer->set('name', 'Harry'); - $this->assertTrue($customer->save()); + $customer->save(); $this->assertSame('Harry', $customer->oldValue('name')); } diff --git a/tests/ActiveRecordTest.php b/tests/ActiveRecordTest.php index e0572ce49..373dc380a 100644 --- a/tests/ActiveRecordTest.php +++ b/tests/ActiveRecordTest.php @@ -267,7 +267,7 @@ public function testSave(): void $this->assertNull($customer->getId()); $this->assertTrue($customer->isNewRecord()); - $this->assertTrue($customer->save()); + $customer->save(); $this->assertFalse($customer->isNewRecord()); $this->assertSame(4, $customer->getId()); @@ -289,7 +289,7 @@ public function testSave(): void $this->assertNull($customer->getId()); $this->assertTrue($customer->isNewRecord()); - $this->assertTrue($customer->save(['email', 'name', 'address'])); + $customer->save(['email', 'name', 'address']); $this->assertFalse($customer->isNewRecord()); $this->assertSame(5, $customer->getId()); @@ -305,11 +305,11 @@ public function testSave(): void $customer->setAddress('address6'); $customer->setStatus(1); - $this->assertTrue($customer->save([ + $customer->save([ 'email' => 'user6@example.com', 'name' => 'user6', 'address', - ])); + ]); $this->assertFalse($customer->isNewRecord()); $this->assertSame(6, $customer->getId()); @@ -324,7 +324,7 @@ public function testSave(): void $customer->setEmail('customer6@example.com'); $customer->setName('customer6'); - $this->assertTrue($customer->save()); + $customer->save(); $this->assertFalse($customer->isNewRecord()); $this->assertSame(6, $customer->getId()); @@ -341,7 +341,7 @@ public function testSave(): void $customer->setName('name6'); $customer->setStatus(1); - $this->assertTrue($customer->save(['email', 'name'])); + $customer->save(['email', 'name']); $customer->refresh(); $this->assertSame(6, $customer->getId()); @@ -353,11 +353,11 @@ public function testSave(): void // update with property values $customer->setAddress('street6'); $customer->setStatus(1); - $this->assertTrue($customer->save([ + $customer->save([ 'email' => 'client6@example.com', 'name' => 'client6', 'address', - ])); + ]); $customer->refresh(); $this->assertSame(6, $customer->getId()); @@ -373,7 +373,7 @@ public function testSaveEmpty(): void $record = new NullValues(); - $this->assertTrue($record->save()); + $record->save(); $this->assertEquals(1, $record->id); } @@ -482,7 +482,11 @@ public function testSetProperties(): void $customer->populateProperties($properties); - $this->assertTrue($customer->save()); + $customer->save(); + + $this->assertTrue( + $this->db()->createQuery()->from('customer')->where(['email' => 'samdark@mail.ru'])->exists(), + ); } public function testSetPropertyNoExist(): void @@ -556,7 +560,7 @@ public function testInsert(): void $this->assertTrue($customer->isNewRecord()); $this->assertNull($customer->getId()); - $this->assertTrue($customer->insert()); + $customer->insert(); $this->assertFalse($customer->isNewRecord()); $this->assertSame(4, $customer->getId()); @@ -578,7 +582,7 @@ public function testInsert(): void $this->assertTrue($customer->isNewRecord()); $this->assertNull($customer->getId()); - $this->assertTrue($customer->insert(['email', 'name', 'address'])); + $customer->insert(['email', 'name', 'address']); $this->assertFalse($customer->isNewRecord()); $this->assertSame(5, $customer->getId()); @@ -594,11 +598,11 @@ public function testInsert(): void $customer->setAddress('address6'); $customer->setStatus(1); - $this->assertTrue($customer->insert([ + $customer->insert([ 'email' => 'user6@example.com', 'name' => 'user6', 'address', - ])); + ]); $this->assertFalse($customer->isNewRecord()); $this->assertSame(6, $customer->getId()); @@ -818,13 +822,6 @@ public function testJoinWithEager(): void $this->assertEquals($eagerItemsCount, $lazyItemsCount); } - public function testSaveWithoutChanges(): void - { - $customer = Customer::findByPk(1); - - $this->assertTrue($customer->save()); - } - public function testPrimaryKeyValue(): void { $customer = Customer::findByPk(1); @@ -943,7 +940,7 @@ public function testGetDirtyValuesOnNewRecord(): void $customer->newValues(['id', 'email', 'address', 'status', 'unknown']), ); - $this->assertTrue($customer->save()); + $customer->save(); $this->assertSame([], $customer->newValues()); $customer->set('address', ''); @@ -1380,7 +1377,7 @@ public function testUpsert( $customer->set($property, $value); } - $this->assertTrue($customer->upsert($insertProperties, $updateProperties)); + $customer->upsert($insertProperties, $updateProperties); $this->assertFalse($customer->isNewRecord()); diff --git a/tests/EventsTraitTest.php b/tests/EventsTraitTest.php index 79f0c0010..fa9de0dde 100644 --- a/tests/EventsTraitTest.php +++ b/tests/EventsTraitTest.php @@ -38,9 +38,8 @@ static function (object $event): void { $model = new CategoryEventsModel(); $model->name = 'Prevented Category'; - $result = $model->insert(); + $model->insert(); - $this->assertFalse($result); $this->assertNull($model->id); $this->assertSame('Prevented Category', $model->name); } @@ -62,9 +61,8 @@ static function (object $event): void { $model = new CategoryEventsModel(); $model->name = 'Custom Return Category'; - $result = $model->insert(); + $model->insert(); - $this->assertTrue($result); $this->assertNull($model->id); $this->assertSame('Custom Return Category', $model->name); } @@ -135,9 +133,8 @@ static function (object $event): void { $model = new CategoryEventsModel(); $model->name = 'Prevented Save'; - $result = $model->save(); + $model->save(); - $this->assertFalse($result); $this->assertNull($model->id); $this->assertSame('Prevented Save', $model->name); } @@ -159,9 +156,8 @@ static function (object $event): void { $model = new CategoryEventsModel(); $model->name = 'Custom Return Save'; - $result = $model->save(); + $model->save(); - $this->assertTrue($result); $this->assertNull($model->id); $this->assertSame('Custom Return Save', $model->name); } @@ -235,9 +231,8 @@ static function (object $event): void { $model = new CategoryEventsModel(); $model->name = 'Prevented Upsert'; - $result = $model->upsert(); + $model->upsert(); - $this->assertFalse($result); $this->assertNull($model->id); $this->assertSame('Prevented Upsert', $model->name); } @@ -259,9 +254,8 @@ static function (object $event): void { $model = new CategoryEventsModel(); $model->name = 'Custom Return Upsert'; - $result = $model->upsert(); + $model->upsert(); - $this->assertTrue($result); $this->assertNull($model->id); $this->assertSame('Custom Return Upsert', $model->name); } diff --git a/tests/MagicActiveRecordTest.php b/tests/MagicActiveRecordTest.php index 898aed9ad..d04b75146 100644 --- a/tests/MagicActiveRecordTest.php +++ b/tests/MagicActiveRecordTest.php @@ -238,7 +238,7 @@ public function testSaveEmpty(): void $record = new NullValues(); - $this->assertTrue($record->save()); + $record->save(); $this->assertEquals(1, $record->id); } @@ -344,7 +344,11 @@ public function testAssignProperties(): void $customer->populateProperties($properties); - $this->assertTrue($customer->save()); + $customer->save(); + + $this->assertTrue( + $this->db()->createQuery()->from('customer')->where(['email' => 'samdark@mail.ru'])->exists(), + ); } public function testSetNoExistProperty(): void @@ -630,13 +634,6 @@ public function testJoinWithEager(): void $this->assertEquals($eagerItemsCount, $lazyItemsCount); } - public function testSaveWithoutChanges(): void - { - $customer = Customer::findByPk(1); - - $this->assertTrue($customer->save()); - } - public function testPrimaryKeyValue(): void { $customer = Customer::findByPk(1); @@ -737,7 +734,7 @@ public function testGetDirtyValuesOnNewRecord(): void $customer->newValues(['id', 'email', 'address', 'status', 'unknown']), ); - $this->assertTrue($customer->save()); + $customer->save(); $this->assertSame([], $customer->newValues()); $customer->set('address', ''); diff --git a/tests/PrivatePropertiesTraitTest.php b/tests/PrivatePropertiesTraitTest.php index 3dd3b9f38..415244f51 100644 --- a/tests/PrivatePropertiesTraitTest.php +++ b/tests/PrivatePropertiesTraitTest.php @@ -46,9 +46,8 @@ public function testInsert(): void $model = new CategoryPrivatePropertiesModel(); $model->setName('Insert Test Category'); - $result = $model->insert(); + $model->insert(); - $this->assertTrue($result); $this->assertNotNull($model->getId()); $this->assertSame('Insert Test Category', $model->getName()); } From b4e801c5b423c59d0feb8f2b7c035c00e65c5382 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Fri, 24 Oct 2025 16:56:50 +0300 Subject: [PATCH 2/2] fix --- tests/Driver/Mssql/ActiveRecordTest.php | 2 +- tests/Driver/Mssql/MagicActiveRecordTest.php | 2 +- tests/Driver/Pgsql/ActiveRecordTest.php | 2 +- tests/Driver/Pgsql/MagicActiveRecordTest.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Driver/Mssql/ActiveRecordTest.php b/tests/Driver/Mssql/ActiveRecordTest.php index c53d5ba1d..acae8cfff 100644 --- a/tests/Driver/Mssql/ActiveRecordTest.php +++ b/tests/Driver/Mssql/ActiveRecordTest.php @@ -52,7 +52,7 @@ public function testSaveWithTrigger(): void $record->stringcol = 'test'; - $this->assertTrue($record->save()); + $record->save(); $this->assertEquals(1, $record->id); $testRecordQuery = TestTriggerAlert::query(); diff --git a/tests/Driver/Mssql/MagicActiveRecordTest.php b/tests/Driver/Mssql/MagicActiveRecordTest.php index c074afff4..f88a37bf5 100644 --- a/tests/Driver/Mssql/MagicActiveRecordTest.php +++ b/tests/Driver/Mssql/MagicActiveRecordTest.php @@ -46,7 +46,7 @@ public function testSaveWithTrigger(): void $record->stringcol = 'test'; - $this->assertTrue($record->save()); + $record->save(); $this->assertEquals(1, $record->id); $testRecordQuery = TestTriggerAlert::query(); diff --git a/tests/Driver/Pgsql/ActiveRecordTest.php b/tests/Driver/Pgsql/ActiveRecordTest.php index d5b0a4ed7..eecde1916 100644 --- a/tests/Driver/Pgsql/ActiveRecordTest.php +++ b/tests/Driver/Pgsql/ActiveRecordTest.php @@ -213,7 +213,7 @@ public function testBooleanDefaultValues(): void $this->assertNull($arClass->bool_col); $this->assertTrue($arClass->default_true); $this->assertFalse($arClass->default_false); - $this->assertTrue($arClass->save()); + $arClass->save(); } public function testPrimaryKeyAfterSave(): void diff --git a/tests/Driver/Pgsql/MagicActiveRecordTest.php b/tests/Driver/Pgsql/MagicActiveRecordTest.php index cb8752cb8..d5edb84cb 100644 --- a/tests/Driver/Pgsql/MagicActiveRecordTest.php +++ b/tests/Driver/Pgsql/MagicActiveRecordTest.php @@ -164,7 +164,7 @@ public function testBooleanDefaultValues(): void $this->assertNull($arClass->bool_col); $this->assertTrue($arClass->default_true); $this->assertFalse($arClass->default_false); - $this->assertTrue($arClass->save()); + $arClass->save(); } public function testPrimaryKeyAfterSave(): void