Re: [hibernate-dev] Some thoughts on possible Binder changes

After discussing this and the current process some more with Gail on IRC I have a more clear understanding of exactly what is needed here. Hardy, the clarification ties directly with your question: What is "complete"? In other words, what exactly are the events used to signify? The answer is complete resolution of a Type. The state of Type resolution as we build the "binding model" is encapsulated in the HibernateTypeDescriptor class. Ultimately the "event" we are interested in is the call to HibernateTypeDescriptor#setResolvedTypeMapping(Type). This is the event that signals that an attribute is fully resolved. At that point it is ok to access its Type (obviously), its relational bindings, etc. HibernateTypeDescriptor is associated with each and every attribute via AttributeBinding#getHibernateTypeDescriptor. Some other things besides attributes have HibernateTypeDescriptor associated with them also (some special id cases, collection elements, map keys, etc). The question then becomes how to model this. The first "difficulty" is that most things understand their dependence on a Type through an Attribute; but HibernateTypeDescriptor does not know the attribute (or collection element or ..) it "comes from". I have added a listener for the HibernateTypeDescriptor#setResolvedTypeMapping call in the HibernateTypeDescriptor, but that still does not get us the ability to route this back to things that are interested in this condition. Let's take a look back at the Person/Address/Zip example as an illustration. When we process Person#address we cannot fully resolve the CompositeType here because the sub-attributes (street, zip, etc) have not yet been bound. So we'd register interest in those attributes becoming complete to finalize the Person#address attribute. But we can't just add a listener to the HibernateTypeDescriptor associated with each of those sub-attribute bindings... the sub-attribute bindings don't exist yet. Here's another situation: @Entity class Order { @EmbeddedId OrderPk pk; @MapsId @ManyToOne Customer customer; ... } @Entity class LineItem { ... @ManyToOne Order order; } Here the situation is that the identifier for Order cannot be fully resolved "up front" because of the @MapsId. Thus resolution of the Orderentity is dependent on the resolution of its "normal" attribute customer. We could mitigate this somewhat by saying that @MapsId attributes are handled specially, when we handle identifiers; but lets for now say we leave it as-is... So we'd need to register a listener to finalize the Order identifier resolution when customer is finalized. Again notice that when processing the Order identifier, the attribute binding for customer does not yet exist, so we can't directly add a listener to the HibernateTypeDescriptor. A similar situation exists for the LineItem order attribute in that it will need to wait for the Order identifier to be finalized. The most straight forward approach for bridging HibernateTypeDescriptor and AttributeBinding would be a whiteboard pattern. Have a third party (Binder) be the listener for all HibernateTypeDescriptor for all created attribute bindings. It would maintain a map of HibernateTypeDescriptor -> AttributeBinding. An alternative would be to make AttributeBinding#getHibernateTypeDescriptor "bi directional". But the externalized mapping allows for some flexibility in the "sources of HibernateTypeDescriptor that are not an attribute" cases I mentioned earlier. Anyway, this third part delegate would listen for HibernateTypeDescriptor#setResolvedTypeMapping events (which get passed the HibernateTypeDescriptor) and map that to listener calls based on the AttributeBinding. Its maybe not the cleanest solution. I think (longer term?) I'd really like to make the binding model immutable after creation the way it was initially intended when we started down the metamodel path. But given the mutable in-flight binding model solution in place, any better suggestions for handling this? On Mon, Apr 14, 2014 at 5:12 PM, Steve Ebersole <steve(a)hibernate.org&gt;wrote: > That document was last updated 9 months ago ("Last edited by Strong Liu, > 9 months ago"). And in the interim we had the merge into master after > which we updated a BUNCH of tests to use @FailureExpectedWithNewMetamodel. > So I am not sure how you think that is up to date. > > TBH, I just go off of usage-searches for > the @FailureExpectedWithNewMetamodel annotation. Initially I used that to > try and categorize the failures because most of the usages simply have > @FailureExpectedWithNewMetamodel() with no useful information (jiraKey or > message). In general I noticed a lot of failures with composite ids, so > thats where I decided to start looking. > > Sure, ideally we would refactor after having a more working set up. But > you have to realize how this is for people coming into this code. Just > take Binder. Like I said, its 4K lines of hard to follow and essentially > non-documented, non-commented code. That's not easy for someone to jump > into. So in my efforts to understand what this code is supposed to be > doing in many places, combined with trying to solve missing or non-working > pieces of functionality I see some ways to improve it. > > Take a simple example, like composite ids with IdClass. At the moment > the code has no understanding that the IdClass mappings come from the > entity; essentially it tries to process it like it does any other > composite. This is because everything routes through the same "build > attribute binding" code no matter where the attribute comes from. I had > already fixed this in the annotation source code using a "attribute builder > strategy" so I had different strategies for the 3 different ways we needed > to build attributes. So that's a natural paradigm to apply here. It's is > the same failure, so its likely the same solution would work. This is not > "refactoring, just to refactor"... this is refactoring to fix a problem. > > > > > On Mon, Apr 14, 2014 at 3:55 PM, Gail Badner <gbadner(a)redhat.com&gt; wrote: > >> Also inline... >> >> ----- Original Message ----- >> > From: "Steve Ebersole" <steve(a)hibernate.org&gt; >> > To: "Hardy Ferentschik" <hardy(a)hibernate.org&gt; >> > Cc: "Hibernate" <hibernate-dev(a)lists.jboss.org&gt; >> > Sent: Saturday, April 12, 2014 11:55:28 AM >> > Subject: Re: [hibernate-dev] Some thoughts on possible Binder changes >> > >> > Thanks for the response. See inline... >> > >> > >> > On Sat, Apr 12, 2014 at 1:15 PM, Hardy Ferentschik >> > <hardy(a)hibernate.org&gt;wrote: >> > >> > > >> > > On 12 Jan 2014, at 18:56, Steve Ebersole <steve(a)hibernate.org&gt; >> wrote: >> > > >> > > > The Background >> > > >> > > ... >> > > >> > > Thanks Steve, for this really nice summary. It is always good to >> share >> > > some basic design/implementation >> > > details. >> > > >> > > > In terms of dealing with composite ids, step (1) really just means >> > > creating >> > > > the Embeddable "shells" (the EmbeddableBinding instance). But at >> this >> > > > point the EmbeddableBinding is not done, we still need its >> attributes >> > > > "resolved" or "bound". To accomplish this, as Binder walks >> through the >> > > > rest of the steps, it continually checks whether the completion of >> the >> > > > attribute it just bound completes the binding of the Embeddable. >> So as >> > > it >> > > > is looping over every attribute, for each attribute it loops over >> every >> > > > known incomplete EmbeddableBinding and checks whether that >> attribute >> > > > "completes" the EmbeddableBinding and if so finalizes it's binding. >> > > >> >> Yes, this is what it does. I agree there are better ways to deal with >> it. I thought the priority was to get functionality working ASAP. It was >> the easiest way to get it working at the time, and I figured we'd refine >> after the alpha. >> >> > > What do you mean by "completes". How do you know that the >> > > EmbeddableBinding is complete. >> > > >> > >> > For embeddables, this boils down to its sub-attributes being fully >> bound. >> > Ultimately we need to be able to generate the Hibernate Type. So >> looking >> > at my example below, ultimately what we care about in regards to >> > Person#address is the resolved Type for that attribute. >> > >> > So here, "completes" is the verb form; the idea being simply.. was the >> > attribute we just finished processing the last unresolved >> sub-attribute for >> > a embeddable; did it "complete" the embeddable in terms of all its >> > sub-attributes now being done. >> > >> > As for how we know that, that depends. In the existing Binder code we >> > literally iterate the attributes making up the embeddable and see if >> the >> > Type for all those sub-attributes has been resolved. >> > See >> > >> org.hibernate.metamodel.internal.binder.Binder#completeCompositeAttributeBindingIfPossible >> > for the current process. >> > >> > I am suggesting this change to use events as outlined below. >> > >> > >> > >> > > >> > > > Which got me to thinking about using events to signal the >> completion of >> > > > things, and the ability to listen for these events. Don't worry, >> I mean >> > > > events here as fairly light weight concept :) >> > > >> > > For what it's worth, Strong had once the same idea. Instead of >> rechecking >> > > and looping he also wanted to >> > > introduce some sort of event based processing. I thought the idea >> sounded >> > > promising. >> > > I am not sure how far he got or whether he even started. I think >> this was >> > > not long before metamodel was put on >> > > ice fore a while. >> > > >> > >> > To be honest, I had the same suggestion for HBMBinder as well even >> back in >> > the day to get out of second passes. I think its a somewhat natural >> > paradigm for the type of problem domain here. >> > >> >> I also remember Strong mentioning an events approach, but I think it was >> after embeddables were already working. >> >> > >> > >> > > >> > > > First, there is the general pros/cons of sequential processing >> versus >> > > > event-driven processing. Some folks view event-driven processing >> as more >> > > > convoluted, harder to follow. >> > > >> > > It can not get much worse than following the 4k Binder as it stands >> now. >> > > Event based processing >> > > can sometimes be tricky. Maybe it would help in this case to >> document the >> > > approach and >> > > algorithm and the main actors. Either in the javadocs or maybe even >> better >> > > in an topical guide (more >> > > dev centric in this case). >> > > >> > >> > True with the "it can't get much worse" aspect. I think sequential >> > processing is fine/great if the thing you are doing is relatively >> simple. >> > I think its safe to say that this is not simple :) >> > >> > >> > > >> > > > Anyway... thoughts? comments? >> > > >> > > For me it is also a question of time and resources. I agree that >> cleaning >> > > up the binding code would be >> > > awesome, but on the other hand I thought most of the details for >> binding >> > > the new metamodel had been >> > > sorted out by now. Is it worth rewriting now. On the other hand, if >> there >> > > are real issues with the code >> > > it might be worth the try. >> > > >> >> I had also assumed that breaking up the Binder would be post-alpha. >> >> > >> > I think "cleaning up" and "paradigm shift" are different beasts. Yes >> > cleaning up can be done any time (even later) relatively easily. >> > Completely shifting the underlying principles by which you attack a >> > problem is altogether different in my mind; I think the approach is >> best >> > ironed out from the onset. >> > >> > That being said, a lot of the actual functionality is already in place. >> > Its just a matter of organizing it slightly differently in most cases. >> > >> >> I agree a paradigm shift is best as soon in the process as possible. >> >> > As for most cases being handled... well the 492 *uses* (not tests mind >> you, >> > uses equate to one or more tests) of FailureExpectedWithNewMetamodel >> would >> > beg to differ. And that's not counting envers in any way which >> currently >> > has tons of failures because of the shift to metamodel. Lots of things >> > simply do not work yet in metamodel. >> >> I would say that most things do work. Strong and I kept track of what >> was left by maintaining this document: >> https://github.com/hibernate/hibernate-orm/wiki/Failing-metamodel-tests. Steve, are you keeping this up-to-date? >> >> >> > _______________________________________________ >> > hibernate-dev mailing list >> > hibernate-dev(a)lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > >> > >