Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeBootstrapContext constructor not called in custom types with Hibernate 6 #30924

Closed
z0mb1ek opened this issue Jul 21, 2023 · 16 comments
Closed
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: feedback-provided Feedback has been provided type: regression A bug that is also a regression
Milestone

Comments

@z0mb1ek
Copy link

z0mb1ek commented Jul 21, 2023

I use spring-orm with hypersistence-utils for additional types. There is class JsonBinaryType that has constructor:

JsonBinaryType(org.hibernate.type.spi.TypeBootstrapContext typeBootstrapContext)

The documentation says that If a custom Type defines a constructor which takes the TypeBootstrapContext argument, Hibernate will use this instead of the default constructor. But there is call on default contructor and it is not working.

I tested version 6.0.9 - it works, but 6.0.10 and 6.0.11 does not. Is it a bug?

relates to vladmihalcea/hypersistence-utils#644

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 21, 2023
@snicoll
Copy link
Member

snicoll commented Jul 24, 2023

@z0mb1ek the documentation you've referenced is from Hibernate and so is the behavior.

I tested version 6.0.9 - it works, but 6.0.10 and 6.0.11 does not. Is it a bug?

Did you test with the same hibernate version? If so can you please share a small sample that reproduces the problem?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jul 24, 2023
@z0mb1ek
Copy link
Author

z0mb1ek commented Jul 24, 2023

@snicoll Yes, i tested with same hibernate version, just change spring-orm version

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 24, 2023
@snicoll
Copy link
Member

snicoll commented Jul 24, 2023

Like I said, please share a small sample we can run ourselves.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 24, 2023
@clemstoquart
Copy link

Hello,

I think I've a similar issue upgrading from Spring Boot 3.0.8 to 3.0.9. The problem can be "fixed" by overriding spring-orm from 6.0.11 to 6.0.10.

After doing some debugging I think that the behaviour change comes from this commit dff7aa4 . For some reason before Spring was using the following constructor to instanciate the bean https://github.com/vladmihalcea/hypersistence-utils/blob/master/hypersistence-utils-hibernate-60/src/main/java/io/hypersistence/utils/hibernate/type/json/JsonBinaryType.java#L56 but now it's using this one https://github.com/vladmihalcea/hypersistence-utils/blob/master/hypersistence-utils-hibernate-60/src/main/java/io/hypersistence/utils/hibernate/type/json/JsonBinaryType.java#L60 .

I've no idea if this new behaviour was intended or not. Can you provide some guidance ?

Thanks.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 24, 2023
@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 25, 2023
@snicoll
Copy link
Member

snicoll commented Jul 25, 2023

It's really hard to say without looking at an actual sample. There are too many guesses at this point.

@snicoll
Copy link
Member

snicoll commented Jul 25, 2023

I assume you are both aware that Spring Boot auto-configures a BeanContainer that delegates to the BeanFactory.

If BeanContainer (from Hibernate) is called for that component, it makes sense to me it attempts to find a constructor matching the beans available in the bean factory. That doesn't help that this class has so many different constructors of course.

As for #30683 I believe it's doing the right thing given its contract. To me it looks like this is working as designed but a sample we can run ourselves can make us revisit that.

@clemstoquart
Copy link

@snicoll Thanks. My understanding of that part of Spring is not very good 😅

I'm going to build a minimal sample to make it easier to understand what's going on.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 25, 2023
@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 25, 2023
@z0mb1ek
Copy link
Author

z0mb1ek commented Jul 25, 2023

@snicoll it is not doing right thing because contract is calling constructor which takes the TypeBootstrapContext argument. Or documentation is not correct

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 25, 2023
@snicoll
Copy link
Member

snicoll commented Jul 26, 2023

@z0mb1ek as I've asked twice already, please share a small sample that will let us make sure we're seeing the same thing. If the BeanContainer is used, there's no such thing in its contract.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 26, 2023
@ooraini
Copy link

ooraini commented Aug 1, 2023

@snicoll

Here is failing test into otherwise an empty application:

package com.example.isssue30924;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import io.hypersistence.utils.hibernate.type.json.JsonType;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import org.hibernate.annotations.Type;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.orm.jpa.EntityManagerFactoryDependsOnPostProcessor;
import org.springframework.boot.autoconfigure.orm.jpa.HibernatePropertiesCustomizer;
import org.springframework.boot.jackson.JsonComponent;
import org.springframework.boot.jackson.JsonObjectSerializer;
import org.springframework.boot.test.autoconfigure.orm.jpa.AutoConfigureTestEntityManager;
import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;
import org.testcontainers.containers.MySQLContainer;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;
import org.testcontainers.utility.DockerImageName;

import java.io.IOException;
import java.util.Map;
import java.util.function.Supplier;

@SpringBootTest(properties = {"spring.jpa.hibernate.ddl-auto=update"})
@AutoConfigureTestEntityManager
@Testcontainers
public class Isssue30924ApplicationTests {

	@Container
	@ServiceConnection
	static final MySQLContainer<?> mySQL = new MySQLContainer<>(DockerImageName.parse("mysql:latest"));

	@Autowired
	TestEntityManager entityManager;

	@Test
	@Transactional
	void contextLoads() {
		SimpleEntity entity = new SimpleEntity();
		entity.id = 1;
		entity.nested = new Nested("smart");
		SimpleEntity simpleEntity = entityManager.merge(entity);
		Assertions.assertEquals(simpleEntity.nested.name, "dummy");
	}

	@Entity
	static class SimpleEntity {
		@Id
		Integer id;

		@Column(columnDefinition = "json")
		@Type(JsonType.class)
		Nested nested;
	}

	@JsonSerialize(using = Config.DummySerializer.class)
	record Nested(String name) {}


	@TestConfiguration
	public static class Config {
		@Component
		static class SomeBean {}

		@JsonComponent
		static class DummySerializer extends JsonObjectSerializer<Object> {
			public DummySerializer(SomeBean someBean) {
			}

			@Override
			protected void serializeObject(Object value, JsonGenerator jgen, SerializerProvider provider) throws IOException {
				jgen.writeStringField("name", "dummy");
			}
		}

		@Component
		static class Properties implements HibernatePropertiesCustomizer {
			@Override
			public void customize(Map<String, Object> hibernateProperties) {
				hibernateProperties.put("hypersistence.utils.jackson.object.mapper", ObjectMapperSupplier.class.getName());
			}
		}

		@Component
		static class DependsOn extends EntityManagerFactoryDependsOnPostProcessor {
			public DependsOn() {
				super("hibernateObjectMapper");
			}
		}

		@Component("hibernateObjectMapper")
		public static class ObjectMapperSupplier implements Supplier<ObjectMapper> {
			private static ObjectMapper objectMapper;

			@Autowired
			void setObjectMapper(ObjectMapper objectMapper) {
				ObjectMapperSupplier.objectMapper = objectMapper;
			}

			@Override
			public ObjectMapper get() {
				return objectMapper;
			}
		}

	}
}

The set up is quite involved, but here my attempt. What we want is to configure the hypersistence library to use the ObjectMapper from the application context, to do that, we store it in a static field and we avoid constructor injection since hypersistence doesn't use the application context. If hypersistence can't instantiate (or you don't configure a custom ObjectMapper) it will create a new one.

To force the test to fail, I register JsonComponent that has a non empty constructor, and since Spring make the application context beans available to Jackson, the ObjectMapper in the application will be able to resolve any @JsonSerialize that uses it.

But, if hypersistence construct it's own ObjectMapper, that one will not be able to create the serializer (non empty constructor) and the test fails.

I use the following build.gradle:

plugins {
	id 'java'
//	id 'org.springframework.boot' version '3.1.0'
//	id 'io.spring.dependency-management' version '1.1.0'
	id 'org.springframework.boot' version '3.1.2'
	id 'io.spring.dependency-management' version '1.1.2'
}

group = 'com.example'
version = '0.0.1-SNAPSHOT'

java {
	sourceCompatibility = '17'
}

repositories {
	mavenCentral()
}

dependencies {
	implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
	implementation 'org.springframework.boot:spring-boot-starter-web'
	implementation 'io.hypersistence:hypersistence-utils-hibernate-62:3.5.1'
	runtimeOnly 'com.mysql:mysql-connector-j'
	testImplementation 'org.springframework.boot:spring-boot-starter-test'
	testImplementation 'org.springframework.boot:spring-boot-testcontainers'
	testImplementation 'org.testcontainers:junit-jupiter'
	testImplementation 'org.testcontainers:mysql'

}

tasks.named('test') {
	useJUnitPlatform()
}

Switch between the versions to see it fail.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 1, 2023
@snicoll
Copy link
Member

snicoll commented Aug 1, 2023

Thanks but that doesn't reproduce the problem that you've described initially (also, why paste all that code in text rather than sharing a sample that I can run? The only thing I am forced to do is to copy paste that to a project anyway).

The original report is about us not calling JsonBinaryType(org.hibernate.type.spi.TypeBootstrapContext typeBootstrapContext) as described in the Hibernate documentation. I need a sample that doesn't do that and calls a different constructor. I've tried to put debug but couldn't manage to find a way to reproduce it.

@ooraini
Copy link

ooraini commented Aug 1, 2023

Thanks but that doesn't reproduce the problem that you've described initially (also, why paste all that code in text rather than sharing a sample that I can run? The only thing I am forced to do is to copy paste that to a project anyway).

Hi @snicoll, Apologies, I though I was saving you the trouble of downloading a full project by putting everything in single file 😅. My bad.

I'm able to verify that the constructor on JsonType is not being called in the new version, can you try that one instead? JsonType(org.hibernate.type.spi.TypeBootstrapContext typeBootstrapContext).

@jhoeller jhoeller self-assigned this Aug 1, 2023
@jhoeller jhoeller added in: data Issues in data modules (jdbc, orm, oxm, tx) type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 1, 2023
@jhoeller jhoeller added this to the 6.0.12 milestone Aug 1, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Aug 1, 2023

After investigating Hibernate's implementation there, this special (User)Type creation behavior used to live in TypeFactory.type(Class<Type> typeClass, Properties parameters) but got promoted to a TypeBeanInstanceProducer which is used as a fallback producer for (User)Type creation in Hibernate 6.x. This is not defined as a contract anywhere but since it is the default behavior, we can try to align with it through specific detection of (User)Type creation in SpringBeanContainer.

@jhoeller
Copy link
Contributor

jhoeller commented Aug 1, 2023

@z0mb1ek this is in 6.0.x now. It would be great if you could give the latest 6.0.12 snapshot a try, specifically the latest version of SpringBeanContainer.

@z0mb1ek
Copy link
Author

z0mb1ek commented Aug 1, 2023

@jhoeller Hi, thank you very much for your work, it starts working, BUT we need this:

spring:
  jpa:
    properties:
      hibernate:
        cdi:
          extensions: false

Very strange property naming, what is this?

@z0mb1ek
Copy link
Author

z0mb1ek commented Aug 1, 2023

I found this #30545

@jhoeller jhoeller changed the title TypeBootstrapContext constructor not called in custom types TypeBootstrapContext constructor not called in custom types with Hibernate 6 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: feedback-provided Feedback has been provided type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

6 participants