glassfish
  1. glassfish
  2. GLASSFISH-1022

Don't cascade operations on "read-only" managed entities

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: 9.1pe
    • Fix Version/s: 9.1pe
    • Component/s: entity-persistence
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      1,022

      Description

      One of our tests produced the situation in the attached testcase Test.java. It's
      important that em's persistence context is cleared after the first transaction.
      The merge operation in the second transaction is cascaded the "new" insurance
      instance, which actually produces the following exception at commit:

      Exception in thread "main" javax.persistence.RollbackException: Exception
      [TOPLINK-7231] (Oracle TopLink Essentials - 2006.8 (Build 060822)):
      oracle.toplink.essentials.exceptions.ValidationException
      Exception Description: Cannot persist detached object [company.Insurance@1cdfd19].
      Class> company.Insurance Primary Key> [2]
      at
      oracle.toplink.essentials.internal.ejb.cmp3.transaction.base.EntityTransactionImpl.commit(EntityTransactionImpl.java:109)
      at
      oracle.toplink.essentials.internal.ejb.cmp3.transaction.EntityTransactionImpl.commit(EntityTransactionImpl.java:45)

      My preliminary analysis is as follows:

      The offending line is in
      RepeatableWriteUnitOfWork#discoverUnregisteredNewObjects:111

      When determinating "new" objects in this UnitOfWork, TopLink iterates the object
      graph. Finding the "unknown" insurance, it tries to register it as new object.
      This fails in

      UnitOfWorkImpl#registerNotRegisteredNewObjectForPersist:3204

      because it's already in the global cache. I don't understand the complete
      situation, but the assumption that "unknown" objects encountered in the object
      graph traversal are always "new" looks like a bug to me.

        Activity

        Hide
        mf125085 added a comment -

        Created an attachment (id=402)
        Test case

        Show
        mf125085 added a comment - Created an attachment (id=402) Test case
        Hide
        marina vatkina added a comment -

        Confirmed

        Show
        marina vatkina added a comment - Confirmed
        Hide
        ijuma added a comment -

        Adding myself to cc list.

        Show
        ijuma added a comment - Adding myself to cc list.
        Hide
        guruwons added a comment -

        This problem is caused by issue 1139 - cascade merge doesn't work for a managed
        entity. That issue is fixed, therefore the problem doesn't occur now.

        Show
        guruwons added a comment - This problem is caused by issue 1139 - cascade merge doesn't work for a managed entity. That issue is fixed, therefore the problem doesn't occur now.
        Hide
        guruwons added a comment -

        But if you do set a detached entity to a managed entity and don't call merge it
        like below, the same exception occurs at commit time.
        ----------
        tx.begin();

        Employee emp = em.find(Employee.class, 1L);
        System.out.println("Carrier: " + emp.getInsurance().getCarrier());

        emp.setInsurance(i2);
        //i2.setEmployee(emp);

        //em.merge(emp);

        tx.commit();
        ----------
        I'm not sure above situation is a bug. A non-managed entity is set in the
        relationship with CascadeType.PERSIST option, so TopLink tries to persist it at
        commit time. If there is no CascadeType.PERSIST option, it doesn't cause the
        exception. This situation is similar when you call em.persist() with a new
        entity which has a detached entity and CascadeType.PERSIST. I think this
        exception is quite normal in this situation.

        Show
        guruwons added a comment - But if you do set a detached entity to a managed entity and don't call merge it like below, the same exception occurs at commit time. ---------- tx.begin(); Employee emp = em.find(Employee.class, 1L); System.out.println("Carrier: " + emp.getInsurance().getCarrier()); emp.setInsurance(i2); //i2.setEmployee(emp); //em.merge(emp); tx.commit(); ---------- I'm not sure above situation is a bug. A non-managed entity is set in the relationship with CascadeType.PERSIST option, so TopLink tries to persist it at commit time. If there is no CascadeType.PERSIST option, it doesn't cause the exception. This situation is similar when you call em.persist() with a new entity which has a detached entity and CascadeType.PERSIST. I think this exception is quite normal in this situation.
        Hide
        mf125085 added a comment -

        > tx.begin();
        >
        > Employee emp = em.find(Employee.class, 1L);
        > System.out.println("Carrier: " + emp.getInsurance().getCarrier());
        >
        > emp.setInsurance(i2);
        > //i2.setEmployee(emp);
        >
        > //em.merge(emp);
        >
        > tx.commit();
        > ----------
        > I'm not sure above situation is a bug.

        Following is a different situation:

        > A non-managed entity is set in the
        > relationship with CascadeType.PERSIST option, so TopLink tries to persist it at
        > commit time. If there is no CascadeType.PERSIST option, it doesn't cause the
        > exception.

        I think the problem is that TopLink cascades to relationship(s) of the
        managed entity, even though no EntityManager operation has been
        applied to the entity (Please see the code example above). TopLink
        should not try to apply any operation to related instances if there's
        no EntityManager operation applied to the managed entity itself, even
        if the relationship is annotated as CASCADE.

        ---------
        > This situation is similar when you call em.persist() with a new
        > entity which has a detached entity and CascadeType.PERSIST. I think this
        > exception is quite normal in this situation.

        Agreed. I'm changing the semantics of this issue to:

        Don't cascade operations on "read-only" managed entities

        Show
        mf125085 added a comment - > tx.begin(); > > Employee emp = em.find(Employee.class, 1L); > System.out.println("Carrier: " + emp.getInsurance().getCarrier()); > > emp.setInsurance(i2); > //i2.setEmployee(emp); > > //em.merge(emp); > > tx.commit(); > ---------- > I'm not sure above situation is a bug. Following is a different situation: > A non-managed entity is set in the > relationship with CascadeType.PERSIST option, so TopLink tries to persist it at > commit time. If there is no CascadeType.PERSIST option, it doesn't cause the > exception. I think the problem is that TopLink cascades to relationship(s) of the managed entity, even though no EntityManager operation has been applied to the entity (Please see the code example above). TopLink should not try to apply any operation to related instances if there's no EntityManager operation applied to the managed entity itself, even if the relationship is annotated as CASCADE. --------- > This situation is similar when you call em.persist() with a new > entity which has a detached entity and CascadeType.PERSIST. I think this > exception is quite normal in this situation. Agreed. I'm changing the semantics of this issue to: Don't cascade operations on "read-only" managed entities
        Hide
        mf125085 added a comment -

        Please forget my previous comment. Please see JPA spec. 3.2.3 "Synchronization
        to the Database":

        a) If the relationship is annotated cascade=PERSIST or cascade=ALL the described
        TopLink behavior is correct.

        b) My test actually throws an exception on the association of a new entity and
        the relationship in not annotated cascade=PERSIST or cascade=ALL, which the
        correct behavior according to the spec.

            • This issue has been marked as a duplicate of 1139 ***
        Show
        mf125085 added a comment - Please forget my previous comment. Please see JPA spec. 3.2.3 "Synchronization to the Database": a) If the relationship is annotated cascade=PERSIST or cascade=ALL the described TopLink behavior is correct. b) My test actually throws an exception on the association of a new entity and the relationship in not annotated cascade=PERSIST or cascade=ALL, which the correct behavior according to the spec. This issue has been marked as a duplicate of 1139 ***

          People

          • Assignee:
            tware
            Reporter:
            mf125085
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: