Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

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

Open
jaapromijn opened this issue Dec 2, 2019 · 12 comments

Comments

@jaapromijn
Copy link

jaapromijn commented Dec 2, 2019

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 #1781: c49cd54#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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

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

@jaapromijn
Copy link
Author

jaapromijn commented Dec 3, 2019

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
Copy link
Contributor

@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
Copy link
Contributor

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

@jaapromijn
Copy link
Author

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
Copy link
Contributor

@pimjansen, I created #1995. I tried my best, but please ping me if you need it to be alterned.

pimjansen pushed a commit that referenced this issue May 11, 2020
Partoo added a commit to Partoo/Faker that referenced this issue Jun 21, 2020
* 'master' of github.com:Partoo/Faker: (294 commits)
  Fix (bug fzaninotto#1862): Do not clear entity manager for doctrine orm populator (fzaninotto#1995)
  Remove persian rude words. (fzaninotto#1879)
  Enhancement: Run builds on master using GitHub Actions
  Fix typo (fzaninotto#1927)
  Fix: Run build against PHP 7.4
  Fix: Aggregate badges in one place (fzaninotto#1866)
  Added changelog for the 1.9.1 release
  Fix: Reduce visibility of setUp() and tearDown() (fzaninotto#1821)
  Enhancement: Collect code coverage (fzaninotto#1824)
  Enhancement: Use all columns when running tests
  Add link to PHPStan extension to readme (fzaninotto#1834)
  Enhancement: Configure verbose output via phpunit.xml.dist
  Update master version
  Bump version to 1.9
  Curly braces for arrays is deprecated in PHP 7.4 (detected by PHPCompatibility)
  Updated indenting changelog
  Update changelog with 1.9.0 release
  Skipped lorumpixel test for the release where the service is pretty unstable
  Fix: Mark test classes as final
  Fix: Remove unnecessary class-level DocBlocks
  ...
@obstschale
Copy link

Is there any ETA when this fix will be released?

@jlekowski
Copy link
Contributor

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

mingalevme pushed a commit to mingalevme/Faker that referenced this issue Sep 29, 2020
GrahamCampbell pushed a commit to FakerPHP/Faker that referenced this issue Oct 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants