php-activerecord icon indicating copy to clipboard operation
php-activerecord copied to clipboard

Model::__isset() to return same as __get()

Open silentworks opened this issue 14 years ago • 17 comments

I had a issue with using ActiveRecord with Twig Templating Engine due to the way how ActiveRecord looks up __isset(), I changed it to match the __get() method and it all seems to work fine now.

public function __isset($name)
    {
        // check for getter
        if (method_exists($this, "get_$name"))
        {
            $name = "get_$name";
            $value = $this->$name();
            return $value;
        }

        return $this->read_attribute($name);
    }

silentworks avatar Jun 08 '11 15:06 silentworks

I chose to solve this problem a different way.

... but I've since deleted the repo. whoops

terite avatar Jun 08 '11 19:06 terite

Thank you for that, this works perfectly. If anymore bugs show up when using ActiveRecord with Twig I will let you know.

silentworks avatar Jun 08 '11 21:06 silentworks

the above link is broken: https://github.com/terite/php-activerecord/commit/68bec6cd93786b26514a9d9ff37b54a67610ea2c

Having a proper test would be fine.

greut avatar May 07 '12 07:05 greut

Thank you for that ;)

guillemcanal avatar Feb 19 '13 10:02 guillemcanal

Thanks a lot!!!!

bshevchenko avatar Mar 29 '13 13:03 bshevchenko

Hmm... i take it back. set_timestamps() checks the updated_at and created_at attributes through the __isset method.

Everything will work fine if you change the set_timestamps() as follows:

public function set_timestamps()
{
    $now = date('Y-m-d H:i:s');

    if (isset($this->attributes['updated_at']))
        $this->updated_at = $now;

    if (isset($this->attributes['created_at']) && $this->is_new_record())
        $this->created_at = $now;
}

bshevchenko avatar Mar 29 '13 14:03 bshevchenko

This is really bad solution. After these changes PHPActiveRecord began throw ActiveRecordException("Call to undefined method: $method") every time I tried to get the value of an empty model attribute. I had to comment out throw and add "return null" instead in Model.php, __call method.

bshevchenko avatar Mar 29 '13 17:03 bshevchenko

I feel like you guys are complicating a lot. This is object oriented programming with a public interface!

You don't need to modify Model.php..., this ruins your ability to upgrade phpactiverecord. Create a subclass that all of your Models that are used in Twig inherit from, especially if you're not planning on writing tests.

and do something similar to this or modify it to what you need.

class TwigModel extends ActiveRecord\Model
 {
    public function __isset($attribute_name)
    {
        try
        {
            if($this->__get($attribute_name))
            {
                return true;
            }
            else
            {
                return false;
            }
        }
        catch(UndefinedPropertyException $e)
        {
            return false;
        }
        catch(ActiveRecordException $e)
        {
            return false;
        }
    }
 }

User.php

User extends TwigModel
{
   $normal phpactiverecord stuff
}

I don't think commenting out exceptions counts as a proper fix ;p.

anther avatar Mar 29 '13 19:03 anther

anther, if you use your decision, you will have to create two identical models for each case, except for the fact that they will inherit from other classes. One for views and one for controllers.

bshevchenko avatar Mar 30 '13 08:03 bshevchenko

Oh, sorry for inattention. I thought that your post contained a specific solution, not advice on the correct extension =)

And that's the way a working solution (using your advice):

use ActiveRecord\ActiveRecordException;

class TwigModel extends \ActiveRecord\Model
{
    public function __isset($attribute_name)
    {
        try
        {
            if($this->__get($attribute_name))
                return true;
            else
                return false;
        }
        catch(UndefinedPropertyException $e)
        {
            return false;
        }
    }

    public function __call($method, $args)
    {
        try
        {
            $result = parent::__call($method, $args);
        }
        catch(ActiveRecordException $e)
        {
            return null;
        }

        return $result;
    }
}

Simply inherit all the models from this class if you are going to use Twig.

bshevchenko avatar Mar 30 '13 09:03 bshevchenko

Hi, is this fix will ever be in this repo ? :) I fixed it in this way:

public function __isset($attribute_name)
    {
        if (array_key_exists($attribute_name,$this->attributes) || array_key_exists($attribute_name,static::$alias_attribute)) {
            return true;
        }
        try {
            $this->__get($attribute_name);
            return true;
        } catch (UndefinedPropertyException $e) {
            return false;
        }
    }

I'm using this version in many projects that use php-activerecord and there are no errors, but I want to keep it up-to-date so I can't still using my fork..

eps90 avatar Jan 18 '14 23:01 eps90

Another version that doesn't rely on catching exceptions and also doesn't call any (potentially expensive) getters.

    /**
     * Determines if an attribute exists for this {@link Model}.
     *
     * @param string $attribute_name
     * @return boolean
     */
    public function __isset($attribute_name)
    {
        // check for aliased attribute
        if (array_key_exists($attribute_name, static::$alias_attribute))
            return true;

        // check for attribute
        if (array_key_exists($attribute_name,$this->attributes))
            return true;

        // check for getters
        if (method_exists($this, "get_${attribute_name}"))
            return true;

        // check relationships if no attribute
        if (array_key_exists($attribute_name,$this->__relationships))
            return true;

        return false;
    }

Edit: note that $attributes and $relationships are private. Therefore it is not possible to use this through inheritance.

dfyx avatar Sep 08 '14 14:09 dfyx

And another one that works through inheritance (uses ReflectionProperty to access private properties)

class Model extends \ActiveRecord\Model {
    /**
     * Determines if an attribute exists for this {@link Model}.
     *
     * @param string $attribute_name
     * @return boolean
     */
    public function __isset($attribute_name)
    {
        if(parent::__isset($attribute_name)) {
            return true;
        }

        // check for getters
        if (method_exists($this, "get_${attribute_name}"))
            return true;

        // check relationships if no attribute
        $parent = new \ReflectionClass('ActiveRecord\Model');
        $relationships = $parent->getProperty('__relationships');
        $relationships->setAccessible(true);

        if (array_key_exists($attribute_name, $relationships->getValue($this)))
            return true;

        return false;
    }
}

dfyx avatar Sep 08 '14 15:09 dfyx

I just noticed that both of the above have a bug when accessing a relationship that hasn't been loaded yet. Here's a fix that works with inheritance and doesn't need reflections:

class Model extends \ActiveRecord\Model {
    /**
     * Determines if an attribute exists for this {@link Model}.
     *
     * @param string $attribute_name
     * @return boolean
     */
    public function __isset($attribute_name) {
        if(parent::__isset($attribute_name))
            return true;

        // check for getters
        if (method_exists($this, "get_${attribute_name}"))
            return true;

        // check for relationships
        if (static::table()->has_relationship($attribute_name))
            return true;

        return false;
    }
}

dfyx avatar Sep 29 '14 08:09 dfyx

@dfyx could you submit a PR for this?

cvanschalkwijk avatar Jan 09 '15 05:01 cvanschalkwijk

thanks @dfyx ! it works now with Twig.

nasrulhazim avatar Oct 14 '15 04:10 nasrulhazim

Aqui aparentemente funcionou tudo certinho. Obrigado a Anther commented on 29 Mar 2013.

renanconstancio avatar Nov 09 '16 14:11 renanconstancio