CompositeUserType

[sebersole]

Similar to UserType, CompositeUserType contract will have to change.

The intention for CompositeUserType ought to be cases which do not fit into the JPA "embeddable" paradigm, which generally means types that need constructor injection. Conceptually it would fit between a basic type and an embeddable type - a value type mapping to multiple columns.

In the broader sense, this is a case where we want to provide a custom org.hibernate.metamodel.spi.EmbeddableRepresentationStrategy, especially its EmbeddableRepresentationStrategy#instantiator. This would also mean changes to Instantiator (and a split from the Instantiator associated with a EntityRepresentationStrategy) to be have access to the composite values

Should such a thing be implicitly considered immutable?

This would be awesome:

@EmbeddableRepresentationStrategyRegistration( MonetaryAmountRepresentationStrategy.class )

which would be roughly equivalent (depending on the exact 2 impls ofc) :

@CompositeUserTypeRegistration( MonetaryAmountType.class )

Spitball:

interface Instantiator {
	/**
	 * Performs and "instance of" check to see if the given object is an
	 * instance of managed structure
	 */
	boolean isInstance(Object object, SessionFactoryImplementor sessionFactory);

	boolean isSameClass(Object object, SessionFactoryImplementor sessionFactory);
}

interface EntityInstantiator<J> extends Instantiator {
	J instantiate(SessionFactoryImplementor sessionFactory);
}

interface CompositeInstantiator<J> extends Instantiator {
	J instantiate(Supplier<Object[]> valuesAccess, SessionFactoryImplementor sessionFactory);
}

interface CompositeUserType<J> {
	void nullSafeSet(PreparedStatement st, Object value, int index, SharedSessionContractImplementor session);

	Object nullSafeGet(
			ResultSet rs,
			int[] positions, 
			SharedSessionContractImplementor session, 
			Object owner) throws SQLException;

	// MutabilityPlan
	boolean isMutable();
	Serializable disassemble(Object value, SharedSessionContractImplementor session);
	Object assemble(Serializable cached, SharedSessionContractImplementor session, Object owner);
	Object deepCopy(Object value);

	// JavaTypeDescriptor
	Class<J> returnedClass();
	boolean equals(Object x, Object y);
	int hashCode(Object x);
}

[beikov]

What do you think about the following?

interface CompositeUserType<J> {

        String[] getPropertyNames();
        java.lang.reflect.Type[] getPropertyTypes();
	Object get(J value, int propertyIndex, SharedSessionContractImplementor session);

        // Maybe this is the same as assemble/disassemble?
        // Maybe we don't need deconstruct/disassemble and can work with `get`?
        J instantiate(Object[] values, SessionFactoryImplementor sessionFactory, Object owner);
        // Or maybe this if we want this to be lazy?
        // J instantiate(IntFunction<Object> valueAccess, SessionFactoryImplementor sessionFactory, Object owner);
	Object[] deconstruct(J value, SessionFactoryImplementor sessionFactory);

	// MutabilityPlan
	boolean isMutable();
	Serializable disassemble(J value, SharedSessionContractImplementor session);
	J assemble(Serializable cached, SharedSessionContractImplementor session, Object owner);
	J deepCopy(J value);

	// JavaTypeDescriptor
	Class<J> returnedClass();
	default Type returnedType() { return returnedClass(); }
	boolean equals(J x, J y);
	int hashCode(J x);
}

This way, a user could define the composite type in terms of java types and make use of existing java and jdbc type descriptors.

[sebersole]

This depends on a number of things, primary of which is how we decide to model this. Is it a special kind of EmbeddableTypeMapping? Or something else?

Which comes around to something we discussed.. do we want to have the general ability to say that an embeddable mapping can be constructor injected?

These all relate to the design at hand.

But overall, yes, we need a way for this to influence:

1. instantiation
2. "scraping" the composed values (your "deconstruct")
3. some means for understanding the model mapping (metadata for the composition and its attributes)

That said, some specifics... I hate this parallel arrarys paradigm. We are converting many of those already in 6 to be more OO and I really don't want to start adding new ones

[sebersole]

See https://hibernate.zulipchat.com/#narrow/stream/132094-hibernate-orm-dev/topic/CompositeUserType for a continuing discussion of this

[sebersole]

A few thoughts after working with this some more...

I think the first thing to consider is whether we want to support these cases via a dedicated contract anymore (aka an update of CompositeUserType) or if we want to move to more of an annotation-driven approach (we already suggest users not use hbm.xml mappings, so a deviation from there is not a concern IMO). Personally, I think we should pick one or the other. It really sucks this has not been resolved in the JPA discussion yet as this work should work with whatever gets decided there. Though I guess us implementing an elegant and simple (from user perspective) solution might help drive that resolution.

We've discussed the annotation-based approach already a bit. Mainly this resolves around the ability to provide (1) an instantiator and (2) a "mapping". I started a preliminary prototype of such a "mapping" akin to @IdClass mappings. Both of these will require quite a bit of work to complete, but I think are very elegant.

I'm not against continuing support for CompositeUserType, but it would require also require a significant amount of work and probably the introduction of new annotations anyway (we discussed this a bit within the Zulip discussion. Long story short, simply re-using @Type, etc as a means to support composites has its own difficulties. But, as discussed there, I think moving away from String-based "type resolution" is a good change even outside of this discussion. See #4207 for this part

[beikov]

I was thinking a bit about this, and since you preferred the approach of an intermediary type (which I find ok), I was remembering a discussion in the JPA issue tracker where a user had proposed something similar: jakartaee/persistence#105

The idea is to have a javax.persistence.Converter and use the auto-apply feature or use-site javax.persistence.Convert annotation. This would nicely align with what you had in mind with the intermediary type and the fact that we already support certain annotations to control mutability etc. on the attribute converters. Heck, I think we might be able to get rid of CompositeUserType which would IMO be great.

Some further thoughts:

• The JPA model, through getJavaType(), should expose the use-site type, not the embeddable intermediary type, but as an embeddable i.e. PersistenceType.EMBEDDABLE
• This embeddable type should expose the attributes as defined in the intermediary type, so the names for access are defined through the intermediary type
• Generate a JPA metamodel for this named after the use-site type i.e. Money_ rather than MoneyEmbeddable_

Maybe we can get rid of the @Type annotation and friends altogether if we jump on the AttributeConverter train :)

[sebersole]

Well first, I don't "prefer" the intermediate embeddable mapping. In terms of user perspective I do think it is fairly natural and consistent with @IdClass.

I'll have to go re-read that JPA thread, but I absolutely hate the idea of AttributeConverter here. We already have cases where that doesn't give us enough information for even basic types, and composites are going to be even worse.

The prototype I started working on acts just like you mentioned. It really does mimic IdClass mappings.

The other "benefit" of the intermediary approach is that itis far more likely to be adopted as the approach in JPA compared to a full-on user contract like our CompositeUserType. And alignment here is very desirable.

[sebersole]

I should mention, I ran into problems implementing the intermediary approach mostly in the "load plan model" stuff which is something that 6 completely replaces anyway. I've run into problems with this code before as well in other cases. And I've had similar problems with some of the methods on Type which need to get removed also. I think a good exercise would be to remove those things as best as possible first

[beikov]

As we discussed during an audio chat, we will go with the "embeddable projection" style of mapping. The implementation for this will be part of #4824