doctrine / data-fixtures

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

Fixing foreign key constraints error #231

Closed mgonyan closed 8 years ago

mgonyan commented 8 years ago

this PR fixes https://github.com/doctrine/data-fixtures/issues/230

Ocramius commented 8 years ago

@mgonyan fix seems good, but isn't backed by test scenario...

peterrehm commented 8 years ago

Awesome. Was investigating in that direction but then ran out of time.

mgonyan commented 8 years ago

Working on it

mgonyan commented 8 years ago

@Ocramius I've added the two different test scenario that covers:

  1. Checks Node dependencies have not been changed, testCommitOrder
  2. Checks purge remove table in the right order, testPurgeTableCorrectOrder
peterrehm commented 8 years ago

It seems the fix needs still some adjustments as I am now getting a CircularReferenceException on the purge. This now happens on a simple relation.

mgonyan commented 8 years ago

@peterhorne could you please give me more details about or an example what's causing a CircularReferenceException in order to add it to the test suite

peterrehm commented 8 years ago

I have a Customer, a ContactPerson and a User entity. The relationship is Customer -> Contactperson -> User. I get the CircularReferenceException between Customer and User.

mgonyan commented 8 years ago

@peterrehm thank you for the example, but I am afraid, I could not find a way to reproduce the issue. I added a test with a similar scenario and it is not failing for me. @Ocramius what do you think?

bendavies commented 8 years ago

This PR fixes most of my issues, but I also get a CircularReferenceException for my entities. I have a fix however. I'll open a PR based on this one.

mgonyan commented 8 years ago

@bendavies could you please tell me how your entities look like and their relationships in order to have a solid test, adjust this PR, thank you

bendavies commented 8 years ago

Sure, I won't open a PR then.

you need this:

Single table inheritance: Parent -> Child

Parent ManyToOne Parent Parent ManyToOne Organisation

That should get you the circular exception,

then to fix it, at https://github.com/mgonyan/data-fixtures/blob/63c795a2591e2f54bfaefe6ee8de5f8f85849133/lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php#L147

if (isset($association['inherited'])) {
    continue;
}

which prevents duplicating the Organisation association on both Parent and Child.

Let me know if that works for you.

peterrehm commented 8 years ago

@mgonyan Can only look at it in detail on monday.

mgonyan commented 8 years ago

Thank you @peterrehm and @bendavies, i've been able to reproduce the issue adding a simple table inheritance as you pointed out, please have a look at it a tell me what you guys think.

bendavies commented 8 years ago

nice one!

bendavies commented 8 years ago

PR looks good. has fixed all my issues.

peterrehm commented 8 years ago

Looks like it does not yet fix my inheritance issue. I am having a Single-Table-Inheritance of BaseUser -> and User. And then the relation is as mentioned above. Customer -> ContactPerson -> User. I am getting CircularReference errors between Customer and User.

mikeSimonson commented 8 years ago

@peterhorne Can you make a PR with a failing test ?

mgonyan commented 8 years ago

@mikeSimonson I have a test that makes the PR failed, However, I am not sure how to solve that issue, EntityA -> EntityB -> EntntyC -> EntityA, I'll push it later

peterrehm commented 8 years ago

@mgonyan Any updates?

mikeSimonson commented 8 years ago

@mgonyan Can you push your test ?

mgonyan commented 8 years ago

@mikeSimonson @peterrehm sorry for the delay, can you please guys have a look at the changes. I am still not sure about the circular reference issue with single table reference.

mikeSimonson commented 8 years ago

@mgonyan The 2 tests that fail now illustrate the BC break right ? testCommitOrder and testPurgeTableCorrectOrder.

mgonyan commented 8 years ago

@mikeSimonson yes I do, if this is the bug which @peterrehm mentioned before I will try to fixing it asap However cycles are possible due to a bad Entity Relation design or particular business requirement. I think this PR https://github.com/doctrine/data-fixtures/pull/233 could help to give us a clue where it is the cycle and rethink the relation model.

mikeSimonson commented 8 years ago

@mgonyan Thanks for your help

mgonyan commented 8 years ago

thank u too. @mikeSimonson , WDYT @peterrehm ?