Search code examples
phpormmodelincrementfuelphp

Fuelphp ORM unexpected results when adding to model property


I have a model that tracks views. It's a very basic model as you can see below. There are currently only 2 methods. One queries the database, and one to add to the views table.

The function that checks the table returns either a view object, or null. It's a static method called inside of the save_view method. If the save_view method returns null, a new view is built, but if it returns an object, the view property is pulled, and a 1 is added to it, then saved to the database.

Here is where it gets weird.

The new build works fine. It just initializes and saves a row with 1 view. If I use the view object that's already been initialized, and add 1 to the view, it is saving as 2. I have no idea what is going on.

If I interrupt the flow with Debug::dump($view); it saves by 1 as expected.

So, for example, if the row currently has 3 views, and I do $view->set('views', ($view->views + 1));, you would think that it would save as views = 4, but it's skipping ahead to views = 5. When interrupted it saves as views = 4.

I've tried this a whole variety of ways, nothing seems to be working. I can get them all to save, but I'm still getting the same unexpected results. Am I missing something basic here, is it possible the code is flowing too fast, or have I discovered a bug? Never seen anything like this before. Thanks for taking a look.

Here is the code:

<?php

class Model_View extends \Orm\Model {

    protected static $_properties = array(
        'id',
        'media_type',
        'media_id',
        'views',
    );
    protected static $_table_name = 'views';

    /**
     * Saves views to the database.
     * @param int $media_id The id of the Media
     * @param int $media_type The key value of the media
     * @return boolean
     */
    public static function save_view($media_id, $media_type) {
        try {
            // Check if view exists //
            $view = self::check_view($media_id, $media_type);

            if (!$view) {
                // If view doesn't exist, create a new row //
                $view = self::forge(array(
                            'media_type' => $media_type,
                            'media_id' => $media_id,
                            'views' => 1
                ));
            } else {
                //Debug::dump($view);
                // Update existing row //
                $view->set(array('views' => ($view->views + 1)));
            }
             // Save results //
             return $view->save();
        } catch (Exception $e) {
            Log::error($e, __METHOD__);
            return false;
        }
    }

    /**
     * Checks the database to see if there is already a matching row
     * for the selected media.
     * @param int $media_id Id of the media
     * @param int $media_type Key value of media
     * @return Model_View|null
     */
    private static function check_view($media_id, $media_type) {
        try {
            // Check database for matching row //
            $view = self::find('first', array('where' => array(
                            'media_id' => $media_id,
                            'media_type' => $media_type
                        )
            ));
            return $view;
        } catch (Exception $e) {
            Log::error($e, __METHOD__);
            return null;
        }
    }

}

For Anyone coming here for what seems like the same issue: I was mistaken, this was not an issue with the Fuelphp ORM. It was an issue with an HTTP request looking for my favicon. I was using a relative path, and it was generating an extra HTTP Request.

Please see the continuation of my issue and the resolution over at the Fuelphp Forums


Solution

  • I have tried that (latest 1.8/develop code), but I can not reproduce this. When I do this in a controller:

    Model_View::save_view(1, 'A');
    Model_View::save_view(1, 'A');
    Model_View::save_view(1, 'A');
    Model_View::save_view(1, 'A');
    Model_View::save_view(1, 'A');
    

    I get a record in the table containing

    1,A,1,5
    

    The only comment I would have about your code is that you should use 'static' and not 'self'. ORM depends a lot on late static binding, so that might cause problems further down the line. It doesn't cause your issue, since it works fine here, and I can't see anything in your code that would suggest otherwise. I would start by adding some Log::info() calls to it, and check your logs. Perhaps something is causing your code to be called twice?