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

Hybrid support for MorphToMany relationship #2690

Merged
merged 20 commits into from
Dec 11, 2023
Merged

Hybrid support for MorphToMany relationship #2690

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ebc193f
Add hybrid support to MorphToMany;
hans-thomas Nov 28, 2023
7983a59
Add test for hybrid MorphToMany;
hans-thomas Nov 28, 2023
95f35d7
Add support for hybrid MorphedByMany;
hans-thomas Nov 28, 2023
f3d8cd3
Add test for hybrid MorphedByMany;
hans-thomas Nov 28, 2023
0309791
apply phpcbf formatting
hans-thomas Nov 28, 2023
3432981
WIP
hans-thomas Nov 28, 2023
33716b5
Add hybrid MorphToMany support;
hans-thomas Nov 28, 2023
77f56b7
Add test for hybrid MorphToMany;
hans-thomas Nov 28, 2023
2c57ebe
Merge branch 'add-hybrid-support-to-morph-to-many' of github.com:hans…
hans-thomas Nov 28, 2023
065d14a
WIP
hans-thomas Nov 28, 2023
aad84cc
apply phpcbf formatting
hans-thomas Nov 28, 2023
108ac9b
Update instance in attaching a model in MorphToMany;
hans-thomas Nov 29, 2023
f275560
Update instance in attaching a model in Inverse MorphToMany;
hans-thomas Nov 29, 2023
cb5778e
Create addIdToParentRelationData method to prevent repeating code;
hans-thomas Nov 29, 2023
8ed3104
apply phpcbf formatting
hans-thomas Nov 29, 2023
b5137a0
Update phpstan-baseline.neon
hans-thomas Nov 29, 2023
94688d4
WIP
hans-thomas Nov 29, 2023
e561201
Fix CS;
hans-thomas Nov 29, 2023
48fdeaa
Merge branch '4.1' into add-hybrid-support-to-morph-to-many
hans-thomas Dec 11, 2023
8440c57
Fix tests;
hans-thomas Dec 11, 2023
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
2 changes: 1 addition & 1 deletion phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ parameters:

-
message: "#^Method Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:push\\(\\) invoked with 3 parameters, 0 required\\.$#"
count: 6
count: 2
path: src/Relations/MorphToMany.php

-
Expand Down
16 changes: 10 additions & 6 deletions src/Eloquent/HybridRelations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,12 +432,16 @@ public function morphedByMany(
$relatedKey = null,
$relation = null,
) {
$foreignPivotKey = $foreignPivotKey ?: Str::plural($this->getForeignKey());

// For the inverse of the polymorphic many-to-many relations, we will change
// the way we determine the foreign and other keys, as it is the opposite
// of the morph-to-many method since we're figuring out these inverses.
$relatedPivotKey = $relatedPivotKey ?: $name . '_id';
// If the related model is an instance of eloquent model class, leave pivot keys
// as default. It's necessary for supporting hybrid relationship
if (is_subclass_of($related, Model::class)) {
// For the inverse of the polymorphic many-to-many relations, we will change
// the way we determine the foreign and other keys, as it is the opposite
// of the morph-to-many method since we're figuring out these inverses.
$foreignPivotKey = $foreignPivotKey ?: Str::plural($this->getForeignKey());

$relatedPivotKey = $relatedPivotKey ?: $name . '_id';
}

return $this->morphToMany(
$related,
Expand Down
122 changes: 99 additions & 23 deletions src/Relations/MorphToMany.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\MorphToMany as EloquentMorphToMany;
use Illuminate\Support\Arr;
use MongoDB\BSON\ObjectId;

use function array_diff;
use function array_key_exists;
Expand All @@ -17,7 +18,9 @@
use function array_merge;
use function array_reduce;
use function array_values;
use function collect;
use function count;
use function in_array;
use function is_array;
use function is_numeric;

Expand Down Expand Up @@ -74,11 +77,20 @@ public function addEagerConstraints(array $models)
protected function setWhere()
{
if ($this->getInverse()) {
$ids = $this->extractIds((array) $this->parent->{$this->table});
if ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
$ids = $this->extractIds((array) $this->parent->{$this->table});

$this->query->whereIn($this->relatedKey, $ids);
$this->query->whereIn($this->relatedKey, $ids);
} else {
$this->query
->whereIn($this->foreignPivotKey, (array) $this->parent->{$this->parentKey});
}
} else {
$this->query->whereIn($this->relatedKey, (array) $this->parent->{$this->relatedPivotKey});
match ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
true => $this->query->whereIn($this->relatedKey, (array) $this->parent->{$this->relatedPivotKey}),
false => $this->query
->whereIn($this->getQualifiedForeignPivotKeyName(), (array) $this->parent->{$this->parentKey}),
};
}

return $this;
Expand Down Expand Up @@ -128,9 +140,25 @@ public function sync($ids, $detaching = true)
// in this joining table. We'll spin through the given IDs, checking to see
// if they exist in the array of current ones, and if not we will insert.
if ($this->getInverse()) {
$current = $this->extractIds($this->parent->{$this->table} ?: []);
$current = match ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
true => $this->parent->{$this->table} ?: [],
false => $this->parent->{$this->relationName} ?: [],
};

if ($current instanceof Collection) {
$current = collect($this->parseIds($current))->flatten()->toArray();
} else {
$current = $this->extractIds($current);
}
} else {
$current = $this->parent->{$this->relatedPivotKey} ?: [];
$current = match ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
true => $this->parent->{$this->relatedPivotKey} ?: [],
false => $this->parent->{$this->relationName} ?: [],
};

if ($current instanceof Collection) {
$current = $this->parseIds($current);
}
}

$records = $this->formatRecordsList($ids);
Expand Down Expand Up @@ -185,15 +213,19 @@ public function attach($id, array $attributes = [], $touch = true)

if ($this->getInverse()) {
// Attach the new ids to the parent model.
$this->parent->push($this->table, [
[
$this->relatedPivotKey => $model->{$this->relatedKey},
$this->morphType => $model->getMorphClass(),
],
], true);
if ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
$this->parent->push($this->table, [
[
$this->relatedPivotKey => $model->{$this->relatedKey},
$this->morphType => $model->getMorphClass(),
],
], true);
} else {
$this->addIdToParentRelationData($id);
}

// Attach the new parent id to the related model.
$model->push($this->foreignPivotKey, $this->parseIds($this->parent), true);
$model->push($this->foreignPivotKey, (array) $this->parent->{$this->parentKey}, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(array) cast should not be used. If the value is an ObjectId, the result is not the one you expect.

var_dump((array) new MongoDB\BSON\ObjectId());
/*
array(1) {
  'oid' =>
  string(24) "6577071f3fded0903a01f030"
}
*/

Here we know it's not a list of ids, so a simple array wrapping is fine.

Suggested change
$model->push($this->foreignPivotKey, (array) $this->parent->{$this->parentKey}, true);
$model->push($this->foreignPivotKey, [$this->parent->{$this->parentKey}], true);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will double-check it later. thanks for the review❤️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ObjectId fields are always string. I tested this with a primary key and a custom key and a custom key was cast to ObjectId.
In all cases, the $this->parent->{$this->parentKey} value is string. Your suggestion is more readable, but I don't see any bug here to open a PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's something we'll have to fix if we manage to store references as ObjectId. But there are many other hurdles to overcome, and I think this notion of "all keys are strings" is too hard-wired into Eloquent anyway.

} else {
// Attach the new parent id to the related model.
$model->push($this->table, [
Expand All @@ -204,7 +236,11 @@ public function attach($id, array $attributes = [], $touch = true)
], true);

// Attach the new ids to the parent model.
$this->parent->push($this->relatedPivotKey, (array) $id, true);
if ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
$this->parent->push($this->relatedPivotKey, (array) $id, true);
} else {
$this->addIdToParentRelationData($id);
}
}
} else {
if ($id instanceof Collection) {
Expand All @@ -221,13 +257,19 @@ public function attach($id, array $attributes = [], $touch = true)
$query->push($this->foreignPivotKey, $this->parent->{$this->parentKey});

// Attach the new ids to the parent model.
foreach ($id as $item) {
$this->parent->push($this->table, [
[
$this->relatedPivotKey => $item,
$this->morphType => $this->related instanceof Model ? $this->related->getMorphClass() : null,
],
], true);
if ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
foreach ($id as $item) {
$this->parent->push($this->table, [
[
$this->relatedPivotKey => $item,
$this->morphType => $this->related instanceof Model ? $this->related->getMorphClass() : null,
],
], true);
}
} else {
foreach ($id as $item) {
$this->addIdToParentRelationData($item);
}
}
} else {
// Attach the new parent id to the related model.
Expand All @@ -239,7 +281,13 @@ public function attach($id, array $attributes = [], $touch = true)
], true);

// Attach the new ids to the parent model.
$this->parent->push($this->relatedPivotKey, $id, true);
if ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
$this->parent->push($this->relatedPivotKey, $id, true);
} else {
foreach ($id as $item) {
$this->addIdToParentRelationData($item);
}
}
}
}

Expand Down Expand Up @@ -276,7 +324,13 @@ public function detach($ids = [], $touch = true)
];
}

$this->parent->pull($this->table, $data);
if ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
$this->parent->pull($this->table, $data);
} else {
$value = $this->parent->{$this->relationName}
->filter(fn ($rel) => ! in_array($rel->{$this->relatedKey}, $this->extractIds($data)));
$this->parent->setRelation($this->relationName, $value);
}

// Prepare the query to select all related objects.
if (count($ids) > 0) {
Expand All @@ -287,7 +341,13 @@ public function detach($ids = [], $touch = true)
$query->pull($this->foreignPivotKey, $this->parent->{$this->parentKey});
} else {
// Remove the relation from the parent.
$this->parent->pull($this->relatedPivotKey, $ids);
if ($this->parent instanceof \MongoDB\Laravel\Eloquent\Model) {
$this->parent->pull($this->relatedPivotKey, $ids);
} else {
$value = $this->parent->{$this->relationName}
->filter(fn ($rel) => ! in_array($rel->{$this->relatedKey}, $ids));
$this->parent->setRelation($this->relationName, $value);
}

// Prepare the query to select all related objects.
if (count($ids) > 0) {
Expand Down Expand Up @@ -390,4 +450,20 @@ public function extractIds(array $data, ?string $relatedPivotKey = null)
return $carry;
}, []);
}

/**
* Add the given id to the relation's data of the current parent instance.
* It helps to keep up-to-date the sql model instances in hybrid relationships.
*
* @param ObjectId|string|int $id
*
* @return void
*/
private function addIdToParentRelationData($id)
{
$instance = new $this->related();
$instance->forceFill([$this->relatedKey => $id]);
$relationData = $this->parent->{$this->relationName}->push($instance)->unique($this->relatedKey);
$this->parent->setRelation($this->relationName, $relationData);
}
}
106 changes: 106 additions & 0 deletions tests/HybridRelationsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
use Illuminate\Database\SQLiteConnection;
use Illuminate\Support\Facades\DB;
use MongoDB\Laravel\Tests\Models\Book;
use MongoDB\Laravel\Tests\Models\Experience;
use MongoDB\Laravel\Tests\Models\Role;
use MongoDB\Laravel\Tests\Models\Skill;
use MongoDB\Laravel\Tests\Models\SqlBook;
use MongoDB\Laravel\Tests\Models\SqlRole;
use MongoDB\Laravel\Tests\Models\SqlUser;
Expand Down Expand Up @@ -36,6 +38,8 @@ public function tearDown(): void
SqlUser::truncate();
SqlBook::truncate();
SqlRole::truncate();
Skill::truncate();
Experience::truncate();
}

public function testSqlRelations()
Expand Down Expand Up @@ -210,4 +214,106 @@ public function testHybridWith()
$this->assertEquals($user->id, $user->books->count());
});
}

public function testHybridMorphToManySqlModelToMongoModel()
{
// SqlModel -> MorphToMany -> MongoModel
$user = new SqlUser();
$user2 = new SqlUser();
$this->assertInstanceOf(SqlUser::class, $user);
$this->assertInstanceOf(SQLiteConnection::class, $user->getConnection());
$this->assertInstanceOf(SqlUser::class, $user2);
$this->assertInstanceOf(SQLiteConnection::class, $user2->getConnection());

// Create Mysql Users
$user->fill(['name' => 'John Doe'])->save();
$user = SqlUser::query()->find($user->id);

$user2->fill(['name' => 'Maria Doe'])->save();
$user2 = SqlUser::query()->find($user2->id);

// Create Mongodb skills
$skill = Skill::query()->create(['name' => 'Laravel']);
$skill2 = Skill::query()->create(['name' => 'MongoDB']);

// MorphToMany (pivot is empty)
$user->skills()->sync([$skill->_id, $skill2->_id]);
$check = SqlUser::query()->find($user->id);
$this->assertEquals(2, $check->skills->count());

// MorphToMany (pivot is not empty)
$user->skills()->sync($skill);
$check = SqlUser::query()->find($user->id);
$this->assertEquals(1, $check->skills->count());

// Attach MorphToMany
$user->skills()->sync([]);
$check = SqlUser::query()->find($user->id);
$this->assertEquals(0, $check->skills->count());
$user->skills()->attach($skill);
$user->skills()->attach($skill); // ignore duplicates
$check = SqlUser::query()->find($user->id);
$this->assertEquals(1, $check->skills->count());

// Inverse MorphToMany (pivot is empty)
$skill->sqlUsers()->sync([$user->id, $user2->id]);
$check = Skill::query()->find($skill->_id);
$this->assertEquals(2, $check->sqlUsers->count());

// Inverse MorphToMany (pivot is empty)
$skill->sqlUsers()->sync([$user->id, $user2->id]);
$check = Skill::query()->find($skill->_id);
$this->assertEquals(2, $check->sqlUsers->count());
}

public function testHybridMorphToManyMongoModelToSqlModel()
{
// MongoModel -> MorphToMany -> SqlModel
$user = new SqlUser();
$user2 = new SqlUser();
$this->assertInstanceOf(SqlUser::class, $user);
$this->assertInstanceOf(SQLiteConnection::class, $user->getConnection());
$this->assertInstanceOf(SqlUser::class, $user2);
$this->assertInstanceOf(SQLiteConnection::class, $user2->getConnection());

// Create Mysql Users
$user->fill(['name' => 'John Doe'])->save();
$user = SqlUser::query()->find($user->id);

$user2->fill(['name' => 'Maria Doe'])->save();
$user2 = SqlUser::query()->find($user2->id);

// Create Mongodb experiences
$experience = Experience::query()->create(['title' => 'DB expert']);
$experience2 = Experience::query()->create(['title' => 'MongoDB']);

// MorphToMany (pivot is empty)
$experience->sqlUsers()->sync([$user->id, $user2->id]);
$check = Experience::query()->find($experience->_id);
$this->assertEquals(2, $check->sqlUsers->count());

// MorphToMany (pivot is not empty)
$experience->sqlUsers()->sync([$user->id]);
$check = Experience::query()->find($experience->_id);
$this->assertEquals(1, $check->sqlUsers->count());

// Inverse MorphToMany (pivot is empty)
$user->experiences()->sync([$experience->_id, $experience2->_id]);
$check = SqlUser::query()->find($user->id);
$this->assertEquals(2, $check->experiences->count());

// Inverse MorphToMany (pivot is not empty)
$user->experiences()->sync([$experience->_id]);
$check = SqlUser::query()->find($user->id);
$this->assertEquals(1, $check->experiences->count());

// Inverse MorphToMany (pivot is not empty)
$user->experiences()->sync([]);
$check = SqlUser::query()->find($user->id);
$this->assertEquals(0, $check->experiences->count());
$user->experiences()->attach($experience);
$user->experiences()->attach($experience); // ignore duplicates
$check = SqlUser::query()->find($user->id);
$this->assertEquals(1, $check->experiences->count());
}
}
6 changes: 6 additions & 0 deletions tests/Models/Experience.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace MongoDB\Laravel\Tests\Models;

use Illuminate\Database\Eloquent\Relations\MorphToMany;
use MongoDB\Laravel\Eloquent\Model as Eloquent;

class Experience extends Eloquent
Expand All @@ -23,4 +24,9 @@ public function skillsWithCustomParentKey()
{
return $this->belongsToMany(Skill::class, parentKey: 'cexperience_id');
}

public function sqlUsers(): MorphToMany
{
return $this->morphToMany(SqlUser::class, 'experienced');
}
}
6 changes: 6 additions & 0 deletions tests/Models/Skill.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@

namespace MongoDB\Laravel\Tests\Models;

use Illuminate\Database\Eloquent\Relations\MorphToMany;
use MongoDB\Laravel\Eloquent\Model as Eloquent;

class Skill extends Eloquent
{
protected $connection = 'mongodb';
protected $collection = 'skills';
protected static $unguarded = true;

public function sqlUsers(): MorphToMany
{
return $this->morphedByMany(SqlUser::class, 'skilled');
}
}
Loading