Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove $throwException from ActiveRecordInterface::relationQuery() #336

Merged
merged 1 commit into from
May 22, 2024
Merged

Remove $throwException from ActiveRecordInterface::relationQuery() #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 16 additions & 23 deletions src/AbstractActiveRecord.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@
$viaClass = null;
$viaTable = null;
$relation = $this->relationQuery($name);
$via = $relation?->getVia();
$via = $relation->getVia();

if ($via !== null) {
if ($this->getIsNewRecord() || $arClass->getIsNewRecord()) {
Expand All @@ -355,9 +355,7 @@
// unset $viaName so that it can be reloaded to reflect the change.
/** @psalm-var string $viaName */
unset($this->related[$viaName]);
}

if ($via instanceof ActiveQueryInterface) {
} else {
$viaRelation = $via;
$from = $via->getFrom();
/** @psalm-var string $viaTable */
Expand All @@ -366,8 +364,7 @@

$columns = [];

/** @psalm-var ActiveQueryInterface|null $viaRelation */
$viaLink = $viaRelation?->getLink() ?? [];
$viaLink = $viaRelation->getLink();

/**
* @psalm-var string $a
Expand All @@ -378,7 +375,7 @@
$columns[$a] = $this->$b;
}

$link = $relation?->getLink() ?? [];
$link = $relation->getLink();

/**
* @psalm-var string $a
Expand All @@ -398,7 +395,7 @@
$columns[$k] = $v;
}

if ($viaClass instanceof ActiveRecordInterface && is_array($via)) {
if ($viaClass instanceof ActiveRecordInterface) {
/**
* @psalm-var string $column
* @psalm-var mixed $value
Expand All @@ -411,7 +408,7 @@
} elseif (is_string($viaTable)) {
$this->db->createCommand()->insert($viaTable, $columns)->execute();
}
} elseif ($relation instanceof ActiveQueryInterface) {
} else {
$link = $relation->getLink();
$p1 = $arClass->isPrimaryKey(array_keys($link));
$p2 = $this->isPrimaryKey(array_values($link));
Expand All @@ -438,13 +435,13 @@
}

// update lazily loaded related objects
if ($relation instanceof ActiveRecordInterface && !$relation->getMultiple()) {
if (!$relation->getMultiple()) {
$this->related[$name] = $arClass;
} elseif (isset($this->related[$name])) {
$indexBy = $relation?->getIndexBy();
$indexBy = $relation->getIndexBy();
if ($indexBy !== null) {
if ($indexBy instanceof Closure) {
$index = $relation?->indexBy($arClass::class);
$index = $relation->$indexBy($arClass::class);

Check warning on line 444 in src/AbstractActiveRecord.php

View check run for this annotation

Codecov / codecov/patch

src/AbstractActiveRecord.php#L444 

Added line #L444 was not covered by tests
} else {
$index = $arClass->$indexBy;
}
Expand Down Expand Up @@ -549,12 +546,8 @@
return $this->retrieveRelation($name);
}

public function relationQuery(string $name, bool $throwException = true): ActiveQueryInterface|null
public function relationQuery(string $name): ActiveQueryInterface

Check warning on line 549 in src/AbstractActiveRecord.php

View check run for this annotation

Codecov / codecov/patch

src/AbstractActiveRecord.php#L549 

Added line #L549 was not covered by tests
{
if (!$throwException) {
return null;
}

throw new InvalidArgumentException(static::class . ' has no relation named "' . $name . '".');
}

Expand Down Expand Up @@ -811,7 +804,7 @@
$viaClass = null;
$viaTable = null;
$relation = $this->relationQuery($name);
$viaRelation = $relation?->getVia();
$viaRelation = $relation->getVia();

if ($viaRelation !== null) {
if (is_array($viaRelation)) {
Expand All @@ -835,7 +828,7 @@
$columns[$a] = $this->$b;
}

$link = $relation?->getLink() ?? [];
$link = $relation->getLink();

foreach ($link as $a => $b) {
/** @psalm-var mixed */
Expand Down Expand Up @@ -893,7 +886,7 @@
}
}

if ($relation instanceof ActiveQueryInterface && !$relation->getMultiple()) {
if (!$relation->getMultiple()) {
unset($this->related[$name]);
} elseif (isset($this->related[$name]) && is_array($this->related[$name])) {
/** @psalm-var array<array-key, ActiveRecordInterface> $related */
Expand Down Expand Up @@ -928,9 +921,9 @@
$viaClass = null;
$viaTable = null;
$relation = $this->relationQuery($name);
$viaRelation = $relation?->getVia();
$viaRelation = $relation->getVia();

if ($relation instanceof ActiveQueryInterface && $viaRelation !== null) {
if ($viaRelation !== null) {
if (is_array($viaRelation)) {
[$viaName, $viaRelation] = $viaRelation;
/** @psalm-var ActiveQueryInterface $viaRelation */
Expand Down Expand Up @@ -976,7 +969,7 @@
$command->update($viaTable, $nulls, $condition)->execute();
}
}
} elseif ($relation instanceof ActiveQueryInterface) {
} else {
$relatedModel = $relation->getARInstance();

$link = $relation->getLink();
Expand Down
7 changes: 3 additions & 4 deletions src/ActiveRecordInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ public function relation(string $name): self|array|null;
* Relations can be defined using {@see hasOne()} and {@see hasMany()} methods. For example:
*
* ```php
* public function relationQuery(string $name, bool $throwException = true): ActiveQueryInterface
* public function relationQuery(string $name): ActiveQueryInterface
* {
* return match ($name) {
* 'orders' => $this->hasMany(Order::class, ['customer_id' => 'id']),
Expand All @@ -337,13 +337,12 @@ public function relation(string $name): self|array|null;
* ```
*
* @param string $name The relation name, for example `orders` (case-sensitive).
* @param bool $throwException Whether to throw exception if the relation doesn't exist.
*
* @throws InvalidArgumentException
*
* @return ActiveQueryInterface|null The relational query object.
* @return ActiveQueryInterface The relational query object.
*/
public function relationQuery(string $name, bool $throwException = true): ActiveQueryInterface|null;
public function relationQuery(string $name): ActiveQueryInterface;

/**
* Resets relation data for the specified name.
Expand Down
21 changes: 1 addition & 20 deletions src/Trait/MagicRelationsTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,14 @@ trait MagicRelationsTrait
* }
* ```
*
* @param string $name The relation name, for example `orders` for a relation defined via `getOrdersQuery()` method
* (case-sensitive).
* @param bool $throwException whether to throw exception if the relation does not exist.
*
* @throws InvalidArgumentException if the named relation does not exist.
* @throws ReflectionException
*
* @return ActiveQueryInterface|null the relational query object. If the relation does not exist and
* `$throwException` is `false`, `null` will be returned.
*/
public function relationQuery(string $name, bool $throwException = true): ActiveQueryInterface|null
public function relationQuery(string $name): ActiveQueryInterface
{
$getter = 'get' . ucfirst($name) . 'Query';

if (!method_exists($this, $getter)) {
if (!$throwException) {
return null;
}

throw new InvalidArgumentException(static::class . ' has no relation named "' . $name . '".');
}

Expand All @@ -66,10 +55,6 @@ public function relationQuery(string $name, bool $throwException = true): Active
$type === null
|| !is_a('\\' . $type->getName(), ActiveQueryInterface::class, true)
) {
if (!$throwException) {
return null;
}

$typeName = $type === null ? 'mixed' : $type->getName();

throw new InvalidArgumentException(
Expand All @@ -82,10 +67,6 @@ public function relationQuery(string $name, bool $throwException = true): Active
$realName = lcfirst(substr($method->getName(), 3, -5));

if ($realName !== $name) {
if (!$throwException) {
return null;
}

throw new InvalidArgumentException(
'Relation names are case sensitive. ' . static::class
. " has a relation named \"$realName\" instead of \"$name\"."
Expand Down
8 changes: 0 additions & 8 deletions tests/ActiveQueryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2331,9 +2331,6 @@ public function testGetRelationInvalidArgumentException(): void

$query = $customer->findOne(1);

/** Without throwing exception */
$this->assertEmpty($query->relationQuery('items', false));

/** Throwing exception */
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage(
Expand All @@ -2350,9 +2347,6 @@ public function testGetRelationInvalidArgumentExceptionHasNoRelationNamed(): voi

$query = $customer->findOne(1);

/** Without throwing exception */
$this->assertEmpty($query->relationQuery('item', false));

$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage(
'Relation query method "Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\Customer::getItemQuery()" should'
Expand All @@ -2369,8 +2363,6 @@ public function testGetRelationInvalidArgumentExceptionCaseSensitive(): void

$query = $customer->findOne(1);

$this->assertEmpty($query->relationQuery('expensiveorders', false));

$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage(
'Relation names are case sensitive. Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\Customer ' .
Expand Down
Loading