From 8adfe47efaf947e585e8c4b31b4c43712cf3158d Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@vmware.com>
Date: Tue, 6 Apr 2021 14:15:16 +0200
Subject: [PATCH] Improve ClassGeneratingEntityInstantiator error message if
 the to be instantiated type is abstract.

We now throw MappingInstantiationException with a nested BeanInstantiationException in alignment to ReflectionEntityInstantiator when attempting to create a new instance of an abstract class.

Closes #2348.
---
 .../ClassGeneratingEntityInstantiator.java    | 82 ++++++++++++++-----
 ...GeneratingEntityInstantiatorUnitTests.java | 12 +++
 2 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiator.java
index 0b997c9e1..2c8022da1 100644
--- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiator.java
+++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiator.java
@@ -26,10 +26,12 @@ import java.util.Map;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.asm.ClassWriter;
 import org.springframework.asm.MethodVisitor;
 import org.springframework.asm.Opcodes;
 import org.springframework.asm.Type;
+import org.springframework.beans.BeanInstantiationException;
 import org.springframework.cglib.core.ReflectUtils;
 import org.springframework.core.NativeDetector;
 import org.springframework.data.mapping.PersistentEntity;
@@ -121,6 +123,10 @@ class ClassGeneratingEntityInstantiator implements EntityInstantiator {
 			return ReflectionEntityInstantiator.INSTANCE;
 		}
 
+		if (Modifier.isAbstract(entity.getType().getModifiers())) {
+			return MappingInstantiationExceptionEntityInstantiator.create(entity.getType());
+		}
+
 		try {
 			return doCreateEntityInstantiator(entity);
 		} catch (Throwable ex) {
@@ -240,30 +246,30 @@ class ClassGeneratingEntityInstantiator implements EntityInstantiator {
 				throw new MappingInstantiationException(entity, Arrays.asList(params), e);
 			}
 		}
+	}
 
-		/**
-		 * Extracts the arguments required to invoke the given constructor from the given {@link ParameterValueProvider}.
-		 *
-		 * @param constructor can be {@literal null}.
-		 * @param provider can be {@literal null}.
-		 * @return
-		 */
-		private <P extends PersistentProperty<P>, T> Object[] extractInvocationArguments(
-				@Nullable PreferredConstructor<? extends T, P> constructor, ParameterValueProvider<P> provider) {
+	/**
+	 * Extracts the arguments required to invoke the given constructor from the given {@link ParameterValueProvider}.
+	 *
+	 * @param constructor can be {@literal null}.
+	 * @param provider can be {@literal null}.
+	 * @return
+	 */
+	static <P extends PersistentProperty<P>, T> Object[] extractInvocationArguments(
+			@Nullable PreferredConstructor<? extends T, P> constructor, ParameterValueProvider<P> provider) {
 
-			if (constructor == null || !constructor.hasParameters()) {
-				return allocateArguments(0);
-			}
-
-			Object[] params = allocateArguments(constructor.getConstructor().getParameterCount());
-
-			int index = 0;
-			for (Parameter<?, P> parameter : constructor.getParameters()) {
-				params[index++] = provider.getParameterValue(parameter);
-			}
-
-			return params;
+		if (constructor == null || !constructor.hasParameters()) {
+			return allocateArguments(0);
 		}
+
+		Object[] params = allocateArguments(constructor.getConstructor().getParameterCount());
+
+		int index = 0;
+		for (Parameter<?, P> parameter : constructor.getParameters()) {
+			params[index++] = provider.getParameterValue(parameter);
+		}
+
+		return params;
 	}
 
 	/**
@@ -276,6 +282,40 @@ class ClassGeneratingEntityInstantiator implements EntityInstantiator {
 		Object newInstance(Object... args);
 	}
 
+	/**
+	 * {@link EntityInstantiator} throwing {@link MappingInstantiationException} upon
+	 * {@link #createInstance(PersistentEntity, ParameterValueProvider)}.
+	 *
+	 * @author Mark Paluch
+	 * @since 2.5
+	 */
+	static class MappingInstantiationExceptionEntityInstantiator implements EntityInstantiator {
+
+		private final Class<?> typeToCreate;
+
+		private MappingInstantiationExceptionEntityInstantiator(Class<?> typeToCreate) {
+			this.typeToCreate = typeToCreate;
+		}
+
+		public static EntityInstantiator create(Class<?> typeToCreate) {
+			return new MappingInstantiationExceptionEntityInstantiator(typeToCreate);
+		}
+
+		/*
+		* (non-Javadoc)
+		* @see org.springframework.data.mapping.model.EntityInstantiator#createInstance(org.springframework.data.mapping.PersistentEntity, org.springframework.data.mapping.model.ParameterValueProvider)
+		*/
+		@Override
+		public <T, E extends PersistentEntity<? extends T, P>, P extends PersistentProperty<P>> T createInstance(E entity,
+				ParameterValueProvider<P> provider) {
+
+			Object[] params = extractInvocationArguments(entity.getPersistenceConstructor(), provider);
+
+			throw new MappingInstantiationException(entity, Arrays.asList(params),
+					new BeanInstantiationException(typeToCreate, "Class is abstract"));
+		}
+	}
+
 	/**
 	 * Generates a new {@link ObjectInstantiator} class for the given custom class.
 	 * <p>
diff --git a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiatorUnitTests.java b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiatorUnitTests.java
index a96726716..00f82f88d 100755
--- a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiatorUnitTests.java
+++ b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiatorUnitTests.java
@@ -39,6 +39,7 @@ import org.springframework.data.mapping.PreferredConstructor;
 import org.springframework.data.mapping.PreferredConstructor.Parameter;
 import org.springframework.data.mapping.model.ClassGeneratingEntityInstantiator.ObjectInstantiator;
 import org.springframework.data.mapping.model.ClassGeneratingEntityInstantiatorUnitTests.Outer.Inner;
+import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.util.ReflectionUtils;
 
 /**
@@ -374,6 +375,13 @@ class ClassGeneratingEntityInstantiatorUnitTests<P extends PersistentProperty<P>
 		assertThat(this.instance.shouldUseReflectionEntityInstantiator(entity)).isTrue();
 	}
 
+	@Test // GH-2348
+	void entityInstantiatorShouldFailForAbstractClass() {
+
+		assertThatExceptionOfType(MappingInstantiationException.class).isThrownBy(() -> this.instance
+				.createInstance(new BasicPersistentEntity<>(ClassTypeInformation.from(AbstractDto.class)), provider));
+	}
+
 	private void prepareMocks(Class<?> type) {
 
 		doReturn(type).when(entity).getType();
@@ -519,4 +527,8 @@ class ClassGeneratingEntityInstantiatorUnitTests<P extends PersistentProperty<P>
 		ClassWithPackagePrivateConstructor() {}
 	}
 
+	public static abstract class AbstractDto {
+
+	}
+
 }