hibernate/hibernate-search — pull request #526 by Sanne (2013-12-20 22:23:46+00:00)
Title: Modules split and new ServiceManager contract

Essentially @hferentschik's work with some additional changes:
- file movements are separate commits
- I've moved all related tests to the new modules too
- most cleanup preparation steps are moved to the initial commits
- engine and orm modules can be built and tested without the extracted modules (good to moving forward and break them all while focusing on some modules only)
- I've reorganized all dependency declarations in the poms

I'm happy with the shape of the new ServiceManager, definitely a good step forward (not to mention the benefits from the modules split), so I'd merge this and move on as none of the comments I have on the new Services approach are blocking, they are minor comments if any. (I had bigger question marks on where some of that code came from but it's clear now).

To summarize the minor remarks:
- the fact that a Service essentially needs to be able to self-start itself rather than returning a fully initialized object makes it imo harder to get around concurrency requirements. Easily solved with a level of indirection: you don't manage the Service but its provider (essentially how we did it before but we can get there with this clearer contract)
- the usage of java.lang.reflect.Constructor confirms the need for a level of indirection
- no idea if the JavaSerialization serializer actually works (but this pull isn't a step backwards in that sense)
- not too happy of having merged various JGroups complex files together as there are many bad defined integrators at that level.. too bad we'll hear from them and iterate.
- I suspect that grabbing the context classloader at bootstrap time is a solid choice but I guess we'll see, and the CL works isn't done yet.
- I'm not too sure of how we'd handle scenarios like when needing a different Service type for an IndexManager (compared to another one). AFAIR we discussed that in the initial design and I couldn't find a reasonable use case. so ok for now. Also, we didn't have that feature before either :-)