DDC-1052: Class table inheritance + OptimisticLocking issue #1644

Closed
doctrinebot opened this Issue Feb 28, 2011 · 6 comments

2 participants

@doctrinebot

Jira issue originally created by user comfortablynumb:

Hi everyone!

I'm having a problem with the optimistic locking feature. I have this (simplified) model tree:

  • BaseElement (abstract) **** Element (abstract) ***** Entity (abstract) ****** Person (abstract) ******* Contact ****** BusinessPartner (abstract) ******* Customer ******* Lead

Now, with MySQL Logging active, when I update a Lead I have these queries executed:

Query   START TRANSACTION
Query   SELECT ...Lead fields... WHERE id = 123
Query       UPDATE business_partner SET ...BusinessPartner fields... WHERE id = 123
Query       UPDATE element SET version = version <ins> 1 WHERE id = 123 AND version = 1
Query       commit

It works perfectly. The same goes when I update a Customer. The problem comes when I want to update a Contact:

Query   START TRANSACTION
Query   SELECT ...Contact fields... WHERE id = 123
Query       UPDATE element SET version = version </ins> 1 WHERE id = 123 AND version = 1
Query       UPDATE person SET ...Person fields... WHERE id = 123
Query       commit

As you can see, the element row (and the version checking) occurs before the update of the person row. I think this is the reason I'm getting an OptimisticLockingException. Maybe after the Person row is updated, version is being checked again? I can't figure out why. My simplified model basically is like this:

BaseElement:

/****
 * @orm:MappedSuperclass
 */
abstract class BaseElement
{
      // ID, Version and other fields
}

Element:

/****
 * @orm:Entity(repositoryClass="ENC\Bundle\ElementModuleBundle\Entity\ElementRepository")
 * @orm:Table(name="element")
 * @orm:InheritanceType("JOINED")
 * @orm:DiscriminatorColumn(name="discriminator", type="string")
 * @orm:DiscriminatorMap({
      "contact"             = "CNE\Bundle\ContactModuleBundle\Entity\Contact",
      "lead"                = "CNE\Bundle\LeadModuleBundle\Entity\Lead",
      "customer"            = "CNE\Bundle\CustomerModuleBundle\Entity\Customer",
   })
abstract class Element extends BaseElement
{
      // Other fields
}

Entity:

/****
 * @orm:Entity(repositoryClass="ENC\Bundle\EntityModuleBundle\Entity\EntityRepository")
 * @orm:Table(name="entity")
 */
abstract class Entity extends Element
{
     // Fields
}

BusinessPartner:

/****
 * @orm:Entity(repositoryClass="ENC\Bundle\BusinessPartnerModuleBundle\Entity\BusinessPartnerRepository")
 * @orm:Table(name="business_partner")
 */
abstract class BusinessPartner extends Entity
{
    // Fields
}

Person

/****
 * @orm:Entity(repositoryClass="ENC\Bundle\PersonModuleBundle\Entity\PersonRepository")
 * @orm:Table(name="person")
 */
abstract class Person extends Entity
{
     // Fields
}

Contact:

/****
 * @orm:Entity(repositoryClass="ENC\Bundle\ContactModuleBundle\Entity\ContactRepository")
 * @orm:Table(name="contact")
 * @orm:HasLifecycleCallbacks
 */
class Contact extends Person
{
    // Fields
}

Lead:

/****
 * @orm:Entity(repositoryClass="ENC\Bundle\LeadModuleBundle\Entity\LeadRepository")
 * @orm:Table(name="lead")
 * @orm:HasLifecycleCallbacks
 */
class Lead extends BusinessPartner
{
    // Fields
}

Customer:

/****
 * @orm:Entity(repositoryClass="ENC\Bundle\CustomerModuleBundle\Entity\CustomerRepository")
 * @orm:Table(name="customer")
 * @orm:HasLifecycleCallbacks
 */
class Customer extends BusinessPartner
{
    // Fields
}

I don't post the annotations of all my models because they're too many and I think they don't have nothing to do with the problem. But in case you need them, please, tell me and I post them :)

Thanks in advance!

@doctrinebot

Comment created by @beberlei:

Can you post the stack trace of the optimistic version exception?

In general This inheritance hierachy scares the hell out of me, why do you need 5 levels of inheritance? Cant you just drop BaseElement, element and Entity into one? Plus this model really fails What happens if a person moves from a lead to customer or contact or back?

@doctrinebot

Comment created by @beberlei:

Can you try to change lib/Doctrine/ORM/Persisters/JoinedEntityPersister.php line 368

if ($this->_class->isVersioned && ! $result) {

to

if ($versioned && ! $result) {
@doctrinebot

Comment created by comfortablynumb:

Thanks for your answer Benjamin! your change indeed solved the problem :) But the file wasn't JoinedEntityPersister (it doesn't exist at least in the latest doctrine copy from "git://github.com/doctrine/doctrine2.git doctrine"). It was on the file BasicEntityPersister.php, located in the same dir you mention.

About my model, I didn't show the entire model because it didn't make sense for this issue. But one reason I have for this hierarchy is that I have lots of models that need to be related with all the instances of my model "Entity". For instance, all Users, Contacts, Leads, Customers and Suppliers need to have Activities, and I need to provide a way to select one between all these models when you, for example, create an Activity. Then I have a model "Opportunity" that needs to be associated only with BusinessPartner's. And, lastly, I have a couple of models that need to be associated with all my "Element"s. BaseElement is a mapped superclass that has only the basic fields for ALL my models (related to Element or not), that have in common all the entities in my model (don't confuse with the model "Entity". Yes, it's a little bit confuse but I couldn't think in another name to relate People and Business Partners :P).

The other point: People can't be a customer or a lead. My requirements say that only business partners can be customers, leads or suppliers. Business partners are companies. They can't be individuals. Business partners can have contacts. And if a user needs to "transform" a lead to a customer, it can create a customer from a lead, and have a reference back to it. I thought I need them to have them separated because this won't be only a CRM. It's going to be a sales system too (I don't know if this is the correct english term for this type of system), so I need to have them in two different entities for a couple of reasons. If I only have a status that describes a customer or a lead, I can't, for example, enforce some business rules at database level I'll have (in a DB portable way at least that I know of). I need to enforce, if it's possible and portable, this type of business rules because there will be other apps that will interact with my DB. For instance, if I need to associate an invoice to a leadorcustomer, how can I enforce this on the DB if I only have a status field that determines if it's a lead or a contact? Separating the entities is the only way I could think of to solve this type of problems. Maybe is not the best way. I'll need to rethink the trade-offs of this decision. Thanks for pointing this out :)

For now it's a CRM. But it's only the beginning. It will have lots of additional models when I'll add modules for a sales system, voIP vinculation, etc. That's why I tried to make a reusable tree on my model. I know the performance risks having too many entities related to each other, having lots of joins and one table in common that will be always touched could be a bottleneck if there are lots of users doing lots of operations. But I'm in the hurry and I need to save development time. I'm the only person that will work on this app, and I have only 6 months from scratch to finish the CRM, which is a really big app in very little time for me (having to work on a pretty big ACL module and a Workflow module too). Inheritance gave me a DRY way in lots of aspects of my app. I can sacrifice some performance to save development time. Then I can optimize if I need it :)

THANKS A LOT for your fix and for quickly look this issue! and for your comments too. I'm not an expert yet and constructive comments like these help me to rethink some points on my decisions.

Best regards.

@doctrinebot

Comment created by @beberlei:

Fixed, merged into 2.0.x branch

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot

Comment created by comfortablynumb:

Great! thanks a lot.

@beberlei beberlei was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot added this to the 2.0.4 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment