Merge remote-tracking branch 'upstream/master' into wip/6.0

hibernate-core/src/main/java/org/hibernate/boot/model/process/spi/MetadataBuildingProcess.java

@@ -32,8 +32,11 @@ import org.hibernate.boot.spi.BootstrapContext;
 import org.hibernate.boot.spi.MetadataBuildingOptions;
 import org.hibernate.boot.spi.MetadataContributor;
 import org.hibernate.boot.spi.MetadataImplementor;
+import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.cfg.MetadataSourceType;
 import org.hibernate.dialect.Dialect;
+import org.hibernate.engine.config.spi.ConfigurationService;
+import org.hibernate.engine.config.spi.StandardConverters;
 import org.hibernate.engine.jdbc.spi.JdbcServices;
 import org.hibernate.type.BasicType;
 import org.hibernate.type.BasicTypeRegistry;
@@ -94,10 +97,16 @@ public class MetadataBuildingProcess {
 			final MetadataSources sources,
 			final BootstrapContext bootstrapContext) {
 		final ManagedResourcesImpl managedResources = ManagedResourcesImpl.baseline( sources, bootstrapContext );
+		final ConfigurationService configService = bootstrapContext.getServiceRegistry().getService( ConfigurationService.class );
+		final boolean xmlMappingEnabled = configService.getSetting(
+				AvailableSettings.XML_MAPPING_ENABLED,
+				StandardConverters.BOOLEAN,
+				true
+		);
 		ScanningCoordinator.INSTANCE.coordinateScan(
 				managedResources,
 				bootstrapContext,
-				sources.getXmlMappingBinderAccess()
+				xmlMappingEnabled ? sources.getXmlMappingBinderAccess() : null
 		);
 		return managedResources;
 	}
@@ -285,7 +294,7 @@ public class MetadataBuildingProcess {
 			final EntityHierarchyBuilder hierarchyBuilder = new EntityHierarchyBuilder();
 //			final MappingBinder mappingBinder = new MappingBinder( true );
 			// We need to disable validation here.  It seems Envers is not producing valid (according to schema) XML
-			final MappingBinder mappingBinder = new MappingBinder( classLoaderService, false );
+			final MappingBinder mappingBinder = options.isXmlMappingEnabled() ? new MappingBinder( classLoaderService, false ) : null;
 			for ( AdditionalJaxbMappingProducer producer : producers ) {
 				log.tracef( "Calling AdditionalJaxbMappingProducer : %s", producer );
 				Collection<MappingDocument> additionalMappings = producer.produceAdditionalMappings(

hibernate-core/src/main/java/org/hibernate/type/CollectionType.java

@@ -737,7 +737,8 @@ public abstract class CollectionType extends AbstractType implements Association
 		// need to put the merged elements in a new collection
 		Object result = ( target == null ||
 				target == original ||
-				target == LazyPropertyInitializer.UNFETCHED_PROPERTY ) ?
+				target == LazyPropertyInitializer.UNFETCHED_PROPERTY ||
+				target instanceof PersistentCollection && ( (PersistentCollection) target ).isWrapper( original ) ) ?
 				instantiateResult( original ) : target;
 
 		//for arrays, replaceElements() may return a different reference, since

hibernate-core/src/test/java/org/hibernate/test/annotations/selectbeforeupdate/UpdateDetachedTest.java

@@ -6,6 +6,10 @@
  */
 package org.hibernate.test.annotations.selectbeforeupdate;
 
+import java.util.HashSet;
+import java.util.Set;
+import javax.persistence.ElementCollection;
+import javax.persistence.Embeddable;
 import javax.persistence.Entity;
 import javax.persistence.Id;
 import javax.persistence.JoinColumn;
@@ -136,6 +140,37 @@ public class UpdateDetachedTest extends BaseCoreFunctionalTestCase{
 		assertEquals( Integer.valueOf( 1 ), foo.getVersion() );
 	}
 
+	@Test
+	@TestForIssue(jiraKey = "HHH-14319")
+	public void testUpdateDetachedWithAttachedPersistentSet() {
+		final Bar bar = new Bar( 5, "Bar" );
+		final Set<Comment> comments = new HashSet<>();
+		comments.add( new Comment( "abc", "me" ) );
+		bar.comments = comments;
+
+		// this should generate versions
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( bar );
+		} );
+		final Bar loadedBar = TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			// We set the comments to the hash set and leave it "dirty"
+			Bar b = session.find( Bar.class, bar.getId() );
+			b.comments = comments;
+
+			// During flushing, the comments HashSet becomes the backing collection of new PersistentSet which replaces the old entry
+			session.flush();
+
+			// Replace the persistent collection with the backing collection in the field
+			b.comments = comments;
+
+			// It's vital that we try merging a detached instance
+			session.detach( b );
+			return (Bar) session.merge( b );
+		} );
+
+		assertEquals( 1, loadedBar.comments.size() );
+	}
+
 	@Entity(name = "Foo")
 	@SelectBeforeUpdate
 	public static class Foo {
@@ -198,6 +233,8 @@ public class UpdateDetachedTest extends BaseCoreFunctionalTestCase{
 		private String name;
 		@Version
 		private Integer version;
+		@ElementCollection
+		private Set<Comment> comments;
 
 		Bar() {
 
@@ -231,5 +268,43 @@ public class UpdateDetachedTest extends BaseCoreFunctionalTestCase{
 		public void setVersion(Integer version) {
 			this.version = version;
 		}
+
+		public Set<Comment> getComments() {
+			return comments;
+		}
+
+		public void setComments(Set<Comment> comments) {
+			this.comments = comments;
+		}
+	}
+
+	@Embeddable
+	public static class Comment {
+		private String comment;
+		private String author;
+
+		public Comment() {
+		}
+
+		public Comment(String comment, String author) {
+			this.comment = comment;
+			this.author = author;
+		}
+
+		public String getComment() {
+			return comment;
+		}
+
+		public void setComment(String comment) {
+			this.comment = comment;
+		}
+
+		public String getAuthor() {
+			return author;
+		}
+
+		public void setAuthor(String author) {
+			this.author = author;
+		}
 	}
 }

hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/ToOneIdMapper.java

@@ -20,14 +20,19 @@ import org.hibernate.envers.internal.tools.EntityTools;
 import org.hibernate.envers.internal.tools.query.Parameters;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.proxy.HibernateProxy;
+import org.jboss.logging.Logger;
+
+import javax.persistence.PersistenceException;
 
 /**
  * @author Adam Warski (adam at warski dot org)
  * @author HernпїЅn Chanfreau
  * @author Michal Skowronek (mskowr at o2 dot pl)
  * @author Chris Cranford
+ * @author Luke Chen
  */
 public class ToOneIdMapper extends AbstractToOneMapper {
+	private static final Logger log = Logger.getLogger( ToOneIdMapper.class );
 	private final IdMapper delegate;
 	private final String referencedEntityName;
 	private final boolean nonInsertableFake;
@@ -58,8 +63,18 @@ public class ToOneIdMapper extends AbstractToOneMapper {
 		// bi-directional relation, we always store the "old", unchanged data, to prevent storing changes made
 		// to this field. It is the responsibility of the collection to properly update it if it really changed.
 		Object entity = nonInsertableFake ? oldObj : newObj;
+
+		// fix HHH-13760 - try to aggressively un-proxy this entity to help get the correct type of data later
+		// in mapToMapFromEntity. But it might fail while getImplementation() if object is deleted or other reasons.
+		// We catch the exception and fallback to call mapToMapFromEntity directly with the HibernateProxy entity
 		if ( lazyMapping && entity instanceof HibernateProxy ) {
-			entity = ( (HibernateProxy) entity ).getHibernateLazyInitializer().getImplementation();
+			try {
+				entity = ((HibernateProxy) entity).getHibernateLazyInitializer().getImplementation();
+			}
+			catch ( PersistenceException e ) {
+				log.debug( "Ignore PersistenceException while initializing the entity, " +
+					"and fallback to call mapToMapFromEntity directly" );
+			}
 		}
 
 		delegate.mapToMapFromEntity( newData, entity );

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytoone/lazy/ChildUser.java

@@ -0,0 +1,38 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.test.integration.manytoone.lazy;
+
+import org.hibernate.envers.NotAudited;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.OneToMany;
+import javax.persistence.Table;
+import java.util.List;
+
+/**
+ * @author Luke Chen
+ */
+@Entity
+@Table(name = "child_user")
+public class ChildUser extends User {
+
+    @OneToMany(fetch = FetchType.LAZY, mappedBy = "user", cascade = CascadeType.REMOVE)
+    @NotAudited
+    private List<Shipment> shipmentList;
+
+    ChildUser() {
+    }
+
+    public long getId() {
+        return id;
+    }
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytoone/lazy/ManyToOneLazyDeleteTest.java

@@ -0,0 +1,88 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.test.integration.manytoone.lazy;
+
+import org.hibernate.Hibernate;
+import org.hibernate.envers.configuration.EnversSettings;
+import org.hibernate.envers.test.BaseEnversFunctionalTestCase;
+import org.hibernate.envers.test.Priority;
+import org.hibernate.testing.TestForIssue;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Map;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests that proxies can still be resolved correctly in ToOneIdMapper even the object is already deleted and can't
+ * find in cache. This can happen if the deleted object is an inherited object, and when the child object is deleted,
+ * we cannot find the object with the parent class name anymore.
+ *
+ * @author Luke Chen
+ */
+@TestForIssue(jiraKey = "HHH-13945")
+public class ManyToOneLazyDeleteTest extends BaseEnversFunctionalTestCase {
+    private Long shipmentId;
+    private User user;
+
+    @Override
+    protected Class<?>[] getAnnotatedClasses() {
+        return new Class<?>[] { Shipment.class, Address.class, AddressVersion.class, User.class, ChildUser.class };
+    }
+
+    @Test
+    @Priority(10)
+    public void initData() {
+
+        this.shipmentId = doInHibernate( this::sessionFactory, session -> {
+            final Shipment shipment = new Shipment( Instant.now(), "system", Instant.now().plus( Duration.ofDays( 3 ) ), "abcd123", null, null );
+            session.persist( shipment );
+            session.flush();
+
+            final Address origin = new Address( Instant.now(), "system", "Valencia#1" );
+            final Address destination = new Address( Instant.now(), "system", "Madrid#3" );
+            final AddressVersion originVersion0 = origin.addInitialVersion( "Poligono Manises" );
+            final AddressVersion destinationVersion0 = destination.addInitialVersion( "Poligono Alcobendas" );
+            user = new ChildUser();
+            session.persist( origin );
+            session.persist( destination );
+            session.persist( user );
+
+            session.flush();
+            shipment.setUser( user );
+            shipment.setOrigin( originVersion0 );
+            shipment.setDestination( destinationVersion0 );
+
+            session.merge( shipment );
+            session.flush();
+
+            return shipment.getId();
+        } );
+
+        doInHibernate( this::sessionFactory, session -> {
+            final Shipment shipment = session.get( Shipment.class, shipmentId );
+            session.remove(shipment);
+            // Cast the User instance to the ChildUser, and delete the child one, so the cache for
+            // the User instance will not be there, and entityNotFound exception will be thrown while envers processing it
+            ChildUser childUser = session.get(ChildUser.class, user.getId());
+            session.remove(childUser);
+
+            session.flush();
+        } );
+    }
+
+    @Override
+    protected void addSettings(Map settings) {
+        super.addSettings( settings );
+
+        settings.put(EnversSettings.STORE_DATA_AT_DELETE, "true");
+    }
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytoone/lazy/ManyToOneLazyFetchTest.java

@@ -32,7 +32,7 @@ public class ManyToOneLazyFetchTest extends BaseEnversFunctionalTestCase {
 
     @Override
     protected Class<?>[] getAnnotatedClasses() {
-        return new Class<?>[] { Shipment.class, Address.class, AddressVersion.class };
+        return new Class<?>[] { Shipment.class, Address.class, AddressVersion.class, User.class, ChildUser.class };
     }
 
     @Test

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytoone/lazy/Shipment.java

@@ -62,6 +62,11 @@ public class Shipment extends BaseDomainEntity {
     @Audited(targetAuditMode = RelationTargetAuditMode.NOT_AUDITED)
     private AddressVersion destination;
 
+    @ManyToOne(optional = true, fetch = FetchType.LAZY)
+    @JoinColumn(name = "user_id", referencedColumnName = "id", nullable = true)
+    @Audited(targetAuditMode = RelationTargetAuditMode.NOT_AUDITED)
+    private User user;
+
     Shipment() {
     }
 
@@ -104,4 +109,8 @@ public class Shipment extends BaseDomainEntity {
     public void setDestination(AddressVersion destination) {
         this.destination = destination;
     }
+
+    public void setUser(User user) {
+        this.user = user;
+    }
 }

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytoone/lazy/User.java

@@ -0,0 +1,43 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.test.integration.manytoone.lazy;
+
+import org.hibernate.envers.NotAudited;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.OneToMany;
+import javax.persistence.Table;
+import java.time.Instant;
+import java.util.List;
+
+/**
+ * @author Luke Chen
+ */
+@Entity
+@Table(name = "user_tbl")
+public class User {
+
+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    protected long id = 0;
+
+    @OneToMany(fetch = FetchType.LAZY, mappedBy = "user", cascade = CascadeType.REMOVE)
+    @NotAudited
+    private List<Shipment> shipmentList;
+
+    User() {
+    }
+
+    public long getId() {
+        return id;
+    }
+}