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

Error with Laravel Eloquent Accessors #6644

Closed
toitzi opened this issue Sep 20, 2024 · 6 comments
Closed

Error with Laravel Eloquent Accessors #6644

toitzi opened this issue Sep 20, 2024 · 6 comments

Comments

@toitzi
Copy link
Contributor

toitzi commented Sep 20, 2024

API Platform version(s) affected: 4.0.1

Description

In Laravel, it's possible to define custom Accessors for your models (see Laravel documentation). These accessors can either modify an existing property or add new attributes that are not tied to database columns.

Several popular Laravel packages, such as Spaite's Laravel Translatable utilize this feature to append or manipulate attributes. However, when using such accessors on models defined as #[ApiResource], API Platform throws an error. This occurs because these custom attributes lack a "primary" key in their metadata array, and their "nullable" value is set to null instead of a boolean (true/false). The issue is exemplified in the screenshot below.

How to reproduce

In any Laravel installation, define a model as #[ApiResource] and add the following accessor:

protected function test(): Attribute
{
    return Attribute::make(
        get: fn () => 'broken',
    );
}

API Platform will then produce an error: undefined array key "primary" on this line

Possible Solution

At first glance, this might seem like a simple fix (adding an isset check on the line mentioned above). However, the error will still occur in the EloquentPropertyMetadataFactory on this line, since $p['nullable'] is null instead of a boolean. A potential fix might involve defaulting it to true ($p['nullable'] ?? true), though I'm uncertain if this is a proper solution or just a quick workaround, given that accessors created in this way might not need to be considered in the first place.

EDIT: A potential workaround is to hide these attributes by adding them to the model's hidden array. However, the error related to the missing isset check would still remain (though the second error would be resolved). In this case, only the isset check, or something similar, would need to be added. It might also be worth documenting that users may need to hide such attributes manually. Automating this process would be ideal, but I’m unsure if there's a reliable way to differentiate between "real" attributes and those that aren't tied to the database (except maybe for the absence of the primary key).

Additional Context
image

@MACscr
Copy link

MACscr commented Sep 23, 2024

Accessors are pretty standard though with laravel and need to be available to the api in addition to other methods defined on the model. Not sure how the api platform is supposed to handle that.

@toitzi
Copy link
Contributor Author

toitzi commented Sep 24, 2024

@MACscr A fix is already in the works (#6633). As far as i can tell they will be available in the API (just unit tests are missing).

@MACscr
Copy link

MACscr commented Sep 28, 2024

ah, i wish this solved it, but when i have any attributes in my model, i still get a 500 error about primary key missing. As soon as i comment out my attributes, it works fine.

@toitzi
Copy link
Contributor Author

toitzi commented Sep 28, 2024

@MACscr please also take a look at #6668 and make sure to use both patches applied. I can confirm it is working. If you continue to have issues even with both patches applied, it would be great if you could share an example of how to reproduce it.

@MACscr
Copy link

MACscr commented Sep 29, 2024

@MACscr please also take a look at #6668 and make sure to use both patches applied. I can confirm it is working. If you continue to have issues even with both patches applied, it would be great if you could share an example of how to reproduce it.

I'm running the latest 4.x, which includes both of those patches and simply doing the example attribute included in this thread causes the same error.

@toitzi
Copy link
Contributor Author

toitzi commented Sep 29, 2024

Latest 4.x does not include those patches!

@soyuka soyuka closed this as completed Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants