nextras / orm

Orm with clean object design, smart relationship loading and powerful collections.
https://nextras.org/orm
MIT License
309 stars 59 forks source link

Repository::getEntityClassNames #138

Closed f3l1x closed 9 years ago

f3l1x commented 9 years ago
    public static function getEntityClassNames()
    {
        // this works
        return [SearchPlace::class, Place::class];
       // this does not work
        return [Place::class, SearchPlace::class];
    }

    public function getEntityClassName(array $data)
    {
        if (isset($data['distance'])) {
            return SearchPlace::class;
        } else {
            return Place::class;
        }
    }

It throws Nextras\Orm\InvalidArgumentException (Undefined property App\Model\Orm\Places\Place::$distance), but entity is instance of SearchPlace.

It depends on the order?

hrach commented 9 years ago

Mentioned code does not throw anything. You probably missed the whole rest - the calling. Which call does cause this?

hrach commented 9 years ago

I suppose this the problem - if you don't provide class, the first possible entity metadata are used. https://github.com/nextras/orm/blob/bbe31658decee763346e8a5c04c4bd170ed82096/tests/cases/integration/Repository/repository.sti.phpt#L24-L25 In case of relationship, ORM always know which entity is targeted and therefore uses correct metadata.

f3l1x commented 9 years ago

@hrach I only do a ordering. Why is needed to provide class? :pensive:

hrach commented 9 years ago

Please stop asking and provide your entity definition and code which cause the exception ;)

f3l1x commented 9 years ago

Entities

/**
 * @property string $name
 */
class Place extends AbstractEntity
{
}

/**
 * @property float|NULL $distance
 */
final class SearchPlace extends Place {
}

Repository

    public function search(...)
    {
        $result = $this->mapper->search(...);
        return $this->mapper->toCollection($result);
    }

    public static function getEntityClassNames()
    {
        // this works
        return [SearchPlace::class, Place::class];
       // this does not work
        return [Place::class, SearchPlace::class];
    }

    public function getEntityClassName(array $data)
    {
        if (isset($data['distance'])) {
            return SearchPlace::class;
        } else {
            return Place::class;
        }
    }

Mapper

    public function search(...)
    {
        $builder = select/wheres/joins..
        return $resultset;
    }

Component

// without orderBy, it works nice
$this->facade->search(...)->orderBy('distance');
f3l1x commented 9 years ago

@hrach Sorry for closing, wrong button :smile:

hrach commented 9 years ago

Of course. Orm checks on "repository layoer" for columns existance, etc. So, orm needs to resolve distance proeprty in calling orderBy(distance). since you didn't specify the entity, it uses the first one defined in getEntityClassNames.

I was thinking about API that would allow to define the current working entity that you could force it in your mapper::search method.

f3l1x commented 9 years ago

Hmmm. So, like this?

->orderBy('SearchPlace->distance') 
hrach commented 9 years ago

Probably FQN is needed, but we may allow just class name without namespace.

f3l1x commented 9 years ago

Is allowed only class name?

hrach commented 9 years ago

No. It's quite low priority.