HHH-19993 Introduce AnnotationBasedUserType and allow UserType constructor accept MemberDetails as parameter

[quaff]

[Please describe here what your change is about]

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

---

https://hibernate.atlassian.net/browse/HHH-19993

[quaff]

@gavinking Please review.

[gavinking]

Please don't pass Properties here. We're nearing the end of a massive mulityear effort to wean everyone off this terrible addiction to passing strings around everywhere.

Using Properties here makes this new interface almost no better than the ParameterizedType interface it's supposed to be replacing. You're just sorta reintroducing ParameterizedType with a different name and much the same lack of type safety.

What makes most sense is a new Context interface analogous to GeneratorCreationContext.

@yrodiere @sebersole How much of a problem is it for Quarkus to have a Member here? We do pass a Member to AnnotationBasedGenerator.initialize() but perhaps it's a problem there too.

[quaff]

Please don't pass Properties here. We're nearing the end of a massive mulityear effort to wean everyone off this terrible addiction to passing strings around everywhere.

Removed.

[quaff]

How much of a problem is it for Quarkus to have a Member here? We do pass a Member to AnnotationBasedGenerator.initialize() but perhaps it's a problem there too.

Currently, this PR accepts Member as constructor's parameter for custom UserType implementation, should I change it to accept MemberDetails instead?

[yrodiere]

@yrodiere @sebersole How much of a problem is it for Quarkus to have a Member here? We do pass a Member to AnnotationBasedGenerator.initialize() but perhaps it's a problem there too.

Passing a Member should be fine as far as GraalVM is concerned.
The only problems would be:

1. Retrieving the Member from its Class in the first place. If done early (~Metadata building) it should work, but anything done later could (will) break.
2. Member just isn't an appropriate representation of attributes on dynamic-Map entities.

I think something like MemberDetails would make more sense indeed, but there may be a better fit for user-facing APIs -- @sebersole would know.

Also... I don't know if it's relevant here, but keep in mind retrieving values using a Member can perform badly. If this Member is intended for retrieving values from the entity, we'd need some other abstraction so that we can hope to leverage org.hibernate.bytecode.spi.ReflectionOptimizer.AccessOptimizer one day. If it's just about looking up the attribute's type, though, we don't need that.

[gavinking]

If this Member is intended for retrieving values from the entity

No, that would definitely be an abuse. It would be used only to reflect on the member and configure the UserType at startup.

I think something like MemberDetails would make more sense indeed

Damn. Because I'm an incredible fucking dumbass, I apparently did not think to declare AnnotationBasedGenerator as @Incubating. 🤬

It would be nice to switch to MemberDetails indeed.

[gavinking]

That looks good.

If you really want your strings, you could also add a getParameters() method to UserTypeCreationContext returning that horrible Properties object, as long as it comes with a big flashing @apiNote that that's no longer the best way to do things.

[quaff]

If you really want your strings, you could also add a getParameters() method to UserTypeCreationContext returning that horrible Properties object, as long as it comes with a big flashing @apiNote that that's no longer the best way to do things.

Parameters is specific to ParameterizedType, I don't think it's applicable for UserTypeCreationContext.

hibernate-orm/hibernate-core/src/main/java/org/hibernate/mapping/MappingHelper.java

Lines 74 to 82 in 8ca24d9

	public static void injectParameters(Object type, Properties parameters) {
	if ( type instanceof ParameterizedType parameterizedType ) {
	parameterizedType.setParameterValues( parameters == null ? EMPTY_PROPERTIES : parameters );
	}
	else if ( parameters != null && !parameters.isEmpty() ) {
	throw new MappingException( "'UserType' implementation '" + type.getClass().getName()
	+ "' does not implement 'ParameterizedType' but parameters were provided" );
	}
	}

[gavinking]

Parameters is specific to ParameterizedType

I don't know what you mean by that.

The @Type annotations and friends all have a parameters member. If we want to keep supporting those, instead of just deprecating them, then we should pass the parameters via the new context object.

[quaff]

Parameters is specific to ParameterizedType

I don't know what you mean by that.

The @Type annotations and friends all have a parameters member. If we want to keep supporting those, instead of just deprecating them, then we should pass the parameters via the new context object.

Do you mean I need to relax the restriction for AnnotationBasedUserType?

throw new MappingException( "'UserType' implementation '" + type.getClass().getName() 
 				+ "' does not implement 'ParameterizedType' but parameters were provided" ); 

[gavinking]

Yes.