On 8 December 2014 at 10:13, Hardy Ferentschik <ha...@hibernate.org> wrote: > 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.
s/ it should not be private/ it should be private/ > 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. Remember this still is "the implementor" for the SearchFactory API so the name is not wrong; what is wrong is the position in terms of modules. So while I disagree on a rename I agree on moving its role: in fact I've been working up to late tonight to move some things across modules; I had to back-off though as it's going to take me longer, and as long as we're clear this is not an SPI we an change it at any point. > > 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. +1 Makes a lot of sense but we need to postpone such internal refactorings by a couple of weeks. > 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. Exactly I just want to rename things to "unclassify" it from 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 _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
