[11.x] Alternate implementation to allow "safe" objects to be used by Eloquent models (without breaking mass assignment protection)

[x7ryan]

This is an alternative implementation of #50190 which will allow the ValidatedInput object to be passed in directly without forcing the user to first cast it to an array themselves without touching mass assignment protection.

In #50190 @jasonmccreary introduces this ability to pass in this object directly into eloquent. However in his implementation he makes the choice that because the input has been validated to disable mass assignment protection when the ValidatedInput object is passed in.

In his PR I commented that I felt that was too hidden and many users unless they dig into it are unlikely to realize that create/update/fill are implicitly changed to disable mass assignment protection based on the type of input used. As other commenters have also raised this same concern I have prepared this PR as an alternative implementation for Taylor to consider.

The first commit brings this to parity with #50190 at the current time, plus added this to the "force" methods as well. The second commit is more opinionated, adding back type hints with the union type. However, since I am unaware of whether Taylor would prefer union types or no types I left that as a separate commit which could easily be reverted.

[jasonmccreary]

To me, this PR doesn't really hold its weight. You're changing several public methods in foundational classes of the framework to save developers a few keystrokes when chaining ->all().

[x7ryan]

To me, this PR doesn't really hold its weight. You're changing several public methods in foundational classes of the framework to save developers a few keystrokes when chaining ->all().

Your own words..."It would be wonderful if I could pass this 'safe' object directly to Eloquent."

I agree, I just don't agree that the framework should implicitly change mass assignment protection based solely on the type passed in as that's too hidden from the developer. It's nothing personal.

It's just an alternative option for Taylor to consider, maybe he agrees with you, maybe he prefers neither.

[fragkp]

I feel the same as @jasonmccreary . Changing all those methods just to save ->all() is not worth it.

[jasonmccreary]

You're quoting a single line from two hours of content about avoiding the "mass assignment dance". Anyway, good on you for opening your own PR with your own ideas from my stream. You tagged me in your PR, so I've left my opinion - as you did on mine. I'll be shocked if either are merged. Especially this one. 😉

[x7ryan]

I feel the same as @jasonmccreary . Changing all those methods just to save ->all() is not worth it.

Idk I can see it go either way.

I'd argue Laravel is as popular as it is in part because of the attention to detail to make the developers life easier, even is subtle little ways. And even though this changes quite a few methods, it just around types accepted, so not a significant maintenance burden or anything, and most importantly doesn't break any current usages of these methods.

I figured it's worth considering separate from the mass assignment issue Jason was making it. But ultimately it's Taylor's call.

[SanderMuller]

Personally I find this implementation a lot cleaner. Having this functionally either way would be great.

[taylorotwell]

How did you decide which methods to update and which to not? For example, why is BelongsToMany@createOrFirst not updated?

[taylorotwell]

Please mark as ready for review once you respond.

[axlon]

This Pr seems kinda weird to me, why are we adding in support for just validatedinput if all we're doing is pull out the underlying array? I feel if this is the way we want to go it should target the arrayable interface instead. This would open up other array-like classes as well.

[donnysim]

I'd agree that neither this nor the original pull targets the correct object - ValidatedInput, and should be ValidatedData interface, but as overall statement I think @axlon has a point that without disabling the mass assignment Arrayable (or maybe directly iterable) might be a better target to support broader inputs, including the ValidatedData.

[jasonmccreary]

I think @axlon has a good point. Maybe the "best" PR for now is modifying the type hint. That's really the main breaking change. Performing specific logic based on the type may not be. Addressing the type now may allow these ideas to be revisited in a minor release of Laravel 11. As opposed to waiting another year for Laravel 12.

[x7ryan]

How did you decide which methods to update and which to not? For example, why is BelongsToMany@createOrFirst not updated?

@taylorotwell That and any other methods I did not include were simply missed, so specific reason to exclude them. That said I agree with some of the later comments about expanding the approach to work with even more types.

Would you consider adding or removing all typehints to constitute "breaking" even if it is not "technically" so? I agree the best way forward is to take some more time to more deeply consider what types make sense to accept here. If you don't want to set the precedent of allowing typehint changes in a minor release, even if technically backwards compatible, then I would close this in favor or a PR that would simply remove the typehints in preparations for future changes in 11.x minor releases.