There has been a lot of discussions on the PR. Let me try and sum up my next steps and bootstrap some discussions.
To comment on a specific subject, create a separate thread per subject or this thread will be come a huge radiated tree :)
## @IndexedEmbedded and null
Apparently both Sanne and I discovered a feature implemented a while ago :)
There are some questions around it so I opened a separate issue to deal with the question https://hibernate.atlassian.net/browse/HSEARCH-1578
## References to field bridges
Despite being on an internal class, some implementations (Infinispan remove query at least) uses direct references to FieldBridge.SOME_BRIDGE.
I’ll reinstate them so that Sanne could integrate continuously HSearch 5 with Infinispan. I would prefer to see them go esp when we have the free form entity support and the idea of field bridge composition done.
https://hibernate.atlassian.net/browse/HSEARCH-1579
BTW can I move these references? Or should they stay on BridgeFactory?
## Handling duplicates
I had in mind the following logic.
Prevent custom bridge providers to offer bridges in the same situation. That would be an error at start up time as the alternative of picking up one of them does not seem very attractive. Then built-in bridges would be taken into account. It means that a custom bridge would be able to override a built-in bridge.
Another alternative is to explicitly give ordering priorities to between providers. I’d rather not go that way if possible.
Sanne questioned the idea of built-in bridge and would rather have all bridges handled equally and not allow duplication.
It seems that the current set of bridges cannot support that approach. When I developed the code, I realised that there is an ordering to respect. If I put the TikeBridgeProvider logic before the date and calendar bridgeProviders, then the DSLTest fails. I could not find why but it shows that we have some interdependencies at the moment.
## Splitting the BridgeProvider in two methods
A way make the inelegant code structure
FieldBridge bridge = provider.provide(…);
if ( bridge != null ) {
return bridge
}
Is to ask of the provider to answer two question:
- boolean canProvideBridgeFor(…)
- FieldBridge createFieldBridge(…)
The code would become
if ( provider.canProvideBridgeFor(…) ) {
return createFieldBridge(…)
}
I will experiment with it to see if it creates in practice too much duplication of code at the provider level. Another concern is that if the answer to can… and create… are inconsistent, we are in trouble.
## AnnotatedElement
Hardy does not like us being tied to the actual physical annotation objects. I tend to agree with him as we would need to fake it for free-form entities.
Since he worked on Jandex and the ORM side of parsing, he will work on a proposal that abstracts us away from AnnotatedElement.
The alternative is to pass the XLayer objects but not everyone is fine of that abstraction.
## BridgeProvider Context object
I’ll change the BridgeProvider to offer some kind of Context object instead of the list of parameters we have today. That will make it more future proof.
## Finish the BridgeProvider conversion
I’ll finish the conversion of the BridgeFactory to delegate as much code to BridgeProviders
## Implementation of the service loader based discovery
Hardy proposes to make each BridgeProvider a Service in the ServiceManager sense. The idea being that when we make a compatible ServiceManager OSGi wise, it will also make a compatible bridge provider discoverer.
It looks fine to me but as Hardy pointed out, we would need to make the ServiceManager accept several implementations per Service.
I’d like to separate the bridge auto-discoverability feature from that ServiceManager improvement to stay focused.
We can converge the service discovery as soon as the SM gains the right capability.
Emmanuel