Re: [hibernate-dev] Redefining the API/SPI/implementation split

Hi, > One of today's issues for Hibernate Search had the goal to move this class: > > org.hibernate.search.engine.spi.SearchFactoryImplementor Well, for me there is more to this than just moving. There are two issues imo. 1. The location. org.hibernate.search.engine.spi implies after our definition that it is and public SPI. However, according to Sanne it should not be private and the main/sole purpose of the class is to integrate the Search engine module with the orm module.

2. The name. SearchFactoryImplementor is something which implements SearchFactory. However, one of the latest changes was to make SearchFactory a stand alone class of the orm module. SearchFactory is now only available in the orm module and has not inheritance link anymore to SearchFactoryImplementor or SearchIntegrator. This is awesome, since now we are able to evolve the engine code in the direction for "free form" entities without affecting the API for the users using Search in combination with Hibernate ORM. However, it also means that the engine module should now be agnostic of the orm module. Having a SearchFactoryImplementor in the engine module, but the SearchFactory defined in the orm module seems wrong.

So, what can be done? Similar to Emmanuel, I am against a internal/friend package. I think we should look at other things we can do. First off, I find the separation between SearchIntegrator (former SearchFactoryIntegrator) and SearchFactoryImplementor quite arbitrary. For example, the integrator contains getIndexBindings, whereas SearchFactoryImplemtor has a getIndexBindings. Why could the latter not be part of SearchIntegrator? If we move all methods of SearchFactoryImplementor into SearchIntegrator, and an integrator cannot implement all of them we could throw an OperationNotSupportedException. Other things we could do is to work with services. We could have something like a ORMInterationService which contains methods which are part of SearchFactoryImplemtor and which we don't want to move to SearchIntegrator. We could also keep SearchFactoryImplementor, but move it to the orm module. During bootstrapping in HibernateSearchSessionFactoryObserver we would create the SearchIntegrator and then wrapp/add the additional functionality required for the SearchFactoryImplementor.

I think we would have a whole range of possibilities to address this. Just moving the class it imo not one of them. If we say that we don't have time to a full fledged refactoring (which would imo be a shame, since that was one of the targets of Search 5), I'd rather just rename the class for now and do a refactoring later. We could rename and deprecate the class to make clear that it is not an SPI.

> So it seemed a straight-forward decision to move it to: > > org.hibernate.search.engine.impl.SearchFactoryImplementor > However then we need to patch the OSGi descriptor to export this package: > org.hibernate.search.engine.impl No, you cannot just move it to org.hibernate.search.engine.impl. If anything you would need to move it into a standalone impl package which does not contains any other class, say org.hibernate.search.engine.orm.impl. If you want to move the class, I'd rather do it this way than creating a 'friend' package. --Hardy