fzaninotto / Faker

Faker is a PHP library that generates fake data for you
MIT License
26.78k stars 3.57k forks source link

Loaded entities in column formatter seem to be treated as new entities in 1.9.0 #1862

Open jaapromijn opened 4 years ago

jaapromijn commented 4 years ago

Summary

When upgrading from 1.8.0 to 1.9.0, using a collection of existing entities in a column formatter results in an error. It seems that the existing entities are now treated as new entities instead.

This seems to be caused by the addition in PR https://github.com/fzaninotto/Faker/pull/1781: https://github.com/fzaninotto/Faker/commit/c49cd5438655043b487f4d986961746ae58edb3a#diff-d38baf8c465e27d0cc5334cf5d8475aaR105

Versions

Version
PHP 7.3.10
fzaninotto/faker 1.9.0

Self-enclosed code snippet for reproduction

        $populator->addEntity(User::class, 4, [
            'roles' => function () use ($generator) {
                $roles = ['ROLE_CONTRACTOR', 'ROLE_USER', 'ROLE_CONTRACTOR_USER'];
                $key   = array_rand($roles);

                return [$roles[$key]];
            },
            'username' => function () use ($generator) {
                return $generator->unique()->userName().'@example.com';
            },
            'magicLinkToken' => function () use ($generator) {
                return md5('test');
            },
        ]);

        $populator->execute($manager);

        $users = $manager->getRepository(User::class)->findAll();

        $populator->addEntity(Status::class, 50, [
            'user' => function () use ($generator, $users) {
                $key = array_rand($users);

                return $users[$key];
            },
        ]);

        $populator->execute($manager);

Expected output

Actual output

  [Doctrine\DBAL\Driver\PDOException (23000)]
  SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'fabienne01@example.com' for key 'UNIQ_8D93D649F85E0677'
pimjansen commented 4 years ago

@jaapromijn i dont think this commit breaks the unique constraint here. It executes an extra flush on Doctrine ORm when it hits the batchsize.

However i see you grab a random user from your existing ORM here in a callback without using a generator?

jaapromijn commented 4 years ago

@pimjansen The $entityManager->clear($class); breaks the code as it was. When removing this call in the code, the code runs again without error.

The doctrine documentation says the following about clear:

When EntityManager#clear() is invoked, all entities that are currently managed by the EntityManager instance become detached.

Could you point me to an example of using the generator to return a random entity from the database?

pimjansen commented 4 years ago

@jaapromijn correct however since all is flushed already, clearing and detaching it should not be a problem? Can you explain the use case, since looking at the code you are quering all users and are populating those again? Isnt it a bit normal that it gives duplicates in that case?

jaapromijn commented 4 years ago

@pimjansen Thanks for your time. We are generating Status objects and assigning a (random) existing user (previously generated with the same populator) to those Status objects.

So for the Status.user field, a User object is returned from the collection.

pimjansen commented 4 years ago

@jaapromijn ok but where is then the unique constraint coming from? Since i dont see you use a check to prevent that?

jaapromijn commented 4 years ago

The users are being created before the statuses:

        $populator->addEntity(User::class, 4, [
            'roles' => function () use ($generator) {
                $roles = ['ROLE_CONTRACTOR', 'ROLE_USER', 'ROLE_CONTRACTOR_USER'];
                $key   = array_rand($roles);

                return [$roles[$key]];
            },
            'username' => function () use ($generator) {
                return $generator->unique()->userName().'@example.com';
            },
            'magicLinkToken' => function () use ($generator) {
                return md5('test');
            },
        ]);

        $populator->execute($manager);

A seperate execute has been called.

jlekowski commented 4 years ago

@pimjansen, I have the same issue as @jaapromijn. Having roughly something like:

$populator = new Populator($generator, $em);
$populator->addEntity(Group::class, 1);
$populator->addEntity(Member::class, 1, ['id' => 999]);
$insertedEntities = $populator->execute();

in 1.9.* generates:

Doctrine\ORM\ORMInvalidArgumentException: A new entity was found through the relationship 'App\Entity\Member#group' that was not configured to cascade persist operations for entity: App\Entity\Group@000000007c79355c00000000627ee445. To solve this issue: Either explicitly call EntityManager#persist() on this unknown entity or configure cascade persist this association in the mapping for example @ManyToOne(..,cascade={"persist"}). If you cannot find out which entity causes the problem implement 'App\Entity\Group#__toString()' to get a clue.

When I comment out the mentioned line $entityManager->clear($class);, it works fine:

// dump from Symfony:
// dd($insertedEntities);

^ array:2 [
  "App\Entity\Group" => array:1 [
    0 => App\Entity\Group^ {#312
      -id: 1
      -name: "Qui et sit itaque et fuga."
      -accountId: 579930900
      -managerName: "Aut illum dolorem et eum nesciunt id. Nostrum in et est eum vero. Fugiat consectetur impedit aut."
      -createdAt: DateTime @816801023 {#317
        date: 1995-11-19 17:10:23.0 UTC (+00:00)
      }
      -members: null
      -limits: null
    }
  ]
  "App\Entity\Member" => array:1 [
    0 => App\Entity\Member^ {#404
      -id: 999
      -group: App\Entity\Group^ {#312}
    }
  ]
]

on (roughly - kept the relevant properties only):

class Group
{
    /**
     * @ORM\OneToMany(targetEntity="Member", mappedBy="group")
     */
    private $members;
}
class Member
{
    /**
     * @ORM\ManyToOne(targetEntity="Group", inversedBy="members")
     * @ORM\JoinColumn(name="group_id", referencedColumnName="id", onDelete="cascade")
     */
    private $group;
}

A similar issue seems to be described here https://stackoverflow.com/questions/18215975/doctrine-a-new-entity-was-found-through-the-relationship.

pimjansen commented 4 years ago

Could someone create a patch PR for this? Lets revert the clear due to sidefx then.

jaapromijn commented 4 years ago

Sorry that I didn't get back to this. I adjusted the code of my fixture generator class in such a way this is no longer an issue for our project.

Our code contained multiple $populator->execute($manager); calls which was unnecessary, and actually generated more objects than intended. With only one execute call left, I just inspect the generated entities and reassign some when needed.

An example of how that would work:

$entities = $populator->execute($manager);
$this->reassignBooks($entities, $manager);

    /**
     * Re-assign books that are not linked to an author.
     */
    private function reassignBooks(array $entities, ObjectManager $manager): void
    {
        $authors = $entities[Author::class];
        $books   = $entities[Book::class];

        foreach ($books as $book) {
            if ($book->getUser()->getAuthor() === null) {
                $key    = array_rand($authors);
                $author = $authors[$key];
                $book->setUser($author->getUser());
                $manager->persist($book);
            }
        }
    }

This is a suitable solution because we don't generate really large datasets.

jlekowski commented 4 years ago

@pimjansen, I created https://github.com/fzaninotto/Faker/pull/1995. I tried my best, but please ping me if you need it to be alterned.

obstschale commented 4 years ago

Is there any ETA when this fix will be released?

jlekowski commented 4 years ago

@pimjansen, following up @obstschale question: any ETA on that?