Hi,
There seem to be a couple of independent issues here.
1. Is it permitted by the specification to persist a non-Entity
subclass of an Entity?
My reading of the spec says no. From 2.1, "The entity class must be
annotated with the Entity annotation or denoted in the XML descriptor
as an entity." From 3.1.1 javadoc for persist(Object), " * @throws
IllegalArgumentException if not an entity". So you cannot persist an
instance of a class that is not an Entity. I cannot find any
reference to a non-Entity subclass as being treated as an Entity.
2. Is it permitted by the specification to cascade persist a non-
Entity subclass of an Entity?
My reading of the spec says no. From 3.2.1, "A new entity instance
becomes both managed and persistent by invoking the persist method on
it or by cascading the persist operation. ". And "For all entities Y
referenced by a relationship from X, if the relationship to Y has
been annotated with the cascade element value cascade=PERSIST or
cascade=ALL, the persist operation is applied to Y." And applying the
persist operation to Y refers to the persist method discussed above.
The issue of making cascade persist operations succeed would depend
on the resolution of these two issues.
I think that changing the spec to allow persisting a non-Entity
subclass B of an Entity class A would violate object persistence basics.
Assume that you treat the non-Entity B as its Entity A superclass.
Instantiating the Entity in a new transaction context loses the
information that it is an instance of non-Entity B. Any B behavior or
fields are lost. To me, this violates the value proposition of
persistence. Additionally, this only works at all if the field is
declared to be of type A, and won't work if the field is declared to
be of type B.
Craig
On Sep 29, 2006, at 10:54 AM, Gordon Yorke wrote:
> Marina,
> I have been quite consistent in my emails. Markus's comments in
> gf issue 1021 was that it was a validation issue because TopLink
> was attempting to persist non-entities. I disagree and issue 1021
> needs to continue to be a bug that must be addressed. The
> importance of issue 1021 can be discussed but it is not a
> validation issue.
> Downgrading 1035 was your suggestion "Let's downgrade this to p4
> until we have a real user complain that things are falling apart
> because we allow this, or until the spec is clarified", as 1035
> deals with preventing persisting non-entities.
> --Gordon
>
> -----Original Message-----
> From: Marina.Vatkina_at_Sun.COM [mailto:Marina.Vatkina_at_Sun.COM]On
> Behalf Of
> Marina Vatkina
> Sent: Thursday, September 28, 2006 3:59 PM
> To: persistence_at_glassfish.dev.java.net
> Subject: Re: Fix for issue 1021
>
>
> Now I'm totally confused...
>
> 1. Isn't the dependency management that causes the order of
> executed sql,
> part of another issue?
>
> 2. If cascade-persist doesn't work for non-entities, and the spec
> is not
> expected to allow this, why should we spend time *fixing* it, while
> it's
> much clearer to throw another exception? If you disagree with an
> IllegalArgumentException, then it can be an
> UnsupportedOptionException.
>
> 3. Issue 1035 is closed currently as 'wontfix', so why should it be
> downgraded instead of being reopen? What else do you like to discuss
> about it?
>
> thanks,
> -marina
>
>
> Gordon Yorke wrote:
>> Right now issue 1021 is reporting an exception case. We should
>> fix that problem directly. Issue 1035 can be downgraded and left
>> for discussion as issue 1035 covers validation of non-entities.
>> --Gordon
>>
>> -----Original Message-----
>> From: Marina.Vatkina_at_Sun.COM [mailto:Marina.Vatkina_at_Sun.COM]On
>> Behalf Of
>> Marina Vatkina
>> Sent: Thursday, September 28, 2006 2:44 PM
>> To: persistence_at_glassfish.dev.java.net
>> Subject: Re: Fix for issue 1021
>>
>>
>> Hello Gordon,
>>
>> Let's compromise ;). Let's downgrade this to p4 until we have a real
>> user complain that things are falling apart because we allow this, or
>> until the spec is clarified.
>>
>> On the other hand, we should then fix 1035. Will you agree?
>>
>> thanks,
>> -marina
>>
>> Gordon Yorke wrote:
>>
>>> Yes Marina,
>>> As I have been mentioning we should be cascading without error.
>>> --Gordon
>>>
>>> -----Original Message-----
>>> From: Marina.Vatkina_at_Sun.COM [mailto:Marina.Vatkina_at_Sun.COM]On
>>> Behalf Of
>>> Marina Vatkina
>>> Sent: Thursday, September 28, 2006 1:47 PM
>>> To: persistence_at_glassfish.dev.java.net
>>> Subject: Re: Fix for issue 1021
>>>
>>>
>>> Gordon,
>>>
>>> The current spec does not define the rules on cascading to non-
>>> entities.
>>> Should we not cascade then?
>>>
>>> thanks,
>>> -marina
>>>
>>> Gordon Yorke wrote:
>>>
>>>
>>>> Although the specification specifically states that non-entities
>>>> can not be passed as arguments to the EntityManager or Query
>>>> interfaces the specification does not define what should happed
>>>> if an operation is cascaded to a non-entity. If this becomes
>>>> clarified then validation will be required, but it is not
>>>> required now and would remove functionality from users. GF
>>>> issue 1021 can not be fixed through adding validation.
>>>> --Gordon
>>>>
>>>> -----Original Message-----
>>>> From: Marina.Vatkina_at_Sun.COM [mailto:Marina.Vatkina_at_Sun.COM]On
>>>> Behalf Of
>>>> Marina Vatkina
>>>> Sent: Wednesday, September 27, 2006 7:11 PM
>>>> To: persistence_at_glassfish.dev.java.net
>>>> Subject: Re: Fix for issue 1021
>>>>
>>>>
>>>> Hi Markus,
>>>>
>>>> I checked with Linda, and she confirmed that the current spec
>>>> requires
>>>> non-entities to cause an exception on EM operations as well as
>>>> cascade
>>>> operations, as from the spec perspective, cascade means applying EM
>>>> api logic to the related instance. She plans to clarify this.
>>>>
>>>> She is also interested in real user cases when the opposite
>>>> behavior
>>>> is needed, so if you know any, please let her know.
>>>>
>>>> thanks,
>>>> -marina
>>>>
>>>> Markus Fuchs wrote On 09/26/06 09:18,:
>>>>
>>>>
>>>>
>>>>> Hi Gordon,
>>>>>
>>>>> Reading section 2.1.9.3 I only find that non-entity super
>>>>> classes can't
>>>>> be persistent:
>>>>>
>>>>> *The non-entity superclass serves for inheritance of behavior
>>>>> only. The
>>>>> state of a non-entity superclass is
>>>>> not persistent. Any state inherited from non-entity super
>>>>> classes is
>>>>> non-persistent in an inheriting entity
>>>>> class. This non-persistent state is not managed by the
>>>>> EntityManager[12]. Any annotations on such
>>>>> super classes are ignored.
>>>>>
>>>>> *This doesn't say anything about non-entity subclasses of entity
>>>>> classes. Further on, the next sentence:
>>>>>
>>>>> *Non-entity classes cannot be passed as arguments to methods of
>>>>> the
>>>>> EntityManager or Query
>>>>> interfaces and cannot bear mapping information.
>>>>>
>>>>> *Says that non-entity classes can't be persisted, etc. Even if not
>>>>> explicitly stated, it would only make sense to apply the above
>>>>> rule to
>>>>> cascaded operations. The situation described in 1021 shows an
>>>>> example,
>>>>> where cascading the persist operation to a non-entity subclass
>>>>> doesn't
>>>>> work in the current implementation.*
>>>>>
>>>>> *Thanks,
>>>>>
>>>>> -- markus.*
>>>>> *
>>>>> Gordon Yorke wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> Hello Markus,
>>>>>> I am a little confused here. There was already a discussion
>>>>>> about
>>>>>> this behaviour and I believe the conclusion was to allow users to
>>>>>> reference non-entity inheritance heirarchy members in their
>>>>>> object
>>>>>> model. TopLink would persist the state from the parent Entity
>>>>>> object.
>>>>>> (discussion thread had subject : "How to prevent stack
>>>>>> overflow" and
>>>>>> was in reference to gf bug 939).
>>>>>> Contrary to the comments in the bug there is nothing the
>>>>>> specification that prevents this behaviour and the
>>>>>> specification even
>>>>>> covers this case in section 2.1.9.3
>>>>>> If we are not validating EntityManager and Query apis then that
>>>>>> needs to be added but this bug is not a validation issue.
>>>>>> --Gordon
>>>>>>
>>>>>> -----Original Message-----
>>>>>> *From:* Markus.Fuchs_at_Sun.COM [mailto:Markus.Fuchs_at_Sun.COM]*On
>>>>>> Behalf Of *Markus Fuchs
>>>>>> *Sent:* Monday, September 25, 2006 8:08 PM
>>>>>> *To:* persistence_at_glassfish.dev.java.net
>>>>>> *Subject:* Fix for issue 1021
>>>>>>
>>>>>> Hi TopLink team,
>>>>>>
>>>>>> I'm trying to fix issue 1021: VALIDATION: Disallow persisting
>>>>>> non-entity subclasses
>>>>>>
>>>>>> Can I assume that there's a one-to-one relationship btw. entity
>>>>>> classes
>>>>>> and descriptors? I.e. is the follow statement true:
>>>>>>
>>>>>> Class is annotated as @Entity <=> There's a ClassDescriptor for
>>>>>> that class.
>>>>>>
>>>>>> Based on this assumption, the attached change throws a
>>>>>> RuntimeException in
>>>>>> AbstractSession.getDescriptor(Class theClass), if the descriptor
>>>>>> for a class
>>>>>> is not found. Adding it there will also change TopLink behavior
>>>>>> for the
>>>>>> non-JPA case. The exception message must still be localized, of
>>>>>> course.
>>>>>> Is there any reason, why TopLink returns the superclass
>>>>>> descriptor, if no
>>>>>> descriptor for the a class can be found?
>>>>>>
>>>>>> Another place to add this validation would be in
>>>>>> EntityManagerImpl,
>>>>>> similar to how em.contains() is called for some JPA
>>>>>> operations. But
>>>>>> that would not cover the cases were the operation is cascaded
>>>>>> to an
>>>>>> instance, as the internal operation is called directly and not/
>>>>>> /through
>>>>>> the EM interface.
>>>>>>
>>>>>> I tested the validation by trying to persist a non-entity
>>>>>> class or a
>>>>>> mapped superclass instance.
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> -- markus.
>>>>>>
Craig Russell
Architect, Sun Java Enterprise System
http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell_at_sun.com
P.S. A good JDO? O, Gasp!
- application/pkcs7-signature attachment: smime.p7s