3:36 p.m.
I think this warranted a separate discussion.
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
Conversion to CascadeType from the various sources then needs to be
multi-valued (CascadeType[]), but that is actually how it is already
anyway if you look at the attribute bindings. It has to be able to
handle custom combinations.
CascadeStyle cannot be made into an enum. Well, that is if we keep the
notion of org.hibernate.engine.spi.CascadeStyle.MultipleCascadeStyle to
not do those "aggregated style" checks everywhere throughout the code base.
I'd also like to see some changes to CascadingAction, but that is out of
the scope of this discussion I guess since it does not affect the
metamodel at all.
On 06/24/2011 05:13 AM, Hardy Ferentschik wrote:
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<steve(a)hibernate.org>
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
--
Steve Ebersole <steve(a)hibernate.org>
http://hibernate.org