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.

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

fix attribute vs relation priorty #2577

Open
wants to merge 27 commits into
base: 4.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2e46a6d
use laravel get attribute
mshamaseen Aug 18, 2023
29935f2
update docker
mshamaseen Aug 19, 2023
a091681
update docker
mshamaseen Aug 20, 2023
07de6e5
Merge branch 'master' of https://github.com/mshamaseen/laravel-mongod…
mshamaseen Aug 20, 2023
9430d08
remove mongo db < 5 support
mshamaseen Aug 20, 2023
77dc9be
replace mongod with mongosh
mshamaseen Aug 20, 2023
80527f5
replace mongod with mongosh
mshamaseen Aug 20, 2023
e4fbb8d
replace mongod with mongosh
mshamaseen Aug 20, 2023
87b6453
replace mongod with mongosh
mshamaseen Aug 20, 2023
0a73a11
detach
mshamaseen Aug 20, 2023
5385aa4
remove mongo version 7 support as it is so new and yield to some errors
mshamaseen Aug 20, 2023
19db89b
Merge branch 'master' of github.com:mshamaseen/laravel-mongodb into n…
mshamaseen Aug 20, 2023
21fac7d
BelongsToMany relation now can't use the same relation name as column…
mshamaseen Aug 21, 2023
d0666b6
update documentation
mshamaseen Aug 21, 2023
dbfa900
Merge pull request #1 from mshamaseen/next-v4
mshamaseen Aug 21, 2023
61842f5
update readme
mshamaseen Aug 21, 2023
c8a6219
Merge branch 'master' of github.com:mshamaseen/laravel-mongodb
mshamaseen Aug 21, 2023
f18bf78
change package name for it to work with packagist
mshamaseen Aug 21, 2023
c0e1ee3
change package name for it to work with packagist
mshamaseen Aug 21, 2023
7a1fb98
add packagist using new package name
mshamaseen Aug 21, 2023
61a0505
Merge branch 'master' of github.com:jenssegers/laravel-mongodb into r…
mshamaseen Aug 22, 2023
09c1a9d
Merge branch 'master' of github.com:mshamaseen/laravel-mongodb into r…
mshamaseen Aug 22, 2023
815f37e
prepare for mering with upstream
mshamaseen Aug 22, 2023
21a46e1
return FUNDING.yml
mshamaseen Aug 22, 2023
cc1631e
fix php cs fixer
mshamaseen Aug 24, 2023
ed45cf7
revision updates
mshamaseen Aug 24, 2023
e3330ae
revert extra
mshamaseen Aug 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 5 additions & 7 deletions .github/workflows/build-ci.yml
Expand Up @@ -36,16 +36,14 @@ jobs:
os:
- ubuntu-latest
mongodb:
- '4.0'
- '4.2'
- '4.4'
- '5.0'
- '6.0'
php:
- '8.1'
- '8.2'
services:
mysql:
image: mysql:5.7
image: mysql:8.0
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
ports:
- 3307:3306
env:
Expand All @@ -58,13 +56,13 @@ jobs:
- name: Create MongoDB Replica Set
run: |
docker run --name mongodb -p 27017:27017 -e MONGO_INITDB_DATABASE=unittest --detach mongo:${{ matrix.mongodb }} mongod --replSet rs --setParameter transactionLifetimeLimitSeconds=5
until docker exec --tty mongodb mongo 127.0.0.1:27017 --eval "db.runCommand({ ping: 1 })"; do
until docker exec --tty mongodb mongosh 127.0.0.1:27017 --eval "db.runCommand({ ping: 1 })"; do
sleep 1
done
sudo docker exec --tty mongodb mongo 127.0.0.1:27017 --eval "rs.initiate({\"_id\":\"rs\",\"members\":[{\"_id\":0,\"host\":\"127.0.0.1:27017\" }]})"
sudo docker exec --tty mongodb mongosh 127.0.0.1:27017 --eval "rs.initiate({\"_id\":\"rs\",\"members\":[{\"_id\":0,\"host\":\"127.0.0.1:27017\" }]})"
- name: Show MongoDB server status
run: |
docker exec --tty mongodb mongo 127.0.0.1:27017 --eval "db.runCommand({ serverStatus: 1 })"
docker exec --tty mongodb mongosh 127.0.0.1:27017 --eval "db.runCommand({ serverStatus: 1 })"
- name: "Installing php"
uses: shivammathur/setup-php@v2
with:
Expand Down
9 changes: 3 additions & 6 deletions Dockerfile
@@ -1,5 +1,5 @@
ARG PHP_VERSION=8.1
ARG COMPOSER_VERSION=2.5.4


FROM php:${PHP_VERSION}-cli

Expand All @@ -9,14 +9,11 @@ RUN apt-get update && \
pecl install xdebug && docker-php-ext-enable xdebug && \
docker-php-ext-install -j$(nproc) pdo_mysql zip

COPY --from=composer:${COMPOSER_VERSION} /usr/bin/composer /usr/local/bin/composer
# Copy the Composer binary from the specified stage
COPY --from=composer:2.5.4 /usr/bin/composer /usr/local/bin/composer

WORKDIR /code

COPY composer.* ./

RUN composer install

COPY ./ ./

RUN composer install
Expand Down
14 changes: 11 additions & 3 deletions README.md
Expand Up @@ -856,19 +856,23 @@ class User extends Model
}
}
```
**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.

### EmbedsMany Relationship

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.

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

**Breaking changes** starting from v4.0 you need to define the return type of EmbedsOne and EmbedsMany relation for it to work
Copy link
Member

Choose a reason for hiding this comment

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

What I did in this PR is, Instead of calling the methods first, check if there is a method that has a return type of EmbedsOne or EmbedsMany and return it, otherwise continue using Laravel attribute/relation workflow (the same logic Laravel team applied for Attribute accessor functions).

Very smart. Can you please provide a link to where it's done like this in laravel.

Copy link
Author

Choose a reason for hiding this comment

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


```php
use Jenssegers\Mongodb\Eloquent\Model;
use Jenssegers\Mongodb\Relations\EmbedsMany;

class User extends Model
{
public function books()
public function books(): EmbedsMany
{
return $this->embedsMany(Book::class);
}
Expand Down Expand Up @@ -937,10 +941,11 @@ Like other relations, embedsMany assumes the local key of the relationship based

```php
use Jenssegers\Mongodb\Eloquent\Model;
use Jenssegers\Mongodb\Relations\EmbedsMany;

class User extends Model
{
public function books()
public function books(): EmbedsMany
{
return $this->embedsMany(Book::class, 'local_key');
}
Expand All @@ -953,12 +958,15 @@ Embedded relations will return a Collection of embedded items instead of a query

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

**Breaking changes** starting from v4.0 you need to define the return type of EmbedsOne and EmbedsMany relation for it to work

```php
use Jenssegers\Mongodb\Eloquent\Model;
use Jenssegers\Mongodb\Relations\EmbedsOne;

class Book extends Model
{
public function author()
public function author(): EmbedsOne
{
return $this->embedsOne(Author::class);
}
Expand Down
14 changes: 10 additions & 4 deletions docker-compose.yml
@@ -1,4 +1,4 @@
version: '3'
version: '3.5'

services:
tests:
Expand All @@ -11,9 +11,10 @@ services:
- .:/code
working_dir: /code
depends_on:
- mongodb
- mysql

mongodb:
condition: service_healthy
mysql:
condition: service_started
mysql:
container_name: mysql
image: mysql:8.0
Expand All @@ -29,3 +30,8 @@ services:
image: mongo:latest
ports:
- "27017:27017"
healthcheck:
test: echo 'db.runCommand("ping").ok' | mongosh mongodb:27017 --quiet
interval: 10s
timeout: 10s
retries: 5
39 changes: 34 additions & 5 deletions src/Eloquent/Model.php
Expand Up @@ -15,15 +15,22 @@
use Illuminate\Support\Str;
use function in_array;
use Jenssegers\Mongodb\Query\Builder as QueryBuilder;
use Jenssegers\Mongodb\Relations\EmbedsMany;
use Jenssegers\Mongodb\Relations\EmbedsOne;
use MongoDB\BSON\Binary;
use MongoDB\BSON\ObjectID;
use MongoDB\BSON\UTCDateTime;
use ReflectionException;
use ReflectionMethod;
use ReflectionNamedType;
use function uniqid;

abstract class Model extends BaseModel
{
use HybridRelations, EmbedsRelations;

public static array $embeddedCache = [];

/**
* The collection associated with the model.
*
Expand Down Expand Up @@ -161,16 +168,38 @@ public function getAttribute($key)

// This checks for embedded relation support.
if (
method_exists($this, $key)
&& ! method_exists(self::class, $key)
&& ! $this->hasAttributeGetMutator($key)
$this->hasEmbeddedRelation($key)
) {
return $this->getRelationValue($key);
}

return parent::getAttribute($key);
}

/**
* Determine if an attribute is an embedded relation.
*
* @param string $key
* @return bool
* @throws ReflectionException
*/
public function hasEmbeddedRelation(string $key): bool
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
{
if (! method_exists($this, $method = Str::camel($key))) {
return false;
}

if (isset(static::$embeddedCache[get_class($this)][$key])) {
return static::$embeddedCache[get_class($this)][$key];
}

$returnType = (new ReflectionMethod($this, $method))->getReturnType();

return $returnType && static::$embeddedCache[get_class($this)][$key] =
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
$returnType instanceof ReflectionNamedType &&
$returnType->getName() === EmbedsOne::class || $returnType->getName() === EmbedsMany::class;
}

/**
* @inheritdoc
*/
Expand Down Expand Up @@ -401,7 +430,7 @@ public function getForeignKey()
/**
* Set the parent relation.
*
* @param \Illuminate\Database\Eloquent\Relations\Relation $relation
* @param Relation $relation
*/
public function setParentRelation(Relation $relation)
{
Expand All @@ -411,7 +440,7 @@ public function setParentRelation(Relation $relation)
/**
* Get the parent relation.
*
* @return \Illuminate\Database\Eloquent\Relations\Relation
* @return Relation
*/
public function getParentRelation()
{
Expand Down
7 changes: 7 additions & 0 deletions tests/ModelTest.php
Expand Up @@ -907,4 +907,11 @@ public function testEnumCast(): void
$this->assertSame(MemberStatus::Member->value, $check->getRawOriginal('member_status'));
$this->assertSame(MemberStatus::Member, $check->member_status);
}

public function testGetFooAttributeAccessor()
{
$user = new User();

$this->assertSame('normal attribute', $user->foo);
}
}
2 changes: 1 addition & 1 deletion tests/Models/Group.php
Expand Up @@ -15,6 +15,6 @@ class Group extends Eloquent

public function users(): BelongsToMany
{
return $this->belongsToMany(User::class, 'users', 'groups', 'users', '_id', '_id', 'users');
return $this->belongsToMany(User::class);
}
}
25 changes: 19 additions & 6 deletions tests/Models/User.php
Expand Up @@ -5,6 +5,7 @@
namespace Jenssegers\Mongodb\Tests\Models;

use DateTimeInterface;
use Carbon\Carbon;
use Illuminate\Auth\Authenticatable;
use Illuminate\Auth\Passwords\CanResetPassword;
use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract;
Expand All @@ -14,6 +15,8 @@
use Illuminate\Support\Str;
use Jenssegers\Mongodb\Eloquent\HybridRelations;
use Jenssegers\Mongodb\Eloquent\Model as Eloquent;
use Jenssegers\Mongodb\Relations\EmbedsMany;
use Jenssegers\Mongodb\Relations\EmbedsOne;

/**
* Class User.
Expand All @@ -23,9 +26,9 @@
* @property string $email
* @property string $title
* @property int $age
* @property \Carbon\Carbon $birthday
* @property \Carbon\Carbon $created_at
* @property \Carbon\Carbon $updated_at
* @property Carbon $birthday
* @property Carbon $created_at
* @property Carbon $updated_at
* @property string $username
* @property MemberStatus member_status
*/
Expand Down Expand Up @@ -76,20 +79,20 @@ public function clients()

public function groups()
{
return $this->belongsToMany(Group::class, 'groups', 'users', 'groups', '_id', '_id', 'groups');
return $this->belongsToMany(Group::class);
}

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

public function addresses()
public function addresses(): EmbedsMany
{
return $this->embedsMany(Address::class);
}

public function father()
public function father(): EmbedsOne
{
return $this->embedsOne(self::class);
}
Expand All @@ -106,4 +109,14 @@ protected function username(): Attribute
set: fn ($value) => Str::slug($value)
);
}

public function getFooAttribute()
{
return 'normal attribute';
}

public function foo()
{
return 'normal function';
}
}
4 changes: 2 additions & 2 deletions tests/RelationsTest.php
Expand Up @@ -344,8 +344,8 @@ public function testBelongsToManyCustom(): void
$group = Group::find($group->_id);

// Check for custom relation attributes
$this->assertArrayHasKey('users', $group->getAttributes());
$this->assertArrayHasKey('groups', $user->getAttributes());
$this->assertArrayHasKey('user_ids', $group->getAttributes());
$this->assertArrayHasKey('group_ids', $user->getAttributes());

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