Remove constructor verification process

ObjectFactory will throw an appropriate error.

Author: harawata

src/main/java/org/apache/ibatis/executor/resultset/DefaultResultSetHandler.java

@@ -380,8 +380,7 @@ private void handleRowValuesForSimpleResultMap(ResultSetWrapper rsw, ResultMap r
           throw new ExecutorException("Expected result object to be a pending constructor creation!");
         }
 
-        createAndStorePendingCreation(resultHandler, resultSet, resultContext, (PendingConstructorCreation) rowValue,
-            lastHandledCreation == null);
+        createAndStorePendingCreation(resultHandler, resultSet, resultContext, (PendingConstructorCreation) rowValue);
         lastHandledCreation = (PendingConstructorCreation) rowValue;
       }
     }
@@ -1069,7 +1068,6 @@ private String prependPrefix(String columnName, String prefix) {
   private void handleRowValuesForNestedResultMap(ResultSetWrapper rsw, ResultMap resultMap,
       ResultHandler<?> resultHandler, RowBounds rowBounds, ResultMapping parentMapping) throws SQLException {
     final boolean useCollectionConstructorInjection = resultMap.hasResultMapsUsingConstructorCollection();
-    boolean verifyPendingCreationResult = true;
     PendingConstructorCreation lastHandledCreation = null;
     if (useCollectionConstructorInjection) {
       verifyPendingCreationPreconditions(parentMapping);
@@ -1090,12 +1088,8 @@ private void handleRowValuesForNestedResultMap(ResultSetWrapper rsw, ResultMap r
       // issue #577, #542 && #101
       if (useCollectionConstructorInjection) {
         if (foundNewUniqueRow && lastHandledCreation != null) {
-          createAndStorePendingCreation(resultHandler, resultSet, resultContext, lastHandledCreation,
-              verifyPendingCreationResult);
+          createAndStorePendingCreation(resultHandler, resultSet, resultContext, lastHandledCreation);
           lastHandledCreation = null;
-          // we only need to verify the first the result for a given result set
-          // as we can assume the next result will look exactly the same w.r.t its mapping
-          verifyPendingCreationResult = false;
         }
 
         rowValue = getRowValue(rsw, discriminatedResultMap, rowKey, null, partialObject);
@@ -1117,8 +1111,7 @@ private void handleRowValuesForNestedResultMap(ResultSetWrapper rsw, ResultMap r
     }
 
     if (useCollectionConstructorInjection && lastHandledCreation != null) {
-      createAndStorePendingCreation(resultHandler, resultSet, resultContext, lastHandledCreation,
-          verifyPendingCreationResult);
+      createAndStorePendingCreation(resultHandler, resultSet, resultContext, lastHandledCreation);
     } else if (rowValue != null && mappedStatement.isResultOrdered()
         && shouldProcessMoreRows(resultContext, rowBounds)) {
       storeObject(resultHandler, resultContext, rowValue, parentMapping, resultSet);
@@ -1298,7 +1291,7 @@ private void createPendingConstructorCreations(Object rowValue) {
       for (Object pendingCreation : pendingCreations) {
         if (pendingCreation instanceof PendingConstructorCreation) {
           final PendingConstructorCreation pendingConstructorCreation = (PendingConstructorCreation) pendingCreation;
-          targetMetaObject.add(pendingConstructorCreation.create(objectFactory, false));
+          targetMetaObject.add(pendingConstructorCreation.create(objectFactory));
         }
       }
     }
@@ -1317,9 +1310,8 @@ private void verifyPendingCreationPreconditions(ResultMapping parentMapping) {
   }
 
   private void createAndStorePendingCreation(ResultHandler<?> resultHandler, ResultSet resultSet,
-      DefaultResultContext<Object> resultContext, PendingConstructorCreation pendingCreation, boolean shouldVerify)
-      throws SQLException {
-    final Object result = pendingCreation.create(objectFactory, shouldVerify);
+      DefaultResultContext<Object> resultContext, PendingConstructorCreation pendingCreation) throws SQLException {
+    final Object result = pendingCreation.create(objectFactory);
     storeObject(resultHandler, resultContext, result, null, resultSet);
     nestedResultObjects.clear();
   }

src/main/java/org/apache/ibatis/executor/resultset/PendingConstructorCreation.java

@@ -15,23 +15,16 @@
  */
 package org.apache.ibatis.executor.resultset;
 
-import java.lang.reflect.Constructor;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
-import java.util.stream.Collectors;
 
 import org.apache.ibatis.executor.ExecutorException;
 import org.apache.ibatis.mapping.ResultMap;
 import org.apache.ibatis.mapping.ResultMapping;
 import org.apache.ibatis.reflection.ReflectionException;
-import org.apache.ibatis.reflection.factory.DefaultObjectFactory;
 import org.apache.ibatis.reflection.factory.ObjectFactory;
 
 /**
@@ -108,87 +101,6 @@ void linkCollectionValue(ResultMapping constructorMapping, Object value) {
     linkedCollectionsByKey.get(creationKey).add(value);
   }
 
-  /**
-   * Verifies preconditions before we can actually create the result object, this is more of a sanity check to ensure
-   * all the mappings are as we expect them to be.
-   * <p>
-   * And if anything went wrong, provide the user with more information as to what went wrong
-   *
-   * @param objectFactory
-   *          the object factory
-   */
-  private void verifyCanCreate(ObjectFactory objectFactory) {
-    // if a custom object factory was supplied, we cannot reasionably verify that creation will work
-    // thus, we disable verification and leave it up to the end user.
-    if (!DefaultObjectFactory.class.equals(objectFactory.getClass())) {
-      return;
-    }
-
-    // before we create, we need to get the constructor to be used and verify our types match
-    // since we added to the collection completely unchecked
-    final Constructor<?> resolvedConstructor = resolveConstructor(resultType, constructorArgTypes);
-    final Type[] genericParameterTypes = resolvedConstructor.getGenericParameterTypes();
-    for (int i = 0; i < genericParameterTypes.length; i++) {
-      if (!linkedCollectionMetaInfo.containsKey(i)) {
-        continue;
-      }
-
-      final PendingCreationMetaInfo creationMetaInfo = linkedCollectionMetaInfo.get(i);
-      final Class<?> resolvedItemType = checkResolvedItemType(creationMetaInfo, genericParameterTypes[i]);
-
-      // ensure we have an empty collection if there are linked creations for this arg
-      final PendingCreationKey pendingCreationKey = creationMetaInfo.getPendingCreationKey();
-      if (linkedCreationsByKey.containsKey(pendingCreationKey)) {
-        final Object emptyCollection = constructorArgs.get(i);
-        if (emptyCollection == null || !objectFactory.isCollection(emptyCollection.getClass())) {
-          throw new ExecutorException(
-              "Expected empty collection for '" + resolvedItemType + "', MyBatis internal error!");
-        }
-      } else {
-        final Object linkedCollection = constructorArgs.get(i);
-        if (!linkedCollectionsByKey.containsKey(pendingCreationKey)) {
-          throw new ExecutorException(
-              "Expected linked collection for key '" + pendingCreationKey + "', not found! MyBatis internal error!");
-        }
-
-        // comparing memory locations here (we rely on that fact)
-        if (linkedCollection != linkedCollectionsByKey.get(pendingCreationKey)) {
-          throw new ExecutorException("Expected linked collection in creation to be the same as arg for resultMap '"
-              + pendingCreationKey + "', not equal! MyBatis internal error!");
-        }
-      }
-    }
-  }
-
-  private static <T> Constructor<T> resolveConstructor(Class<T> type, List<Class<?>> constructorArgTypes) {
-    try {
-      if (constructorArgTypes == null) {
-        return type.getDeclaredConstructor();
-      }
-
-      return type.getDeclaredConstructor(constructorArgTypes.toArray(new Class[0]));
-    } catch (Exception e) {
-      String argTypes = Optional.ofNullable(constructorArgTypes).orElseGet(Collections::emptyList).stream()
-          .map(Class::getSimpleName).collect(Collectors.joining(","));
-      throw new ReflectionException(
-          "Error resolving constructor for " + type + " with invalid types (" + argTypes + ") . Cause: " + e, e);
-    }
-  }
-
-  private static Class<?> checkResolvedItemType(PendingCreationMetaInfo creationMetaInfo, Type genericParameterTypes) {
-    final ParameterizedType genericParameterType = (ParameterizedType) genericParameterTypes;
-    final Class<?> expectedType = (Class<?>) genericParameterType.getActualTypeArguments()[0];
-    final Class<?> resolvedItemType = creationMetaInfo.getArgumentType();
-
-    if (!expectedType.isAssignableFrom(resolvedItemType)) {
-      throw new ReflectionException(
-          "Expected type '" + resolvedItemType + "', while the actual type of the collection was '" + expectedType
-              + "', ensure your resultMap matches the type of the collection you are trying to inject");
-    }
-
-    return resolvedItemType;
-  }
-
   @Override
   public String toString() {
     return "PendingConstructorCreation(" + this.hashCode() + "){" + "resultType=" + resultType + '}';
@@ -199,16 +111,10 @@ public String toString() {
    *
    * @param objectFactory
    *          the object factory
-   * @param verifyCreate
-   *          should we verify this object can be created, should only be needed once
    *
    * @return the new immutable result
    */
-  Object create(ObjectFactory objectFactory, boolean verifyCreate) {
-    if (verifyCreate) {
-      verifyCanCreate(objectFactory);
-    }
-
+  Object create(ObjectFactory objectFactory) {
     final List<Object> newArguments = new ArrayList<>(constructorArgs.size());
     for (int i = 0; i < constructorArgs.size(); i++) {
       final PendingCreationMetaInfo creationMetaInfo = linkedCollectionMetaInfo.get(i);
@@ -228,7 +134,7 @@ Object create(ObjectFactory objectFactory, boolean verifyCreate) {
         final List<PendingConstructorCreation> linkedCreations = linkedCreationsByKey.get(pendingCreationKey);
 
         for (PendingConstructorCreation linkedCreation : linkedCreations) {
-          emptyCollection.add(linkedCreation.create(objectFactory, verifyCreate));
+          emptyCollection.add(linkedCreation.create(objectFactory));
         }
 
         newArguments.add(emptyCollection);

src/test/java/org/apache/ibatis/immutable/ImmutableConstructorTest.java

@@ -143,9 +143,9 @@ void shouldSelectBlogWithPostsButNoCommentsOrTags() {
   void shouldFailToSelectBlogWithMissingConstructorForPostComments() {
     try (SqlSession session = sqlSessionFactory.openSession()) {
       ImmutableBlogMapper mapper = session.getMapper(ImmutableBlogMapper.class);
-      assertThatThrownBy(() -> mapper.retrieveAllBlogsWithMissingConstructor()).isInstanceOf(PersistenceException.class)
+      assertThatThrownBy(mapper::retrieveAllBlogsWithMissingConstructor).isInstanceOf(PersistenceException.class)
           .hasCauseInstanceOf(ReflectionException.class).hasMessageContaining(
-              "Error resolving constructor for class org.apache.ibatis.domain.blog.immutable.ImmutablePost with invalid types");
+              "Error instantiating class org.apache.ibatis.domain.blog.immutable.ImmutablePost with invalid types");
     }
   }
 }