[hibernate-dev] Some thoughts on possible Binder changes

[Steve Ebersole]

Some more follow-up on this...

Ordered processing of entity hierarchies based on id dependencies

One of the "wait on" scenarios I discussed above where events would help is
in the case of identifier which are made up of one or more many-to-one
associations.  What ends up happening currently is that we iterate the
entity hierarchies in a undefined order.  So in a normal Customer/Order
like described in the earlier emails, its possible that we process Order
before we process Customer.  One thing I just finished as a pre-step
(during the indexing we do of the sources anyway) was to build an ordered
list of the hierarchies based on the to-one dependencies expressed by their
identifiers.  I am not sure this works in all cases yet, but its at least a
best effort.


@MapsId attributes

In the case of EmbeddedId composite ids, we also have this notion in JPA of
"maps-id":

@Embeddable
class OrderPK implements Serializable {
  public Integer customerId;
  public Integer orderNumber;
  ...
}

@Entity
class Customer {
  @Id
  private Integer id;
  ..
}

@Entity
class Order {
  @EmbeddedId
  private OrderPK pk;

  @MapsId("customerId")
  @ManyToOne
  private Customer customer;

  ...
}

@MapsId is accounted for when determining the "ordered processing of entity
hierarchies based on id dependencies".  However, there is still some
difficulty here in that Binder tries to process all identifiers before
moving on to "non-identifier" attributes.  Atm it does not consider
@MapsId attributes
to be part of the "identifier".  The trouble comes from the fact that we
need to have the Order#customer attribute resolved before we can fully
resolve the Order identifier.

One proposal to get around that is that we process attributes marked as
@MapsId as we process the identifiers in terms of binding.  In the binding
model, there would still be a non-identifier attribute for Order#customer;
I just am talking about timing in the Binder.  The other option is to wait
using events; though that seems far more likely to lead to indefinite wait
scenarios.




On Tue, Apr 15, 2014 at 9:25 PM, Steve Ebersole <steve at hibernate.org> wrote:

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