framework icon indicating copy to clipboard operation
framework copied to clipboard

Prevent Attribute accessors from running for non-arrayable attributes

Open elliota43 opened this issue 1 week ago • 1 comments

When serializing models to arrays/JSON, Eloquent currently considers all Attribute-based accessors via getMutatedAttributes(), even for attributes that are not arrayable (hidden or not visible).

This can:

  • Invoke accessors that will never appear in the serialized output,
  • Trigger unnecessary work, and
  • Cause unexpected exceptions when those accessors touch unloaded relationships or other states.

Changes

  • attributesToArray() now derives mutated attributes only from the model's arrayable attributes via a new helper:
protected function getArrayableMutatedAttributes(array $attributes): array
{
	return array_values(array_filter(array_keys($attributes), function ($key) {
		return $this->hasAnyGetMutator($key);
	}));
}
  • mutateAttributeForArray():
    • Applies class casts first,
    • Uses Attribute-based get mutators when hasAttributeGetMutator() is true (and serializes DateTimeInterface values),
    • Falls back to legacy getFooAttribute mutators.
  • getAttributeMarkedMutatorMethods() no longer invokes Attribute methods; it only inspects their return types to see if they return Attribute.
  • hasAttributeGetMutator() now lazily resolves and caches the presence of a get closure, and only invokes the underlying method when required.

Behavior

  • toArray()/toJson() now invoke Attribute accessors only for attributes that:
    • are arrayable for that instance, and
    • define a get mutator (legacy or Attribute-based)
  • Hidden / non-visible Attribute accessors are no longer executed as a side-effect of serialization.
  • Set-only Attribute mutators (no get) continue to work as before and are not treated as get mutators.

Tests

Added tests to DatabaseEloquentModelTest:

  • testAccessorsNotCalledForNonVisibleAttributes
  • testAccessorsCalledForVisibleAttributes
  • testAccessorsNotCalledForHiddenAttributes
  • testSetOnlyAttributeMutatorDoesNotBreakSerialization

These ensure that only visible/arrayable attributes trigger Attribute accessors during toArray().

Fixes #55067

elliota43 avatar Dec 07 '25 18:12 elliota43

Please mark as ready for review when comment above has been answered.

taylorotwell avatar Dec 10 '25 16:12 taylorotwell

I noticed the CI run is failing on Illuminate\Tests\Queue\FileFailedJobProviderTest::testCanRetrieveAllFailedJobs due to a one-second difference / swapped failed_at timestamps between two failed jobs.

This PR only changes Eloquent’s HasAttributes serialization path and a corresponding Eloquent test; it doesn’t touch the queue failed-job provider or its tests, so this looks unrelated / timing-sensitive.

elliota43 avatar Dec 11 '25 20:12 elliota43

I am unable to recreate this issue, and looking through the code I don't it supported there.

On current 12.x branch, getMutatedAttributes doesn't actually invoke the attributes. It just caches the names of the mutated attributes. attributesToArray calls addMutatedAttributes to array which skips over mutated attributes that aren't present in the given $attributes, which has already had hidden attributes removed by getArraybleAttributes.

Am I missing something?

User model:

<?php

namespace App\Models;

// use Illuminate\Contracts\Auth\MustVerifyEmail;
use Illuminate\Database\Eloquent\Casts\Attribute;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;

class User extends Authenticatable
{
    use HasFactory, Notifiable;

    protected $guarded = [];

    protected $visible = ['id'];

    public function name(): Attribute
    {
        return Attribute::make(
            get: function () {
                info('In attribute...');

                return 'EXAMPLE USER NAME';
            },
        );
    }

    protected function casts(): array
    {
        return [
            'email_verified_at' => 'datetime',
            'password' => 'hashed',
        ];
    }
}

Route:

<?php

use App\Models\User;
use Illuminate\Support\Facades\Route;

Route::get('/', function () {
    $user = User::find(1);

    return $user->toArray();
});

Nothing is written in the logs.

taylorotwell avatar Dec 11 '25 21:12 taylorotwell