Skip to content

Commit 4958cfa

Browse files
TigrovTigrov
authored
Remove flags from getPrimaryKey() and getOldPrimaryKey() methods (#422)
1 parent 921bb4d commit 4958cfa

File tree

4 files changed

+210
-74
lines changed

4 files changed

+210
-74
lines changed

‎src/AbstractActiveRecord.php‎

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public function equals(ActiveRecordInterface $record): bool
106106
return false;
107107
}
108108

109-
return $this->getTableName() === $record->getTableName() && $this->getPrimaryKey() === $record->getPrimaryKey();
109+
return $this->getTableName() === $record->getTableName() && $this->getPrimaryKeys() === $record->getPrimaryKeys();
110110
}
111111

112112
public function get(string $propertyName): mixed
@@ -180,25 +180,33 @@ public function oldValues(): array
180180
return $this->oldValues ?? [];
181181
}
182182

183-
/**
184-
* @throws InvalidConfigException
185-
* @throws Exception
186-
*/
187-
public function getOldPrimaryKey(bool $asArray = false): mixed
183+
public function getOldPrimaryKey(): float|int|string|null
184+
{
185+
$keys = $this->primaryKey();
186+
187+
return match (count($keys)) {
188+
1 => $this->oldValues[$keys[0]] ?? null,
189+
0 => throw new Exception(
190+
static::class . ' does not have a primary key. You should either define a primary key for '
191+
. $this->getTableName() . ' table or override the primaryKey() method.'
192+
),
193+
default => throw new Exception(
194+
static::class . ' has multiple primary keys. Use getOldPrimaryKeys() method instead.'
195+
),
196+
};
197+
}
198+
199+
public function getOldPrimaryKeys(): array
188200
{
189201
$keys = $this->primaryKey();
190202

191203
if (empty($keys)) {
192204
throw new Exception(
193205
static::class . ' does not have a primary key. You should either define a primary key for '
194-
. 'the corresponding table or override the primaryKey() method.'
206+
. $this->getTableName() . ' table or override the primaryKey() method.'
195207
);
196208
}
197209

198-
if ($asArray === false && count($keys) === 1) {
199-
return $this->oldValues[$keys[0]] ?? null;
200-
}
201-
202210
$values = [];
203211

204212
foreach ($keys as $name) {
@@ -208,12 +216,31 @@ public function getOldPrimaryKey(bool $asArray = false): mixed
208216
return $values;
209217
}
210218

211-
public function getPrimaryKey(bool $asArray = false): mixed
219+
public function getPrimaryKey(): float|int|string|null
212220
{
213221
$keys = $this->primaryKey();
214222

215-
if ($asArray === false && count($keys) === 1) {
216-
return $this->get($keys[0]);
223+
return match (count($keys)) {
224+
1 => $this->get($keys[0]),
225+
0 => throw new Exception(
226+
static::class . ' does not have a primary key. You should either define a primary key for '
227+
. $this->getTableName() . ' table or override the primaryKey() method.'
228+
),
229+
default => throw new Exception(
230+
static::class . ' has multiple primary keys. Use getPrimaryKeys() method instead.'
231+
),
232+
};
233+
}
234+
235+
public function getPrimaryKeys(): array
236+
{
237+
$keys = $this->primaryKey();
238+
239+
if (empty($keys)) {
240+
throw new Exception(
241+
static::class . ' does not have a primary key. You should either define a primary key for '
242+
. $this->getTableName() . ' table or override the primaryKey() method.'
243+
);
217244
}
218245

219246
$values = [];
@@ -554,7 +581,7 @@ public function populateRelation(string $name, array|ActiveRecordInterface|null
554581
*/
555582
public function refresh(): bool
556583
{
557-
$record = $this->query()->findByPk($this->getPrimaryKey(true));
584+
$record = $this->query()->findByPk($this->getOldPrimaryKeys());
558585

559586
return $this->refreshInternal($record);
560587
}
@@ -758,7 +785,7 @@ public function updateAllCounters(array $counters, array|string $condition = '',
758785
*/
759786
public function updateCounters(array $counters): bool
760787
{
761-
if ($this->updateAllCounters($counters, $this->getOldPrimaryKey(true)) === 0) {
788+
if ($this->updateAllCounters($counters, $this->getOldPrimaryKeys()) === 0) {
762789
return false;
763790
}
764791

@@ -864,7 +891,7 @@ public function unlink(string $relationName, ActiveRecordInterface $arClass, boo
864891
/** @psalm-var array<array-key, ActiveRecordInterface> $related */
865892
$related = $this->related[$relationName];
866893
foreach ($related as $a => $b) {
867-
if ($arClass->getPrimaryKey() === $b->getPrimaryKey()) {
894+
if ($arClass->getPrimaryKeys() === $b->getPrimaryKeys()) {
868895
unset($this->related[$relationName][$a]);
869896
}
870897
}
@@ -1044,7 +1071,7 @@ protected function deleteInternal(): int
10441071
* We don't check the return value of deleteAll() because it is possible the record is already deleted in
10451072
* the database and thus the method will return 0
10461073
*/
1047-
$condition = $this->getOldPrimaryKey(true);
1074+
$condition = $this->getOldPrimaryKeys();
10481075

10491076
if ($this instanceof OptimisticLockInterface) {
10501077
$lock = $this->optimisticLockPropertyName();
@@ -1130,7 +1157,7 @@ protected function updateInternal(array|null $properties = null): int
11301157
return 0;
11311158
}
11321159

1133-
$condition = $this->getOldPrimaryKey(true);
1160+
$condition = $this->getOldPrimaryKeys();
11341161

11351162
if ($this instanceof OptimisticLockInterface) {
11361163
$lock = $this->optimisticLockPropertyName();

‎src/ActiveRecordInterface.php‎

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -127,47 +127,48 @@ public function propertyValues(array|null $names = null, array $except = []): ar
127127
public function getIsNewRecord(): bool;
128128

129129
/**
130-
* Returns the old primary key value(s).
130+
* Returns the old value of the primary key as a scalar.
131131
*
132-
* This refers to the primary key value that's populated into the record after executing a find method (for example,
133-
* `findOne()`).
132+
* This refers to the primary key value that is populated into the record after data is retrieved from the database
133+
* by an instance of {@see ActiveQueryInterface}.
134134
*
135-
* The value remains unchanged even if the primary key property is manually assigned with a different value.
135+
* The value remains unchanged while the record will not be {@see update() updated}.
136136
*
137-
* @param bool $asArray Whether to return the primary key value as an array. If true, the return value will be an
138-
* array with property name as key and property value as value. If this is `false` (default), a scalar value will be
139-
* returned for a non-composite primary key.
137+
* @throw Exception If multiple primary keys or no primary key.
140138
*
141-
* @return mixed The old primary key value. An array (property name => property value) is returned if the primary
142-
* key is composite or `$asArray` is true. A string is returned otherwise (`null` will be returned if the key value
143-
* is `null`).
139+
* @see getOldPrimaryKeys()
140+
*/
141+
public function getOldPrimaryKey(): float|int|string|null;
142+
143+
/**
144+
* Returns the old values of the primary key as an array with property names as keys and property values as values.
145+
*
146+
* This refers to the primary key values that is populated into the record after data is retrieved from the database
147+
* by an instance of {@see ActiveQueryInterface}.
148+
*
149+
* The value remains unchanged while the record will not be {@see update() updated}.
144150
*
145-
* @psalm-return (
146-
* $asArray is true
147-
* ? array<string, mixed>
148-
* : mixed|null
149-
* )
151+
* @throw Exception If no primary key or multiple primary keys.
152+
*
153+
* @see getOldPrimaryKeys()
150154
*/
151-
public function getOldPrimaryKey(bool $asArray = false): mixed;
155+
public function getOldPrimaryKeys(): array;
152156

153157
/**
154-
* Returns the primary key value(s).
158+
* Returns the scalar value of the primary key.
155159
*
156-
* @param bool $asArray Whether to return the primary key value as an array. If true, the return value will be an
157-
* array with property names as keys and property values as values. Note that for composite primary keys, an array
158-
* will always be returned regardless of this parameter value.
160+
* @throw Exception If multiple primary keys or no primary key.
159161
*
160-
* @return mixed The primary key value. An array (property name => property value) is returned if the primary key
161-
* is composite or `$asArray` is true. A string is returned otherwise (`null` will be returned if the key value is
162-
* `null`).
162+
* @see getPrimaryKeys()
163+
*/
164+
public function getPrimaryKey(): float|int|string|null;
165+
166+
/**
167+
* Returns the values of the primary key as an array with property names as keys and property values as values.
163168
*
164-
* @psalm-return (
165-
* $asArray is true
166-
* ? array<string, mixed>
167-
* : mixed|null
168-
* )
169+
* @see getPrimaryKey()
169170
*/
170-
public function getPrimaryKey(bool $asArray = false): mixed;
171+
public function getPrimaryKeys(): array;
171172

172173
/**
173174
* Return the name of the table associated with this AR class.

‎tests/ActiveRecordTest.php‎

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -634,34 +634,88 @@ public function testJoinWithEager(): void
634634

635635
public function testSaveWithoutChanges(): void
636636
{
637-
$this->reloadFixtureAfterTest();
638-
639-
$customerQuery = new ActiveQuery(Customer::class);
640-
641-
$customer = $customerQuery->findByPk(1);
637+
$customer = Customer::findByPk(1);
642638

643639
$this->assertTrue($customer->save());
644640
}
645641

646642
public function testGetPrimaryKey(): void
647643
{
648-
$customerQuery = new ActiveQuery(Customer::class);
649-
650-
$customer = $customerQuery->findByPk(1);
644+
$customer = Customer::findByPk(1);
651645

652646
$this->assertSame(1, $customer->getPrimaryKey());
653-
$this->assertSame(['id' => 1], $customer->getPrimaryKey(true));
647+
$this->assertSame(['id' => 1], $customer->getPrimaryKeys());
648+
649+
$orderItemQuery = new ActiveQuery(OrderItem::class);
650+
$orderItem = $orderItemQuery->findByPk([1, 2]);
651+
652+
$this->assertSame(['order_id' => 1, 'item_id' => 2], $orderItem->getPrimaryKeys());
653+
654+
$this->expectException(Exception::class);
655+
$this->expectExceptionMessage(OrderItem::class . ' has multiple primary keys. Use getPrimaryKeys() method instead.');
656+
657+
$orderItem->getPrimaryKey();
654658
}
655659

656-
public function testGetOldPrimaryKey(): void
660+
public function testGetPrimaryKeyWithoutPrimaryKey(): void
657661
{
658-
$customerQuery = new ActiveQuery(Customer::class);
662+
$orderItem = new OrderItemWithNullFK();
659663

660-
$customer = $customerQuery->findByPk(1);
664+
$this->expectException(Exception::class);
665+
$this->expectExceptionMessage(OrderItemWithNullFK::class . ' does not have a primary key.');
666+
667+
$orderItem->getPrimaryKey();
668+
}
669+
670+
public function testGetPrimaryKeysWithoutPrimaryKey(): void
671+
{
672+
$orderItem = new OrderItemWithNullFK();
673+
674+
$this->expectException(Exception::class);
675+
$this->expectExceptionMessage(OrderItemWithNullFK::class . ' does not have a primary key.');
676+
677+
$orderItem->getPrimaryKeys();
678+
}
679+
680+
public function testGetOldPrimaryKey(): void
681+
{
682+
$customer = Customer::findByPk(1);
661683
$customer->setId(2);
662684

663685
$this->assertSame(1, $customer->getOldPrimaryKey());
664-
$this->assertSame(['id' => 1], $customer->getOldPrimaryKey(true));
686+
$this->assertSame(['id' => 1], $customer->getOldPrimaryKeys());
687+
688+
$orderItemQuery = new ActiveQuery(OrderItem::class);
689+
$orderItem = $orderItemQuery->findByPk([1, 2]);
690+
$orderItem->setOrderId(3);
691+
$orderItem->setItemId(4);
692+
693+
$this->assertSame(['order_id' => 1, 'item_id' => 2], $orderItem->getOldPrimaryKeys());
694+
695+
$this->expectException(Exception::class);
696+
$this->expectExceptionMessage(OrderItem::class . ' has multiple primary keys. Use getOldPrimaryKeys() method instead.');
697+
698+
$orderItem->getOldPrimaryKey();
699+
}
700+
701+
public function testGetOldPrimaryKeyWithoutPrimaryKey(): void
702+
{
703+
$orderItem = new OrderItemWithNullFK();
704+
705+
$this->expectException(Exception::class);
706+
$this->expectExceptionMessage(OrderItemWithNullFK::class . ' does not have a primary key.');
707+
708+
$orderItem->getOldPrimaryKey();
709+
}
710+
711+
public function testGetOldPrimaryKeysWithoutPrimaryKey(): void
712+
{
713+
$orderItem = new OrderItemWithNullFK();
714+
715+
$this->expectException(Exception::class);
716+
$this->expectExceptionMessage(OrderItemWithNullFK::class . ' does not have a primary key.');
717+
718+
$orderItem->getOldPrimaryKeys();
665719
}
666720

667721
public function testGetDirtyValuesOnNewRecord(): void

0 commit comments

Comments
 (0)