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

[Sanne Grinovero]

Thanks for the quick feedback!

On 5 December 2014 at 21:35, Emmanuel Bernard <emmanuel at hibernate.org> wrote:
> If hibernate-search-orm needs it, it's likely that infinispan-query
> could also make use of that contract. After all, infinispan-query is
> very similar conceptually to our ORM integration albeit simpler.

I had a similar thought, in fact I'm going through each and every use
of these APIs by hibernate-search-ORM to see if it's really needed and
cleaning up all those which can live with the proper SPI.
By doing this, I'm "promoting" some method (one at the moment) from
the Implementor to the proper SPI API.

There are some cases though which are all of
 a)Required
 b)Highly ORM specialized
 c)Experimental
And I don't feel comfortable exposing these on a clean SPI - nor I
expect anyone to need them.

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.

Also, Infinispan Query (and everyone else) can use the new "unwrap"
method: at least accessing such experimental functionality would have
a warning sign.

> 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.

There is a remarkable difference in requirements between an SPI meant
for "our own modules" (as in: versioned in the same repository) and
one meant to be used/extended by a project which has an independent
release cycle.
 - we can break our "internal" SPI in an atomic commit which also
fixes all other modules so this never introduces compatibility
problems
 - Running a single testsuite - with a single git checkout -
immediately highlights if something is wrong
 - CI can prevent mistakes early on rather than the costly "revert
after the release" process

The way I previously understood our definition, the "SPI" is a rather
stable contract meant for third party integrations, such as a second
level cache for Hibernate (not all implementations live in the ORM
source repository) or Infinispan Query / CapeDwarf integrating, so
there is an expectation of stability there which we're not bound to
for internal-only SPIs.
Also I was expecting our "SPI" contract to be more stable than what we
need across modules in the same repository; for example in the
Infinispan Query case it's not desirable for me to keep all methods in
mind which they use, nor having to run the Query testsuite on a Search
snapshot every time I make a change to a method. The modules "owned"
in the same repository don't have these drawbacks, so it can and
should be treated differently.


> If you don't change your mind, I am not a big fan of the internal name.
> If nothing because ORM uses it with a different semantic.
> I don't have very good alternatives
> - friend
> - sharedimpl

My goal was to send the "keep away - here be monsters" kind of message
but sounding a bit more professional, like an *internal implementation
detail*.
I don't see how the term "friend" helps? Also, anything is an
"Implementation" so any "impl" postfix doesn't suggest at all that you
should steer away from it.. "shared" making it worse :)
How is "internal" different than the meaning ORM uses it for?

Proposal: "private" ?
At lest it will be familiar to Java visibility, but personally I still
prefer the "internal". N.B. It's not a big deal how we call it as it's
just an internal detail :-)

Sanne

>
> Emmanuel
>
> On Fri 2014-12-05 20:30, Sanne Grinovero wrote:
>> One of today's issues for Hibernate Search had the goal to move this class:
>>
>> org.hibernate.search.engine.spi.SearchFactoryImplementor
>>
>> to not have the "SPI" package postfix as we never meant this to be
>> part of the SPI but rather an internal contract.
>> While it's an internal contract, it's functionality is needed by other
>> modules *in our same repository*, for example hibernate-search-orm
>> needs to access it.
>> So we consider it an integration contract across our own shards, but
>> it's not meant to be used by anyone else.
>>
>> 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
>>
>> By doing so this will export more than what is strictly necessary - as
>> there are other classes in there - and this gets Karaf to blow up with
>> many nasty errors as there are dependent classloaders being triggered
>> by those other internal components. So we really need to keep those
>> sealed into the module.
>>
>> So I was thinking to create a new package classification "internal":
>>
>> org.hibernate.search.engine.internal.SearchFactoryImplementor
>>
>> We would export this package via OSGi descriptors, but still the name
>> should be self-explanatory enough as a warning to other consumers?
>>
>> Sanne
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev