Re: [hibernate-dev] Another @Access quandry

So the spec does specifically say "It is not permitted to specify a field as Access(PROPERTY) or a property as Access(FIELD)". I understanding making usability choices when the spec is unclear. Do you think there is non-clarity in that quote though? Even if you say making usability choices when the spec is very clear, but "wrong"... again I can live with that. But we need to be clear about that. On Wed, Mar 26, 2014 at 11:21 AM, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; wrote: My take on the spec as always been that I'd rather follow the intent, the common sense above the letter. Likewise, I favored user experience over spec strict adherence. I did clash numerous time with the TCK in these targets but I still prefer that over just doing something stupid but spec to the letter. (this is general and not specific to that case). Anyway so my take is pretty much as it was when I first implemented @AccessType even if it steps over the spec at the margin. BTW I'm also happy if we all decide I made a usability mistake that should be fixed. Emmanuel On Wed 2014-03-26 11:14, Steve Ebersole wrote: > On Wed, Mar 26, 2014 at 10:12 AM, Steve Ebersole <steve(a)hibernate.org&gt;wrote: > > > It does violate the spec though, that's the problem: > > > > Well it *could* be read to violate the spec. That's inherently the problem > with specs that use unclear wording; they can be read and argued multiple > ways. > > > > > > > "... It is not permitted to specify a field as Access(PROPERTY) or a > > property as Access(FIELD)..." > > > > which imo is exactly what this is doing (specifying a property as FIELD): > > > > @Id > > @GeneratedValue > > @Access(AccessType.FIELD) > > public long getId() { > > return id; > > } > > > > > > > > On Wed, Mar 26, 2014 at 9:56 AM, Sanne Grinovero <sanne(a)hibernate.org&gt;wrote: > > > >> I do of course agree that people should use a single strategy and > >> stick to it, so I agree with your reading about what the "general > >> expectation" is. > >> > >> But the original test represents a quite naturally looking example and > >> it's hard to justify why that should be considered illegal; I'd > >> probably be more inclined in making user's life easier than try to > >> lecture them about how a proper mapping should look like. > >> > >> Ignoring any annotation leads to waste of time and debugging > >> frustration, so rather than silently discarding a mis-positioned > >> annotation I'd prefer a fail-fast approach; that said I think just > >> applying them all - as long as there are no obvious conflicting > >> annotations - would be even more user friendly and doesn't seem to > >> violate any specific wording of the spec. > >> > >> Sanne > >> > >> > >> On 26 March 2014 13:57, Steve Ebersole <steve(a)hibernate.org&gt; wrote: > >> > Again from the spec (still discussing class-level Access(PROPERTY)) : > >> "The > >> > behavior is undefined if mapping annotations are placed on any instance > >> > variables defined by the class for which Access(FIELD) is not > >> specified". > >> > > >> > Which to me implies that the expectation for switching access for a > >> > particular field within such a class is to annotate the *field* with > >> > Access(FIELD). > >> > > >> > Also the footnote to this sections seems very relevant: > >> > > >> > "[8] ... It is not permitted to specify a field as Access(PROPERTY) or a > >> > property as Access(FIELD)..." > >> > > >> > > >> > > >> > On Wed, Mar 26, 2014 at 8:02 AM, Emmanuel Bernard < > >> emmanuel(a)hibernate.org&gt; > >> > wrote: > >> >> > >> >> My reading at the time and what I did find more intuitive is what the > >> >> test represents. > >> >> > >> >> Entity level @AccessType expresses where the annotations should > >> >> be. Otherwise the position of @Id is used to find the access type to > >> >> consider annotation wise. > >> >> > >> >> If for a few attributes I wish to use the alternative property access, > >> I > >> >> can add @AccessType next to the other annotations but expressing that > >> >> the actual property value access is based on the alternative access. > >> >> That way, all annotations are in the same place. > >> >> > >> >> On Wed 2014-03-26 11:12, Sanne Grinovero wrote: > >> >> > As a user I would not expect the @Access annotation to be treated as > >> a > >> >> > special case by the framework in terms of when an annotation is > >> >> > ignored, as for example that I can put this on either properties or > >> >> > fields, and it would not be ignored, while other annotations could be > >> >> > ignored depending on the position. > >> >> > > >> >> > Also I highly doubt that there is a practical use case to "comment" a > >> >> > mapping annotation by moving it to the wrong position (say I move a > >> >> > @GeneratedValue from a field to a property when using FIELD access): > >> >> > that would be extremely confusing to maintain. > >> >> > > >> >> > The spec's wording states "When Access(PROPERTY) is applied to an > >> >> > [...] mapping annotations **may** be placed on .." > >> >> > I'd stress that it doesn' t say "must" but "may", and also doesn't > >> >> > seem to strictly ban the opposite. > >> >> > > >> >> > As a user if I put a mapping annotation anywhere I expect it to be > >> >> > respected, so I would expect the framework to work on the union of > >> the > >> >> > possible positions, and probably even to throw an exception on > >> >> > conflicting options. The @Access property would then only be used to > >> >> > state which access strategy should be used (and a nice effect is tha > >> >> > the name becomes particularly self-explanatory too). > >> >> > > >> >> > Also there are many types of possible contradictions in the mapping > >> >> > options: > >> >> > > >> >> > public class Course { > >> >> > @Id @GeneratedValue(strategy=TABLE) > >> >> > private long id; > >> >> > ... > >> >> > > >> >> > @Id @GeneratedValue(strategy=SEQUENCE) > >> >> > public long getId() { > >> >> > return id; > >> >> > } > >> >> > > >> >> > Or you could have a stronger conflict which isn't solvable via > >> >> > AccesType "rules" either: > >> >> > > >> >> > public class Course { > >> >> > @Id @GeneratedValue(strategy=TABLE) > >> >> > @Access(AccessType.FIELD) > >> >> > private long id; > >> >> > ... > >> >> > > >> >> > @Id @GeneratedValue(strategy=SEQUENCE) > >> >> > @Access(AccessType.PROPERTY) > >> >> > public long getId() { > >> >> > return id; > >> >> > } > >> >> > > >> >> > This last example is the reason why I think you should always > >> >> > consistently look at both to collect mapping options, and possibly > >> >> > throw runtime exceptions. > >> >> > > >> >> > Sanne > >> >> > > >> >> > > >> >> > > >> >> > > >> >> > On 26 March 2014 04:13, Steve Ebersole <steve(a)hibernate.org&gt; wrote: > >> >> > > >From the test > >> >> > > > >> >> > > > >> org.hibernate.test.annotations.access.jpa.AccessMappingTest#testExplicitPropertyAccessAnnotationsWithHibernateStyleOverride > >> >> > > we have the following: > >> >> > > > >> >> > > > >> >> > > @Entity > >> >> > > @Access(AccessType.PROPERTY) > >> >> > > public class Course3 { > >> >> > > private long id; > >> >> > > ... > >> >> > > > >> >> > > @Id > >> >> > > @GeneratedValue > >> >> > > @Access(AccessType.FIELD) > >> >> > > public long getId() { > >> >> > > return id; > >> >> > > } > >> >> > > ... > >> >> > > } > >> >> > > > >> >> > > The test asserts that this is a valid mapping. Granted that the > >> spec > >> >> > > is > >> >> > > very unclear here, so I might be missing something. The pertinent > >> >> > > spec > >> >> > > section here states: > >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > *<quote>When Access(PROPERTY) is applied to an entity class, mapped > >> >> > > superclass, or embeddableclass, mapping annotations may be placed > >> on > >> >> > > the > >> >> > > properties of that class, and the persistenceprovider runtime > >> accesses > >> >> > > persistent state via the properties defined by that class. All > >> >> > > proper-ties > >> >> > > that are not annotated with the Transient annotation are > >> persistent. > >> >> > > WhenAccess(PROPERTY) is applied to such a class, it is possible to > >> >> > > selectively designate indi-vidual attributes within the class for > >> >> > > instance > >> >> > > variable access. To specify a persistent instancevariable for > >> access > >> >> > > by the > >> >> > > persistence provider runtime, that instance variable must be > >> >> > > desig-nated > >> >> > > Access(FIELD).</quote>* > >> >> > > > >> >> > > > >> >> > > I can see a few different ways to read that: > >> >> > > > >> >> > > 1) @Access can be placed on the attribute to define both where to > >> look > >> >> > > for > >> >> > > mapping annotations and the runtime access strategy for a given > >> >> > > attribute. > >> >> > > Here, we'd do: > >> >> > > > >> >> > > @Entity > >> >> > > @Access(AccessType.PROPERTY) > >> >> > > public class Course3 { > >> >> > > @Id > >> >> > > @GeneratedValue > >> >> > > @Access(AccessType.FIELD) > >> >> > > private long id; > >> >> > > ... > >> >> > > > >> >> > > public long getId() { > >> >> > > return id; > >> >> > > } > >> >> > > ... > >> >> > > } > >> >> > > > >> >> > > 2) @Access can be placed on the attribute to define the runtime > >> access > >> >> > > strategy for a given attribute, but the class/hierarchy AccessType > >> >> > > controls > >> >> > > where to look for mapping annotations. This would lead to: > >> >> > > > >> >> > > @Entity > >> >> > > @Access(AccessType.PROPERTY) > >> >> > > public class Course3 { > >> >> > > @Access(AccessType.FIELD) > >> >> > > private long id; > >> >> > > ... > >> >> > > > >> >> > > @Id > >> >> > > @GeneratedValue > >> >> > > public long getId() { > >> >> > > return id; > >> >> > > } > >> >> > > ... > >> >> > > } > >> >> > > > >> >> > > The test seems to illustrate that our legacy code made yet a 3rd > >> >> > > reading of > >> >> > > this passage such that @Access is still considered a "mapping > >> >> > > annotation" > >> >> > > even though that seems to directly contradict "To specify a > >> persistent > >> >> > > instance > >> >> > > variable for access by the persistence provider runtime, that > >> instance > >> >> > > variable must be desig- > >> >> > > nated Access(FIELD)." > >> >> > > > >> >> > > > >> >> > > Is there some other passage I am missing that bears on what to do > >> >> > > here? > >> >> > > How do y'all feel about that passage and its implications on this > >> >> > > test > >> >> > > mapping? > >> >> > > _______________________________________________ > >> >> > > hibernate-dev mailing list > >> >> > > hibernate-dev(a)lists.jboss.org > >> >> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >> >> > _______________________________________________ > >> >> > hibernate-dev mailing list > >> >> > hibernate-dev(a)lists.jboss.org > >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >> > > >> > > >> > > > >