Sahoo,
Your rules were well-written and quite clear, and I had no problems
seeing the rationale behind them, but thanks for your message, anyway.
:-)
-Mike
> -----Original Message-----
> From: Sanjeeb.Sahoo_at_Sun.COM [mailto:Sanjeeb.Sahoo_at_Sun.COM]On Behalf Of
> Sanjeeb Kumar Sahoo
> Sent: Thursday, August 24, 2006 7:05 AM
> To: persistence_at_glassfish.dev.java.net; Mike Keith
> Cc: Linda.Demichiel_at_Sun.COM
> Subject: Justifications for rule #3 & #4
>
>
> Hi Mike,
>
> Thanks for a prompt reply. Glad to know that we have *finally* agreed.
> Let me try to explain the rationale behind rule #3 & #5 as I felt you
> are not totally convinced by them. I know I don't have to do
> it since we
> have already agreed, but I just think this needs to be done so that we
> can clarify this in the future spec.
>
> *Rule #3:* It was added to avoid surprises that may be caused to users
> for a use case like this:
> @Embeddable public class C {
> // has no metadata. It has two fields, but one property!
> int i; int j;
> int getI() { return i;}
> void setI(int i) { this.i = i;}
> }
>
> @Entity public class A {
> @Id String id; // entity class uses FIELD access
> @Embedded C c;
> }
>
> @Entity public class B {
> String id;
> C c;
> @Id String getId() { return id} // entity class uses PROPERTY access
> void setId(String id) { this.id = id;}
>
> @Embedded C getC() { return c;}
> void setC(C c) { this.c = c;}
> }
>
> If we allow this use case, then Class C will have two different
> persistence views in the same PU. From A's point of view, it has two
> persistence fields. From B's point of view, it has one persistence
> property. I sincerely think, this kind of inconsistent
> sharing should be
> disallowed. User could have easily annotated either a field
> or property
> of C and that would have *not only* allowed sharing of the
> embeddable in
> A & B, it would *also* have provided a consistent view of C
> to both the
> enclosing entity classes. Or user could have had two different
> embeddable classes if she wanted two different database
> representations.
> Of course, we can write extensive verification logic that checks for
> inconsistencies, but as you said the use case is so weak that I don't
> see any good reasons to write them.
>
> *Rule #4:* This use case leads to similar confusion that arises when
> both fields and properties are annotated in a class. So
> having this rule
> avoids more *complex* rules. The current overriding rules are already
> quite *complex*. I *can't* write a program that uses mapping
> xml without
> having the spec in front of me. IMHO overriding of metadata
> using XML is
> supposed to be *sparingly* used. For such a small number of
> users, why
> should make the spec overly complex?
>
> Thanks & regards,
> Sahoo
>
> Mike Keith wrote:
> > 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) {
> >> Thanks,
> >> Sahoo
>
>