persistence@glassfish.java.net

RE: code review for issue #831

From: Mike Keith <michael.keith_at_oracle.com>
Date: Wed, 23 Aug 2006 16:32:21 -0400

Sahoo,

Rules 1 and 2, and 5 are great.
Rule 3 is not what I thought we were doing but the use case is so weak
to doing it the other way in any case that I am perfectly fine with this rule.
Rule 4 is fine for now. My plan has always to provide a way to override the
access type in XML, but I don't think that has been done yet in the entity, so
it is not needed in embeddables yet, either.

In summary, full steam ahead...

Thanks for your patience and diligence on this one, Sahoo.
-Mike

> -----Original Message-----
> From: Sanjeeb.Sahoo_at_Sun.COM [mailto:Sanjeeb.Sahoo_at_Sun.COM]On Behalf Of
> Sanjeeb Kumar Sahoo
> Sent: Wednesday, August 23, 2006 2:52 PM
> To: Marina Vatkina; Mike Keith; Tom Ware
> Cc: persistence_at_glassfish.dev.java.net; Linda.Demichiel_at_Sun.COM
> Subject: Re: code review for issue #831
>
>
> Mike, Tom, Marina,
>
> The modified rules are in the javadoc of the following method which is
> going to be used to decide access-type of an embeddable class. Please
> review it and get back soon. AccessType is defined like this:
>
> enum AccessType {FIELD, PROPERTY, UNDEFINED, MIXED};
>
>
> /**
> * This method is responsible for determining the
> access-type for an
> * Embeddable class represented by emDescr that is passed to
> * this method. This method should *not* be called more
> than once as
> this is
> * quite expensive. Now the rules:
> *
> * Rule 1: In the *absence* of metadata in embeddable class,
> access-type of
> * an embeddable is determined by the access-type of the enclosing
> entity.
> *
> * Rule 2: In the presence of metadata in embeddable class,
> access-type of
> * an embeddable is determined using that metadata. This
> allows sharing
> * of the embeddable in entities with conflicting access-types.
> *
> * Rule 3: It is an error to use a *metadata-less*
> embeddable class in
> * entities with conflicting access-types as that might result in
> * different database mapping for the same embeddable class.
> *
> * Rule 4: It is an error if metadata-complete == false, and
> * metadata is present *both* in annotations and XML, and
> * access-type as determined by each of them is *not* same.
> *
> * Rule 5: It is an error if *both* fields and properties of an
> embeddable
> * class are annotated and metadata-complete == false.
> *
> * @param emDesc descriptor for the Embeddable class
> */
> private AccessType determineAccessTypeOfEmbedded(
> MetadataDescriptor emDesc) {
> final AccessType entityAccessType =
> m_descriptor.usesPropertyAccess() ?
> PROPERTY : FIELD;
> // access-type of embeddable defaults to that of
> enclosing entity's
> AccessType accessType = UNDEFINED;
> boolean metadataComplete =emDesc.ignoreAnnotations();
> AccessType accessTypeUsingAnnottaion =
> computeAccessTypeFromAnnotation(emDesc);
> AccessType accessTypeUsingXML =
> computeAccessTypeFromXML(emDesc);
> if (metadataComplete) {
> // metadata-complete is true, then use XML access-type if
> defined,
> // else use enclosing entity's access-type.
> accessTypeUsingXML = accessTypeUsingXML != UNDEFINED ?
> accessTypeUsingXML : entityAccessType;
> } else {// metadata-complete is false
> if (accessTypeUsingAnnottaion == UNDEFINED
> && accessTypeUsingAnnottaion == UNDEFINED) {
> // metadata is absent, so we use enclosing entity's
> access-type
> accessType = entityAccessType;
> } else if (accessTypeUsingXML == UNDEFINED
> && accessTypeUsingAnnottaion != UNDEFINED) {
> // annottaion is present in embeddable class
> accessType = accessTypeUsingAnnottaion;
> } else if (accessTypeUsingAnnottaion == UNDEFINED
> && accessTypeUsingXML != UNDEFINED) {
> // access is defined using XML for embeddable class
> accessType = accessTypeUsingXML;
> } else if (accessTypeUsingAnnottaion ==
> accessTypeUsingXML) {
> // annotation is present as well as access is defined
> using XML
> // and they are same. So use it.
> accessType = accessTypeUsingAnnottaion;
> } else {
> // annotation is present as well as access is defined
> using XML
> // and they are different. So report an exception.
> m_validator.throwIncorrectOverridingOfAccessType(
> emDesc.getJavaClass(),
> accessTypeUsingAnnottaion.toString(),
> accessTypeUsingXML.toString());
> }
> }
> if (accessType == MIXED) {
>
> m_validator.throwBothFieldsAndPropertiesAnnotatedException(emD
> esc.getJavaClass());
> }
> return accessType;
> }
>
> Thanks,
> Sahoo
>
> Marina Vatkina wrote:
> > Sahoo, Team,
> >
> > We discussed with Tom the short term requirements to fix
> this issue, and
> > agreed that for now we should not change the behavior
> supported by the
> > current code. Tom, please correct me if I miss anything.
> >
> > TopLink is able to support sharing an Embeddable between 2
> entities with
> > different access type, because it creates a copy of the
> model for each
> > owner. Which means that the best current approach to fix
> this bug (we can
> > continue philosophical discussions about the features) should be:
> >
> > Rule 1: If an Embeddable, does not define annotations, its
> access type
> > is determined by the access type of the owning entity.
> >
> > Rule 3: It's an error for a verifier mode to detect a
> conflict access
> > type (non-portable application) either between an entity and an
> > embeddable,
> > or between two entities sharing the same embeddable class.
> >
> > Sahoo, I'll leave it for you to determine how the rules above affect
> > rule #2.
> >
> > regards,
> > -marina
> >
> > Sanjeeb Kumar Sahoo wrote:
> >> How about these rules for *our implementation*:
> >>
> >> Rule 1. Annotation processor *always* uses access-type of enclosing
> >> entity to decide access-type of embeddable *irrespective* of
> >> existence of annotations in embeddable class. Since this is a rule
> >> for annotation processor, it does *not* come into picture when
> >> embeddable has metadata-complete = true in mapping XML file. In
> >> future, we can always have our own TopLink specific annotation to
> >> override this rule, or when then spec officially changes
> the rule, we
> >> can change our implementation.
> >>
> >> Rule 2: Allow mapping XML to provide a conflicting
> access-type for an
> >> embeddable only with *metadata-complete = true*. It is an
> *error* if
> >> XML provides a conflicting access-type for an embeddable with
> >> metadata-complete = false.
> >> Allowing XML to override access-type when metadata-complete = false
> >> requires elaborate rules which users may find difficult to
> >> understand, so why have it?
> >>
> >> Rule 3: It is an *error* to use an embeddable in entity
> classes with
> >> conflicting access-types in a persistence unit when the embeddable
> >> does not have metadata-complete = true. A
> ValidationException will be
> >> thrown for this case.
> >>
> >> I think, these rules keeps the behavior spec compliant as
> far as the
> >> current wordings in the spec go and yet we are able to provide the
> >> extra feature of sharing of an embeddable in entity classes with
> >> conflicting access type. Only difference is that, as of
> now user must
> >> use mapping XML to use this extra feature.
> >>
> >> The fix I sent for code review was already doing #1 and
> #3. I need to
> >> do #2 and send the code for review again. I hope not to see many
> >> objections to these rules.
> >>
> >> I am deliberately not mentioning behavior of verifier. Let's keep
> >> verifier out of this discussion.
> >>
> >> Thanks,
> >> Sahoo
> >>
> >> Marina Vatkina wrote:
> >>
> >>> Mike,
> >>>
> >>> This is not what the spec says in section "2.1.5
> Embeddable Classes":
> >>>
> >>> "The access type for an embedded object is
> >>> determined by the access type of the entity in which it
> is embedded."
> >>>
> >>> There is nothing else in the spec that states otherwise
> (I searched
> >>> for embeddable), so the only option to support a mixed access type
> >>> correctly will be to introduce a product-specific annotation which
> >>> will make it absolutely clear that a developer uses a
> product-specific
> >>> feature.
> >>>
> >>> thanks,
> >>> -marina
> >>>
> >>> Mike Keith wrote On 08/09/06 12:23,:
> >>>
> >>>
> >>>> Right. The combination of access types is undefined,
> >>>> but an embeddable and an entity are two separate
> >>>> classes and putting annotations on an embedded object has nothing
> >>>> to do with combining access types with the entity. They are
> >>>> completely separate objects and do not share, merge or combine
> >>>> persistent state.
> >>>>
> >>>> We actually did have technical reasons for not supporting
> >>>> different access types in inheritance hierarchies, and this
> >>>> was the reason why the limitation exists. The confusion
> factor went
> >>>> up significantly when we tried to define the rules for how field
> >>>> and property mappings were defaulted in an entity
> >>>> hierarchy with and without mapped superclasses.
> >>>>
> >>>> I agree that in the next release we will need to clarify.
> >>>> I also do not want to alienate features that make
> perfect sense and
> >>>> are essentially just miswordings of the spec. Non-portability
> >>>> is one thing, making them appear wrong is another.
> >>>>
> >>>> -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 2:23 PM
> >>>>> To: Mike Keith
> >>>>> Cc: Sanjeeb Kumar Sahoo; persistence_at_glassfish.dev.java.net
> >>>>> Subject: Re: code review for issue #831
> >>>>>
> >>>>>
> >>>>>
> >>>>> The spec is very consistent in treating the combination of
> >>>>> access types as undefined. The fact that it *could* work
> >>>>> does not make it any less undefined, or the spec any more
> >>>>> or less 'incorrect'. We didn't have any technical reasons
> >>>>> for not supporting different access types in mapped superclasses
> >>>>> either.
> >>>>>
> >>>>> At any rate, I do think we should look at clarifying this
> >>>>> in a future release, systematically with the other cases
> >>>>> for which more flexibility makes sense.
> >>>>>
> >>>>> Linda
> >>>>>
> >>>>>
> >>>>> 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
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>
> >>>
> >>>
> >>
> >
>