Skip to:

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

CollectionUpdateEventTest.testWithClassBridge test fails with ORM 5.2.11-SNAPSHOT

Description

When upgrading ORM to current master (5.2.11-SNAPSHOT), the CollectionUpdateEventTest.testWithClassBridge test fails with:

testWithClassBridge(org.hibernate.search.test.engine.optimizations.CollectionUpdateEventTest) Time elapsed: 0.103 sec <<< FAILURE!
java.lang.AssertionError: catalogItems should have been initialized
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.assertTrue(Assert.java:41)
at org.hibernate.search.test.engine.optimizations.CollectionUpdateEventTest.testScenario(CollectionUpdateEventTest.java:100)
at org.hibernate.search.test.engine.optimizations.CollectionUpdateEventTest.testWithClassBridge(CollectionUpdateEventTest.java:51)

git bisect says it is related to this commit: https://github.com/hibernate/hibernate-orm/commit/4235d756107d00bd4007c53eaa8d85fd1382cc55

Activity

Yoann RodièreSeptember 4, 2017 at 1:59 PM

I just checked, and the fix for https://hibernate.atlassian.net/browse/HSEARCH-2868#icft=HSEARCH-2868 is enough to make our build pass with Hibernate ORM 5.2.11-SNAPSHOT: https://travis-ci.org/yrodiere/hibernate-search/builds/271673072?utm_source=email&utm_medium=notification . Let's wait for the PR to be merged.

Yoann RodièreSeptember 4, 2017 at 11:41 AM

I opened https://hibernate.atlassian.net/browse/HSEARCH-2868#icft=HSEARCH-2868 and I'm about to send a PR. It seems fixing that bug is enough to also fix the issue described in this ticket, i.e. to make our tests pass with Hibernate ORM 5.2.11-SNAPSHOT. I'll ping you when we merge HSEARCH-2868, maybe you can confirm everything is okay then.

Yoann RodièreSeptember 4, 2017 at 9:15 AM

I think there's actually a bug in Hibernate Search, which previously was hidden in our test case due to the unnecessary initializations in Hibernate ORM.

The currently failing test (CollectionUpdateEventTest.testWithClassBridge) sets up this chain of associations:

Catalog <--(OneToMany)--> CatalogItem <--(ManyToOne)--> Item

And assumes (though it's far from explicit) this chain of dependency from the point of view of index content:

Catalog --(is contained in)--> CatalogItem --(is contained in)--> Item

These "contained in" relationships are due to both the presence of @IndexedEmbedded annotations, and more importantly to the the presence of a class bridge on Item, which could theoretically use about anything in Catalog. Hence, modifying anything in Catalog should result in the associated Item instances to be reindexed: we simply don't know which data the class bridge depends on.

The thing is, due to https://hibernate.atlassian.net/browse/HSEARCH-782#icft=HSEARCH-782 we have an "optimization" in place that will make us skip reindexing when the collection being modified wasn't initialized. This can happen when simply adding an element to an uninitialized collection, for instance: the added element is simply added to a queue under the hood (see org.hibernate.collection.internal.PersistentBag.add(Object)).

The https://hibernate.atlassian.net/browse/HSEARCH-782#icft=HSEARCH-782 optimization is located in FullTextIndexEventListener:

protected void processCollectionEvent(AbstractCollectionEvent event) { // ... PersistentCollection persistentCollection = event.getCollection(); final String collectionRole; if ( persistentCollection != null ) { if ( !persistentCollection.wasInitialized() ) { // non-initialized collections will still trigger events, but we want to skip them // as they won't contain new values affecting the index state return; } collectionRole = persistentCollection.getRole(); } // ... }

But this optimization is, in my opinion, pure nonsense: additions to the collection could totally affect other indexes containing the collection owner. This is true in our case, where Item has a class bridge which could depend on practically anything, but it's also true without a class bridge. See for instance this mapping:

public class Catalog { // ... @ContainedIn @OneToMany(mappedBy = "catalog", cascade = { CascadeType.REMOVE, CascadeType.REFRESH }, fetch = FetchType.LAZY) private Set<CatalogItem> catalogItems = new HashSet<CatalogItem>(); @ManyToMany(fetch = FetchType.LAZY, mappedBy = "catalogs", cascade = { CascadeType.PERSIST }) @Field(bridge = @FieldBridge(impl = SomeBridge.class)) private List<Consumer> consumers = new ArrayList<Consumer>(); // ... }

Adding a consumer would keep the consumer list uninitialized, but would obviously still affect the index of catalog items, since there is a field bridge on "consumers".

I'm currently investigating https://hibernate.atlassian.net/browse/HSEARCH-782#icft=HSEARCH-782 to see if we could fix it another way... But I suspect there's a bug here, which may affect Hibernate Search even without the Hibernate ORM upgrade. I'll open another ticket if necessary.

Former userAugust 31, 2017 at 6:43 PM

I believe the assertion you are seeing is expected, as the commit you mention specifically made a change so that uninitialized collections are no longer initialized when cascading PERSIST operations to uninitialized collections.

I think the test just needs to be updated to change the assertion.

Fixed

Details

Assignee

Reporter

Components

Fix versions

Priority

Created August 31, 2017 at 3:48 PM
Updated September 13, 2017 at 10:15 PM
Resolved September 4, 2017 at 10:18 PM

Flag notifications