Improve the mapper-javabean loading API and common code

Description

First, we really should merge the way we configure loading for search and for massindexing. Having a single EntityLoadingStrategy type that handles the creation of search loaders *and* mass indexing loaders would be ideal.

Second, loading should be configured when creating the mapper, not when creating the session. We should assign loading strategies to entities when they are added to the mapping builder (org.hibernate.search.mapper.javabean.mapping.SearchMappingBuilder#addEntityType), and optionally allow users to pass session-specific context through LoadingOptions's interceptors. LoadingOptions#registerLoader and LoadingOptions#massIndexingLoadingStrategy should disappear.

Third, ideally we should not expose loading-related APIs in mapper-pojo-base. We should have SPIs in mapper-pojo-base, with corresponding APIs and adapters in mapper-orm/mapper-javabean. See for example EntityLoadingTypeGroupStrategy or MassIndexingMonitor, but there are probably others.

Fourth, EntityLoadingTypeGroupStrategy is complex and dangerous, and should probably be moved to SPI. I wouldn’t expose it in the mapper-javabean API in particular.

Also, I would make the strategy global to the mapper, so that we can avoid having to depend on the typeContextProvider in AbstractHibernateOrmLoadingStrategy, which leads to very dodgy code in HibernateOrmTypeContextContainer#init (we pass to the type context builders a reference to the container while we are in the constructor of that container…).

There are also various details that could be improved:

  • EntityLoader should accept a Deadline

  • EntityLoadingTypeGroupStrategy should be EntityLoadingTypeGroupingStrategy, and the corresponding getter in EntityLoadingStrategy should be groupingStrategy, not groupStrategy.

  • LoadingInvocationContext seems unnecessarily complex; why are there multiple proceed methods?

  • JavaBeanSearchLoadingContext , when it looks for a loader/loadingStrategy, walks upwards through the type hierachy and picks the first loader/loadingStrategy it finds. See org.hibernate.search.mapper.javabean.loading.impl.JavaBeanSearchLoadingContext#createLoader for example. That’s… acceptable, I suppose, but should be done when the loaders/loadingStrategies are registered.

  • The implementation org.hibernate.search.mapper.orm.loading.impl.HibernateOrmMassIndexingContext#entityIdentifier seems wrong, as it returns the entity ID instead of the ID used for indexing (which may be different if using @DocumentId). We should use the same implementation as in the JavaBean mapper.

  • MassIndexingEntityLoadingStrategy should have a type parameter I for the type of identifiers, so that we can guarantee that the identifier scroll returns identifiers of the same type expected by the EntityLoader.

  • HibernateOrmMassIndexingDocumentProducerInterceptor and HibernateOrmMassIndexingIdentifierProducerInterceptor share a lot of code related to transactions. We should factorize that.

Environment

None

Assignee

Yoann Rodière

Reporter

Yoann Rodière

Labels

None

Suitable for new contributors

None

Pull Request

None

Feedback Requested

None

Fix versions

Priority

Major