On Wed, Mar 26, 2014 at 10:12 AM, Steve Ebersole <st...@hibernate.org>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 <sa...@hibernate.org>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 <st...@hibernate.org> 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 < >> emman...@hibernate.org> >> > 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 <st...@hibernate.org> 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@lists.jboss.org >> >> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> > _______________________________________________ >> >> > hibernate-dev mailing list >> >> > hibernate-dev@lists.jboss.org >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > >> > >> > > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev