[hibernate-dev] CascadeStyle, CascadeType etc

[Steve Ebersole]

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 at 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev

-- 
Steve Ebersole <steve at hibernate.org>
http://hibernate.org