New fix for #22. Better throw an exception to ensure the user knows what

is doing and enables it explicitly.

Author: emacarron

src/main/java/org/apache/ibatis/builder/xml/XMLConfigBuilder.java

@@ -83,7 +83,7 @@ private XMLConfigBuilder(XPathParser parser, String environment, Properties prop
 
   public Configuration parse() {
     if (parsed) {
-      throw new BuilderException("Each MapperConfigParser can only be used once.");
+      throw new BuilderException("Each XMLConfigBuilder can only be used once.");
     }
     parsed = true;
     parseConfiguration(parser.evalNode("/configuration"));
@@ -208,6 +208,7 @@ private void settingsElement(XNode context) throws Exception {
       configuration.setLocalCacheScope(LocalCacheScope.valueOf(props.getProperty("localCacheScope", "SESSION")));
       configuration.setJdbcTypeForNull(JdbcType.valueOf(props.getProperty("jdbcTypeForNull", "OTHER")));
       configuration.setLazyLoadTriggerMethods(stringSetValueOf(props.getProperty("lazyLoadTriggerMethods"), "equals,clone,hashCode,toString"));
+      configuration.setSafeResultHandlerEnabled(booleanValueOf(props.getProperty("safeResultHandlerEnabled"), true));
       configuration.setDefaultScriptingLanguage(resolveClass(props.getProperty("defaultScriptingLanguage")));
       configuration.setCallSettersOnNulls(booleanValueOf(props.getProperty("callSettersOnNulls"), false));
       configuration.setLogPrefix(props.getProperty("logPrefix"));

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

@@ -17,7 +17,6 @@
 
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -45,7 +44,6 @@ public class NestedResultSetHandler extends FastResultSetHandler {
 
   private final Map<CacheKey, Object> objectCache = new HashMap<CacheKey, Object>();
   private final Map<CacheKey, Object> ancestorCache = new HashMap<CacheKey, Object>();
-  private final List<Object> unsortedRowHandlerElements = new ArrayList<Object>();
 
   public NestedResultSetHandler(Executor executor, MappedStatement mappedStatement, ParameterHandler parameterHandler, ResultHandler resultHandler, BoundSql boundSql, RowBounds rowBounds) {
     super(executor, mappedStatement, parameterHandler, resultHandler, boundSql, rowBounds);
@@ -65,11 +63,20 @@ private void ensureNoRowBounds(RowBounds rowBounds) {
   // HANDLE RESULT SETS
   //
 
+  @Override
+  protected void handleResultSet(ResultSet rs, ResultMap resultMap, List<Object> multipleResults, ResultColumnCache resultColumnCache) throws SQLException {
+    if (resultHandler != null && configuration.isSafeResultHandlerEnabled() && !mappedStatement.isResultOrdered()) {
+      throw new ExecutorException("Mapped Statements with nested result mappings cannot be safely used with a custom ResultHandler. "
+          + "Use safeResultHandlerEnabled=false setting to bypass this check " 
+          + "or ensure your statement returns ordered data and set resultOrdered=true on it.");
+    }
+    super.handleResultSet(rs, resultMap, multipleResults, resultColumnCache);
+  }
+
   @Override
   protected void cleanUpAfterHandlingResultSet() {
     super.cleanUpAfterHandlingResultSet();
     objectCache.clear();
-    unsortedRowHandlerElements.clear();
   }
 
   //
@@ -85,25 +92,21 @@ protected void handleRowValues(ResultSet rs, ResultMap resultMap, ResultHandler
       final ResultMap discriminatedResultMap = resolveDiscriminatedResultMap(rs, resultMap, null);
       final CacheKey rowKey = createRowKey(discriminatedResultMap, rs, null, resultColumnCache);
       Object partialObject = objectCache.get(rowKey);
-      if (mappedStatement.isResultOrdered()) { // issue #577       
+      if (mappedStatement.isResultOrdered()) { // issue #577 && #542
         if (partialObject == null && rowValue != null) {
-          objectCache.clear(); 
+          objectCache.clear();
           callResultHandler(resultHandler, resultContext, rowValue);
-        } 
+        }
         rowValue = getRowValue(rs, discriminatedResultMap, rowKey, rowKey, null, resultColumnCache, partialObject);
       } else {
         rowValue = getRowValue(rs, discriminatedResultMap, rowKey, rowKey, null, resultColumnCache, partialObject);
         if (partialObject == null) {
-          unsortedRowHandlerElements.add(rowValue); // issue #542
+          callResultHandler(resultHandler, resultContext, rowValue);
         }
       }
     }
-    if (mappedStatement.isResultOrdered()) {
-      if (rowValue != null) callResultHandler(resultHandler, resultContext, rowValue);
-    } else {
-      for (Object o : unsortedRowHandlerElements) {
-        callResultHandler(resultHandler, resultContext, o);
-      }
+    if (rowValue != null && mappedStatement.isResultOrdered()) {
+      callResultHandler(resultHandler, resultContext, rowValue);
     }
   }
 

src/main/java/org/apache/ibatis/session/Configuration.java

@@ -218,12 +218,10 @@ public void setDatabaseId(String databaseId) {
     this.databaseId = databaseId;
   }
 
-  @Deprecated
   public boolean isSafeResultHandlerEnabled() {
     return safeResultHandlerEnabled;
   }
 
-  @Deprecated // "Not needed as of the fix for issue #542"
   public void setSafeResultHandlerEnabled(boolean safeResultHandlerEnabled) {
     this.safeResultHandlerEnabled = safeResultHandlerEnabled;
   }

src/test/java/org/apache/ibatis/submitted/nestedresulthandler/Mapper.xml

@@ -28,7 +28,7 @@
 		</collection>
 	</resultMap>
 
-	<select id="getPersons" resultMap="personResult">
+	<select id="getPersons" resultMap="personResult" resultOrdered="true">
 		select p.id as person_id, p.name as person_name, i.id as item_id, i.name as item_name
 		from persons p, items i
 		where p.id = i.owner

src/test/java/org/apache/ibatis/submitted/nestedresulthandler/NestedResultHandlerTest.java

@@ -19,6 +19,7 @@
 import java.sql.Connection;
 import java.util.List;
 
+import org.apache.ibatis.exceptions.PersistenceException;
 import org.apache.ibatis.io.Resources;
 import org.apache.ibatis.jdbc.ScriptRunner;
 import org.apache.ibatis.session.ResultContext;
@@ -98,6 +99,23 @@ public void handleResult(ResultContext context) {
     }
   }
 
+  @Test(expected=PersistenceException.class)
+  public void testUnorderedGetPersonWithHandler() {
+    SqlSession sqlSession = sqlSessionFactory.openSession();
+    try {
+      sqlSession.select("getPersonsWithItemsOrdered", new ResultHandler() {
+        public void handleResult(ResultContext context) {
+          Person person = (Person) context.getResultObject();
+          if ("grandma".equals(person.getName())) {
+            Assert.assertEquals(2, person.getItems().size());
+          }
+        }
+      });
+    } finally {
+      sqlSession.close();
+    }
+  }
+
   /**
    * Fix bug caused by issue #542, see new issue #22 on github If we order by a
    * nested result map attribute we can miss some records and end up with