Property container factory

[Mikulas]

I'm refactoring implementation of postgres composite types in our app for the third time. Final solution I've arrived at is generic PostgresCompositeType class that can parse and be serialized into pg composite type. Those are linked with IEntities via PostgresTypePropertyProxy which implements IPropertyContainer.

This PostgresTypePropertyProxy is same of all properties, but differs in which internal class it uses, which is set via constructor.

AbstractEntity::initProperty does not know what type it needs to create container with, so I though I would do something alongside those lines:

function __construct() {
    parent::__construct();
    $this->address = new PostgresTypePropertyProxy($this, $this->getMetadata()->getProperty('address'), AddressType::class);
}

however, this a/ does obviously not work, b/ but neither does setRawValue (which sets raw value of container, not the value one level above, which I need), c/ would be smelly anyway as the first container would be overwritten immediately.

What solutions I though of:

• [ ] ease AbstractEntity::initProperty visibility to protected (simple)
• [ ] replace new $class() in initProperty with call to injected property container factory (clean, but quite involved)
• [ ] replace new $class() in initProperty with call to protected method that would by default do the same as now (clean, simple)

[hrach]

Could you elaborate on the second possible solution? Sadly, I don't understand the beginning at all. However, I'm plased that such advanced things are almost possible :)

[Mikulas]

Also the last two solutions have the added benefit of removing

public function __construct(IEntity $entity, PropertyMetadata $propertyMetadata);

which currently makes it hard to extend.

---

I haven't really though it through with the injected container factory. I am however very much in favour of

call to protected method that would by default do the same as now

I will prepare a patch just to illustrate the point (without any expectations of merge-ability).

---

such advanced things are almost possible

Yeah very much so! GJ :)

[hrach]

Also the last two solutions have the added benefit of removing

Yes, removing the needed constructor parameters would be great!

[Mikulas]

Alright this is what I hacked up quickly: https://github.com/nextras/orm/compare/master...Mikulas:preview/container-factory

IInstantiableProperty is not a great name, but other that that it would greatly improve the usability of IProperty.

[hrach]

What about:

if (!is_subclass_of($class, IPropertyFactory::class, TRUE)) { 
     return $class::create(...);
} else {
     return $class(...);
}

by passing this as the first arg the factory will have an access to injected dependencies.

[Mikulas]

Or what if IProperty had public static create(IEntity, PropertyMetadata)? (It would have to anyway to allow this construct). I've though of that, but there is (as of php < 7) no guarantee what the method returns, there would have to be an instanceof check in private initProperty.

I'm not sure that would solve the original issue though. What I think I need is

class MyEntity extends Orm\Entity {
    protected function createPropertyContainer(PropertyMetadata $metadata) {
        if ($metadata->name === 'address') {
            return new PostgresTypePropertyProxy($this, $metadata, … my additional args …);
        }
        return parent::createPropertyContainer($metadata);
    }
}

[hrach]

I see, you desperately want to construct the property in context of the entity. In such case, I see no use for another interface. Let's just extract that functionality into own method. Ok?

[Mikulas]

Luckily @JanTvrdik found out I already can get everything from $metadata, so we don't really need this interface patch anymore to implement this.

I'm sending a patch with createPropertyContainer though.

[hrach]

Thanks.