Intializer Manager and DataFixture integration

[dantleech]

BC Break: Yes
Doc PR: symfony-cmf/symfony-cmf-docs#337

This PR fixes the issue about the initializer not being called when loading fixtures.

• New IntializerManager service which aggregates and executes Initializers
• Have placed a new PHPCRExecutor in this bundle - the old one is in data-fixtures

We can:

• Keep the PHPCRExecutor here (why not?)
• Move this executor to data-fixtures
• (in data-fixtures) insert the EventManager into the Executors and have them fire an event after purging.

Console looks like this:

$ php app/console doctrine:phpcr:fixtures:load --no-interaction 
  > purging database
  > Executing initializer: Doctrine\Bundle\PHPCRBundle\Initializer\GenericInitializer
  > Executing initializer: Doctrine\Bundle\PHPCRBundle\Initializer\GenericInitializer
  > Executing initializer: DTL\WebBundle\Initializer\SiteInitializer
  > loading DTL\WebBundle\DataFixtures\PHPCR\LoadPageData
  > loading DTL\WebBundle\DataFixtures\PHPCR\LoadBlockData
  > loading DTL\WebBundle\DataFixtures\PHPCR\LoadPostData

I think we should also add a getName method to the initializers -- but maybe later (or rather in 1.1)

[dbu]

this is a BC break (an acceptable one imo) - meaning it needs to be documented in the changelog.

[dantleech]

Why is it a BC break?

[dbu]

because we no longer define the doctrine_phpcr.initialize.initializers parameter. if somebody manually ran the initializer using this parameter, things will stop working for him.

i suggest we only change this thing in the 1.1 version to avoid troubles.

[dbu]

i agree with the direction this is taking. the initiializer manager makes sense to me.

i think it makes sense to keep the executor in data-fixtures, so that the lib can be used without symfony bundles. what we probably should do is extend the base executor here to overwrite the constructor and the purge method, to use the initializer. initializer is a phpcr-bundle feature, changing the executor in data-fixtures would also be a violation of concerns.

[dantleech]

There seems to be very little value in extending the base executor, we cannot call the parent execute method because it does both a purge and fixture load, so we have to overwrite it completely - and there is nothing else in the class really.

The only value might be in a testing by proxy - if we extend the data-fixtures executor here then at least it is in the class heierarchy and we are more likely to notice if it breaks (e.g. with a BC break in the API). But then it should be tested anywway.

We do however extend the AbstractExecutor which has most of the logic.

[dantleech]

Of course the other option is to add the even dispatcher to the data-fixtures executor. Maybe this can be done later.

[dbu]

i think you would not need to overwrite the execute method, but could overwrite the purge method to do the init after parent::purge() returns.

having the event manager in the executor and triggering something right before the first fixture is loaded sounds reasonable - what do other doctrines do? /cc @beberlei

[dantleech]

hmm, there is no purge method in the executor .. and moving this logic to the Purger would be bad as it should certainly be the Executor class which "executes" the initializer (and purgers should only purge)

[dbu]

i see that method. its in the AbstractExecutor: https://github.com/doctrine/data-fixtures/blob/master/lib/Doctrine/Common/DataFixtures/Executor/AbstractExecutor.php#L128

if you extend PHPCRExecutor you can still overwrite that method of the base class.

[dantleech]

Ah yes .. ok sounds good.

[dantleech]

ok -- updated have also updated docs as linked above. @dbu @lsmith77

[dbu]

looking good to me. can you please add a changelog entry saying that fixture loading now runs the initializer and that instead of a parameter doctrine_phpcr.initialize.initializers containing initializers, people should use the initializer manager servcie?
then its good to merge - i switched master to 1.1 now so thats fine.

[dantleech]

ok, changelog added, squashed and rebased...

[dbu]

tne => the

[dbu]

great, thanks a lot for this cleanup!

[dbu]

are you sure this is going to work in for example the sandbox? i think we now really need #97 and have to adapt the bundles. see for example https://github.com/symfony-cmf/cmf-sandbox/blob/master/src/Sandbox/MainBundle/DataFixtures/PHPCR/LoadSimpleCmsData.php#L27 - according to https://github.com/symfony-cmf/SimpleCmsBundle/blob/master/DependencyInjection/CmfSimpleCmsExtension.php#L144 this will evaluate to /cms and then we try to create the simplecms root node at /cms/simple - if we have an initializer creating an empty node at that location, fixture loading will fail.

[dantleech]

Ah ok, so /cms/simple is a Document - but there are no compatability problems with this merge now are there?

[dbu]

there is this BC break of that initializers now are run when they where not run. we have to adapt simplecmsbundle 1.1 to either check if /cms/simple exists and delete it otherwise, or to provide an odm initializer if we implement that.