Find out Whether Model Observers in Laravel are a Bad Practice

Laravel provides an interesting way to automate common model events inside your app with . dispatched events, closure events, and observers While it sounds cool to have this kind of plug-and-play solution — there are certain cases when this will backfire on your project if you tend to overload this feature with business logic. TL;DR I think observers and model events are fine for MVP and/or smaller projects. When you have 2+ devs working and/or 100+ test cases — they become an issue (not absolutely will). can For very large projects that will be an issue for sure. You would need to spend a lot of time refactoring, QAing, and regress-testing your app. So think ahead and refactor early. Reason: model events create hidden side effects, sometimes unexpected and not required by the executed action. The most common side effects can be observed when writing and running Unit tests and Feature tests for your Laravel app. This article will demonstrate this scenario. Our example Processing temperature measures from IoT devices, storing them in a database and doing some additional calculations after each sample is consumed. Our business requirements: store a sample consumed via exposed API for each stored sample update and average temperature for the last 10 measures This is our Sample model and migration: id(); $table->string('device_id'); $table->float('temp'); $table->timestamp('created_at')->useCurrent(); }); } public function down(): void { Schema::dropIfExists('samples'); } }; <?php declare(strict_types=1); namespace App\Models; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; class Sample extends Model { use HasFactory; public $timestamps = false; protected $fillable = [ 'device_id', 'temp', 'created_at', ]; } Now every time we store a sample we want to store the average temperature for the last 10 samples in another model, : Avg Temperature id(); $table->string('device_id'); $table->float('temp'); $table->timestamps(); }); } public function down(): void { Schema::dropIfExists('avg_temperatures'); } }; <?php declare(strict_types=1); namespace App\Models; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; class AvgTemperature extends Model { use HasFactory; protected $fillable = [ 'device_id', 'temp', ]; } We can achieve this simply by attaching an event to state of the model: the created Sample <?php declare(strict_types=1); namespace App\Models; use App\Events\SampleCreatedEvent; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; class Sample extends Model { use HasFactory; public $timestamps = false; protected $fillable = [ 'device_id', 'temp', 'created_at', ]; /** * @var array */ protected $dispatchesEvents = [ 'created' => SampleCreatedEvent::class, ]; } Now we add a listener with average recalculation logic: class EventServiceProvider extends ServiceProvider { /** * @var array > */ protected $listen = [ SampleCreatedEvent::class => [ RecalcAvgTemperatureListener::class, ], ]; } limit(10) ->avg('temp'); AvgTemperature::updateOrCreate([ 'device_id' => $event->sample->device_id, ], [ 'temp' => $average ?? 0, ]); } } Now, our naive controller implementation, , would look like this: **skipping validation and any good development patterns** all(), ['created_at' => now()]) ); } } We can also write a feature test that confirms that our API route works as expected — sample is stored and avg sample is stored: post('/sample', [ 'device_id' => 'xyz', 'temp' => 10.5, ]); // assert $sample = Sample::first(); $this->assertSame('xyz', $sample->device_id); $this->assertSame(10.5, $sample->temp); } /** @test */ public function when_sample_is_sent_then_avg_model_is_stored(): void { Sample::factory()->create(['device_id' => 'xyz', 'temp' => 20]); // act $this->post('/sample', [ 'device_id' => 'xyz', 'temp' => 10, ]); // assert $sample = AvgTemperature::first(); $this->assertSame('xyz', $sample->device_id); $this->assertSame(15.0, $sample->temp); } } That looks perfectly fine, right? Now when things go wrong Imagine a second developer on your team is going to write a Unit test where he wants to check average temperature calculations. He extracts a service from the listener class to do this job: recalcAvg->withLatestTenSamples($event->sample); } } device_id) ->orderByDesc('created_at') ->limit(10) ->pluck('temp') ->avg(); AvgTemperature::updateOrCreate([ 'device_id' => $sample->device_id, ], [ 'temp' => $average ?? 0, ]); } } He has this unit test written where he wants to seed 100 samples at once at 1-minute intervals: <?php declare(strict_types=1); namespace Tests\Original; use App\Models\AvgTemperature; use App\Models\Sample; use App\Services\AvgTemperatureRecalcService; use Tests\TestCase; class AvgTemperatureRecalcServiceTest extends TestCase { /** @test */ public function when_has_existing_100_samples_then_10_last_average_is_correct(): void { for ($i = 0; $i create([ 'device_id' => 'xyz', 'temp' => 1, 'created_at' => now()->subMinutes($i), ]); } $sample = Sample::factory()->create(['device_id' => 'xyz', 'temp' => 11, 'created_at' => now()]); // pre assert // this will FAIL because average was already recounted 100x times when factory was creating 100x samples $this->assertCount(0, AvgTemperature::all()); // act $service = new AvgTemperatureRecalcService(); $service->withLatestTenSamples($sample); // assert $avgTemp = AvgTemperature::where('device_id', 'xyz')->first(); $this->assertSame((float)((9 + 11) / 10), $avgTemp->temp); } } This is a pretty simple example and can be fixed by disabling the model event or faking the whole Event facade on an ad-hoc basis. Event::fake(); // or Sample::unsetEventDispatcher(); For any more or less large project such options are painful — you always need to remember your model creates side effects. Imagine such an event creates side effects in another database or an external service via API call. Every time you create a sample with a factory you’d have to deal with mocking external calls. We have a combination of a bad development pattern of model events and not enough code decoupling. Refactoring and decoupling our example For better visibility, we will create a second set of models in our project and a new route. First, we remove the model event from our Sample model, now it looks like this: <?php declare(strict_types=1); namespace App\Models; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; class RefactoredSample extends Model { use HasFactory; protected $table = 'samples'; public $timestamps = false; protected $fillable = [ 'device_id', 'temp', 'created_at', ]; } Then we create a service that will be responsible for consuming new samples: $sample->device_id, 'temp' => $sample->temp, 'created_at' => now(), ]); event(new SampleCreatedEvent($sample)); return $sample; } } Notice our service is now responsible for firing an event in case of success. Our new route handler will look like this: newSample(SampleDto::fromRequest($request)); } } Request class: > */ public function rules(): array { return [ 'device_id' => ['required', 'string'], 'temp' => ['required', 'numeric'], ]; } } Now we replicate our second developer’s tests with the new route and can confirm it passes: <?php declare(strict_types=1); namespace Tests\Refactored; use App\Models\AvgTemperature; use App\Models\RefactoredSample; use App\Services\RefactoredAvgTemperatureRecalcService; use Tests\TestCase; class AvgTemperatureRecalcServiceTest extends TestCase { /** @test */ public function when_has_existing_100_samples_then_10_last_average_is_correct(): void { for ($i = 0; $i create([ 'device_id' => 'xyz', 'temp' => 1, 'created_at' => now()->subMinutes($i), ]); } $sample = RefactoredSample::factory()->create(['device_id' => 'xyz', 'temp' => 11, 'created_at' => now()]); // pre assert $this->assertCount(0, AvgTemperature::all()); // act $service = new RefactoredAvgTemperatureRecalcService(); $service->withLatestTenSamples($sample); // assert $avgTemp = AvgTemperature::where('device_id', 'xyz')->first(); $this->assertSame((float)((9 + 11) / 10), $avgTemp->temp); } } Conclusion What was improved: We decoupled our controller from the database model. We decoupled sample processing (business logic) from the framework. The firing of is more controllable and will not trigger when not expected. SampleCreatedEvent How this helps: Developers are happier when working with your code. You can now mock sample processing when testing the sample controller. CI/CD runs faster and costs less as we don’t do unnecessary work (valid for large projects). The repository with code can be found here: . https://github.com/dkhorev/model-observers-bad-practice Also Published Here

HackerNoon Mobile

Find out Whether Model Observers in Laravel are a Bad Practice

Too Long; Didn't Read

@dkhorev

RELATED STORIES