[11.x] Introduce freezing Model

[cosmastech]

In this PR, we introduce the concept of a frozen model.

A frozen model is disallowed to do the following:

• Fill attributes
• Set attributes
• Load new relations
• Update
• Touch
• Save, SaveQuietly
• Delete
• Increment, incrementQuietly
• decrement, decrementQuietly
• pre-emptively freeze loaded relations
• disallow loading on Eloquent\Collection
• disallow "fresh" on Eloquent\Collection
• other Eloquent\Collection concerns?

Basically, the model becomes an inert object which disallows fetching relations and modifying the underlying table.

Why?

The goal here is to allow passing models as "dumb objects." One of the biggest pains I have experienced is models get passed to resources and or actions/services and end up performing side effects. These side effects can cause things like N+1 problems, plus hard to follow/maintain logic. The lazy-loading violation kind of helps us here, but it has the weird gotcha that it can still load relations if it was loaded singly.

A frozen model communicates intent: you can look, but you don't touch.

Alternatives approaches as of today

Map the model to a data object and pass that around instead. I think that's honestly probably the best, since they are inert. But mapping from a model to a DTO can bring its own headaches.

Alternate approaches to this PR

Create a FrozenModel class

This would quite possibly be better, as it could leverage strong type-hints, but would only be able to indicate the model is frozen via a template. Something like:

/**
 * @template TModel of Model
 */
class FrozenModel
{
    /** @var TModel $model */
    protected Model $model;

    public function __construct(Model $model)
    {
        $this->model = $model;
    }

    public function __call($method, $parameters)
    {
        if (in_array($method, ['save', 'fill', 'update', 'touch', 'loadMissing', /* ... */])) {
            throw new FrozenModelException("Cannot call [$method] on frozen model[$model->getKey()]");
        }
        return $this->model->__call($method, $parameters);
    }
}

and give the model class a simple method.

class Model // ...
{
    /** all the use stuff */

   /**
    * @return FrozenModel<$this>
    */
   public function freeze(): FrozenModel
   {
       return new FrozenModel($this);
   }
}

As I'm writing the description, I think I kind of prefer this approach, but figured I'd float out an idea and get maintainer/community feedback.

I do realize while this is a draft, there won't be a formal review.

[github-actions]

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[josephkerkhof]

I like this idea. What if the Immutable Object Pattern is used to achieve this effect? It should be familiar to Laravel developers. DateTime and Carbon objects follow this pattern.

You could take an instance of a model and do a ->toImmutable() on it.

[calebdw]

The frozen state needs to be specifiable and enforceable by the type system / static analysis and not a runtime property (think a separate class like Carbon/CarbonImmutable, or a intersection type like Model&Immutable) ---otherwise every model mutation would need to be guarded by if (! $model->isFrozen()). That's not to say that you can't throw runtime exceptions for invalid mutations, but the type system needs to know if models are writable.

Also note that the CarbonImmutable instance allows changes, it just returns a new static instance instead of mutating the original---this is different from throwing exceptions when attempting to change the instance.

If a model becomes frozen, does this also recursively freeze all loaded relations? I would expect so as I would not want to pass a frozen model to a consumer and then have them mutate a related model.

Some other methods to potentially look at restricting are: __set/offsetSet, __call, all of the load* methods, save, {re}fresh, all other set* methods, etc.

I like the idea because I don't necessarily care to create DTOs that basically contain the same properties, just some food for thought.

Best,

[cosmastech]

Thanks for the reply @calebdw!

The frozen state needs to be specifiable and enforceable by the type system / static analysis and not a runtime property (think a separate class like Carbon/CarbonImmutable, or a intersection type like Model&Immutable) ---otherwise every model mutation would need to be guarded by if (! $model->isFrozen()). That's not to say that you can't throw runtime exceptions for invalid mutations, but the type system needs to know if models are writable.

I've been thinking about this a lot. I hadn't considered the Immutable marker interface as an option, so thank you for that.

The problem I see with most of the non-runtime solutions is that I don't see a way to avoid some kind of code generation.

Immutable marker interface

Take for instance some code like this:

public function loginAs(User&Immutable $user): void
{
  // ...
}

loginAs(User::first()->toFrozen()); // ❌ User does not implement `Immutable`

There would be two options:

• Every Model implements Immutable, but then the type-safety benefit is lost. (ie: any time I pass User I'm passing User&Immutable)
• Return an anonymous class:

class User extends Model
{
    public function toFrozen(): User&Immutable
    {
        return new class extends User implements Immutable {
            // some logic here for constructing the object
        };
    }
}

Problem is that then Models cannot be marked as final. Maybe this isn't a big deal. Also, anonymous classes are kind of ugly to debug when things break.

I don't see a way to get a leverage the PHP type system without creating a separate class (probably employing code generation to do so). At that point, it seems like this would fall better in line with something like spatie's laravel-data than it would within the framework itself.

Create a FrozenModel class

/**
 * @param FrozenModel<User> $user
 */
public function loginAs(FrozenModel $user) { /* ... */ }
loginAs(User::first()->toFrozen()); // ✅ by the type-system && static analysis
loginAs(Team::first()->toFrozen()); // ✅ by the type-system, but ❌  fails by static analysis.

The benefit is only here if we enforce static analysis.

A real-time Immutable object

The other option is to do something like real-time facades, but I found the support for that was wanting in PHPStorm & Laravel IDEA (though I think that may have changed recently). It also looks like larastan doesn't support them. This may have the same problems with marking a class as final, haven't thought about it fully.

---

If a model becomes frozen, does this also recursively freeze all loaded relations? I would expect so as I would not want to pass a frozen model to a consumer and then have them mutate a related model.

I forgot about that. Good call. 👍 I have added it to the checklist.

I like the idea because I don't necessarily care to create DTOs that basically contain the same properties, just some food for thought.

I'm glad you think it could be valuable as well. Part of me just doesn't want to spend the time writing this without some guidance on which direction the maintainers/community would prefer, or if the idea is totally unacceptable. It's probably another 4+ hours of dev with any of these approaches.

[juampi92]

Hey,

My two cents on this PR: while the idea is promising, more evidence is needed for the framework to maintain this as a core feature. The way to prove this need is by creating a package. Packages are meant to be a choose-your-own-adventure where you tweak the framework until it does exactly what you want. If the community appreciates the feature and understands its purpose, it could eventually become a first-party feature.

There are more limitations when you can't edit the framework's source code, so you will need to find other strategies to achieve the same thing, which can be more challenging. If it eventually becomes incorporated by the framework as a first-party feature, it will be integrated with the core features, which overall increase the complexity of maintaining the framework.

I suggest starting with a static analysis approach (like a frozen model with generics) because it’s the least intrusive, and this feature can't be a runtime check. If you want to sell this idea further, PHPStan/Pest architectural rules to make sure JSON Resources are not using mutable models would make this easy to adopt and benefit immediately.

The point I'm trying to make is that to integrate something into the framework, you need to build a strong case for why it’s needed and why many people will benefit from it. Making a PR on the framework will already grant you a lot of exposition to the community, so I understand the benefits of making this PR, but it's not the way these ideas are usually tested.

It's good to get this idea started, I wish we had a place in our Laravel community to do RFCs for packages :/

[cosmastech]

It's good to get this idea started, I wish we had a place in our Laravel community to do RFCs for packages :/
I wish there was a sort of RFC process for Laravel proposals. I know it's a bummer to spend many hours writing something and having it closed.

I'm leaning towards closing this PR, as I think the likelihood of this getting accepted with the massive number of changes is probably close to 0. I may pivot and try just taking the static analysis approach, because like you said, it's less invasive.

[calebdw]

This Laravel news article looks interesting / related:

https://laravel-news.com/zero-to-prod-data-model-php-package