[hibernate-dev] [Search] Regression with @ContainedIn between 4.3 and 4.4

[Hardy Ferentschik]

Hi,

I think we dropped the ball on this one. I basically had a look at Guillaume’s pull request.
His analysis was correct and his proposed patch brings back the old pre 4.4 behaviour
with minimal changes.

There is still the question of the indented use cases of @ContainedIn. As discussed in this thread
afaik the indent was as a sole counterpart to @IndexedEmbedded. The implementation (probably
as a side effect) made it also work when @IndexedEmbedded was not used. This was changed with
the metamodel refactoring where contained in was indeed treated as counterpart of indexed embedded. 

I think we should go ahead and apply Guillaume’s pull request for 4.4 and 4.5, basically reinstating
old 4.x behaviour, side effect or not.

The next question is then what to do in Search 5. I think we can just carry forward the change/patch.
I don’t think that @ContainedIn needs another attribute or that we should introduce a whole new annotation.
@ContainedIn seems quite natural provided we documented it intend and behaviour. Basically all what @ContainedIn
is saying, is that here we have a reference to another entity which needs reindexing as well when the current
entity gets reindexed. Whether the inclusions happens via @IndexEmbedded or via a custom bridge is irrelevant. 

Thoughts?

—Hardy

P.S. Sanne, are you planning 4.4 and 4.5 releases as well? Just asking due to the recent back ports. If so, we should
include this patch as well.





On 28 Jan 2014, at 14:32, Guillaume Smet <guillaume.smet at gmail.com> wrote:

> Hi again,
> 
> I posted a pull request for the 4.5 branch:
> https://github.com/hibernate/hibernate-search/pull/590 . Comments
> welcome!
> 
> I included a rather large test case I have extracted from our app.
> It's simplified but I think you should get the idea by looking at it.
> 
> All tests passes.
> 
> FWIW, I find the new way to do it more consistent with the names of
> the methods as updateContainedInMaxDepths now does exactly what its
> name says.
> 
> It would be nice if it could also be backported to 4.4.x.
> 
> Glad to discuss further enhancements if needed but I think the old
> behavior should be brought back before considering any further
> enhancements/features.
> 
> -- 
> Guillaume
> 
> On Fri, Mar 28, 2014 at 1:28 PM, Guillaume Smet
> <guillaume.smet at gmail.com> wrote:
>> Hi Sanne,
>> 
>> On Fri, Mar 28, 2014 at 12:56 PM, Sanne Grinovero <sanne at hibernate.org> wrote:
>>> However - I might not have fully grasped this yet - but I'm thinking
>>> that this is a feature request and not a bugfix that should be hastily
>>> applied on 4.4.
>> 
>> It's not a feature request. 4.4 changed this behavior. It was working
>> well with 4.3 (and every other versions before that).
>> 
>>> Guillaume, I suspect you might be a happier user if you would keep
>>> using a more bleeding edge version, so to catch any such thing earlier
>>> rather than the day before you go in production. I can only promise
>>> you that the migration to 5.0 will be a nightmare if you don't follow
>>> along in small iterations :-(
>> 
>> I don't think we can be more bleeding edge than us. We upgrade our
>> core framework to new versions as soon as they are released (we coded
>> https://www.artifact-listener.org/ for this purpose). That's why we
>> spend a considerable amount of time doing QA on the components we use
>> - you see a part of this work on ORM + Search but we also do it for
>> other components. You might have noticed that we are often the first
>> company out there to catch regressions.
>> 
>> When we have the ability to dedicate cycles to it, we also test CR.
>> 
>> But... we develop a lot of applications for a lot of customers and
>> they have different lifecycles: we can upgrade some easily and for
>> others we have to wait for the right opportunity to do it.
>> 
>> All in all, we have the following policy:
>> - all the applications in development: they use the latest versions of
>> each components, except if we catch a regression when upgrading. If so
>> we try to help fix it for the next version and we upgrade ASAP;
>> - the applications in production are updated regularly for minor
>> changes; If the changes are big, we wait for our customers to ask
>> changes on the app and we do it then.
>> 
>> The fact is that we caught this one while upgrading an app which was
>> using 4.3.0.Final (which is not that old) but I think we have other
>> applications already upgraded to 4.4.2 which have that bug, it's just
>> that we haven't caught it when we upgraded.
>> 
>> FWIW, we couldn't upgrade to 4.4.x before 4.4.2.Final because of a few
>> regressions we reported and helped getting fixed (I have at least
>> these ones in mind: HSEARCH-1442 and HSEARCH-1462 but IIRC we had
>> others).
>> 
>> (Sorry for the noise but I thought it was a good opportunity to
>> explain how we work, considering we're involved in this community for
>> quite a long time now)
>> 
>> --
>> Guillaume
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev