Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.x — as no identity/no id values set. It cannot be added to the identity map. #777

Closed
iBasit opened this issue Aug 9, 2017 · 26 comments

Comments

@iBasit
Copy link

iBasit commented Aug 9, 2017

I have two entities.

user (entity 1)

  • id (primary key)
  • name

detail (entity 2)

  • user (primary key, which is from user entity)
  • address

When running fixtures with

detail_{1..10}:
    user:             "@user_<current()>"

I get the following error

[Doctrine\ORM\ORMInvalidArgumentException]
The given entity of type 'AppBundle\Entity\Detail' (AppBundle\Entity\Detail@0000000007f1464d000000006748ef3
e) has no identity/no id values set. It cannot be added to the identity map.

Please note that this was working fine on 2.x

Stackoverflow links
https://stackoverflow.com/a/30404503/75799

@theofidry
Copy link
Member

That sounds weird. There is indeed a difference at the persistence level for Alice 3.x: it has been moved to theofidry/AliceDataFixtures, but as you can see, it doesn't do much: https://github.com/theofidry/AliceDataFixtures/blob/master/src/Loader/PersisterLoader.php#L77.

Did you use a special config option in 2.x or something?

@iBasit
Copy link
Author

iBasit commented Aug 9, 2017

Nope, nothing special other than Custom Faker providers, which was easily switched over with new implementation.

Not sure what to do for above error, which is very important to fix. Any suggestions?

@theofidry
Copy link
Member

I've never seen that error before, did you find a way to reproduce it somewhere?

@iBasit
Copy link
Author

iBasit commented Aug 11, 2017

I have reproduced the problem at https://github.com/iBasit/alice-problem.git

Getting up and running is easy as 123.

  1. composer install
  2. bin/console database:create:database
  3. bin/console doctrine:schema:update --force

and then just test with bin/console hautelook:fixtures:load

image

I hope you can help on this. Thank you

@iBasit
Copy link
Author

iBasit commented Aug 13, 2017

Any luck how I can fix this?

@theofidry
Copy link
Member

Soz I'm a bit busy this w/we with my moving I couldn't check yet.

@iBasit
Copy link
Author

iBasit commented Aug 14, 2017

Off Topic
That must be soo much hassle arranging dates and movers van and help. If you haven't booked any van, then try https://www.deliveryd2d.com they have good customer service, which helps a lot, they take care of everything on low cost with a van.

Back on Topic
Please take a look at it when you get a chance, our work is being held because of this deadlock. Anyway, Thank you and appreciate all your help.

@theofidry
Copy link
Member

theofidry commented Aug 14, 2017

Off Topic

Thanks I'll check it out :)

Back on Topic

I'll try to have a proper look in the evening, but otherwise it will take longer.

Is it expected that Detail has no id? that looks weird. Also doctrine/orm#4584 might help

@iBasit
Copy link
Author

iBasit commented Aug 14, 2017

Thanks.

Yes detail is using parent primary key as the primary key of detail, rather than its own. It's one to one relationship.

@iBasit
Copy link
Author

iBasit commented Aug 15, 2017

I did have a look at the doctrine2 issue before I posted here, they head phpdoc error.

I was trying to dig in and I found the error is generated by following line
https://github.com/theofidry/AliceDataFixtures/blob/788b8364e5e7fdd527376c06863c4dce2550b456/src/Loader/PersisterLoader.php#L85

I'm not sure how to fix this issue, this issue did not exist previously on 2.x

@theofidry
Copy link
Member

Ok I pinned down the issue.

The issue is really what is described in the StackOverflow question. In you case what you need to do in raw PHP is:

$manager->persist($user);
$manager->flush();

$manager->persist($detail);
$manager->flush();

Now to make it work, there is several solutions. A first one would be to create your own PersisterLoader:

use AppBundle\Entity\Detail;
use Fidry\AliceDataFixtures\LoaderInterface;
use Fidry\AliceDataFixtures\Persistence\PersisterAwareInterface;
use Fidry\AliceDataFixtures\Persistence\PersisterInterface;
use Fidry\AliceDataFixtures\Persistence\PurgeMode;
use Fidry\AliceDataFixtures\ProcessorInterface;
use Nelmio\Alice\IsAServiceTrait;

/**
 * Loader decorating another loader to add a persistence layer.
 *
 * @author Jordi Boggiano <j.boggiano@seld.be>
 * @author Théo FIDRY <theo.fidry@gmail.com>
 *
 * @final
 */
/*final*/ class PersisterLoader implements LoaderInterface, PersisterAwareInterface
{
    use IsAServiceTrait;

    /**
     * @var LoaderInterface
     */
    private $loader;

    /**
     * @var PersisterInterface
     */
    private $persister;

    /**
     * @var ProcessorInterface[]
     */
    private $processors;

    /**
     * @param LoaderInterface      $decoratedLoader
     * @param PersisterInterface   $persister
     * @param ProcessorInterface[] $processors
     */
    public function __construct(LoaderInterface $decoratedLoader, PersisterInterface $persister, array $processors)
    {
        $this->loader = $decoratedLoader;
        $this->persister = $persister;
        $this->processors = (function (ProcessorInterface ...$processors) {
            return $processors;
        })(...$processors);
    }

    /**
     * @inheritdoc
     */
    public function withPersister(PersisterInterface $persister): self
    {
        return new self($this->loader, $persister, $this->processors);
    }

    /**
     * Pre process, persist and post process each object loaded.
     *
     * {@inheritdoc}
     */
    public function load(array $fixturesFiles, array $parameters = [], array $objects = [], PurgeMode $purgeMode = null): array
    {
        $objects = $this->loader->load($fixturesFiles, $parameters, $objects, $purgeMode);

        foreach ($objects as $id => $object) {
            if ($object instanceof Detail) {
                continue;   // Persist it at the second pass
            }

            foreach ($this->processors as $processor) {
                $processor->preProcess($id, $object);
            }
            $this->persister->persist($object);
        }
        $this->persister->flush();

        foreach ($objects as $id => $object) {
            if (!$object instanceof Detail) {
                continue;
            }

            foreach ($this->processors as $processor) {
                $processor->preProcess($id, $object);
            }
            $this->persister->persist($object);
        }
        $this->persister->flush();

        foreach ($objects as $id => $object) {
            foreach ($this->processors as $processor) {
                $processor->postProcess($id, $object);
            }
        }

        return $objects;
    }
}

As you can see there is two persist() pass. However this is not enough. Indeed the Doctrine persister ObjectManagerPersister does a clear() in the flush to mitigate some memory leaks. So that means you need to override it to not call the clear() call as otherwise when persisting the details, the User entity attached will be perceived as a new entity.

A cleaner way although a bit more hacky woud be to create your own persister and skip the Detail entity in the persist() and create a post-processor (they are executed just after the flush) to persist the details.

This IMO is a workaround however. It would be nice to tackle the issue at the core, but it requires to considerate more complex scenarios where you have a chain of 1to1 relationships for example.

@theofidry
Copy link
Member

Also note that if it was working in 2.x, that was by cheer luck.

@iBasit
Copy link
Author

iBasit commented Aug 15, 2017

I have tried the second solution with the above code. I got following error stating user is new entity and not the one that has already been persist/saved.

image

I have also committed the above code and pushed to the repo.

iBasit added a commit to iBasit/alice-problem that referenced this issue Aug 15, 2017
@theofidry
Copy link
Member

You missed that bit:

However this is not enough. Indeed the Doctrine persister ObjectManagerPersister does a clear() in the flush to mitigate some memory leaks. So that means you need to override it to not call the clear() call as otherwise when persisting the details, the User entity attached will be perceived as a new entity.

But I would rather recommend the second solution

iBasit added a commit to iBasit/alice-problem that referenced this issue Aug 15, 2017
@iBasit
Copy link
Author

iBasit commented Aug 15, 2017

I have overwritten the class, but on run time, it loads the overwritten class and then after loading all the datafixtures yml files it loads the old ObjectManagerPersister, which then gives an error.

I have committed the changes. Do you think there is still some other bug?

@theofidry
Copy link
Member

I need to take a proper look but I can't right now. If you override the service (i.e. declare you own service with the same service ID) it should work though.

@iBasit
Copy link
Author

iBasit commented Aug 15, 2017

I agree, If you check following service file which I committed on the the alice-problem git repo, it has same service id. it loads fine on first load, but on second load it takes the old service..

service names
iBasit/alice-problem@ef0cf73#diff-1728bfc1c274f341afc1a0275fca694d

@theofidry
Copy link
Member

What do you mean by second load? Just in case also double check you deleted the container

@iBasit
Copy link
Author

iBasit commented Aug 15, 2017

Sorry, what I mean is when it runs PersisterLoader it loads the new ObjectManagerPersister fine, but after loading all the datafixtures yml files, then it loads old ObjectManagerPersister, which uses persist and flush method. Not sure why its loading old ObjectManagerPersister after it has already loaded the new ObjectManagerPersister.

@theofidry
Copy link
Member

Sorry I forgot to post my message yesterday, see theofidry/alice-problem@14bf2e0.

Arguably there is some design issues:

  • Missing alias for persister in FidryAliceDataFixtures to easily override it
  • Add a persister factory for e.g. DoctrineOrmLoader in HautelookAliceBundle

Can't do much atm, but PRs are welcome :)

@iBasit
Copy link
Author

iBasit commented Aug 18, 2017

Thank you so much, this helped a lot. I had an issue with purger also, but manage to fix it.

@iBasit iBasit closed this as completed Aug 18, 2017
@theofidry
Copy link
Member

Let's keep the issue open as they are some changes to be made to make it easier :)

@theofidry theofidry reopened this Aug 18, 2017
@iBasit
Copy link
Author

iBasit commented Aug 18, 2017

Great! — Just heads up, if there is a relationship in sync with detail to other entities, this fix will effect them also, so one would have to persist them as well.

My example was user to affiliate and then also affiliate sub relationships

also on the note, --purge-with-truncate does not work at all (actually same problem here as before, setting and then unsetting...)

@theofidry
Copy link
Member

Moved the issue to the right repository: theofidry/AliceDataFixtures#50 :)

@Mahmoudz
Copy link

Here's a quick solution:

Edit the fixtures loader to skip certain keys, like this embeddable Point:

Infrastructure\Persistence\Doctrine\Extension\Point:
  local__blablabla:
    __construct: [53.2164587474, -0.9746272299]
    private function persistFixtures(array $fixtures, ObjectManager $objectManager)
    {
        $loader    = new AppNativeLoader();
        $objectSet = $loader->loadFiles($fixtures);

        foreach ($objectSet->getObjects() as $key => $object)
        {
            $this->addReference($key, $object);

            // skip keys that starts with 'local__' mainly used for embeddables
            if ($this->isLocal($key) === false)
            {
                $objectManager->persist($object);
            }
        }
        // Note: the flashing happens later when all fixtures (ORM/ODM) are persisted
    }

    private function isLocal($key) : bool
    {
        return strpos($key, 'local__') === 0;
    }

Here I choose local__ but you can customise it.

@theofidry
Copy link
Member

@Mahmoudz do note however that embeddables are properly ignored already. If that's not the case then it's a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants