[Feature] Allow dependency injection

[internalsystemerror]

Further to https://github.com/API-Skeletons/zf-doctrine-data-fixture/issues/12, this approach implements a new Loader which overrides the createFixture method to obtain fixtures via the DataFixtureManager.

In order to ensure full coverage of this feature, I had to fix the tests somewhat. This has been tested with the lowest possible dependency versions allowed by composer as well as the latest.

• Convert arrays to shorthand syntax
• Enforce PSR-2
• Fix lowest dependency versions
• Ensure tests work with lowest & latest dependency versions
• Improve test suite class naming scheme for easier readability
• Make test suite modules PSR-4
• Add docblocks
• Add declare(strict_types=1); to all php files
• Add return types
• Improve to use class name as a constant where possible
• Add loader to allow use of dependency injection with dependent fixtures
• Update README.md

I apologise for the amount in this PR, it is mostly due to CS fixes. I can resubmit with a smaller changeset, however I thought I would offer the choice.

[TomHAnderson]

This is great. As you can see this is only PR#13 so it's good to have others taking an interest in this.

I'll review when the week starts and give you notes then.

[internalsystemerror]

I'm happy to help, I had a fork of this myself that I'd upgraded to support zf3, but I like your approach to remove Doctrine\Orm to a dev dependency, One thing I'd like to add to this is a test for Doctrine\Odm I suspect that The OrmExecutor and OrmPurger may have to switch depending upon the instance of the object manager.

Edit: I can also add a BC break test to this PR to ensure that using the standard Doctrine loader still works but for right now I'm off to bed! o/

[TomHAnderson]

Here's something I need help with. Zend is phasing out zend-mvc-console. I've known about this for some time but haven't taken any steps for it. Now would be a good time:

https://github.com/zendframework/zend-console/issues/34#issuecomment-307123395

[TomHAnderson]

For testing complex tools like this locally I use a docker approach: http://zf-doctrine-audit.readthedocs.io/en/latest/unittest.html If you want to add ODM support I suggest using this pattern for local testing and of course Travis has support for installing mongodb at test run.

[internalsystemerror]

Do you have a preference as to whether you go with zfcampus/zf-console or symfony/console? Also I haven't tried using docker with travis yet, is that possible? Or are you suggesting docker locally and travis for CI?

[TomHAnderson]

Docker is used to locally test an environment. Travis has the ability to configure an environment. So you're not using Docker on Travis. Docker is just for local development and testing.

I prefer symfony/console.

[TomHAnderson]

https://github.com/internalsystemerror/zf-doctrine-data-fixture/pull/1/files#diff-3864ffe37f3a1de39876ea62cae525f3R36

Will you help me understand the circumstance for which a fixture would not be in the DataFixtureManager?

[TomHAnderson]

I've pushed a coding standard to master. Please rebase and use that for phpcs.

[internalsystemerror]

done

[internalsystemerror]

If the order of the loaded fixtures is such that the dependent is loaded before the dependency, the standard doctrine loader will add it via $this->addFixture(new $class), in 1.3 this was updated to call $this->addFixture($this->createFixture($class)) with createFixture simply returning new $class. This PR overrides the standard Loader's createFixture method, to relay back to the DataFixtureManager to ensure that the responsibility for instantiating a fixture lies there.

[internalsystemerror]

I detail it in more detail here: https://github.com/API-Skeletons/zf-doctrine-data-fixture/issues/12

[TomHAnderson]

I up'd the doctrine/data-fixture to 1.3 today. Unit tests pass with your changes. Are you OK with this change?

[internalsystemerror]

Yep, I upped the version to 1.3 as part of this PR, I want to add another test before merging, to ensure there is no BC break.

[internalsystemerror]

Added the test, this uses the default doctrine loader so as to make sure that anyone using the standard method before this PR will continue to be able to do so.

The README is also updated to show the new usage. The composer.json changes (such as reduce zend-test to 3.0) is to ensure that when running the phpunit tests with the --prefer-lowest flag on composer, that the same dependency versions are installed (zend-servicemanager, zend-mvc) as if the --no-dev flag was set. Otherwise it would be possible to obtain versions via --prefer-lowest --no-dev that haven't been tested.

[internalsystemerror]

Travis needs to be updated to use the new phpcs.xml.dist

[internalsystemerror]

There is still the line in the README that says not to use dependency injection. How would you like to reword this? I can add that the api-skeleton loader is optional and only required if using dependency injection? Or I can simply remove the line.

[TomHAnderson]

Remove it

[internalsystemerror]

done

[TomHAnderson]

I'm going to merge this because it passes unit tests and we need to move to smaller PRs.

It's great to see this library getting so much attention. The contribution is appreciated!

[internalsystemerror]

Yeah thats my bad due to the CS changes, PHPStorm likes to get a bit eager with my own projects, should probably turn that off for the future... Thanks for merging, I'll look at what we can do about zend-mvc-console for the future.