On 06/23/2011 05:42 PM, Gail Badner wrote: > I just ran into an issue where EntityBinding.getOptimisticLockMode() returns an ordinal from a org.hibernate.annotations.OptimisticLockType enum that is inconsistent with the static constants in org.hibernate.engine.internal.Versioning. > > Jira issue: http://opensource.atlassian.com/projects/hibernate/browse/HHH-6362 > > I'm not clear on what conventions there are for enums that are source-specific. > > My preference would be to: > > 1) Extract the static constants for version type from org.hibernate.engine.internal.Versioning into a new enum that is an SPI, org.hibernate.engine.spi.OptimisticLockMode. > > 2) Change EntityBinding.getOptimisticLockMode() and EntityBindingState.getOptimisticLockMode() to return org.hibernate.engine.spi.OptimisticLockMode. > > 3) Change sources to be responsible for converting from a source-specific value to org.hibernate.engine.spi.OptimisticLockMode. Aside from the enum name, I think this is all goodness. These are distinctly different from LockModes and we should avoid that confusion. I tend to like to use the term "strategy" with pluggable interfaces, but I think OptimisticLockingStrategy is definitely descriptive here. Anyone else with better names? > In the same vein, I see there is org.hibernate.metamodel.binding.CascadeType, which is not source-specific. In that class, there's a TODO to integrate it with org.hibernate.engine.spi.CascadeStyle. > > I also see that AttributeBindingState.getCascadeTypes() returns Set<CascadeType>. > > My preference would be to remove org.hibernate.metamodel.binding.CascadeType and to change AttributeBindingState.getCascadeTypes() and AbstractAttributeBinding.getCascadeTypes() to: > > org.hibernate.engine.spi.CascadeStyle getCascadeStyle(); I think this is fine. The comment is Hardy's though, so lets let him respond to see if he had something else in mind. -- Steve Ebersole <steve(a)hibernate.org&gt; http://hibernate.org _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

Hi, I am also for moving to enums. I am not sure whether or why this should go to org.hibernate.engine.spi though. I am still at odds with this package refactoring. I think we discussed once before that there are several places in the code which would benefit from the introduction of an enum which can be used by annotations and hbm. I think we will find more than just OptimisticLockMode. Regarding CascadeStyle I definitely had a consolidation in mind as well. Really org.hibernate.engine.spi.CascadeStyle has to become a enum. The reason I did not do it was that this class is used outside the new metamodel code (obviously) and I tried so far to stay away from changes which affect existing/running code. Now that the integration work into the Persisters and so on has started it might be the right time to start doing this type of things as well. Another thought regarding this class is that it currently contains some logic. I am not a big fan of having a logic in an emum. CascadeStyle might be borderline, but it is something to keep in mind. --Hardy On Fri, 24 Jun 2011 02:43:16 +0200, Steve Ebersole&lt;steve(a)hibernate.org&gt; wrote: > On 06/23/2011 05:42 PM, Gail Badner wrote: >> I just ran into an issue where EntityBinding.getOptimisticLockMode() >> returns an ordinal from a org.hibernate.annotations.OptimisticLockType >> enum that is inconsistent with the static constants in >> org.hibernate.engine.internal.Versioning. >> >> Jira issue: >> http://opensource.atlassian.com/projects/hibernate/browse/HHH-6362 >> >> I'm not clear on what conventions there are for enums that are >> source-specific. >> >> My preference would be to: >> >> 1) Extract the static constants for version type from >> org.hibernate.engine.internal.Versioning into a new enum that is an >> SPI, org.hibernate.engine.spi.OptimisticLockMode. >> >> 2) Change EntityBinding.getOptimisticLockMode() and >> EntityBindingState.getOptimisticLockMode() to return >> org.hibernate.engine.spi.OptimisticLockMode. >> >> 3) Change sources to be responsible for converting from a >> source-specific value to org.hibernate.engine.spi.OptimisticLockMode. > > Aside from the enum name, I think this is all goodness. These are > distinctly different from LockModes and we should avoid that confusion. > I tend to like to use the term "strategy" with pluggable interfaces, > but I think OptimisticLockingStrategy is definitely descriptive here. > Anyone else with better names? > > >> In the same vein, I see there is >> org.hibernate.metamodel.binding.CascadeType, which is not >> source-specific. In that class, there's a TODO to integrate it with >> org.hibernate.engine.spi.CascadeStyle. >> >> I also see that AttributeBindingState.getCascadeTypes() returns >> Set<CascadeType>. >> >> My preference would be to remove >> org.hibernate.metamodel.binding.CascadeType and to change >> AttributeBindingState.getCascadeTypes() and >> AbstractAttributeBinding.getCascadeTypes() to: >> >> org.hibernate.engine.spi.CascadeStyle getCascadeStyle(); > > I think this is fine. The comment is Hardy's though, so lets let him > respond to see if he had something else in mind. _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

On Fri, 24 Jun 2011 15:36:28 +0200, Steve Ebersole&lt;steve(a)hibernate.org&gt; wrote: > First, I think CascadeType should be changed up a bit. Currently it > contains some "aggregate" values (ALL, ALL_DELETE_ORPHAN). I'd like to > keep this limited to just the "atomic" cascade-types. Rather than the > map lookups here based on the enum value, I'd much rather see the enum > expose that "alternate value". For example: > https://gist.github.com/1044751 Ok. The cascade style to cascade type map is new to me anyway. Btw, why do you prefer the "enum expose that "alternate value"" approach?