[10.x] ClientFactory breaks when using UUIDs and quiet creation

[axlon]

• Passport Version: 10.1.3
• Laravel Version: 8.63.0
• PHP Version: 8.0.10
• Database Driver & Version: n/a

Description:

Laravel 8.59 introduced the ability to create models quietly (without events) using model factories. Because Laravel Passport currently uses a model event listener to set a UUID on the Client model, Passport's model factory will fail to create client models when using both quiet creation and the UUIDs option.

Example error:

Integrity constraint violation: 19 NOT NULL constraint failed: oauth_clients.id (SQL: insert into "oauth_clients" ("user_id", "name", "secret", "redirect", "personal_access_client", "password_client", "revoked", "updated_at", "created_at") values (?, Broekenmagazijn, $2y$10$SJO8Kazjw5qFI6gGkST81O1AJT7hsqBMS71CqdA/INkFDdHXkO1Je, http://www.dewit.com/perspiciatis-accusantium-voluptas-esse-aut-debitis-officia-velit, 0, 0, 0, 2021-10-15 07:41:31, 2021-10-15 07:41:31)

Steps To Reproduce:

• Clone this demo repository
• Run Composer install and create an .env file
• Run PHPUnit (its configured to use in-memory database), you should see the error immediately

Possible fixes:

One way to address this would be to move the UUID check out of the model event listener and into the ClientFactory and ClientCommand. This would make Passport's client model behave like any other model that has a non-incremental primary key, in that you would always have to explicitly define it before inserting the model into the database. I propose making this change on master as people might be relying on current behaviour in their applications.

Another solution could be to build a fallback into the model factory, by leaving the model event listener as it is and also adding the check to the model factory. This would have the added benefit of being a non-breaking change (as far as I can tell), but the downside of having the same check in two places.

I'm able to make a PR for either change (or another fix) if any are deemed necessary/acceptable.

[driesvints]

Hmm, yes I see what you mean. This indeed doesn't plays nicely with each other. Maybe we can do both? First send in your current non breaking change to 10.x so it's fixed on the current stable release and then afterwards when we get that into master send in a PR to remove the event behavior from the model. I'm not sure what the exact changes would be so if there's many there's a chance that Taylor might not accept them. But I think it's worth exploring this. Thanks!

[driesvints]

I'm going to close this because I technically don't consider this a bug but rather a limitation. Still want to thank you for reporting it and welcoming your PR if you want to send that in.

[axlon]

@driesvints agree its not technically a bug, as far as making a PR goes do you have a preference between the fixes I listed?

Edit: nevermind I overlooked your first comment, will be sending PRs shortly