[5.6] Disable model relationship touching #23469

tonysm · 2018-03-09T19:31:00Z

Added

Add the ability to disable relationship model touching functionality for a given callback scope
Add Model::isIgnoringTouch() method to check if the model is in the withoutTouching scope

TL;DR

We can now disable the relationship model touching in a given callback lifetime for specific models when we want to with the new Model::withoutTouching and Model::withoutTouchingOn methods.

I saw this functionality in one of DHH's videos (here) and figured I would give it a try to implement in Laravel. Not sure if you find it useful or not, I was mainly interested in what it would take to write it.

ActiveRecord documentation can be found here, for reference.

Examples

Let's say a Post belongs to some User and has the user relationship set in the touches array.

use Illuminate\Database\Eloquent\Model;

Model::withoutTouching(function () use ($user, $post) {
    $user->touch(); # updates user timestamps.
    $post->touch(); # updates post, but not the related models thru relationships.

    // Updates the model name and timestamps. However, It will not update 
    // any relationships timestamps that are in the no touching target.
    $post->update(['name' => 'Lorem']);
});

User::withoutTouching(function () use ($user, $post) {
    $user->touch(); # updates user timestamps.
    $post->touch(); # updates timestamp, but not the related user's

    // Updates post timestamps, but not the related user's.
    $post->update(['name' => 'Lorem']);
});

// You can also nest calls
User::withoutTouching(function () use ($user, $post) {
    Post::withoutTouching(function () use ($user, $post) {
        $user->touch(); # updates user timestamps.
        $post->touch(); # updates post, but not the related models thru relationships.

        // Updates the model name and timestamps. However, It will not update 
        // any relationships timestamps that are in the no touching target.
        $post->update(['name' => 'Lorem']);
    });
});

// Alternatively, you can avoid the nesting by specifying the models you want to ignore at once:
Model::withoutTouchingOn([User::class, Post::class], function () use ($user, $post) {
    $user->touch(); # updates user timestamps.
    $post->touch(); # updates post, but not the related models thru relationships.

    // Updates the model name and timestamps. However, It will not update 
    // any relationships timestamps that are in the no touching target.
    $post->update(['name' => 'Lorem']);
});

// Check if model is in the `withoutTouching` scope
$this->assertFalse(Model::isIgnoringTouch());
$this->assertFalse(User::isIgnoringTouch());

Model::withoutTouching(function () {
    $this->assertTrue(Model::isIgnoringTouch());
    $this->assertTrue(User::isIgnoringTouch());
});

User::withoutTouching(function () {
    $this->assertFalse(Model::isIgnoringTouch());
    $this->assertTrue(User::isIgnoringTouch());
});

$this->assertFalse(Model::isIgnoringTouch());
$this->assertFalse(User::isIgnoringTouch());

As I'm not that familiar with the Eloquent internals, I'm not entirely sure about the current implementation.

Miguel-Serejo · 2018-03-09T23:07:34Z

I get wanting to override $touches in some cases, but explicitly calling ->touch() should never just silently do nothing. It should either throw an exception (TouchedUntouchableModelException or similar) or actually touch.

tonysm · 2018-03-10T00:36:43Z

Yeah, I also had that feeling. At first I was implementing it only for the relationship touching, but looking at the Rails API, looks like they do both.

taylorotwell · 2018-03-10T19:47:26Z

What is this used for?

tonysm · 2018-03-10T21:35:36Z

When you use the touching feature of Eloquent and you are doing some batch operation dealing with lots of models at the same time (like the incineration feature in Basecamp - deleting lots of data), the touching might generate lots of unnecessary queries, and could potentially slow down the whole process.

But, this is not a use case I've encountered myself, I just saw it on Rails and got curious about what it would take to bring it to Eloquent.

dwightwatson · 2018-03-12T02:00:04Z

Would like to see something like this go in. I've had to use $model->timestamps = false as a hack to get around it in the past.

My use-case is an app that shows updated timestamps to users to they can see how recently activity has occurred. We have background jobs and a moderation team that make changes to these models, but these changes aren't public-facing so we don't want it to appear as though the record has been updated to other users.

Being able to wrap it up in a withoutTouching callback would make it clearer and show more intent than having to use the $model->timestamps = false approach which isn't as obvious.

taylorotwell · 2018-03-12T14:00:14Z

@tonysm so were there more changes you wanted to make based on @36864's comments? I didn't quite follow.

tonysm · 2018-03-12T14:12:01Z

so were there more changes you wanted to make based on @36864's comments? I didn't quite follow.

Depends on how you want it to behave. Right now, in the withoutTouching scope:

Touching timestamps through relationships when a model is updated is disabled; and
Direct calls to $model->touch() are also disabled

The comment was about the last option, whether it should be a noop, throw an exception, or not disabling at all.

I think the Rails API for this feature is as I implemented, by looking at their API and documentation. I'm gonna try to run a small experiment here to confirm that. But let me know what you think.

tonysm · 2018-03-12T15:33:42Z

@taylorotwell just confirmed the way it's implemented right now matches the Rails API. So if you are ok with it, nothing else needs to be done. Unless you can think of any side effects that I need to take into account.

Below you can find how Rails implements it.

irb(main):014:0> p = Post.first
  Post Load (0.6ms)  SELECT  "posts".* FROM "posts" ORDER BY "posts"."id" ASC LIMIT ?  [["LIMIT", 1]]
=> #<Post id: 1, title: "Lorem", user_id: 1, created_at: "2018-03-12 15:27:45", updated_at: "2018-03-12 15:27:45">
irb(main):015:0> p.title = "Lorem ipsum"
=> "Lorem ipsum"
irb(main):016:0> p.save
   (0.2ms)  begin transaction
  User Load (0.4ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
  SQL (1.0ms)  UPDATE "posts" SET "title" = ?, "updated_at" = ? WHERE "posts"."id" = ?  [["title", "Lorem ipsum"], ["updated_at", "2018-03-12 15:28:35.939259"], ["id", 1]]
  SQL (0.5ms)  UPDATE "users" SET "updated_at" = '2018-03-12 15:28:35.946283' WHERE "users"."id" = ?  [["id", 1]]
   (18.1ms)  commit transaction
=> true
irb(main):017:0> User.no_touching do
irb(main):018:1*   p.title = "Relationship touching"
irb(main):019:1>   p.save
irb(main):020:1> end
   (0.2ms)  begin transaction
  SQL (0.9ms)  UPDATE "posts" SET "title" = ?, "updated_at" = ? WHERE "posts"."id" = ?  [["title", "Relationship touching"], ["updated_at", "2018-03-12 15:29:09.085786"], ["id", 1]]
   (8.6ms)  commit transaction
=> true
irb(main):021:0> User.first.touch
  User Load (0.4ms)  SELECT  "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT ?  [["LIMIT", 1]]
   (0.2ms)  begin transaction
  SQL (0.7ms)  UPDATE "users" SET "updated_at" = '2018-03-12 15:29:19.736664' WHERE "users"."id" = ?  [["id", 1]]
   (9.7ms)  commit transaction
=> true
irb(main):022:0> User.no_touching do
irb(main):023:1*   User.first.touch
irb(main):024:1> end
  User Load (0.6ms)  SELECT  "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT ?  [["LIMIT", 1]]
=> nil

tonysm · 2018-03-12T16:09:05Z

@dwightwatson

Being able to wrap it up in a withoutTouching callback would make it clearer and show more intent than having to use the $model->timestamps = false approach which isn't as obvious.

Actually, I don't think it fits your usecase quite well. You're updating the model itself, which will update the timetamps of the model, for example:

>>> $p = Post::create(['title' => 'Lorem', 'user_id' => 1])
"insert into "posts" ("title", "user_id", "updated_at", "created_at") values (?, ?, ?, ?)"
"update "users" set "updated_at" = ? where "users"."id" = ?"
"select * from "users" where "users"."id" = ? limit 1"
>>> Post::withoutTouching(function () use ($p) {
         $p->update(['title' => 'Something else']);
    });
"update "posts" set "title" = ?, "updated_at" = ? where "id" = ?"
"update "users" set "updated_at" = ? where "users"."id" = ?"

As you can see, the model itself got the updated_at field touched. That's also true for the Rails API

irb(main):001:0> p = Post.first
  Post Load (0.3ms)  SELECT  "posts".* FROM "posts" ORDER BY "posts"."id" ASC LIMIT ?  [["LIMIT", 1]]
irb(main):003:0> Post.no_touching do
irb(main):004:1*     p.update title: "Lorem"
irb(main):005:1> end
   (0.2ms)  begin transaction
  SQL (0.8ms)  UPDATE "posts" SET "title" = ?, "updated_at" = ? WHERE "posts"."id" = ?  [["title", "Lorem"], ["updated_at", "2018-03-12 16:00:45.502981"], ["id", 1]]
  SQL (0.6ms)  UPDATE "users" SET "updated_at" = '2018-03-12 16:00:45.507653' WHERE "users"."id" = ?  [["id", 1]]
   (8.9ms)  commit transaction
=> true

But that being said, I think we should not update the model's timestamps in the withoutTouching scope. What do you think @taylorotwell ?

unstoppablecarl · 2018-03-12T16:24:27Z

Is the idea to disable all touching for all models? Or just specific models?

tonysm · 2018-03-12T16:31:17Z

@unstoppablecarl both. You can disable specific models touching or globally.

specific:

User::withoutTouching(function () use ($post) {
    // creates the comment without updating related user from post, but post is touched.
    $post->comments()->create(...); 

    Post::withoutTouching(function () use ($post) {
         // creates the comment without touching the post or the user 
         // related to the post (because of the nested scope).
         $post->comments()->create(...);
    });
});

globally:

use Illuminate\Database\Eloquent\Model;

Model::withoutTouching(function () use ($post) {
    // creates the comment without touching any relationship
    $post->comments()->create(...);
});

tonysm · 2018-03-12T16:54:19Z

@dwightwatson I was wrong. In my implementation, we are not touching the model's timestamp, I had a cached revision here in my testing application. So calls to $model->update() do not update the updated_at field when the model class is in the withoutTouching target.

Example:

>>> $p = Post::first()
"select * from "posts" limit 1"
=> App\Post {#761
     id: "1",
     title: "lorem",
     user_id: "1",
     created_at: "2018-03-12 15:55:19",
     updated_at: "2018-03-12 16:25:34",
   }
>>> $p->update(['title' => 'ipsum']);                                                                                                                                                         "update "posts" set "title" = ?, "updated_at" = ? where "id" = ?"
"update "users" set "updated_at" = ? where "users"."id" = ?"
"select * from "users" where "users"."id" = ? limit 1"
=> true
>>> Post::withoutTouching(function () use ($p) {                                                                                                                                                
      $p->update(['title' => 'lorem']);                                                                                                                                                           
    });
"update "posts" set "title" = ? where "id" = ?"
"update "users" set "updated_at" = ? where "users"."id" = ?"
=> null

But this differs from the Rails implementation. (see the Rails implementation int he comment above #23469 (comment)).

Any thoughts on whether we should stick to the Rails API and update the model's updated_at when calling $model->update(), or stick to the current implementation and just update the dirty fields, skiping the timestamps (again, when in then withoutTouching scope)?

taylorotwell · 2018-03-13T13:19:37Z

If it's going to be the same method name as Rails probably it would need to behave the same way so as not to surprise people.

tonysm · 2018-03-13T14:05:12Z

@taylorotwell Done. Don't think there's anything else to do here. Going to send another PR to cover the usecase @dwightwatson mentioned later today.

afraca · 2018-03-14T10:25:05Z

Nitpick, but any particular reason you use self:: over static::? I've had to override properties like those on the fly myself and would find the self:: here a bit annoying.

tonysm · 2018-03-14T12:16:33Z

@afraca good point, I thought I had to use self, but turns out it was not the case.

unstoppablecarl · 2018-03-17T09:39:32Z

Can you call model::withoutTouching if Model is abstract?

unstoppablecarl · 2018-03-17T09:52:27Z

I really like this idea. I think this may be a better api for it though.

Model::withoutTouching([Foo::class, Bar::class, ...], function(){}); where the array is all model classes you do not want touched.

Foo::withoutTouchingRelations(['posts', 'users'], function(){}); where the array are relations of the foo model.

I have had some cases where I needed to conditionally touch some models in the collection of related models but not others. How would you write that?

JosephSilber · 2018-03-25T03:23:37Z

If it's going to be the same method name as Rails probably it would need to behave the same way so as not to surprise people.

Is that true for every other method in Laravel as well? I'm a little surprised by this opinion.

Regarding the general feature: I feel quite strongly that calling touch directly should definitely touch the model. To me, it doesn't make mush sense to disable a direct call to touch.

tonysm · 2018-03-26T11:25:23Z

Regarding the general feature: I feel quite strongly that calling touch directly should definitely touch the model. To me, it doesn't make mush sense to disable a direct call to touch.

@JosephSilber I think you are right. I thought of introducing a forceTouch method instead, but makes sense that directly calling $model->touch() bypasses the protection. Was trying to mimic the Rails API, but it does make more sense than introducing a new method.

tonysm · 2018-03-26T11:42:50Z

@unstoppablecarl

Can you call model::withoutTouching if Model is abstract?

The implementation is based on instanceof, so if you do:

class ParentModel extends Model {}

class ChildModel extends ParentModel {}

ParentModel::withoutTouching(function () {
   // If something triggers updating timestamps of child model relationships,
   // it won't touch it because ChildModel instanceof ParentModel === true.
});

Model::withoutTouching([Foo::class, Bar::class, ...], function(){}); where the array is all model classes you do not want touched.

I like it. Wish PHP had method overloading. Anyways, so for this API it should not include the class being class? Like, ignore the caller, and use only the given classes?

Foo::withoutTouchingRelations(['posts', 'users'], function(){}); where the array are relations of the foo model.

Not sure about this one. It's doable, but don't think it's that useful, tbh. It would only make it much more complex that it should be, IMO. It should just be "if you encounter relationships of this class, don't touch it".

I have had some cases where I needed to conditionally touch some models in the collection of related models but not others. How would you write that?

Can you provide an example? If I got it right, you want to conditionally touch models based on some criteria. Like, if you encounter relationships of class "User" and the ID is 123, don't touch it, if not this ID it's ok to touch it. Can't think of a use case where you are using the relationship touching feature, but you don't want to touch specific models of a hasMany relationship.

tonysm · 2018-03-26T11:47:57Z

Gonna make some changes here:

withoutTouching scope should only be for relationship model touching. We should not ignore direct calls to $model->touch();
Will add a new withoutTouchingOn([A::class, B::class], function () {}) API method to avoid nested calls of withoutTouching, but should behave exactly the same as nested calls.

tonysm · 2018-03-27T00:27:56Z

I think it's all done here.

unstoppablecarl · 2018-03-27T13:11:42Z

src/Illuminate/Database/Eloquent/Model.php

+        static::$ignoreOnTouch = array_values(array_merge(static::$ignoreOnTouch, $models));
+
+        try {
+            call_user_func($callback);


Is call_user_func($callback) better than $callback() ?

unstoppablecarl · 2018-03-27T13:15:19Z

src/Illuminate/Database/Eloquent/Model.php

+    public function shouldTouch()
+    {
+        foreach (static::$ignoreOnTouch as $ignoredClass) {
+            if ($this instanceof $ignoredClass) {


I think this could cause some surprise to users that extend models. I think it would be better to check if(get_class($this) === $ignoredClass)

I would disagree, I would be surprised if extending an ignored class causes the child class to not be ignored.
Also, this allows you to have a parent "IgnoredModel" class and extend all your models from that instead of having to explicitly ignore every model.

Yeah, this also allows us to globally ignore relationship touching by invoking it from the base model class. That's the desired behavior.

olivernybroe

Maybe add a getter for $ignoreOnTouch?

unstoppablecarl · 2018-05-21T13:35:01Z

Maybe add a getter for $ignoreOnTouch?

I like that. maybe isIgnoringTouch() ?

taylorotwell · 2018-05-21T14:38:35Z

@tonysm where are we at with this?

…thod

tonysm · 2018-05-21T20:38:48Z

@olivernybroe @unstoppablecarl Added the Model::isIgnoringTouch() method.

@taylorotwell It's ready

taylorotwell · 2018-05-23T13:03:02Z

Just curious, do we have actual use cases for this waiting for this to be merged? Like packages being developed, etc. Or is this mainly something be done "in theory"? 😄

tonysm · 2018-05-23T13:44:47Z

All in theory. I haven't seen anyone needing it in the Laravel world. As I said:

this is not a use case I've encountered myself, I just saw it on Rails and got curious about what it would take to bring it to Eloquent.

Can be closed, no big deal. haha

negoziator · 2018-06-02T05:38:56Z

We do have a use case.
In our company, we are migrating millions of rows to our Laravel application within months.

We have been running into a performance problem, where touching of related models is actually a problem! We have hacked ourself around it, but this change would really be appreciated.

tonysm added 6 commits March 8, 2018 21:29

Allow disabling touching models and their children temporarily

8483ce8

Fix touching the model directly not respecting the without touch

efa95a6

Make sure we can update a model without touching its timestamps

435320d

Test deletion of children also respects the parent no touching

5708067

Fix style issue

84cc466

More tests for the direct model touching and the chain

ddb1192

tonysm changed the title ~~Disable model touching~~ [5.6] Disable model touching Mar 9, 2018

tonysm added 3 commits March 13, 2018 10:54

Test nested relationships are touched when direct parent is ignored

5cda08e

Touch models own timestamps when in withoutTimestamps scope

940aeab

Remove unused conditional

a3889bc

Use static instead of self

7371846

tonysm changed the title ~~[5.6] Disable model touching~~ [5.6] Disable model relationship touching Mar 15, 2018

tonysm added 3 commits March 26, 2018 20:50

Remove ignoring direct touch calls in withoutTouching scope

881921a

Add withoutTouchingOn that receives all models classes to ignore at once

620eee1

Add unit testing for the withoutTouchingOn method

53510ea

unstoppablecarl reviewed Mar 27, 2018

View reviewed changes

olivernybroe reviewed Apr 4, 2018

View reviewed changes

tonysm added 5 commits May 21, 2018 17:14

Allow checking if class is in the ignore touching scope

4a7a25f

Remove shouldTouch and replace its usage with the isIgnoredOnTouch me…

3629625

…thod

Align use statements

e46179a

Remove extra line

8d1ade1

Rename method

1d85125

taylorotwell merged commit 1d85125 into laravel:5.6 Jun 20, 2018

derekmd mentioned this pull request Apr 23, 2021

[8.x] Add timestamps to model save options #37104

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[5.6] Disable model relationship touching #23469

[5.6] Disable model relationship touching #23469

tonysm commented Mar 9, 2018 •

edited

Loading

Miguel-Serejo commented Mar 9, 2018

tonysm commented Mar 10, 2018 •

edited

Loading

taylorotwell commented Mar 10, 2018

tonysm commented Mar 10, 2018 •

edited

Loading

dwightwatson commented Mar 12, 2018

taylorotwell commented Mar 12, 2018 •

edited

Loading

tonysm commented Mar 12, 2018

tonysm commented Mar 12, 2018 •

edited

Loading

tonysm commented Mar 12, 2018 •

edited

Loading

unstoppablecarl commented Mar 12, 2018

tonysm commented Mar 12, 2018 •

edited

Loading

tonysm commented Mar 12, 2018 •

edited

Loading

taylorotwell commented Mar 13, 2018

tonysm commented Mar 13, 2018

afraca commented Mar 14, 2018

tonysm commented Mar 14, 2018

unstoppablecarl commented Mar 17, 2018

unstoppablecarl commented Mar 17, 2018 •

edited

Loading

JosephSilber commented Mar 25, 2018

tonysm commented Mar 26, 2018

tonysm commented Mar 26, 2018

tonysm commented Mar 26, 2018 •

edited

Loading

tonysm commented Mar 27, 2018

unstoppablecarl Mar 27, 2018

unstoppablecarl Mar 27, 2018

Miguel-Serejo Mar 27, 2018

tonysm Mar 27, 2018

olivernybroe left a comment

unstoppablecarl commented May 21, 2018

taylorotwell commented May 21, 2018

tonysm commented May 21, 2018

taylorotwell commented May 23, 2018

tonysm commented May 23, 2018

negoziator commented Jun 2, 2018 •

edited

Loading

[5.6] Disable model relationship touching #23469

[5.6] Disable model relationship touching #23469

Conversation

tonysm commented Mar 9, 2018 • edited Loading

Added

TL;DR

Examples

Miguel-Serejo commented Mar 9, 2018

tonysm commented Mar 10, 2018 • edited Loading

taylorotwell commented Mar 10, 2018

tonysm commented Mar 10, 2018 • edited Loading

dwightwatson commented Mar 12, 2018

taylorotwell commented Mar 12, 2018 • edited Loading

tonysm commented Mar 12, 2018

tonysm commented Mar 12, 2018 • edited Loading

tonysm commented Mar 12, 2018 • edited Loading

unstoppablecarl commented Mar 12, 2018

tonysm commented Mar 12, 2018 • edited Loading

tonysm commented Mar 12, 2018 • edited Loading

taylorotwell commented Mar 13, 2018

tonysm commented Mar 13, 2018

afraca commented Mar 14, 2018

tonysm commented Mar 14, 2018

unstoppablecarl commented Mar 17, 2018

unstoppablecarl commented Mar 17, 2018 • edited Loading

JosephSilber commented Mar 25, 2018

tonysm commented Mar 26, 2018

tonysm commented Mar 26, 2018

tonysm commented Mar 26, 2018 • edited Loading

tonysm commented Mar 27, 2018

unstoppablecarl Mar 27, 2018

Choose a reason for hiding this comment

unstoppablecarl Mar 27, 2018

Choose a reason for hiding this comment

Miguel-Serejo Mar 27, 2018

Choose a reason for hiding this comment

tonysm Mar 27, 2018

Choose a reason for hiding this comment

olivernybroe left a comment

Choose a reason for hiding this comment

unstoppablecarl commented May 21, 2018

taylorotwell commented May 21, 2018

tonysm commented May 21, 2018

taylorotwell commented May 23, 2018

tonysm commented May 23, 2018

negoziator commented Jun 2, 2018 • edited Loading

tonysm commented Mar 9, 2018 •

edited

Loading

tonysm commented Mar 10, 2018 •

edited

Loading

tonysm commented Mar 10, 2018 •

edited

Loading

taylorotwell commented Mar 12, 2018 •

edited

Loading

tonysm commented Mar 12, 2018 •

edited

Loading

tonysm commented Mar 12, 2018 •

edited

Loading

tonysm commented Mar 12, 2018 •

edited

Loading

tonysm commented Mar 12, 2018 •

edited

Loading

unstoppablecarl commented Mar 17, 2018 •

edited

Loading

tonysm commented Mar 26, 2018 •

edited

Loading

negoziator commented Jun 2, 2018 •

edited

Loading