Change Alice API

[theofidry]

#601 (comment)

I think it would be more beneficial for the user to propose a new API to write down fixtures, e.g.:

<?php

use Is\Bundle\PlanBundle\Entity\Event;
use Nelmio\Alice\Alice;

return [
  Event::class => [
    'foo1' => [
      'title' => Alice::faker()->sentence(3),
      'show' => Alice::reference('@show_*'),
      'startDateTime': Alice::faker()->dateTimeBetween('-1 month', '+4 month'),
      'isDraft' => false,
      'version' => Alice::optional(10, Alice::reference('@version_*')),
      '__calls': [
        'setRevenue (25%?)': [Alice::faker()->moneyBetween(10000, 300000)]
      ],
    ],
    'foo2' => new Foo(),
  ],
];

There is three immediate advantages there:

• (Almost) no more YAML DSL to learn, things could easily be discoverable via that Alice class
• In some cases, you can ditch the Alice API altogether and just provide your object instantiated as a regular object (cf. foo2 in the example above)
• It could be introduced as a new feature in 3.x, providing a soft migration path for the users for 4.x (which can wait plenty of time for now at least)

[kgilden]

Just throwing out a suggestion of creating a light-weight DSL to make it even easier. I've seen developers getting confused with setRevenue (25%?): vs setRevenue: (25%?). Something along the lines of the following, perhaps?

<?php

use Is\Bundle\PlanBundle\Entity\Event;
use Nelmio\Alice\Alice;

return [
    Alice::fixture(Event::class)
      ->pattern('foo_{0..99}')
      ->constructor([Alice::reference('@show_*')])
      ->props([
        'isDraft' => false,
        'startDateTime' => Alice::faker()->dateTimeBetween('-1 month', '+4 month'),
      ])
      ->calls([
          'setRevenue' => Alice::sometimesCalled(0.25, [Alice::faker()->moneyBetween(10000, 300000)]),
          'setFoo' => Alice::sometimesNull(0.5, true),
      ])
];

Very rough and there's definitely room for improvement.

---

Also while we're at it, would it be crazy to abstract away whether the arguments are for the constructor, props or need to call a setter? The system could follow a convention of "constructor args" > "setters" > "properties via reflection". Just to show in code what I mean.

    /**
     * Creates a new object with the given values. Values must be a list mapped
     * by property names. Constructor arguments must also be passed inside $values.
     *
     *     class Ex { private $foo; public function __construct($bar) {}}
     *
     *     // Creates an instance of Ex with $foo set to 23 and $bar 'whatever'.
     *     $factory = new ObjectFactory('Ex');
     *     $ex = $factory->create('Ex', array(
     *         'foo' => 23,
     *         'bar' => 'whatever',
     *     ));
     *
     * @param array $values
     *
     * @return object
     *
     * @throws \RuntimeException If a required constructor argument is missing
     * @throws \Excetpion        If a property was not found
     */
    public function create(array $values = [])
    {
        $class = new \ReflectionClass($this->fqcn);

        $constructorArgs = [];

        if (($constructor = $class->getConstructor())) {
            foreach ($constructor->getParameters() as $arg) {
                if (isset($values[$arg->getName()])) {
                    $constructorArgs[] = $values[$arg->getName()];
                    unset($values[$arg->getName()]);
                } elseif ($arg->isDefaultValueAvailable()) {
                    $constructorArgs[] = $arg->getDefaultValue();
                } else {
                    throw new \RuntimeException(sprintf('Missing required constructor argument "$%s" for "%s".', $arg->getName(), $this->fqcn));
                }
            }
        }

        $object = $class->newInstanceArgs($constructorArgs);
        $this->setProperties($object, $values);

        return $object;
    }

    /**
     * Sets the properties of an object using reflection.
     *
     * @param object $object
     * @param array  $properties
     *
     * @throws \Exception If any of the properties was not found in the object
     */
    private function setProperties($object, array $properties)
    {
        foreach ($properties as $name => $value) {
            try {
                PropertyAccess::createPropertyAccessor()->setValue($object, $name, $value);
            } catch (NoSuchPropertyException $e) {
                if ($this->setProperty($object, $name, $value)) {
                    continue;
                }

                // Try snake case instead of camel case (i.e. "foo_bar" vs "fooBar").
                if ($name === ($altName = Inflector::tableize($name))) {
                    throw new \Exception(sprintf('Could not set "%s" to "%s" on "%s" using property access nor reflection.', $name, $value, get_class($object)), null, $e);
                }

                if ($this->setProperty($object, $altName, $value)) {
                    continue;
                }

                throw new \Exception(sprintf('Could not set "%s" or "%s" to "%s" on "%s" using property access nor reflection.', $name, $altName, $value, get_class($object)), null, $e);
            }
        }
    }

    /**
     * Sets the property of an object using reflection.
     *
     * @param object $object
     * @param string $name
     * @param mixed  $value
     *
     * @return bool Whether the property was found and successfully changed
     *
     * @throws \Exception If the property was not found in the object
     */
    private function setProperty($object, $name, $value)
    {
        $class = new \ReflectionClass($object);

        while ($class && !$class->hasProperty($name)) {
            $class = $class->getParentClass();
        }

        if ($class) {
            $property = $class->getProperty($name);
            $property->setAccessible(true);
            $property->setValue($object, $value);

            return true;
        }

        return false;
    }

[theofidry]

For the first suggestion 👍.

Also while we're at it, would it be crazy to abstract away whether the arguments are for the constructor, props or need to call a setter? The system could follow a convention of "constructor args" > "setters" > "properties via reflection". Just to show in code what I mean.

I don't think it's a good idea because:

• It involves a bit less transparency
• Since PHP doesn't allow bidirectional relationships at instantiation time, in order to link all the entities, Alice needs to do two passes:
  • 1 to instantiate all the fixtures
  • 2 to hydrate them (it actually also do a 3rd one IIRC)

cf. https://github.com/nelmio/alice/blob/master/CONTRIBUTING.md#generator for more on that part

[c33s]

-1 for deprecating yaml, as it removes the option to let non developers write fixtures

to abstract away whether the arguments are for the constructor, props or need to call a setter?

this is exactly what i would need also see #840 (comment)

[theofidry]

-1 for deprecating yaml, as it removes the option to let non developers write fixtures

I would argue that the current YAML is more oriented towards developers and falls short in that regard, hence a PHP alternative which would offer both a more robust and auto-discoverable API.

From what I can see in #840 (comment), even the current YAML is quite far from it. But although it would require some changes to the Fixtures objects, I think this could be achieved with custom data loader (see https://github.com/nelmio/alice/blob/master/CONTRIBUTING.md#architecture) and possibly bypassing completely the expression language which is the problematic part at the moment.

In other words: deprecating the current YAML as you know it does not mean you could not achieve #840 (comment)

[tacman]

I quite like the YAML format, in fact, I think Alice Fixtures are useful even when not leveraging all the cool dynamic options.

In particular, sometimes I just want to dump a small set of fixtures in YAML, then reload them unto another machine or fork. Basically, use the YAML files as an archive. But it's maddening that there's no way to represent a simple strings (like a URL) that contain special symbols, like $, #, /, etc.

[goetas]

I would keep yaml. In our projects, fixtures are written by non php-devs, often with minimal dev experience. forcing them to learn PHP is a no go for us...