gabordemooij / redbean

ORM layer that creates models, config and database on the fly
https://www.redbeanphp.com
2.31k stars 279 forks source link

Unable to use __call() with Model #838

Closed benmajor closed 4 years ago

benmajor commented 4 years ago

For convenience, I'd sometimes like to use getters and setters to manipulate Beans. This allows us to extract out data management within a system to Repositories and maintain testable code.

For some reason, using the __call magic method on a Bean Model doesn't seem to work. For example, my Models all extend a base class (Entity), which looks like this:

<?php

namespace Example\API\Entity;

use Example\API\Exception\EntityException;

class Entity extends \RedBeanPHP\SimpleModel
{
  # Handle get* and set*:
  function __call($method, $params)
  {
    $start = substr($method, 0, 3);

    if( $start == 'get' || $start == 'set' )
    {
      $property = lcfirst(substr($method, 3));

      # It's a getter:
      if( $start == 'get' )
      {
        return $this->bean->$property;
      }

      # No need for else, can only be reached if setter:
      if( count($params) != 1 )
      {
        throw new EntityExcpetion('Setter method for '.$property.' expects 1 parameter.');
      }

      $this->bean->$property = $params[0];

      return $this;
    }
  } 
}

When we extend the base class, the __call method is never executed, for example:

$user = R::load('user', 1);

echo $user->getForename(); # never called
echo $user->forename;      # correctly returns 'Ben'

Similarly, calling a method that does not exist on a Bean Model does not leak an error or exception:

echo $user->nonExistentMethod(); # returns null

This is obviously a design feature in RedBean, but is there any way for us to still leverage __call() on a Model?

Lynesth commented 4 years ago

The way RedBean allows a bean to call its model's methods is by checking if the model has the specific function declared or not, using:

if ( !method_exists( $this->__info['model'], $method ) ) {

In your case, $method is equal to getForename, and since that method doesn't exist, it doesn't work.

We probably should modify it that way, actually:

if ( !is_callable( array( $this->__info['model'], $method ) ) ) {

Just need to make sure it doesn't break anything, but it probably shouldn't.

Meanwhile you can either:


Edit: Actually made a commit for that, let me know if d114877ad4977c1fd5e527ac1182960f90b88ea3 works for you.

benmajor commented 4 years ago

Thanks for fast reply, @Lynesth! I think that it would be much better to check against is_callable, as it would be nice to utilise the __call method as indicated.

I've just tried your latest commit, and it seems to work exactly fine. I don't think that there should be any implications to adding it to the core, and I look forward to seeing in the next release if you think it's worthwhile!

Thanks again :)