Hi Mike,
This is an interesting argument, as I thought it was a typo in the
spec (copy-and-paste type) that only class hierarchy is mentioned.
thanks,
-marina
Mike Keith wrote:
> The section in 2.1.1 applies to entity/class hierarchies and not
> really to embeddables. There is no technical reason to disallow
> an embeddable having a different access type from its owning entity
> since there is no conflict between it and the owning class
> (like there is in entity hierarchies where fields/properties are
> inherited). The problem was in the wording of that one line that
> discusses how the access type should be defined for an embeddable
> in the absence of metadata (since there is no PK and hence no
> requirement for there to be any metadata at all).
>
> Sahoo once asked me whether something like the following code
> would work:
>
> @Entity class Building {
> @Id int id;
> @Embedded Address address;
> }
> @Entity class Person {
> @Id int getId() { ... }
> @Embedded public Address getAddress() { ... }
> ...
> }
> @Embeddable class Address {
> String street;
> String city;
> String getStreet() { ... };
> String getCity() { ... };
> ...
> }
>
> The answer that I gave was that it would work fine and that
> to map the column names, etc, @AttributeOverride annotations
> should be used on the entities.
>
> The example should be able to work just as well, though, if the
> address were annotated, as in:
>
> @Embeddable class Address {
> @Column(name="STR")
> String street;
> String city;
> ...
> }
>
> This would simply mean that the embedded object is just being
> accessed using field access, which neither concerns nor affects
> the owning entity. In reality I can't even think of a really good
> use case for overriding the access type of an embedded object to match
> the entity that happens to be pointing to it.
>
> The *only* reason why the above example might not be permitted is
> because of the one incorrectly worded line in the spec that I pointed
> out. As it is there does not need to be any vendor-specific annotations,
> since just annotating the class is the obvious way to do this.
>
> -Mike
>
>
>>-----Original Message-----
>>From: Linda.Demichiel_at_Sun.COM
>>[mailto:Linda.Demichiel_at_Sun.COM]On Behalf
>>Of Linda DeMichiel
>>Sent: Wednesday, August 09, 2006 12:13 AM
>>To: Sanjeeb Kumar Sahoo
>>Cc: Mike Keith; persistence_at_glassfish.dev.java.net
>>Subject: Re: code review for issue #831
>>
>>
>>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
>>>>>
>>>>>
>>>>
>>>>