Sahoo,
I agree with your assessment in your earlier email.
As stated in section 2.1.5, the access type for an embedded
object is determined by the access type of the entity in which
it is embedded, and as stated in section 2.1.1, the behavior
is unspecified if mapping annotations are applied to both
persistent fields and properties or if the XML descriptor
specifies use of different access types within a class hierarchy.
Specifying a different access type in an embeddable class than
in the entity which embeds it is undefined, and the verifier
should flag this case. Defined behavior exists only for the
case where a single access type applies for an entire entity
hierarchy.
In the next rev of the spec, we can look at whether or not
we should define annotations on embeddables that imply a
different access type to be 'overriding' (and if so, whether
for the embedded class as a whole or for a specific field
or property). If we do so, we should do this more generally
(i.e., also handle the cases of subclasses defining different
access types as well).
In the mean time, if it is important to support a vendor-specific
mixing of access types, we should use a product-specific annotation
for that purpose.
-Linda
Sanjeeb Kumar Sahoo wrote:
> Mike,
>
> If that was the intent, then why should an application be non-portable
> if it uses different access-type in the embeddable? Or are we also
> missing a line like this in that same section of the spec: /a portable
> app should not use access-type that is different from the enclosing
> entity class./
>
> More over, were similar lines missing for MappedSuperclass? How about
> mixed access-type in an entity hierarchy?
>
> Thanks,
> Sahoo
> Mike Keith wrote:
>
>> // /When there is no metadata specified for an embedded object, the
>> access type for an embedded object *is* determined by the access type
>> of the entity in which it is embedded./
>>
>> -----Original Message-----
>> *From:* Mike Keith [mailto:michael.keith_at_oracle.com]
>> *Sent:* Tuesday, August 08, 2006 1:45 PM
>> *To:* Sanjeeb Kumar Sahoo; persistence_at_glassfish.dev.java.net;
>> Linda DeMichiel
>> *Subject:* RE: code review for issue #831
>>
>> The intent was never to disallow a vendor form doing so on their
>> own. Our wording in the first section might have
>> been better stated as:
>>
>> -----Original Message-----
>> *From:* Sanjeeb.Sahoo_at_Sun.COM
>> [mailto:Sanjeeb.Sahoo_at_Sun.COM]*On Behalf Of *Sanjeeb Kumar Sahoo
>> *Sent:* Tuesday, August 08, 2006 1:27 PM
>> *To:* persistence_at_glassfish.dev.java.net; Mike Keith; Linda
>> DeMichiel
>> *Subject:* Re: code review for issue #831
>>
>> Hi Gordon,
>>
>> Thanks for reviewing. Are there any other comments?
>>
>> I totally agree with you that an embeddable should be allowed
>> to have its own access-type and that would have given more
>> options to the user, but unfortunately, the expert group
>> ignored this point when it was raised. More over, the spec is
>> *very very* confusing on this subject. In 2.1.5, it says:
>> /The access type for an embedded object *is* determined by the
>> access type of the entity in which it is embedded./
>> (Note, it says *is determined by* as opposed to *can be
>> determined*.). Yet, *orm_1_0.xsd* allows an access-type to be
>> specified for an embeddable, there by *contradicting* the
>> previous rule. Moreover, only non-portable applications can
>> have different access-type for an embeddable as the spec says
>> the following in section #10.1.5.2:
>> /Portable applications must not specify the access attribute
>> if mapping annotations have been applied
>> to the fields or properties of the embeddable class or the
>> entity with which it is associated and the value
>> differs from the access type defined by means of annotations.
>> Portable applications must not use more than one access type
>> within an entity hierarchy.
>> /
>> So, until it is officially clarified in the spec, I think, we
>> *must not* use annotations in embeddable class to determine
>> access-type. I am also not in favor of allowing access-type of
>> an embeddable to be overridden in orm.xml unless we support
>> the same extension for MappedSuperclass. Are you proposing to
>> support this for MappedSuperclass as well? Looking at the
>> code, I think, supporting this for MappedSuperclass will be
>> tougher than that for Embeddable.
>>
>> Thanks,
>> Sahoo
>>
>> Gordon Yorke wrote:
>>
>>> Hello Sahoo,
>>> I have reviewed your code and have a suggestion. The
>>> Access Type of the Embeddable should still be determined based on
>>> annotations or XML (XML can define access type directly). This
>>> allows users to specify the access type of the Embeddable. If the
>>> access type is not available from the Embeddable then use the access
>>> type of the parent. The specification states that the access type
>>> can be 'determined' from that of the parent but is not mandated to be
>>> that of the parent( Mike, is that not the intention of the
>>> specification?) This would give users much more flexibility for
>>> sharing Embeddables.
>>> Also based on how TopLink treats Embeddable mappings we
>>> could, in the future, support a different access type for each target
>>> of an Embeddable mapping if the AccesType of the Embeddable was not
>>> supplied by the user.
>>> --Gordon
>>>
>>> -----Original Message-----
>>> From: Sanjeeb.Sahoo_at_Sun.COM [mailto:Sanjeeb.Sahoo_at_Sun.COM]On
>>> Behalf Of
>>> Sanjeeb Kumar Sahoo
>>> Sent: Monday, August 07, 2006 9:00 AM
>>> To: Tom Ware
>>> Cc: persistence_at_glassfish.dev.java.net
>>> Subject: code review for issue #831
>>>
>>>
>>> Hi Tom,
>>>
>>> Attached here with the suggested fix for
>>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=831 .
>>> The jar file contains modified sources in new folder,
>>> original sources
>>> in orig folder and differences in diffs folder.
>>> Please review them. I have update issue tracker with the
>>> details.
>>>
>>> Tests run:
>>> entity-persistence-tests(all of them passed)
>>> JPA TCK v1.0: 3 tests failed but they fail on a clean
>>> workspace as well
>>> (Yes, I have run those 3 tests in a clean workspace and they
>>> failed).
>>> So, my changes are not causing those failures.
>>>
>>> Appreciate a quick reply.
>>>
>>> Thanks,
>>> Sahoo
>>>
>>>
>>
>>
>