Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-1052

Class table inheritance + OptimisticLocking issue

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.4
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None
    • Environment:
      MySQL 5.1 + Ubuntu server 64 bits + PHP 5.3.2

      Description

      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 + 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 + 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!

        Activity

        Hide
        Benjamin Eberlei added a comment - - edited

        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?

        Show
        Benjamin Eberlei added a comment - - edited 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?
        Hide
        Benjamin Eberlei added a comment -

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

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

        to

        if ($versioned && ! $result) {
        
        Show
        Benjamin Eberlei added a comment - Can you try to change lib/Doctrine/ORM/Persisters/JoinedEntityPersister.php line 368 if ($ this ->_class->isVersioned && ! $result) { to if ($versioned && ! $result) {
        Hide
        Gustavo Falco added a comment -

        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 ).

        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 lead_or_customer, 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.

        Show
        Gustavo Falco added a comment - 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 ). 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 lead_or_customer, 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.
        Hide
        Benjamin Eberlei added a comment -

        Fixed, merged into 2.0.x branch

        Show
        Benjamin Eberlei added a comment - Fixed, merged into 2.0.x branch
        Hide
        Gustavo Falco added a comment -

        Great! thanks a lot.

        Show
        Gustavo Falco added a comment - Great! thanks a lot.

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Gustavo Falco
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: