[hibernate-dev] Some thoughts on possible Binder changes

[Steve Ebersole]

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
Orderidentifier resolution when
customer is finalized.  Again notice that when processing the
Orderidentifier, 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 at hibernate.org> 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 at redhat.com> wrote:
>
>> Also inline...
>>
>> ----- Original Message -----
>> > From: "Steve Ebersole" <steve at hibernate.org>
>> > To: "Hardy Ferentschik" <hardy at hibernate.org>
>> > Cc: "Hibernate" <hibernate-dev at lists.jboss.org>
>> > 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 at hibernate.org>wrote:
>> >
>> > >
>> > > On 12 Jan 2014, at 18:56, Steve Ebersole <steve at hibernate.org> 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 at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> >
>>
>
>