Improve Property populator

[enumag]

The Property populator currently can set values to private properties of the class itself but not to private properties of parent classes. It caused unexpected behaviour for me when I refactored one of my doctrine entitites to multiple entities with common parent.

Let me know if this is wanted in Alice. I'll add test in that case.

[tshelburne]

My personal opinion (and unrequested advice): A private property is private because it's not supposed to be accessible to the outside world - when I'm making fixture files, I always try to use them to build objects the way I intend them to be built in the application itself. Setting private properties is asking for a mess if you depend on them being private for state validity.

That said, the fact that private properties can be set on instances, but only on the child class, is silly. Add the test and we'll merge it in.

[enumag]

Thanks for your advice although I'm aware of it myself. I just don't know how else I can configure fixtures for my entities. If you can advice me about that I'd be greatful.

Here is the the problematic part of my entities:

/**
 * @ORM\Entity
 */
class Demand
{
    // some other properties + their getters & setters

    /**
     * @ORM\OneToOne( targetEntity = "DemandSchedule", inversedBy = "demand")
     * @var DemandSchedule
     */
    private $schedule;

    /**
     * @param bool $need
     * @return DemandSchedule
     */
    public function getSchedule($need = false)
    {
        if ($need && !$this->schedule) {
            $this->schedule = new DemandSchedule($this);
        }
        return $this->schedule;
    }
}

/**
 * @ORM\Entity
 */
class DemandSchedule
{
    // some other properties + their getters & setters

    /**
     * @ORM\OneToOne( targetEntity = "Demand", mappedBy = "schedule" )
     * @var Demand
     */
    private $demand;

    public function __construct(Demand $demand)
    {
        $this->demand = $demand;
    }

    /**
     * @return Demand
     */
    public function getDemand()
    {
        return $this->demand;
    }
}

I need a fixture for Demand (with schedule), currently I've this.

App\Entity\Demand:
    demand1:
        schedule: @schedule1 // this is the problem - setting private property
        // other fields

App\Entity\DemandSchedule:
    schedule1:
        __construct: false // to avoid error
        // other fields

I've tried it the other way around as well:

App\Entity\DemandSchedule:
    schedule1:
        __construct: [ @demand1 ]
        // other fields

The problem is that since Demand is the owning side of the relation it won't work - both entities are created correctly but the relation between them is not saved.

In retrospect the owning side probably should have been DemandSchedule but it would not be easy for me to change that now (I've the same problem on multiple places in my application).

Is there a way to configure such fixtures without relying on setting private properties / adding a setter Demand::setSchedule($schedule) / changing the relation owning side? It pretty much means creating @schedule1 entity by @demand1->getSchedule() method - which I don't know how to achieve.

[tshelburne]

Part of your struggle, I think, is in the fact that your getter is acting as a setter. You are attempting to make things work in ways that I personally would avoid. Your intuition in the last code snippet you pasted is correct - you really should be passing a demand to the demand schedule via the constructor. My suggestion, if it would be too much work to reorganize your data structure, would be to add a short reconciliation pass following creation of your entities. In other words, after you get your objects out, do something akin to the following (apologies for pseudo code, I've been in Purescript / Ruby world for months now):

$objects = $loader->load($fixtures);
foreach($objects as $object) {
  if ($object instanceof DemandSchedule) {
    // save relationship between $object and $object->getDemand();
  }
}

This may feel like a nasty solution, but it does solve your problem without requiring setting private properties, depending on a framework solution, or fixing a much larger problem (database design). In my projects that use Alice, I have many such reconciliation steps to deal with different ORMs, environmental requirements, and business rules.

[enumag]

Tanks for your advice, I think I'd rather use the improved property populator for now...

Test added. While working on it I also noticed that composer.lock is commited as well for some reason and that PHPUnit is way outdated so I fixed that as well.

[tshelburne]

@enumag Let's leave this as PluralProperties - I don't really see a benefit to making this change, and it makes it look important if it's part of the history. Also, it's marking the other class as a modification of the existing class, instead of as a new class. Just generally confusing. Make sense?

[enumag]

Sure, it just didn't make sense to use a class called PluralProperties for test that doesn't work with arrays. But as you wish, I'll change it back.

[enumag]

@tshelburne All done, can you review it again? :-)