doctrine / data-fixtures

Doctrine2 ORM Data Fixtures Extensions
http://www.doctrine-project.org
MIT License
2.77k stars 224 forks source link

DependentFixtureInterface->getDependencies not allowed to be an empty array. #309

Closed bavragor closed 4 years ago

bavragor commented 5 years ago

Should be allowed to return an empty array. In our current project we have an Abstract Fixture which implements the interface, but sometimes children won't have any dependencies.

Could be fixed in Doctrine\Common\DataFixtures\Loader Line 257, instead of throwing an exception when the array is empty set $sequenceForClasses[$fixtureClass] = 0;

Should I open a PR for this?

alcaeus commented 5 years ago

Moving to doctrine/data-fixtures. Also, this has been discussed at length in https://github.com/doctrine/data-fixtures/pull/252 and it was decided to not allow this. I don't think any of the significant points changed since then. (ping @Ocramius @weaverryan @javiereguiluz)

javiereguiluz commented 5 years ago

@Bavragor in Symfony we used a trick to overcome this limitation. See: https://github.com/doctrine/DoctrineFixturesBundle/commit/3b56051149b0baf783849b70c21afd2c69afe047

alcaeus commented 5 years ago

That "trick" is long gone though, it was removed in 3.0 (see https://github.com/doctrine/DoctrineFixturesBundle/pull/209) in favour of not implementing DependentFixtureInterface by default. See https://github.com/doctrine/DoctrineFixturesBundle/pull/209#discussion_r153280533.

bavragor commented 5 years ago

Actually it is not gone. I'm using doctrine/doctrine-fixtures-bundle 3.1.0 and have a EmptyDependencyFixture as a workaround. It represents a problem for us because this kinda prevents fixture inheritance.

alcaeus commented 5 years ago

I meant that we removed it from the bundle and it's no longer included by default: as per the discussion in the linked issue, your fixture should implement the interface if it has dependencies and not otherwise. There are some edge cases but those did not warrant changing the logic at the time. IMO, none of that has changed.