[hibernate-dev] Some thoughts on possible Binder changes

[Steve Ebersole]

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
> >
>