On the importance of parentheses

Sep 30, 2020

When building software, developers more often than not rely on libraries and abstractions to keep their focus on their own code rather than getting caught up in details that have been solved time and time again.

At Ziik, we use Laravel as the basis for our REST API - it is well documented, easy to use, very flexible and all in all a very good framework to develop a flexible system.

Laravel suits us well. We build a SaaS with a decent number of entity types and quite a bit of logic happening, when content is added, modified or removed. For example, we send notifications to our smartphone app users, when new content is published. These notifications go through various steps, most notably Firebase that in turn pass them on to either Google's or Apple's push notification services.

With these steps through both internal and external systems, troubleshooting notifications has always been a challenge. Last week, notifications were suddenly very unreliable and more users than usual complained that they were not getting notifications on new content. Here is why:

The Cause

Laravel has Eloquent - an ingrained abstraction layer on the database querying that makes it simple to write short, easy to read lines of code that do a lot. However, there are also caveats to using this abstraction, namely that the implementation details are hidden from view.

If you take a look at the following class:

class Content extends Model
    public function files()
        return $this->morphMany(File::class, 'owner');

It is a Content class that has a relationship with the File class. This makes it easy to operate on the files that belong to a certain piece of content, especially for simple operations. It does not have to be all that complicated before you need some degree of concentration, though.

  ->where('uploaded', false)
  ->orWhere('processing', true)

The above example counts the number of files connected to a piece of content that are not yet uploaded or are being processing after upload.

Or so it seemed

Actually, the code counts the number of files connected to a piece of content that are not yet uploaded or any files that are being processed after upload. The "or" clause did not take the relationship between Content and File into consideration.

The Solution

To get the code to do, what we expected it to do, we had to nest the where and orWhere operations.

    function (Builder $query) { 
      $query->where('uploaded', false)
        ->orWhere('processing', true);

The Connection

And how did all this relate to the notifications not being sent?

Well, notifications are sent when content is published, and we recently made changes to wait until all files are uploaded and processed, before publishing content.

The code to check if all files are uploaded and processed malfunctioned as described above - if there was a single file anywhere in the system currently being processed (e.g. a video file being transcoded for playback on multiple types of devices) all content would "think" that there were files still being uploaded/processed and therefore it would not be published and no notifications were sent.

The Lesson

We have a large number of automated tests that are run before any deployment of new code to our production environment, so why did it not find this problem?

Automated tests only test the scenarios that developers write the tests for - and while we did have scenarios to test that content was published when all files were uploaded/processed and that it was not published if there were still files on the way, we lacked the scenario of another file for other content being processed at the same time.

We do have that scenario now.

by Jan Keller

Long time developer, architect and CTO with a real love for the backend. On this blog, I give out insight gathered through working with Vue, Nuxt, Laravel and more, when building and maintaining our SaaS.

Catch me on Twitter and Github

Restructuring a Laravel application using Domains, Actions and more

In coding tutorials, much is often left out in order not to be too verbose, to keep the complexity down or because it feels over engineering to follow through with the examples. I will give a full structural suggestion based on my experience that can be applied directly, regardless of the project size.

Jan Keller Jun 8, 2022

Mutation Testing - the missing link

Mutation testing can give your tests that extra confidence level not achieved simply by measuring code test coverage.

Jan Keller Dec 15, 2021

TDD without tests first approach

TDD is synonymous with writing tests first and implementations later. I want to challenge that approach as a catch-all do-all approach.

Jan Keller Dec 15, 2021

Lessons learned migrating to Laravel Vapor

When moving to Laravel Vapor, there are things to consider. In this article, I name the obstacles I have experienced in a recent project.

Jan Keller Jun 4, 2021