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

SecondCacheLevel association and inheritance update problem #5793

Closed
mpoiriert opened this issue Apr 19, 2016 · 11 comments
Closed

SecondCacheLevel association and inheritance update problem #5793

mpoiriert opened this issue Apr 19, 2016 · 11 comments
Assignees
Milestone

Comments

@mpoiriert
Copy link

I have a entities like this (I just document the minimum needed):

/**
 * @Entity
 * @Cache("NONSTRICT_READ_WRITE")
 * @DiscriminatorColumn(name="type", type="string")
 * @DiscriminatorMap({"child" = "ChildClass"})
 */
class BaseClass
{
}

/**
 * @Entity
 * @Cache("NONSTRICT_READ_WRITE")
 */
class ChildClass extends BaseClass
{
  /**
  * @Cache("NONSTRICT_READ_WRITE")
  */
  private $associations;
}

When changing the ChildClass->associations value and persisting it the data in the cache is not removed.

Investigating I found that the Doctrine\ORM\Cache\Persister\Entity\AbstractEntityPersister is using the entityName (that will be ChildClass) but that the Doctrine\ORM\Cache\Persister\Collection\NonStrictReadWriteCachedCollectionPersister is using the $this->sourceEntity->rootEntityName (that will be BaseClass) So it doesn't clear the proper key entry.

Pretty easy to change but might be risky...

If you consider that I am on the good track I can give it a try to make the unit test for it.

@Metabor
Copy link
Contributor

Metabor commented May 9, 2016

Looks for me similar to the problem reported here:
#5562

@mpoiriert
Copy link
Author

Yes i think the the problem is the same thing since both is related to inheritance and second level cache.

@FabioBatSilva FabioBatSilva self-assigned this Jun 28, 2016
@duytbp
Copy link

duytbp commented Sep 27, 2016

Hi all, is there any update on this bug?

@lcobucci
Copy link
Member

I believe that #6028 fixes this.

@mpoiriert
Copy link
Author

No it doesn't

@lcobucci
Copy link
Member

lcobucci commented Sep 27, 2016

@mpoiriert weird, what kind of associations do you have? I can investigate this today.

@mpoiriert
Copy link
Author

mpoiriert commented Nov 1, 2016

Sorry for the delay really busy at the time.

This is the class override I have to fix the issue on my side

use Doctrine\ORM\Cache\CollectionCacheKey;
use Doctrine\ORM\PersistentCollection;

/**
 * This is to fix this bug:
 * @see https://github.com/doctrine/doctrine2/issues/5793
 */
class NonStrictReadWriteCachedCollectionPersister extends \Doctrine\ORM\Cache\Persister\Collection\NonStrictReadWriteCachedCollectionPersister
{
    /**
     * {@inheritdoc}
     */
    public function delete(PersistentCollection $collection)
    {
        $ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
        $key     = new CollectionCacheKey($this->sourceEntity->name, $this->association['fieldName'], $ownerId);

        $this->persister->delete($collection);

        $this->queuedCache['delete'][spl_object_hash($collection)] = $key;
    }

    /**
     * {@inheritdoc}
     */
    public function update(PersistentCollection $collection)
    {
        $isInitialized = $collection->isInitialized();
        $isDirty       = $collection->isDirty();

        if ( ! $isInitialized && ! $isDirty) {
            return;
        }

        $ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
        $key     = new CollectionCacheKey($this->sourceEntity->name, $this->association['fieldName'], $ownerId);

       // Invalidate non initialized collections OR ordered collection
        if ($isDirty && ! $isInitialized || isset($this->association['orderBy'])) {
            $this->persister->update($collection);

            $this->queuedCache['delete'][spl_object_hash($collection)] = $key;

            return;
        }

        $this->persister->update($collection);

        $this->queuedCache['update'][spl_object_hash($collection)] = array(
            'key'   => $key,
            'list'  => $collection
        );
    }
}

and

use Doctrine\ORM\Cache\CollectionCacheKey;
use Doctrine\ORM\PersistentCollection;

/**
 * This is to fix this bug:
 * @see https://github.com/doctrine/doctrine2/issues/5793
 */
class ReadWriteCachedCollectionPersister extends \Doctrine\ORM\Cache\Persister\Collection\ReadWriteCachedCollectionPersister
{
    /**
     * {@inheritdoc}
     */
    public function delete(PersistentCollection $collection)
    {
        $ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
        $key     = new CollectionCacheKey($this->sourceEntity->name, $this->association['fieldName'], $ownerId);
        $lock    = $this->region->lock($key);

        $this->persister->delete($collection);

        if ($lock === null) {
            return;
        }

        $this->queuedCache['delete'][spl_object_hash($collection)] = array(
            'key'   => $key,
            'lock'  => $lock
        );
    }

    /**
     * {@inheritdoc}
     */
    public function update(PersistentCollection $collection)
    {
        $isInitialized = $collection->isInitialized();
        $isDirty       = $collection->isDirty();

        if ( ! $isInitialized && ! $isDirty) {
            return;
        }

        $this->persister->update($collection);

        $ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
        $key     = new CollectionCacheKey($this->sourceEntity->name, $this->association['fieldName'], $ownerId);
        $lock    = $this->region->lock($key);

        if ($lock === null) {
            return;
        }

        $this->queuedCache['update'][spl_object_hash($collection)] = array(
            'key'   => $key,
            'lock'  => $lock
        );
    }
}

As you can see the only thing I had to change is to use

$this->sourceEntity->name

instead of

$this->sourceEntity->rootEntityName

I never did a pull request with that fix since I didn't have time to follow protocol and I did want to make sure it's working. We are using this fix since the 20th April

@lcobucci
Copy link
Member

@mpoiriert did you try your mapping with 2.6.x-dev?
The persisters must use the rootEntityName so your fix might break something else we build.

You could also send us the mapping you have so we can try it out and find the problem.

@lcobucci lcobucci assigned lcobucci and unassigned FabioBatSilva Jan 19, 2017
@lcobucci lcobucci added this to the 2.6.0 milestone Jan 19, 2017
@lcobucci
Copy link
Member

lcobucci commented Jan 19, 2017

@mpoiriert sorry, you were right that the problem wasn't solved.
I'm sending a PR today that fixes it for good 😉

@mpoiriert
Copy link
Author

Great thanks. We are planning to upgrade all our stack in the next couples of months so I'll be glad to remove all my factory/override when I'll pass to 2.6.0

@lcobucci
Copy link
Member

@mpoiriert nice, I'll close this issue since we merged #6244.
Would be amazing if you could test your mapping using 2.6.x-dev and report us any other issue.

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

No branches or pull requests

5 participants