Please everyone and especially Sanne, read this branch of the exchange Hardy and I have.
We need another pair of eyes to make sure we don’t mess things up as it is an important
departure compared to how things are done today (and in my prototype).
Hardy, I am starting to come along your side on this discussion. They key things that made
me swing is that if a user needs a special annotation to customize the bridge, then it’s
equivalent to a @FieldBridge. So a feature like CDI stereotypes or BV’s composition (by
annotating a custom annotation with a @FieldBridge) would solve that problem more
elegantly.
I am still a bit sceptical to move the bridge discovery to AMP - at least when explicit
annotations are at play because:
- AMP is complex
- BridgeFactory seems a nice focal point to everything bridge related
- a few common rules must be applied to bridges (like the IterableBridges / MapBridges and
ArrayBridges wrappers)
Nevertheless the idea of inferring types via @FieldBridge, @TikeBridge, @DateBridge,
@Spatial in a separate method than guessType has merits.
BUT
In practice for Date related add-ons like JodaTime and Java 8 date, I am wondering if we
should ensure that someone can use @DateBridge. After all a resolution is likely
conceptually needed and forcing another annotation seems wrong.
Likewise for "Hibernate Spatial” types, it will probably make sense to support them
as annotated with @Spatial like we do for Coordinates.
So what do we do about there?
- are they legit use cases (I think they are)
- is that supposed to be supported by some other features unrelated to BridgeProvider?
- should we design BridgeProvider in a way that let them react to these built in
annotations? (via explicit methods I suppose).
Emmanuel
PS: I’ve spend around 20hrs on that feature so if we could converge, that would be good
:)
On 02 Apr 2014, at 21:18, Hardy Ferentschik <hardy(a)hibernate.org> wrote:
On 2 Jan 2014, at 17:37, Emmanuel Bernard <emmanuel(a)hibernate.org> wrote:
> Yes your analysis is correct.
Cool, so we seem to be on the same page then.
> I do think 3 is the most valuable but that 2c is a relatively close second.
+1 for case #3 being the most important one. I would even go so far to say that this is
the only one we should address with the BridgeProvider.
> Now the @Spatial and @TikaBridge annotations do have attributes which will influence
how the underlying bridge is created.
> I don’t think you are proposing to move the spatial and Tika bridge creation up on
AnnotationMetadataProvider.
That’s what I am proposing. Also the processing of the @Bridge annotation (standalone or
as part of @Field) should move. Really
what happens in BridgeFactory#findExplicitFieldBridge (which is funny enough called by
something like BridgeFactory#guessType)
is annotation and hence AnnotationMetadataProvider specific. BridgeFactory#guessType
could really just become the handler of the described scenario 3.
> That would AFAIU duplicate the bridge creation logic between the AMP and some
ProgrammaticMetadataProvider.
No, not really or at most temporarily. Remember, using commons annotations and pseudo
annotations are just a crutch. It would be much easier to
instantiate the right metadata and bridges directly. After all the user does explicitly
something like .property( "name", ElementType.FIELD ).spatial()
There is not need to guess, you could create the appropriate bridge directly.
> Also I have a hard time navigating and understanding AnnotationMetadataProvider, so
I’m not sure we should make it more complex.
Sure AnnotationMetadataProvider is a lot of code, but it is actually still very similar
to the code you originally wrote for DocumentBuilder. It just has moved.
An indexed class is processed by creating the class hierarchy for this class and then
iterating the class hierarchy finding indexed properties. You should
be able to follow the flow quite easily from AnnotationMetadataProvider#initializeClass.
Also have a look at the call sites for BridgeFactory#guessType.
> So somehow, the AMP should convert @Spatial, @NumericField, @TikaBridge, and
@DateBridge into some non annotation based representation and pass that information to the
bridgeFactory.
Somehow you would create the bridge directly. Either from the AnnotationMetadataProvider
or we need some additional methods on BridgeFactory.
> Your approach would be to call explicitly buildXxxBridge() - like buildDateBridge() -
methods form AMP.
That would be one way of doing it. If these methods would be in BridgeFactory they could
even be called from the programmatic config
> These would be hosted on the BridgeFactory. Is that correct?
Right
> And the same explicit call logic would be done on a programmatic API equivalent.
Right
> Now how would you pass and to these kind of explicit calls in the 2c case when the
annotation is unknown a priori?
A couple of things here. buildXxxBridge() are specific to our internal bridges (date,
spatial, tika). There is no equivalent for custom (user provided) bridges. They are either
explicitly configured or go via the BridgeProvider contract. If we truly want to support
2c, we need something like the AnnoatedElement in the contract of BridgeProvider.
On the other hand, 2c can also be solved by the user by specifying his bridge directly.
If we just think about case #3 BridgeProvider could almost become:
FieldBridge returnFieldBridgeIfMatching(Class<?> returnType, ServiceManager
serviceManager);
For me it has many benefits:
1) It separates completely the annotation processing from the bridge creation, something
which is currently in the way for a refactoring of the programmatic config and probably a
problem
for non entity based indexing
2) Probably future proof for non entity based indexing
3) From a development point it treats case #1, #2a and #2b the same way - as explicitly
known bridges (which is really the case)
4) There is no major loss in functionality
—Hardy