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 - getRelated() on null for relationship discovery #790

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

coderkoala
Copy link
Contributor

@coderkoala coderkoala commented Feb 7, 2024

Namaskar,

This PR addresses an issue by introducing an additional instanceof check for the $relation to see if it's a valid relationship, in the EloquentEntitySet driver. If not, bail early.

$relatedModel = get_class($relation->getRelated());

I'm not sure as to why even in case of a null exception, the get_class is not bailing, but doing a check to bail seems to work.

Here's a stack trace where I was able to encounter this issue: https://flareapp.io/share/J7oxAYx5

Here's the offending model snippet, the ... denotes parts that have nothing to do with the bug:

...

use Flat3\Lodata\Attributes\LodataRelationship;
use Illuminate\Database\Eloquent\Factories\HasFactory;

class Companies extends Model
{
    use HasFactory;
    use InputScopeTrait;

    protected $table = 'companies';

    protected $fillable = [
        ...
        'domain_id',
        'organization_id',
    ];

    protected $guarded = ['id'];

    public $timestamps = true;

    /**
     * Define the relationship where each row belongs to one domain.
     *
     * @return \Illuminate\Database\Eloquent\Relations\hasOne
     */
    #[LodataRelationship]
    public function domain()
    {
        return $this->hasOne(Domains::class, 'id', 'domain_id');
    }

    /**
     * Define the relationship where each row belongs to one organization.
     *
     * @return \Illuminate\Database\Eloquent\Relations\hasOne
     */
    #[LodataRelationship]
    public function organization()
    {
        return $this->hasOne(Organizations::class, 'id', 'organization_id');
    }

    ...

    /**
     * Define the relationship where each row can have multiple payment information.
     *
     * @return \Illuminate\Database\Eloquent\Relations\hasMany
     */
    #[LodataRelationship]
    public function paymentInformation()
    {
        return $this->hasMany(PaymentInformation::class, 'owner_id', 'id')
            ->where('owner_type', 'Organization')
            ->where('domain_id', $this->domain_id)
            ->where('organization_id', $this->organization_id);
    }
}

OdataCompanies Class:

...
namespace App\Providers\Odata;

use Illuminate\Support\ServiceProvider;
use App\Models\Account\Companies;

/**
 * Lodata Service Provider
 * @package App\Providers
 */
class ODataCompanies extends ServiceProvider
{
    public function boot()
    {
        if (app()->runningInConsole() || app()->environment('testing')) {
            return;
        }

        \Lodata::discover(Companies::class);
    }
}

Essentially loaded into as a provider.

Additional information:

  • PHP 8.2.15 (built: Jan 20 2024 14:17:05) (NTS)
  • Laravel Framework 10.43.0
  • MySQL Community version 8@latest

Note it's essentially the stock Laravel 10.x sail docker image.

Addresses an issue by introducing an additional instanceof check for the `$relation` to see if it's a valid relationship. If not, bail early.
@27pchrisl 27pchrisl merged commit 53c8c3f into flat3:5.x Feb 7, 2024
16 checks passed
@27pchrisl
Copy link
Contributor

Thanks @coderkoala !

@coderkoala coderkoala deleted the patch-1 branch February 7, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants