Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

Collection fields in @MappedSuperclasses are ignored on update

Description

When a indexed collection field is defined in a @MappedSuperclass, a change to this collection only will not trigger reindexing on entities extending the @MappedSuperclass.

The problems seems to lie somewhere around AbstractDocumentBuilder#collectionChangeRequiresIndexUpdate. Hibernate sets the role of a collection property defined in a mappedsuperclass as my.package.MyConcreteEntity.myCollectionProperty, whereas the TypeMetadata#collectionRoles only contains my.package.MyMappedSuperclass.myCollectionProperty (regardless of the class of the actual entity), which definitely does not match.

Note that this bug only affects properties defined in a @MappedSuperclass. It does not seem to arise when the property is defined in a superclass which is an @Entity.

I am going to provide a test case soon (just need the ticket number).

Attachments

1
  • 02 Apr 2014, 07:46 PM

Activity

Guillaume SmetApril 8, 2014 at 1:14 PM

Hardy FerentschikApril 8, 2014 at 12:11 PM

Here is my attempt to fix the issue: https://github.com/openwide-java/hibernate-search/commit/8ddad809c82b6b21817dcdc5bb3cc262dbe8f855 using the idea of Yoann.

Ok. I added some comments. The change looks simple enough, but is of course also a bit sneaky. It creates role names which actually do not exist. However, since I cannot have the same property name more than once in the hierarchy it should just work. I guess this approach is easier to understand and maintain than a regexp, so let's go for this.

If you're OK with it, I can push a PR.

+1 If you can please use my test classes. Basically the same, but I refactored the test a bit and in particular let it extend SearchTestCaseJUnit4.

To illustrate the problem I have with your patch, consider the following pseudo code: ...

You counter example is correct. A conflict like this in the package name would atm fail. I guess one could tweak the regexp a bit, but it only gets more complicated. On the other hand, I think this case would be extremely rare, but anyways.

I still like the idea. If there were a deterministic way to split off the class name, I think I would prefer the regexp (it would also be a much easier one). I guess ORM 5 will introduce a new naming pattern, but that's not what we are aligned with for now.

Guillaume SmetApril 7, 2014 at 9:00 PM

Hi Hardy,

Here is my attempt to fix the issue: https://github.com/openwide-java/hibernate-search/commit/8ddad809c82b6b21817dcdc5bb3cc262dbe8f855 using the idea of Yoann.

If you're OK with it, I can push a PR.

To illustrate the problem I have with your patch, consider the following pseudo code:

@MappedSuperClass com.company.collection1.MyAbstractClass @CollectionOfElements @Field(fieldBridge...) private List<String> collection1;

and

@Entity com.company.collection1.MyEntity @CollectionOfElements private List<String> myNonIndexedCollection;

Hibernate Search knows that collection1 should be considered an indexed collection and myNonIndexedCollection should not.

Consider the following update operation:

  • I update myNonIndexedCollection

  • I don't update collection1
    -> Search should decide that it doesn't have to reindex the object.

But when it tries to do so with your patch, it checks if com.company.collection1.MyEntity.myNonIndexedCollection is matching ^(\w+\.)collection1(\.\w)*$ and it does. So it's going to reindex the entity even if it's not necessary.

Hardy FerentschikApril 7, 2014 at 7:46 PM

I'm pretty sure it is. The collection role coming from ORM is a fully qualified class name.

Sure, but as said, I don't think we would need to worry about conflicts here, but proof me wrong

Working on a patch right now. Will post if on Github soon.

Sure. We'll squash this bug one way or the other

Guillaume SmetApril 7, 2014 at 7:38 PM

Not sure whether this is really the case, but we can add some tests.

I'm pretty sure it is. The collection role coming from ORM is a fully qualified class name.

Not quite sure I follow (the second part).

Working on a patch right now. Will post if on Github soon.

Fixed

Details

Assignee

Reporter

Components

Fix versions

Affects versions

Priority

Created April 2, 2014 at 3:28 PM
Updated April 10, 2014 at 10:40 PM
Resolved April 10, 2014 at 10:40 PM