Fixture lifecycle

[theofidry]

Calling for validation @tshelburne & @Seldaek as it's a big userland change. Short story long, it's a review of the current process and what I would like to change for v3. Main changes are support for multiple calls, external factories

/cc @soullivaneuh @ogizanagi

Points to review:

• multi call support (Add the factory keyword #729)
• unique values with function calls
• factory (Add the factory keyword #729)
• [ ] accessors
• calls (Add the factory keyword #729)
• configurator

Currently in v2

• the object described by fixture is instantiated
• the instantiated object is populated

Instantiation process

If no __constructor option is given, the most appropriate constructor will be searched in the following order:

1. the class public constructor (assuming it doesn't have any required arguments)
2. the class private constructor (assuming it doesn't have any required arguments)
3. the class protected constructor (assuming it doesn't have any required arguments)
4. without constructor and arguments (via reflection ofc)

It is possible to change that behaviour by specifying constructor arguments, which allows the user to:

• force the instantiation by passing the constructor, (using the value false)
• use a factory: is supported static call with a simplified syntax if the factory class is the same as the fixture one
• specify arguments

Population process

For each of the property described, the more suitable accessor to set the property is looked for in the following order:

• if the value is an array, look for an adder with some variants
• custom setter if one has been added
• set the property directly
• look for a setter

Accessing properties

While generating values, some may be a reference to a fixture property, e.g. owner: '@user1->username'. This is done by looking at the first appropriate accessor (direct or getter)

Limitations:

• partial support for calls after instantiation (Multiple method call (adders) #313) (is done withing the population process)
• no support for unique flag with the constructor
• partial support for unique flag for arrays (Unique values handling #387)
• no support

v3

After "population" step

To handle the limitation described in #313, a new step is added in the cycle:

• the object described by fixture is instantiated
• the instantiated object is populated
• (new) the instantiated and populated object is configured

This allows to add full support for calling methods. The old call method feature is deprecated in v2 and removed in v3. The syntax will be:

Nelmio\Entity\User:
    user1:
       __calls:
            - setLocation: [0.49, 0.5]

⚠️ There is a behaviour difference with the old syntax:

Nelmio\Entity\User:
    user1:
       setLocation: [0.49, 0.5]

As in v3, the call setLocation will be done after the object has been fully populated (before was part of the "population" process).

Unique values support for function calls

For both function calls and constructor, unique values should be able to be specified like so:

Nelmio\Entity\User:
    user{1..10}:
        __construct:
            0 (unique): <username()> # the index does not matter, it is just here for adding a flag

🚨 Problem: it becomes hard to distinguish when the key is an index for a flag and when it is a constructor method.

Proposal:

• A. Check for the value: it is not that hard to tell the difference between '0 (unique)' and 'create'.
• B. Deprecate __construct for being used as a factory and introduce another factory option. See below for further description.

Factory support

If accepted, this would deprecate the usage of __construct used as a factory which is a BC change in userland 🚨.

Syntax:

FQCN:
    fixture_reference:
        __factory:
            - methodName
            - [ ...args ]

Example:

Nelmio\Entity\User:
    user1:
        __factory: [ create, [ <username()> ] ]

It would benefit of the unique flag as well:

Nelmio\Entity\User:
    user{1..10}:
        __factory:
            create:
              0 (unique): <username()>

Supporting static factories would be easy (it is an already supported feature with __construct):

Nelmio\Entity\User:
    user1:
        __factory:
            Nelmio\Entity\UserFactory::create: [ <username()> ]

🔥 New feature: supporting non-static factories, i.e. an injected service:

Nelmio\Entity\User:
    user1:
        __factory:
            @user.factory::create: [ <username()> ] # the mask must be @serviceName::methodName

Accessors

The __set option could be deprecated in favour of allowing the user to specify accessors for some properties:

Nelmio\Entity\User:
    user1:
        username: <username()>
        name1: <(strtolower($username))>
        name2: <(strtolower(@self->username))>
        __accessors:
            username:
                 read: get_user_name
            name1:
                 write: set_name_1
            name2:
                 write: withName2

As shown in the example above, __accessors could help to specify read/write accessors on a property basis. If none is declared, the old behaviour is preserved, i.e. looking for the most suitable one.

As a bonus, immutable mutators could be supported (would be consistent with the support of call methods done on immutable objects cf. #313 (comment)).

Note that there is a subtlety here:

• $username refers to the value generated for the username property, which may differ from object property if it's passing by a setter altering the input
• @self->username will ignore the value generated for the username property and instead will look for an accessor

Optional method calls

As expressed in #249, it would be a nice feature to have optional method calls:

Nelmio\Entity\User:
    user1:
        __calls:
          - setLocation: 80%? [40.689269, -74.044737]

i.e. there is a probability of 80% that the setLocation method is called with the given parameters. But the sad thing is that it would be impossible to do with the array syntax:

Nelmio\Entity\User:
    user1:
        __calls:
          - setLocation:  80%?  # would generate an error when parsing the YAML
            - 40.689269
            - -74.044737

Proposal:

• A. Go with it, would limit this feature to the inline syntax
• B. put that as a flag instead (but is inconsistent with the syntax for properties):

Nelmio\Entity\User:
    user1:
        __calls:
          - setLocation (80%?):
            - 40.689269
            - -74.044737

Configurators (i.e. external service calls)

Update:

See #629 (comment) and #629 (comment).

With the support of multiple method calls and factory with external services, it would make sense to support configurators as well

Nelmio\Entity\User:
    user{1..10}:
        __configurator: @user.configurator::config # the mask must be @serviceName::methodName

Obviously support for static calls could be provided. The only thing expected from a configurator method is to take the object as an input and return the configured object.

⁉️ Question: this ask for another step in the cycle, should it be done before or after the call step?

[tshelburne]

Just my opinions:

1. I support multi-calls post construction
2. Would __construct still be supported as is, and you are adding __factory?
3. Indifferent about accessors - that seems like it's pushing the simplicity of the library, but if people want it, perhaps it's helpful.
4. Restrict to inline
5. Configurator should be after call step - that makes the lifecycle increase in complexity as it moves along, rather than jumping around.

[theofidry]

Would __construct still be supported as is, and you are adding __factory?

If we decide to add __factory yeah, it would just reduce __constructor to specify:

• don't use the constructor, i.e. false
• specify the arguments to give to the __construct function

Tbh I don't feel strongly about this, if you feel this should stay in __construct I wouldn't mind but that could make it more logical. Another thing is maybe we could support both at first and deprecate this usage of __construct, as it would not be possible to change this in v2 this would make the migration easier from a user POV.

Indifferent about accessors - that seems like it's pushing the simplicity of the library, but if people want it, perhaps it's helpful.

Let's keep it as an idea then. I would prefer to finish the first alpha version of v3 before considering to add this.

Configurator should be after call step - that makes the lifecycle increase in complexity as it moves along, rather than jumping around.

👍

Thanks for taking the time to take a look at it @tshelburne

[tshelburne]

I think we should keep __construct and simplify it as you said - given that, I think the __factory syntax makes a lot of sense. I don't see a reason to deprecate __construct at all - the syntax is meaningful, clear, and probably handles most use-cases well. We don't want to force downstream devs to adhere to our idea of good design, but I think encouraging simplicity and clarity through time-tested interfaces is a good idea.

BTW, one note - the library is changing so rapidly and has evolved so much that I can't really keep up with it - I've reduced my notifications to direct @ mentions only so that I'll see the meaningful parts instead of losing it in the bulk. Let me know if it seems to cause problems, but it seems the only reasonable way for me to keep up with it, TBH.

[theofidry]

I don't see a reason to deprecate __construct at all

I meant deprecating the usage of __construct as a factory, i.e. specifying the static constructor method. To be more clear:

Nelmio\Entity\User:
    user{1..10}:
        __construct: # deprecate that for the factory statement below
            create:
              0 (unique): <username()>
        __factory:
            create:
              0 (unique): <username()>

But having __construct would still be perfectly valid:

Nelmio\Entity\User:
    user{1..10}:
        __construct:
              - 0 (unique): <username()>

[theofidry]

BTW, one note - the library is changing so rapidly and has evolved so much that I can't really keep up with it - I've reduced my notifications to direct @ mentions only so that I'll see the meaningful parts instead of losing it in the bulk. Let me know if it seems to cause problems, but it seems the only reasonable way for me to keep up with it, TBH.

Perfectly ok with me :)

[theofidry]

Regarding the Optional method calls problem:

As expressed in #249, it would be a nice feature to have optional method calls:

Nelmio\Entity\User:
    user1:
        __calls:
          - setLocation: 80%? [40.689269, -74.044737]

i.e. there is a probability of 80% that the setLocation method is called with the given parameters. But the sad thing is that it would be impossible to do with the array syntax:

Nelmio\Entity\User:
    user1:
        __calls:
          - setLocation:  80%?  # would generate an error when parsing the YAML
            - 40.689269
            - -74.044737

Proposal:

• A. Go with it, would limit this feature to the inline syntax
• B. put that as a flag instead (but is inconsistent with the syntax for properties):

Nelmio\Entity\User:
    user1:
        __calls:
          - setLocation (80%?):
            - 40.689269
            - -74.044737

Having it as a value complicates a bit things to be honest:

• It means altering the call "configuration" after evaluating the arguments
• Technically speaking, it's not the same as for properties [1]

[1]: in the case of properties, '80%? foo: bar' is "have 80% of chances to be'foo'" and otherwise if'bar'`. Whereas here we are talking of "have 80% of chances to call this function". Actually we could extend it to properties as well:

Nelmio\Entity\User:
    user1:
        username: '80%? foo'
    user2:
        username (80%?): '50%? foo'

The difference between the two is that there is 20% of chances that username will be set to null for user1 (by eventually calling a setter, which may assign another value...), whereas for user2 there is 20% of chances that username is not set at all (mutator or not).

[Evertt]

Hey will the optional methods ever get implemented? 🙏

[theofidry]

I'm not really interested in actively implementing new features for alice. So no unless someone is willing to step up to contribute to it

[Evertt]

Too bad, but I get it.