1) Yes, we can allow vendor extensions, but that should not make a
portable app behave differently.
2) What do you suggest we should support? I hope, it's overriding of
access-type of an embeddable in orm.xml and *not* determining
access-type of embeddable based on annotation. I say this because, no
where in the spec is it written that the following code is incorrect or
non-portable:
@Entity class Foo {
@Id int id;
@Embedded Bar bar;
}
@Embeddable Bar {
int f;
@Basic getF(){ ...;}
void setF(int f){...}
}
One can argue that user put that annotation in the getter because that's
how that embeddable class is used in some other application. The spec
does not prohibit such use, it just says that provider will not use them
when Bar is referenced from Foo. This means, when Bar is used in Foo, a
provider must use the field access-type irrespective of vendor-extension
property set or not.
IMHO, the spec needs to be lot more clearer before we decide to add such
extension.
Thanks,
Sahoo
Marina Vatkina wrote:
> Just a thought:
> We can still allow vendor extensions, right? So if a property (e.g.
> "toplink.allow-mixed-access-type") is set to true in a particular PU,
> we allow it. By default, don't.
>
> thanks,
> -marina
>
> Sanjeeb Kumar Sahoo wrote:
>> 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
>>>
>>>
>>>
>>