Laravel 5.1 Eloquent ORM randomly returning incorrect relationship - *major update*

Question

I have a Laravel app that powers an ecommerce website with moderate traffic. This website allows people to place orders via the frontend, but it also has backend functionality for taking orders over the phone via a call centre.

An order is related to a customer, and a customer can optionally be a user - a user being someone with a login to the frontend. A customer with no user account is only ever created as a result of an order being taken via the call centre.

The issue I have encountered is very odd, and I believe might be some kind of Laravel bug.

It only occurs very occasionally, but what is happening is that when an order is taken via the call centre for a customer with no user account, an order confirmation is being sent out to a random user - literally random, as far as I can tell, just plucked out of the database despite no relation in the data.

These are the relevant parts of the models in the project:

class Order extends Model
{
    public function customer()
    {
        return $this->belongsTo('App\Customer');
    }
}

class Customer extends Model
{
    public function orders()
    {
        return $this->hasMany('App\Order');
    }

    public function user()
    {
        return $this->belongsTo('App\User');
    }
}

class User extends Model
{ 
    public function customer()
    {
        return $this->hasOne('App\Customer');
    }
}

These are the database migrations for the above (edited for brevity):

   Schema::create('users', function (Blueprint $table) {
        $table->increments('id');
        $table->string('first_name');
        $table->string('last_name');
        $table->string('email')->unique();
        $table->string('password', 60);
        $table->boolean('active');
        $table->rememberToken();
        $table->timestamps();
        $table->softDeletes();
    });

    Schema::create('customers', function(Blueprint $table)
    {
        $table->increments('id');
        $table->integer('user_id')->nullable->index();
        $table->string('first_name');
        $table->string('last_name');
        $table->string('telephone')->nullable();
        $table->string('mobile')->nullable();
        $table->timestamps();
        $table->softDeletes();
    });

    Schema::create('orders', function(Blueprint $table)
    {
        $table->increments('id');
        $table->integer('payment_id')->nullable()->index();
        $table->integer('customer_id')->index();
        $table->integer('staff_id')->nullable()->index();
        $table->decimal('total', 10, 2);
        $table->timestamps();
        $table->softDeletes();
    });

The logic that sends the order confirmation is within an event handler that gets fired after the order has been paid.

Here is the OrderSuccess event (edited for brevity):

namespace App\Events;

use App\Events\Event;
use App\Order;
use Illuminate\Queue\SerializesModels;
use Illuminate\Contracts\Broadcasting\ShouldBroadcast;


class OrderSuccess extends Event
{
    use SerializesModels;

    public $order;

    /**
     * Create a new event instance.
     *
     * @return void
     */
    public function __construct(Order $order)
    {
        $this->order = $order;
    }
}

As can be seen, this event is passed an Order model object.

Here is the event handler (edited for brevity):

/**
 * Handle the event.
 *
 * @param  OrderSuccess  $event
 * @return void
 */
public function handle(OrderSuccess $event)
{
    // set order to paid
    $order = $event->order;
    $order->paid = date('Y-m-d H:i:s');
    $order->save();

    if(!is_null($order->customer->user)) {

        App_log::add('customer_order_success_email_sent', 'Handlers\Events\OrderSuccessProcess\handle', $order->id, print_r($order->customer, true).PHP_EOL.print_r($order->customer->user, true));

        // email the user the order confirmation
        Mail::send('emails.order_success', ['order' => $order], function($message) use ($order)
        {
            $message->to($order->customer->user->email, $order->customer->first_name.' '.$order->customer->last_name)->subject('Order #'.$order->id.' confirmation');
        });
    }

}

There is a check to see if the $order->customer->user object is not null and, if true, an order confirmation is sent. If it is null (which it frequently is), then no confirmation is sent.

As can be seen from the above, I added a log to record the objects when an email is sent. Here is an example of an erroneous one (again, truncated for brevity):

App\Customer Object
(
[attributes:protected] => Array
    (
        [id] => 10412
        [user_id] => 
        [first_name] => Joe
        [last_name] => Bloggs
        [telephone] => 0123456789
        [created_at] => 2015-09-14 13:09:45
        [updated_at] => 2015-10-24 05:00:01
        [deleted_at] => 
    )

[relations:protected] => Array
    (
        [user] => App\User Object
            (
                [attributes:protected] => Array
                    (
                        [id] => 1206
                        [email] => [email protected]
                        [password] => hashed
                        [remember_token] => 
                        [created_at] => 2015-09-19 09:47:16
                        [updated_at] => 2015-09-19 09:47:16
                        [deleted_at] => 
                    )
            )

    )

[morphClass:protected] => 
[exists] => 1
[wasRecentlyCreated] => 
[forceDeleting:protected] => 
)

App\User Object
(
[attributes:protected] => Array
    (
        [id] => 1206
        [email] => [email protected]
        [password] => hashed
        [remember_token] => 
        [created_at] => 2015-09-19 09:47:16
        [updated_at] => 2015-09-19 09:47:16
        [deleted_at] => 
    )

[morphClass:protected] => 
[exists] => 1
[wasRecentlyCreated] => 
[forceDeleting:protected] => 
)

As you can see, there is no user_id for Customer, and yet Laravel has returned a User object.

What is more, if I manually fire the exact same OrderSuccess event, the above is not reproducible - it does not send the email, and it does not load a User object.

As I said before, this issue is happening very infrequently - there are on average about 40 orders a day via the call centre for customers with no user account, and the highlighted issue might only occur once or twice a week.

I'm not familiar enough with Laravel to know what might be the issue here - is it some form of model caching, a problem with Eloquent ORM, or some other gremlin in the system?

Any ideas appreciated - I may post this problem in the Laravel github issue tracker if it appears to be some form of bug.

Update Relating to some of the answers / comments put forward, I have tried to remove any potential Eloquent ORM issues, to retrieve the data manually, like so:

$customer = Customer::find($order->customer_id);
$user = User::find($customer->user_id);

if(!is_null($user)) {
    // send email and log actions etc
}

The above is still producing the same random results - unrelated users are being retrieved even when the customer has no user_id (it's NULL in this instance).

Update 2 As the first update did not help in any way, I reverted to using the original Eloequent approach. To try another solution, I took my event code out of the event handler, and placed it in my controller - I was previously firing by OrderSuccess event using Event::fire(new OrderSuccess ($order));, and instead I commented this line out and just placed the event handler code in the controller method:

$order = Order::find($order_id);

//Event::fire(new OrderSuccess ($order));

// code from the above event handler
$order->paid = date('Y-m-d H:i:s');
$order->save();

if(!is_null($order->customer->user)) {

    App_log::add('customer_order_success_email_sent', 'Handlers\Events\OrderSuccessProcess\handle', $order->id, print_r($order->customer, true).PHP_EOL.print_r($order->customer->user, true));

    // email the user the order confirmation
    Mail::send('emails.order_success', ['order' => $order], function($message) use ($order)
    {
        $message->to($order->customer->user->email, $order->customer->first_name.' '.$order->customer->last_name)->subject('Order #'.$order->id.' confirmation');
    });
}

The above change has been on the production site for over a week now - and since that change, there has not been a single instance of the issue.

The only possible conclusion I can reach is some kind of bug in the Laravel event system, somehow corrupting the passed object. Or could something else be at play?

Update 3 It seems I was premature to state that moving my code outside the event fixed the issue - in fact, via my logging, in the last 2 days I can see some more incorrect order confirmation were sent out (a total of 5, after almost 3 weeks of no issues).

I noticed that the user ids that had received the rogue order confirmations appeared to be incrementing (not without gaps, but still in ascending order).

I also noticed that each of the problem orders had been paid for via cash and account credit - most are cash only. I looked further into this, and the user ids are actually the ids of the related credit transactions!

The above is the first cast iron breakthrough in trying to resolve this issue. On closer inspection, I can see that the issue is still random - there are quite a few (at least 50%) orders that have been paid via account credit for customer's with no user account, but that have not caused incorrect emails to be sent out (despite the associated credit transaction id having a user id match).

So, the problem is still random, or seemingly so. My credit redemption event is triggered like so:

Event::fire(new CreditRedemption( $credit, $order ));

The above is called just prior to my OrderSuccess event - as you can see, both events are passed the $order model object.

My CreditRedemption event handler looks like this:

public function handle(CreditRedemption $event)
{
    // make sure redemption amount is a negative value
    if($event->credit < 0) {
        $amount = $event->credit;
    }
    else {
        $amount = ($event->credit * -1);
    }

    // create the credit transaction
    $credit_transaction = New Credit_transaction();
    $credit_transaction->transaction_type = 'Credit Redemption';
    $credit_transaction->amount = $amount; // negative value
    $credit_transaction->customer_id = $event->order->customer->id;
    $credit_transaction->order_id = $event->order->id;

    // record staff member if appropriate
    if(!is_null($event->order->staff)) {
        $credit_transaction->staff_id = $event->order->staff->id;
    }

    // save transaction
    $credit_transaction->save();

    return $credit_transaction;
}

The $credit_transaction->save(); is generating the id in my credit_transactions table that is somehow being used by Laravel to retrieve a user object. As can be seen in the above handler, I'm not updating my $order object at any point.

How is Laravel using (remember, still randomly, some < 50% of the time) the id of my newly created $credit_transaciton to populate the $order->customer->user model object?

Answer 1

I can't help you get to the root of whats causing your issue, but I can offer a possible work around based on the logic you provided in Update 1 of your question.

Original Logic

$customer = Customer::find($order->customer_id);
$user = User::find($customer->user_id);

if(!is_null($user)) {
    // send email and log actions etc
}

Revised Logic

Because a customers user_id can be null, it may be more effective to limit the returned customers to those who have a user_id. This can be achieved by using the whereNotNull() method. We can then go on to check if a customer was returned, and if so send the email, etc.

$customer = Customer::whereNotNull('user_id')->find($order->customer_id); 

if (!$customer->isEmpty()) { 
    // send email and log actions etc 
}

By not giving the application a chance to return a customer with a null user_id this should hopefully solve your issue, but unfortunately it doesn't shine any light on why it is happening in the first place.

Answer 2

Shouldn't your migrations have

->unsigned()

for example:

$table->integer('user_id')->unsinged()->index();

As mentioned in the Laravel Doc?

Laravel also provides support for creating foreign key constraints, which are used to force referential integrity at the database level. For example, let's define a user_id column on the posts table that references the id column on a users table. http://laravel.com/docs/5.1/migrations#foreign-key-constraints

Answer 3

I am not sure if your migrations are defined correctly specially when compared to your Models. If you are using belongsTo and hasOne relationship, you should use foreign key references in migrations.

Schema::create('customers', function(Blueprint $table)
    {
        $table->increments('id');
        $table->integer('user_id')->nullable();
        $table->foreign('user_id')->references('id')->on('users');

        $table->string('first_name');
        $table->string('last_name');
        $table->string('telephone')->nullable();
        $table->string('mobile')->nullable();
        $table->timestamps();
        $table->softDeletes();
    });



Schema::create('orders', function(Blueprint $table)
{
   $table->increments('id');
   $table->integer('payment_id')->nullable()->index();

   $table->integer('customer_id')->nullable();
   $table->foreign('customer_id')->references('id')->on('customers');
   $table->integer('staff_id')->nullable()->index();
   $table->decimal('total', 10, 2);
   $table->timestamps();
   $table->softDeletes();
    });

Now you will need to set this column whenever an actual user exists when the customer record is being created. But you don't actually have to set this column manually. You can do this below as you are using relationships:

Step 1: Save the customer first.

$customer->save();

Step 2: Now we will set the user_id on the customer if exists. To do this, you can get the user object in $user and then just call

$customer->user->save($user);

The code above will automatically set the user_id on customers table

Then I will check if user record exists in this way below:

$user_exists = $order->customer()->user();

if($user_exists)
{
    //email whatever
}