Skip to content

Commit e450d14

Browse files
author
Pantea Marius-ciclistu
committed
POC for laravel/framework#31778 cover changes in created updated saved that got into $original but not in db
1 parent 703390e commit e450d14

File tree

2 files changed

+108
-76
lines changed

2 files changed

+108
-76
lines changed

‎src/Models/BaseModel.php‎

Lines changed: 54 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ abstract class BaseModel extends Model
5656
*/
5757
protected ?array $tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
5858

59+
/**
60+
* Temporary original cache to prevent changes in created,updated,saved events from getting
61+
* into $original without being saved into DB
62+
*/
63+
protected ?array $tmpOriginalBeforeAfterEvents = null;
64+
5965
private array $incrementsToRefresh = [];
6066

6167
/**
@@ -586,6 +592,14 @@ public function getAttributes(): array
586592
return $this->attributes;
587593
}
588594

595+
/**
596+
* @inheritdoc
597+
*/
598+
protected function getAttributesForInsert(): array
599+
{
600+
return $this->tmpOriginalBeforeAfterEvents = $this->getAttributes();
601+
}
602+
589603
/**
590604
* @inheritdoc
591605
*/
@@ -608,16 +622,6 @@ public function isDirty($attributes = null): bool
608622
return [] !== $this->getDirty(\is_array($attributes) ? $attributes : \func_get_args());
609623
}
610624

611-
/**
612-
* @inheritdoc
613-
*/
614-
public function syncOriginal(): static
615-
{
616-
$this->original = $this->getAttributes(isset($this->tmpDirtyIfAttributesAreSyncedFromCashedCasts));
617-
618-
return $this;
619-
}
620-
621625
/**
622626
* Get the attributes that have been changed since the last sync.
623627
* @param string|array $attributes
@@ -685,10 +689,9 @@ public function save(array $options = []): bool
685689
try {
686690
/** We will try to optimize the execution by caching $dirty array BUT,
687691
multiple set calls will be needed (https://github.com/laravel/framework/discussions/31778)
688-
WHEN $this->usesTimestamps() or WHEN there are listeners for the following events:
689-
- updating event can do changes so, $this->getDirtyForUpdate() and $this->syncChanges() will call
690-
$this->getDirty(),
691-
- updated/saved events can do changes so, $this->syncOriginal() will call $this->getAttributes() */
692+
WHEN $this->usesTimestamps() or WHEN there are listeners for updating event because they can do changes
693+
so, $this->getDirtyForUpdate() and $this->syncChanges() will call $this->getDirty() which will call
694+
getAttributes() */
692695
if (!$this->getEventDispatcher()->hasListeners('eloquent.updating: ' . $this::class)) {
693696
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = $dirty;
694697
unset($dirty);
@@ -703,6 +706,7 @@ public function save(array $options = []): bool
703706
return false;
704707
} finally {
705708
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
709+
$this->tmpOriginalBeforeAfterEvents = null;
706710
}
707711
}
708712

@@ -711,20 +715,24 @@ public function save(array $options = []): bool
711715

712716
/** Multiple set calls are needed (https://github.com/laravel/framework/discussions/31778) because:
713717
- $this->performInsert can do changes,
714-
- creating event can do changes so, $this->getAttributesForInsert() will call $this->getAttributes(),
715-
- created/saved events can do changes so, $this->syncOriginal() will call $this->getAttributes() */
718+
- creating event can do changes so,
719+
$this->getAttributesForInsert() and $this->syncOriginal() will call $this->getAttributes() */
716720

717-
$saved = $this->performInsert($query);
721+
try {
722+
$saved = $this->performInsert($query);
718723

719-
if (
720-
'' === (string)$this->getConnectionName() &&
721-
($connection = $query->getConnection()) instanceof Connection
722-
) {
723-
$this->setConnection($connection->getName());
724-
}
724+
if (
725+
'' === (string)$this->getConnectionName() &&
726+
($connection = $query->getConnection()) instanceof Connection
727+
) {
728+
$this->setConnection($connection->getName());
729+
}
725730

726-
if ($saved) {
727-
$this->finishSave($options + ['touch' => $isDirty]);
731+
if ($saved) {
732+
$this->finishSave($options + ['touch' => $isDirty]);
733+
}
734+
} finally {
735+
$this->tmpOriginalBeforeAfterEvents = null;
728736
}
729737

730738
return $saved;
@@ -735,19 +743,19 @@ public function save(array $options = []): bool
735743
*/
736744
protected function finishSave(array $options): void
737745
{
738-
if (
739-
isset($this->tmpDirtyIfAttributesAreSyncedFromCashedCasts)
740-
&& $this->getEventDispatcher()->hasListeners('eloquent.saved: ' . $this::class)
741-
) {
742-
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
743-
}
744-
745746
$this->fireModelEvent('saved', false);
746747

747748
if ($options['touch'] ?? true) {
748749
$this->touchOwners();
749750
}
750751

752+
if (isset($this->tmpOriginalBeforeAfterEvents)) {
753+
$this->original = $this->tmpOriginalBeforeAfterEvents;
754+
$this->tmpOriginalBeforeAfterEvents = null;
755+
756+
return;
757+
}
758+
751759
$this->syncOriginal();
752760
}
753761

@@ -781,19 +789,27 @@ protected function performUpdate(Builder $query): bool
781789

782790
$this->syncChanges();
783791

784-
if (
785-
isset($this->tmpDirtyIfAttributesAreSyncedFromCashedCasts)
786-
&& $this->getEventDispatcher()->hasListeners('eloquent.updated: ' . $this::class)
787-
) {
788-
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
789-
}
792+
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
793+
$this->tmpOriginalBeforeAfterEvents = $this->attributes;
790794

791795
$this->fireModelEvent('updated', false);
792796
}
793797

794798
return true;
795799
}
796800

801+
/**
802+
* @inheritdoc
803+
*/
804+
protected function insertAndSetId(Builder $query, $attributes): void
805+
{
806+
$id = $query->insertGetId($attributes, $keyName = $this->getKeyName());
807+
808+
$this->setAttribute($keyName, $id);
809+
810+
$this->tmpOriginalBeforeAfterEvents = $this->attributes;
811+
}
812+
797813
/**
798814
* Get the attributes that have been changed since the last sync for an update operation.
799815
*

‎src/Models/ExcessiveSetOptimizerOnSaveTrait.php‎

Lines changed: 54 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ trait ExcessiveSetOptimizerOnSaveTrait
1515
*/
1616
protected ?array $tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
1717

18+
/**
19+
* Temporary original cache to prevent changes in created,updated,saved events from getting
20+
* into $original without being saved into DB
21+
*/
22+
protected ?array $tmpOriginalBeforeAfterEvents = null;
23+
1824
public function __call($method, $parameters)
1925
{
2026
$lowerMethod = \strtolower($method);
@@ -51,6 +57,14 @@ public function getAttributes(): array
5157
return $this->attributes;
5258
}
5359

60+
/**
61+
* @inheritdoc
62+
*/
63+
protected function getAttributesForInsert(): array
64+
{
65+
return $this->tmpOriginalBeforeAfterEvents = $this->getAttributes();
66+
}
67+
5468
/**
5569
* @inheritdoc
5670
*/
@@ -73,16 +87,6 @@ public function isDirty($attributes = null): bool
7387
return [] !== $this->getDirty(\is_array($attributes) ? $attributes : \func_get_args());
7488
}
7589

76-
/**
77-
* @inheritdoc
78-
*/
79-
public function syncOriginal(): static
80-
{
81-
$this->original = $this->getAttributes(isset($this->tmpDirtyIfAttributesAreSyncedFromCashedCasts));
82-
83-
return $this;
84-
}
85-
8690
/**
8791
* Get the attributes that have been changed since the last sync.
8892
* @param string|array $attributes
@@ -150,10 +154,9 @@ public function save(array $options = []): bool
150154
try {
151155
/** We will try to optimize the execution by caching $dirty array BUT,
152156
multiple set calls will be needed (https://github.com/laravel/framework/discussions/31778)
153-
WHEN $this->usesTimestamps() or WHEN there are listeners for the following events:
154-
- updating event can do changes so, $this->getDirtyForUpdate() and $this->syncChanges() will call
155-
$this->getDirty(),
156-
- updated/saved events can do changes so, $this->syncOriginal() will call $this->getAttributes() */
157+
WHEN $this->usesTimestamps() or WHEN there are listeners for updating event because they can do changes
158+
so, $this->getDirtyForUpdate() and $this->syncChanges() will call $this->getDirty() which will call
159+
getAttributes() */
157160
if (!$this->getEventDispatcher()->hasListeners('eloquent.updating: ' . $this::class)) {
158161
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = $dirty;
159162
unset($dirty);
@@ -168,6 +171,7 @@ public function save(array $options = []): bool
168171
return false;
169172
} finally {
170173
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
174+
$this->tmpOriginalBeforeAfterEvents = null;
171175
}
172176
}
173177

@@ -176,20 +180,24 @@ public function save(array $options = []): bool
176180

177181
/** Multiple set calls are needed (https://github.com/laravel/framework/discussions/31778) because:
178182
- $this->performInsert can do changes,
179-
- creating event can do changes so, $this->getAttributesForInsert() will call $this->getAttributes(),
180-
- created/saved events can do changes so, $this->syncOriginal() will call $this->getAttributes() */
183+
- creating event can do changes so,
184+
$this->getAttributesForInsert() and $this->syncOriginal() will call $this->getAttributes() */
181185

182-
$saved = $this->performInsert($query);
186+
try {
187+
$saved = $this->performInsert($query);
183188

184-
if (
185-
'' === (string)$this->getConnectionName() &&
186-
($connection = $query->getConnection()) instanceof Connection
187-
) {
188-
$this->setConnection($connection->getName());
189-
}
189+
if (
190+
'' === (string)$this->getConnectionName() &&
191+
($connection = $query->getConnection()) instanceof Connection
192+
) {
193+
$this->setConnection($connection->getName());
194+
}
190195

191-
if ($saved) {
192-
$this->finishSave($options + ['touch' => $isDirty]);
196+
if ($saved) {
197+
$this->finishSave($options + ['touch' => $isDirty]);
198+
}
199+
} finally {
200+
$this->tmpOriginalBeforeAfterEvents = null;
193201
}
194202

195203
return $saved;
@@ -200,19 +208,19 @@ public function save(array $options = []): bool
200208
*/
201209
protected function finishSave(array $options): void
202210
{
203-
if (
204-
isset($this->tmpDirtyIfAttributesAreSyncedFromCashedCasts)
205-
&& $this->getEventDispatcher()->hasListeners('eloquent.saved: ' . $this::class)
206-
) {
207-
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
208-
}
209-
210211
$this->fireModelEvent('saved', false);
211212

212213
if ($options['touch'] ?? true) {
213214
$this->touchOwners();
214215
}
215216

217+
if (isset($this->tmpOriginalBeforeAfterEvents)) {
218+
$this->original = $this->tmpOriginalBeforeAfterEvents;
219+
$this->tmpOriginalBeforeAfterEvents = null;
220+
221+
return;
222+
}
223+
216224
$this->syncOriginal();
217225
}
218226

@@ -246,19 +254,27 @@ protected function performUpdate(Builder $query): bool
246254

247255
$this->syncChanges();
248256

249-
if (
250-
isset($this->tmpDirtyIfAttributesAreSyncedFromCashedCasts)
251-
&& $this->getEventDispatcher()->hasListeners('eloquent.updated: ' . $this::class)
252-
) {
253-
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
254-
}
257+
$this->tmpDirtyIfAttributesAreSyncedFromCashedCasts = null;
258+
$this->tmpOriginalBeforeAfterEvents = $this->attributes;
255259

256260
$this->fireModelEvent('updated', false);
257261
}
258262

259263
return true;
260264
}
261265

266+
/**
267+
* @inheritdoc
268+
*/
269+
protected function insertAndSetId(Builder $query, $attributes): void
270+
{
271+
$id = $query->insertGetId($attributes, $keyName = $this->getKeyName());
272+
273+
$this->setAttribute($keyName, $id);
274+
275+
$this->tmpOriginalBeforeAfterEvents = $this->attributes;
276+
}
277+
262278
/**
263279
* Get the attributes that have been changed since the last sync for an update operation.
264280
*

0 commit comments

Comments
 (0)