Re: [hibernate-dev] Redefining the API/SPI/implementation split
After reading this thread, `family` or `private` feel better than `friend` BTW, can’t you isolate those classes in a dedicated package still marked `impl` but have it exposed to OSGi? After all all this "shared but not shared because we decided to cut our software in many pieces“ is a bit of an implementation detail from a user PoV. > On 08 Dec 2014, at 02:14, Sanne Grinovero wrote: > > Thanks all, > so I went for the "friend" package name, adding a brief notice in the > javadocs. > > https://github.com/Sanne/hibernate-search/blob/HSEARCH-1739/engine/src/main/java/org/hibernate/search/engine/friend/package-info.java > > Sanne > > On 6 December 2014 at 16:02, Steve Ebersole wrote: >>> Sometimes it's just convenience; we have an "util.JNDIHelper" which is >>> needed in several modules (Massindexer statistics, JMS lookups, etc..) >>> and I don't think we should expose it as an API nor that another >>> project like Infinispan Query should ever use it. >> >> >> It's funny you bring up JNDIHelper specifically. ORM used to have a >> JNDIHelper class as well and I went through this same exercise. In ORM >> JNDIHelper is now the JndiService (I treat "well known acronyms" as words >> for the purpose of camel casing). But the important point is that it is a >> service, available from the service registry. To me, access to the service >> registries and their services overall, is an SPI concern. If some 3rd party >> integration wants to use Hibernate's JndiService as part of its integration >> (and some cache integrations do), I am more than ok with that. >> >> >>> Isn't it our definition of a SPI? >>> >>> No. I just realised our definition of SPI isn't adequate as there are >>> two different kinds of integration points. >> >> >> In my opinion we have many more "levels" of SPI then just 2. We have even >> discussed how to handle some of the differences a few times in f2f or irc. >> >> An SPI describes a contract meant for integration. But there are many parts >> to that. You mentioned caching, so the main contract there is the >> RegionFactory. So obviously an integrator is implementing that. The >> RegionFactory is returning Region instances. those 2 contracts need to >> remain stable. Since the integrations implement those contracts, any >> changes of any kind to the contracts require changes to the implementation >> (aside from certain cases involving supplied base implementations). Most of >> the methods on RegionFactory to build a Region accept a CacheDataDescription >> argument. CacheDataDescription is a "parameter object". Changes to >> CacheDataDescription do not generally require changes to the implementation. >> I like to think of it in terms of a component and the data passed between >> Hibernate and that component. So even though RegionFactorty, Region and >> CacheDataDescription are all "spi contracts", they fulfill very different >> roles in the concept of an SPI. Some need to be more stable than others. >> >> I also think a lot in terms of "audience" when it comes to contracts. I >> might write a class intending it for use by a number of different roles: >> >> Another Hibernate class >> >> In the same module >> In another module >> In another Hibernate project, although I prefer to think of this more in >> line with an "Integration" >> >> Integration >> >> As the contract for an integration >> As the communication related to that integration >> >> Application - SessionFactory, Session, etc >> >> Just wanted to clarify... >> >> Personally, I like the phrase "friend" or "family" when describing the type >> of thing you are discussing. Not sure either of those work great as a >> package name though to denote the idea of "stay away". > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
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 pgprpOLzo2BoD.pgp Description: PGP signature ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Redefining the API/SPI/implementation split
Hi, On Mon, Dec 08, 2014 at 10:47:25AM +0100, Emmanuel Bernard wrote: > BTW, can’t you isolate those classes in a dedicated package still marked > `impl` but have it exposed to OSGi? That was my suggestion as well. It is probably not the worst alternative. It moves the class out of the spi package and makes it an impl class (properly indicating that it is not to be used by anyone else). The downside is that we still need to expose this impl package via OSGi. However, since it would be a "dedicated" package just for this integration code, the impact would be minimal. Mind you, I still believe that we could address the whole issue in a much cleaner way as well. --Hardy pgpIwTcMyjaVl.pgp Description: PGP signature ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Redefining the API/SPI/implementation split
Ok that sounds like the best compromise. I'll adjust my open PR. Thanks! Sanne On 8 December 2014 at 10:18, Hardy Ferentschik wrote: > Hi, > > On Mon, Dec 08, 2014 at 10:47:25AM +0100, Emmanuel Bernard wrote: >> BTW, can’t you isolate those classes in a dedicated package still marked >> `impl` but have it exposed to OSGi? > > That was my suggestion as well. It is probably not the worst alternative. It > moves the class out of the spi > package and makes it an impl class (properly indicating that it is not to be > used by anyone else). > The downside is that we still need to expose this impl package via OSGi. > However, since it would be a "dedicated" > package just for this integration code, the impact would be minimal. > > Mind you, I still believe that we could address the whole issue in a much > cleaner way as well. > > --Hardy ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Redefining the API/SPI/implementation split
On 8 December 2014 at 10:13, Hardy Ferentschik 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
Re: [hibernate-dev] Redefining the API/SPI/implementation split
On Mon, Dec 08, 2014 at 12:07:45PM +, Sanne Grinovero wrote: > > 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. How can an interface be the "implementor" of anything. It just defines an API, it does not implement anything. This is exactly what I am saying, it is a misnomer to start with. --Hardy pgp2OiSjMQ5zC.pgp Description: PGP signature ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Redefining the API/SPI/implementation split
> On 08 Dec 2014, at 14:34, Hardy Ferentschik wrote: > > On Mon, Dec 08, 2014 at 12:07:45PM +, Sanne Grinovero wrote: >>> 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. > > How can an interface be the "implementor" of anything. It just defines an > API, it does not implement > anything. This is exactly what I am saying, it is a misnomer to start with. It a contract of all implementors of SearchFactory, I don’t see anything wrong with the name. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
