https://github.com/hibernate/hibernate-orm/pull/591
On Thu 05 Sep 2013 10:06:14 PM CDT, Shaozhuang Liu wrote: > I can give it a try > ------------------------- > Best Regards, > > Strong Liu <stliu at hibernate.org> > http://about.me/stliu/bio > > On 2013Sep 6, at 8:15 AM, Steve Ebersole <st...@hibernate.org> wrote: > >> I have something mostly working and not causing regressions. Its quite >> fugly, but I blame that on binders and hcann :) >> >> I'd really like to do a pull request for this one and have y'all take a >> look. But at the same time I'd also really like to do the 4.3 Beta4 release >> tomorrow. Anyone familiar with those bits of code willing to look this over >> in the next 12 hours if I go through the steps of creating the PR? >> >> >> On 09/05/2013 12:10 PM, Steve Ebersole wrote: >>> On Thu 05 Sep 2013 11:52:22 AM CDT, Shaozhuang Liu wrote: >>>>> Also, I am not sure that iterating properties yet again is a great idea. >>>>> >>>>> One alternative I had considered was to hook this in with >>>>> PropertyBinder, where it calls SimpleValueBinder. That needs to happen >>>>> anyway; the plan was to delegate the call to determine the Convert to >>>>> use PropertyHolder and to pass that Convert into SimpleValueBinder. >>>>> That code would need to pass in the XProperty anyway. So currently >>>>> PropertyBinder does: >>>>> >>>>> simpleValueBinder.setType( property, returnedClass, containerClassName ); >>>>> >>>>> My plan was to instead have it do: >>>>> >>>>> simpleValueBinder.setType( property, returnedClass, containerClassName, >>>>> holder.resolveAttributeConverter( property ) ); >>>>> >>>>> I *think* that when resolveAttributeConverter() would be called, we'd >>>>> have enough info to fully resolve the converter to use properly. >>>>> >>>>> There are similar concerns in ComponentPropertyHolder, but maybe a >>>>> discussion of the above will shed light on those concerns too. >>>> >>>> doesn't this work? >>> >>> What exactly? >>> >>> >>>> at this time, the PropertyHolder is resolved, so it knows the convert it >>>> holds, >>>> then the convert defined on the attribute get applied first, then the one >>>> in PropertyHolder >>> >>> I am not sure what you mean by "resolved" here. At no time that is useful >>> in this process does PropertyHolder have full access to all the properties >>> the thing holds. >>> >>> If by "the convert it holds" you mean the converts specifically defined on >>> the @Entity or @Embedded (and therefore @Embeddable), then yes. >>> >>> But "then the convert defined on the attribute get applied first" is >>> actually wrong for composite paths. For composites, you actually want the >>> one higher up to have higher precedence. In the Person.homeAddress.city >>> example, a convert defined on Address#city should actually have the >>> *lowest* precedence. >>> >>> I think what we'll have to end up doing is to not normalize the composite >>> paths to one place unfortunately. Then we'd have to pass the XProperty >>> for which we are trying to resolve the converter to use into the >>> PropertyHolder method. For composite paths this will just have to mean >>> walking multiple levels (one per path-part). The down side to this is that >>> at no time do we have an overview of the overall converts for a property >>> path (aside from walking the composite path to actually resolve the >>> converter to use). >>> >>> >>>> >>>> any problem with this approach ? >>>> >>>>> >>>>> >>>>> >>>>> On Wed 04 Sep 2013 02:44:40 PM CDT, Steve Ebersole wrote: >>>>>> >>>>>> I am still a bit confused on how to apply the normalization to make >>>>>> sure it happens in the proper order. >>>>>> >>>>>> Let's look at: >>>>>> >>>>>> @Entity >>>>>> class Person { >>>>>> ... >>>>>> @Embedded >>>>>> @Convert( attributeName="city", converter=Converter2.class ) >>>>>> public Address homeAddress; >>>>>> } >>>>>> >>>>>> @Embeddable >>>>>> class Address { >>>>>> ... >>>>>> @Convert( converter=Converter1.class ) >>>>>> public String city; >>>>>> } >>>>>> >>>>>> >>>>>> So here, the Converter2 class ought to be the one applied to >>>>>> "homeAddress.city" path. >>>>>> >>>>>> Now granted there are a few different ways to skin this cat, but the >>>>>> plan I had was to normalize these all on the root of the path. So >>>>>> here, both of these conversions would get stored on >>>>>> ClassPropertyHolder<Person> under the "homeAddress.city" path. When I >>>>>> go to build the SimpleValue for Person.homeAddress.city, I'd ask the >>>>>> CompositePropertyHolder for Person.homeAddress to resolve the explicit >>>>>> (non-autoApply) conversion for its city attribute (the simple value). >>>>>> The trouble I have though is applying the conversions in the proper >>>>>> order. For example, here, I'd want to apply the conversions defined >>>>>> directly on the Embeddable first (the Embeddable conversions should >>>>>> always act as the baseline), then the conversions at its Embedded >>>>>> point (via XML or annotations). >>>>>> >>>>>> One idea was to hook into org.hibernate.cfg.PropertyHolder#addProperty >>>>>> in terms of normalizing the paths. I am just not sure of the timing >>>>>> between these PropertyHolder#addProperty (and how populated the passed >>>>>> Property objects are at that point) and the calls to >>>>>> SimpleValueBinder/PropertyBinder. >>>>>> >>>>>> Interestingly, I still am not sure we have enough here to report an >>>>>> error in cases like: >>>>>> >>>>>> @Entity >>>>>> @Convert( attributeName="homeAddress.city", converter=Converter3.class ) >>>>>> class Person { >>>>>> ... >>>>>> @Embedded >>>>>> @Convert( attributeName="city", converter=Converter2.class ) >>>>>> public Address homeAddress; >>>>>> } >>>>>> >>>>>> Unless we somehow kept "proximity info" or "location info" about the >>>>>> conversion in PropertyHolder. >>>>>> >>>>>> >>>>>> On Wed 04 Sep 2013 01:27:21 AM CDT, Emmanuel Bernard wrote: >>>>>>> >>>>>>> >>>>>>> On Tue 2013-09-03 17:22, Steve Ebersole wrote: >>>>>>>> >>>>>>>> >>>>>>>> 2.a) It seems like there are times when >>>>>>>> org.hibernate.cfg.AbstractPropertyHolder#parent would be useful for >>>>>>>> what I need to do. But there appears to be times when this is null. >>>>>>>> For entity mappings (ClassPropertyHolder) thats fine. But for the >>>>>>>> others, that would be problematic. Going back to the >>>>>>>> Person#homeAddress example, I'd really need the >>>>>>>> ComponentPropertyHolder for Person#homeAddress to register the >>>>>>>> converts with ClassPropertyHolder<Person> under the "homeAddress" >>>>>>>> base key ("homeAddress.city" for example). Is there a time here >>>>>>>> where AbstractPropertyHolder#parent would be null for >>>>>>>> ComponentPropertyHolder/CollectionPropertyHolder? >>>>>>> >>>>>>> >>>>>>> >>>>>>> I looked around the code base and the only case I could find is for >>>>>>> ClassPropertyHolder whose parent is indeed null. >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> 2.b) Is this AbstractPropertyHolder#parent the best way you see to >>>>>>>> handle path-based converts? Or do you see a better option? >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Yes I still think it's the best aproach and frankly I don't quite see >>>>>>> alternatives. >>>>> _______________________________________________ >>>>> 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
