Strange dirty behavior in PersistentCollection

mmoreram commented 8 years ago

Scenario

Using CTI with a ManyToMany relationship in the abstract class (Not sure if is related...)
All entities involved in this scenario are persisted and flushed previously (fixtures)
In this field I have a valid Collection with some entities (Categories)
I do flush($product) of the main entity (Product)
I do flush($product) again of the main entity.
An exception is thrown.

An exception occurred while executing 'INSERT INTO purchasable_category
(purchasable_id, category_id) VALUES (?, ?)' with params [6, 1]:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate
entry '6-1' for key 'PRIMARY'

The mapping code

We have the Purchasable mapping file (abstract one)

Elcodi\Component\Product\Entity\Purchasable:
    type: entity
    repositoryClass: Elcodi\Component\Product\Repository\PurchasableRepository
    table: purchasable
    inheritanceType: joined
    discriminatorColumn:
        name: purchasable_type
        type: string
    id:
        id:
            type: integer
            generator:
                strategy: AUTO

    fields:
        name:
            column: name
            type: string
            length: 255
            nullable: true
        sku:
            column: sku
            type: string
            length: 255
            nullable: true
        slug:
            column: slug
            type: string
            length: 255
            nullable: true
        shortDescription:
            column: short_description
            type: string
            length: 255
            nullable: true
        description:
            column: description
            type: text
            nullable: true
        showInHome:
            column: show_in_home
            type: boolean
            nullable: true
        dimensions:
            column: dimensions
            type: string
            length: 255
            nullable: true
        stock:
            column: stock
            type: integer
            nullable: true
        price:
            column: price
            type: integer
            nullable: true
        reducedPrice:
            column: reduced_price
            type: integer
            nullable: true
        height:
            column: height
            type: integer
            nullable: true
        width:
            column: width
            type: integer
            nullable: true
        depth:
            column: depth
            type: integer
            nullable: true
        weight:
            column: weight
            type: integer
            nullable: true
        imagesSort:
            column: images_sort
            type: string
            length: 2048
            nullable: true
        metaTitle:
            column: meta_title
            type: string
            length: 255
            nullable: true
        metaDescription:
            column: meta_description
            type: string
            length: 255
            nullable: true
        metaKeywords:
            column: meta_keywords
            type: string
            length: 255
            nullable: true
        createdAt:
            column: created_at
            type: datetime
            nullable: false
        updatedAt:
            column: updated_at
            type: datetime
            nullable: false
        enabled:
            column: enabled
            type: boolean
            nullable: false

    manyToOne:
        principalCategory:
            targetEntity: Elcodi\Component\Product\Entity\Interfaces\CategoryInterface
            joinColumn:
                name: principal_category_id
                referencedColumnName: id
                nullable: true
                onDelete: "SET NULL"
        principalImage:
            targetEntity: Elcodi\Component\Media\Entity\Interfaces\ImageInterface
            joinColumn:
                onDelete: SET NULL
                name: principal_image_id
                referencedColumnName: id
                nullable: true
        priceCurrency:
            targetEntity: Elcodi\Component\Currency\Entity\Interfaces\CurrencyInterface
            joinColumn:
                 name: price_currency_iso
                 referencedColumnName: iso
                 nullable: true
        reducedPriceCurrency:
            targetEntity: Elcodi\Component\Currency\Entity\Interfaces\CurrencyInterface
            joinColumn:
                 name: reduced_price_currency_iso
                 referencedColumnName: iso
                 nullable: true
        manufacturer:
            targetEntity: Elcodi\Component\Product\Entity\Interfaces\ManufacturerInterface
            inversedBy: purchasables
            joinColumn:
                name: manufacturer_id
                referencedColumnName: id
                nullable: true
                onDelete: "SET NULL"

    manyToMany:
        images:
            targetEntity: Elcodi\Component\Media\Entity\Interfaces\ImageInterface
            joinTable:
                name: purchasable_image
                joinColumns:
                    product_id:
                        referencedColumnName: id
                        onDelete: "CASCADE"
                inverseJoinColumns:
                    image_id:
                        referencedColumnName: id
                        onDelete: "CASCADE"

        categories:
            targetEntity: Elcodi\Component\Product\Entity\Interfaces\CategoryInterface
            inversedBy: purchasables
            joinTable:
                name: purchasable_category
                joinColumns:
                    purchasable_id:
                        referencedColumnName: id
                        onDelete: "CASCADE"
                inverseJoinColumns:
                    category_id:
                        referencedColumnName: id
                        onDelete: "CASCADE"

    lifecycleCallbacks:
        preUpdate: [loadUpdateAt]
        prePersist: [loadUpdateAt]

And we have the specific Product mapping file (Extends Purchasable)

Elcodi\Component\Product\Entity\Product:
    type: entity
    repositoryClass: Elcodi\Component\Product\Repository\ProductRepository
    table: product

    manyToOne:
        principalVariant:
            targetEntity: Elcodi\Component\Product\Entity\Interfaces\VariantInterface
            joinColumn:
                name: principal_variant_id
                referencedColumnName: id
                nullable: true
                onDelete: "SET NULL"

    oneToMany:
        variants:
            targetEntity: Elcodi\Component\Product\Entity\Interfaces\VariantInterface
            mappedBy: product
            cascade: [ "all" ]
            orphanRemoval: true

    manyToMany:
        attributes:
            targetEntity: Elcodi\Component\Attribute\Entity\Interfaces\AttributeInterface
            joinTable:
                name: product_attribute
                joinColumns:
                    product_id:
                        referencedColumnName: id
                        onDelete: "CASCADE"
                inverseJoinColumns:
                    attribute_id:
                        referencedColumnName: id
                        onDelete: "CASCADE"

The crashing code

This code is part of a service. productObjectManager can be considered as the default entity manager. The exception is produced by the second call.

$this
    ->productObjectManager
    ->flush($purchasable);

$this
    ->productObjectManager
    ->flush($purchasable);

Some tips

When I do flush in the first time, the Collection of Category instances is set to dirty. After some investigation, https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/UnitOfWork.php#L607 this line is the one that set this value.

// Inject PersistentCollection
$value = new PersistentCollection(
    $this->em, $this->em->getClassMetadata($assoc['targetEntity']), $value
);
$value->setOwner($entity, $assoc);
$value->setDirty( ! $value->isEmpty());

$class->reflFields[$name]->setValue($entity, $value);

Commenting this line, everything works as expected (I know that that's not an option), but I would like to understand what implies this line and why is there.

Any help please? Any idea of what could be the issue?

billschaller commented 8 years ago

Can you make a gist containing the entities and persistance code leading to the error?

mmoreram commented 8 years ago

@zeroedin-bill Updated :)

billschaller commented 8 years ago

Hmm. I haven't seen someone use interfaces as targetEntity before, but I really don't know what is going on here. @Ocramius, @guilhermeblanco, any ideas?

mmoreram commented 8 years ago

@zeroedin-bill the resolve target entity does the work :)

http://symfony.com/doc/current/cookbook/doctrine/resolve_target_entity.html

mmoreram commented 8 years ago

ping @acasademont

mmoreram commented 8 years ago

I got it. Look at this piece of code.

$categories = $product->getCategories();
$newCategories = new ArrayCollection();
foreach ($categories as $category) {
    $newCategories->add($category);
}
$product->setCategories($newCategories);

What happens here? Even if categories are the same, because the object is no longer a PersistentCollection but an ArrayCollection, the element if marked as dirty and is recomputed again as inserts.

I would love to understand why, so I will continue digging in the project, but if you can give me some light, would be nice :)

@zeroedin-bill @Ocramius

billschaller commented 8 years ago

@mmoreram I think we'd really need a failing test case to fully understand what's going on here...

mmoreram commented 8 years ago

Yes. That's what I working on now :)

billschaller commented 8 years ago

:+1:

doctrine / orm