Skip to content

Commit 68afe15

Browse files
mshamaseenGromNaN
authored andcommitted
Fix priority of embeds vs attributes
1 parent 9d36d17 commit 68afe15

File tree

8 files changed

+79
-19
lines changed

8 files changed

+79
-19
lines changed

Diff for: README.md

+11-3
Original file line numberDiff line numberDiff line change
@@ -805,19 +805,23 @@ class User extends Model
805805
}
806806
}
807807
```
808+
**Warning:** naming the foreign key same as the relation name will prevent the relation for being called on dynamic property, i.e. in the example above if you replaced `group_ids` with `groups` calling `$user->groups` will return the column instead of the relation.
808809

809810
### EmbedsMany Relationship
810811

811812
If you want to embed models, rather than referencing them, you can use the `embedsMany` relation. This relation is similar to the `hasMany` relation but embeds the models inside the parent object.
812813

813814
**REMEMBER**: These relations return Eloquent collections, they don't return query builder objects!
814815

816+
**Breaking changes** starting from v4.0 you need to define the return type of EmbedsOne and EmbedsMany relation for it to work
817+
815818
```php
816819
use MongoDB\Laravel\Eloquent\Model;
820+
use MongoDB\Laravel\Relations\EmbedsMany;
817821

818822
class User extends Model
819823
{
820-
public function books()
824+
public function books(): EmbedsMany
821825
{
822826
return $this->embedsMany(Book::class);
823827
}
@@ -886,10 +890,11 @@ Like other relations, embedsMany assumes the local key of the relationship based
886890

887891
```php
888892
use MongoDB\Laravel\Eloquent\Model;
893+
use MongoDB\Laravel\Relations\EmbedsMany;
889894

890895
class User extends Model
891896
{
892-
public function books()
897+
public function books(): EmbedsMany
893898
{
894899
return $this->embedsMany(Book::class, 'local_key');
895900
}
@@ -902,12 +907,15 @@ Embedded relations will return a Collection of embedded items instead of a query
902907

903908
The embedsOne relation is similar to the embedsMany relation, but only embeds a single model.
904909

910+
**Breaking changes** starting from v4.0 you need to define the return type of EmbedsOne and EmbedsMany relation for it to work
911+
905912
```php
906913
use MongoDB\Laravel\Eloquent\Model;
914+
use MongoDB\Laravel\Relations\EmbedsOne;
907915

908916
class Book extends Model
909917
{
910-
public function author()
918+
public function author(): EmbedsOne
911919
{
912920
return $this->embedsOne(Author::class);
913921
}

Diff for: src/Eloquent/EmbedsRelations.php

+30
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@
55
use Illuminate\Support\Str;
66
use MongoDB\Laravel\Relations\EmbedsMany;
77
use MongoDB\Laravel\Relations\EmbedsOne;
8+
use ReflectionMethod;
9+
use ReflectionNamedType;
810

911
/**
1012
* Embeds relations for MongoDB models.
1113
*/
1214
trait EmbedsRelations
1315
{
16+
/**
17+
* @var array<class-string, array<string, bool>>
18+
*/
19+
private static array $hasEmbeddedRelation = [];
20+
1421
/**
1522
* Define an embedded one-to-many relationship.
1623
*
@@ -76,4 +83,27 @@ protected function embedsOne($related, $localKey = null, $foreignKey = null, $re
7683

7784
return new EmbedsOne($query, $this, $instance, $localKey, $foreignKey, $relation);
7885
}
86+
87+
/**
88+
* Determine if an attribute is an embedded relation.
89+
*
90+
* @param string $key
91+
* @return bool
92+
* @throws \ReflectionException
93+
*/
94+
private function hasEmbeddedRelation(string $key): bool
95+
{
96+
if (! method_exists($this, $method = Str::camel($key))) {
97+
return false;
98+
}
99+
100+
if (isset(self::$hasEmbeddedRelation[static::class][$key])) {
101+
return self::$hasEmbeddedRelation[static::class][$key];
102+
}
103+
104+
$returnType = (new ReflectionMethod($this, $method))->getReturnType();
105+
106+
return self::$hasEmbeddedRelation[static::class][$key] = $returnType instanceof ReflectionNamedType
107+
&& in_array($returnType->getName(), [EmbedsOne::class, EmbedsMany::class], true);
108+
}
79109
}

Diff for: src/Eloquent/HybridRelations.php

+4
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,10 @@ public function belongsToMany(
272272

273273
$instance = new $related;
274274

275+
if ($relatedPivotKey === $relation) {
276+
throw new \LogicException(sprintf('In %s::%s(), the key cannot be identical to the relation name "%s". The default key is "%s".', static::class, $relation, $relation, $instance->getForeignKey().'s'));
277+
}
278+
275279
$relatedPivotKey = $relatedPivotKey ?: $instance->getForeignKey().'s';
276280

277281
// If no table name was provided, we can guess it by concatenating the two

Diff for: src/Eloquent/Model.php

+5-7
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use MongoDB\BSON\ObjectID;
1919
use MongoDB\BSON\UTCDateTime;
2020
use MongoDB\Laravel\Query\Builder as QueryBuilder;
21+
use ReflectionException;
2122
use function uniqid;
2223

2324
abstract class Model extends BaseModel
@@ -154,6 +155,7 @@ public function getTable()
154155

155156
/**
156157
* @inheritdoc
158+
* @throws ReflectionException
157159
*/
158160
public function getAttribute($key)
159161
{
@@ -172,11 +174,7 @@ public function getAttribute($key)
172174
}
173175

174176
// This checks for embedded relation support.
175-
if (
176-
method_exists($this, $key)
177-
&& ! method_exists(self::class, $key)
178-
&& ! $this->hasAttributeGetMutator($key)
179-
) {
177+
if ($this->hasEmbeddedRelation($key)) {
180178
return $this->getRelationValue($key);
181179
}
182180

@@ -474,7 +472,7 @@ public function getForeignKey()
474472
/**
475473
* Set the parent relation.
476474
*
477-
* @param \Illuminate\Database\Eloquent\Relations\Relation $relation
475+
* @param Relation $relation
478476
*/
479477
public function setParentRelation(Relation $relation)
480478
{
@@ -484,7 +482,7 @@ public function setParentRelation(Relation $relation)
484482
/**
485483
* Get the parent relation.
486484
*
487-
* @return \Illuminate\Database\Eloquent\Relations\Relation
485+
* @return Relation
488486
*/
489487
public function getParentRelation()
490488
{

Diff for: tests/ModelTest.php

+7
Original file line numberDiff line numberDiff line change
@@ -1035,4 +1035,11 @@ public function testEnumCast(): void
10351035
$this->assertSame(MemberStatus::Member->value, $check->getRawOriginal('member_status'));
10361036
$this->assertSame(MemberStatus::Member, $check->member_status);
10371037
}
1038+
1039+
public function testGetFooAttributeAccessor()
1040+
{
1041+
$user = new User();
1042+
1043+
$this->assertSame('normal attribute', $user->foo);
1044+
}
10381045
}

Diff for: tests/Models/Group.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ class Group extends Eloquent
1515

1616
public function users(): BelongsToMany
1717
{
18-
return $this->belongsToMany(User::class, 'users', 'groups', 'users', '_id', '_id', 'users');
18+
return $this->belongsToMany(User::class, 'users', 'groupIds', 'userIds', '_id', '_id', 'users');
1919
}
2020
}

Diff for: tests/Models/User.php

+19-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace MongoDB\Laravel\Tests\Models;
66

7+
use Carbon\Carbon;
78
use DateTimeInterface;
89
use Illuminate\Auth\Authenticatable;
910
use Illuminate\Auth\Passwords\CanResetPassword;
@@ -14,6 +15,8 @@
1415
use Illuminate\Support\Str;
1516
use MongoDB\Laravel\Eloquent\HybridRelations;
1617
use MongoDB\Laravel\Eloquent\Model as Eloquent;
18+
use MongoDB\Laravel\Relations\EmbedsMany;
19+
use MongoDB\Laravel\Relations\EmbedsOne;
1720

1821
/**
1922
* Class User.
@@ -23,9 +26,9 @@
2326
* @property string $email
2427
* @property string $title
2528
* @property int $age
26-
* @property \Carbon\Carbon $birthday
27-
* @property \Carbon\Carbon $created_at
28-
* @property \Carbon\Carbon $updated_at
29+
* @property Carbon $birthday
30+
* @property Carbon $created_at
31+
* @property Carbon $updated_at
2932
* @property string $username
3033
* @property MemberStatus member_status
3134
*/
@@ -76,20 +79,20 @@ public function clients()
7679

7780
public function groups()
7881
{
79-
return $this->belongsToMany(Group::class, 'groups', 'users', 'groups', '_id', '_id', 'groups');
82+
return $this->belongsToMany(Group::class, 'groups', 'userIds', 'groupIds', '_id', '_id', 'groups');
8083
}
8184

8285
public function photos()
8386
{
8487
return $this->morphMany(Photo::class, 'has_image');
8588
}
8689

87-
public function addresses()
90+
public function addresses(): EmbedsMany
8891
{
8992
return $this->embedsMany(Address::class);
9093
}
9194

92-
public function father()
95+
public function father(): EmbedsOne
9396
{
9497
return $this->embedsOne(self::class);
9598
}
@@ -106,4 +109,14 @@ protected function username(): Attribute
106109
set: fn ($value) => Str::slug($value)
107110
);
108111
}
112+
113+
public function getFooAttribute()
114+
{
115+
return 'normal attribute';
116+
}
117+
118+
public function foo()
119+
{
120+
return 'normal function';
121+
}
109122
}

Diff for: tests/RelationsTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,8 @@ public function testBelongsToManyCustom(): void
344344
$group = Group::find($group->_id);
345345

346346
// Check for custom relation attributes
347-
$this->assertArrayHasKey('users', $group->getAttributes());
348-
$this->assertArrayHasKey('groups', $user->getAttributes());
347+
$this->assertArrayHasKey('userIds', $group->getAttributes());
348+
$this->assertArrayHasKey('groupIds', $user->getAttributes());
349349

350350
// Assert they are attached
351351
$this->assertContains($group->_id, $user->groups->pluck('_id')->toArray());

0 commit comments

Comments
 (0)