Fixes mybatis#146. Avoid using caches that are dirty but use the rest.

Author: emacarron

src/main/java/org/apache/ibatis/cache/TransactionalCacheManager.java

@@ -28,6 +28,10 @@ public void clear(Cache cache) {
     getTransactionalCache(cache).clear();
   }
 
+  public Object getObject(Cache cache, CacheKey key) {
+    return getTransactionalCache(cache).getObject(key);
+  }
+  
   public void putObject(Cache cache, CacheKey key, Object value) {
     getTransactionalCache(cache).putObject(key, value);
   }

src/main/java/org/apache/ibatis/cache/decorators/TransactionalCache.java

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2012 the original author or authors.
+ *    Copyright 2009-2014 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -44,7 +44,13 @@ public int getSize() {
   }
 
   public Object getObject(Object key) {
-    return delegate.getObject(key);
+    if (clearOnCommit) return null;
+    delegate.getReadWriteLock().readLock().lock();
+    try {
+      return delegate.getObject(key);
+    } finally {
+      delegate.getReadWriteLock().readLock().unlock();
+    }
   }
 
   public ReadWriteLock getReadWriteLock() {

src/main/java/org/apache/ibatis/executor/CachingExecutor.java

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2012 the original author or authors.
+ *    Copyright 2009-2014 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -34,18 +34,10 @@
 public class CachingExecutor implements Executor {
 
   private Executor delegate;
-  private boolean autoCommit; // issue #573. No need to call commit() on autoCommit sessions
   private TransactionalCacheManager tcm = new TransactionalCacheManager();
 
-  private boolean dirty;
-
   public CachingExecutor(Executor delegate) {
-    this(delegate, false);
-  }
-
-  public CachingExecutor(Executor delegate, boolean autoCommit) {
     this.delegate = delegate;
-    this.autoCommit = autoCommit;
   }
 
   public Transaction getTransaction() {
@@ -54,9 +46,8 @@ public Transaction getTransaction() {
 
   public void close(boolean forceRollback) {
     try {
-      //issue #499. Unresolved session handling
-      //issue #573. Autocommit sessions should commit
-      if (dirty && !autoCommit) { 
+      //issues #499, #524 and #573
+      if (forceRollback) { 
         tcm.rollback();
       } else {
         tcm.commit();
@@ -81,28 +72,23 @@ public <E> List<E> query(MappedStatement ms, Object parameterObject, RowBounds r
     return query(ms, parameterObject, rowBounds, resultHandler, key, boundSql);
   }
 
-  public <E> List<E> query(MappedStatement ms, Object parameterObject, RowBounds rowBounds, ResultHandler resultHandler, CacheKey key, BoundSql boundSql) throws SQLException {
+  public <E> List<E> query(MappedStatement ms, Object parameterObject, RowBounds rowBounds, ResultHandler resultHandler, CacheKey key, BoundSql boundSql)
+      throws SQLException {
     Cache cache = ms.getCache();
     if (cache != null) {
       flushCacheIfRequired(ms);
-      if (ms.isUseCache() && resultHandler == null) { 
+      if (ms.isUseCache() && resultHandler == null) {
         ensureNoOutParams(ms, parameterObject, boundSql);
-        if (!dirty) {
-          cache.getReadWriteLock().readLock().lock();
-          try {
-            @SuppressWarnings("unchecked")
-            List<E> cachedList = (List<E>) cache.getObject(key);
-            if (cachedList != null) return cachedList;
-          } finally {
-            cache.getReadWriteLock().readLock().unlock();
-          }
+        @SuppressWarnings("unchecked")
+        List<E> list = (List<E>) cache.getObject(key);
+        if (list == null) {
+          list = delegate.<E> query(ms, parameterObject, rowBounds, resultHandler, key, boundSql);
+          tcm.putObject(cache, key, list); // issue #578. Query must be not synchronized to prevent deadlocks
+          return list;
         }
-        List<E> list = delegate.<E> query(ms, parameterObject, rowBounds, resultHandler, key, boundSql);
-        tcm.putObject(cache, key, list); // issue #578. Query must be not synchronized to prevent deadlocks
-        return list;
       }
     }
-    return delegate.<E>query(ms, parameterObject, rowBounds, resultHandler, key, boundSql);
+    return delegate.<E> query(ms, parameterObject, rowBounds, resultHandler, key, boundSql);
   }
 
   public List<BatchResult> flushStatements() throws SQLException {
@@ -112,13 +98,11 @@ public List<BatchResult> flushStatements() throws SQLException {
   public void commit(boolean required) throws SQLException {
     delegate.commit(required);
     tcm.commit();
-    dirty = false;
   }
 
   public void rollback(boolean required) throws SQLException {
     try {
       delegate.rollback(required);
-      dirty = false;
     } finally {
       if (required) {
         tcm.rollback();
@@ -154,8 +138,7 @@ public void clearLocalCache() {
 
   private void flushCacheIfRequired(MappedStatement ms) {
     Cache cache = ms.getCache();
-    if (cache != null && ms.isFlushCacheRequired()) {
-      dirty = true; // issue #524. Disable using cached data for this session
+    if (cache != null && ms.isFlushCacheRequired()) {      
       tcm.clear(cache);
     }
   }

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

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2013 the original author or authors.
+ *    Copyright 2009-2014 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -471,10 +471,6 @@ public Executor newExecutor(Transaction transaction) {
   }
 
   public Executor newExecutor(Transaction transaction, ExecutorType executorType) {
-    return newExecutor(transaction, executorType, false);
-  }
-
-  public Executor newExecutor(Transaction transaction, ExecutorType executorType, boolean autoCommit) {
     executorType = executorType == null ? defaultExecutorType : executorType;
     executorType = executorType == null ? ExecutorType.SIMPLE : executorType;
     Executor executor;
@@ -486,7 +482,7 @@ public Executor newExecutor(Transaction transaction, ExecutorType executorType,
       executor = new SimpleExecutor(this, transaction);
     }
     if (cacheEnabled) {
-      executor = new CachingExecutor(executor, autoCommit);
+      executor = new CachingExecutor(executor);
     }
     executor = (Executor) interceptorChain.pluginAll(executor);
     return executor;

src/main/java/org/apache/ibatis/session/defaults/DefaultSqlSession.java

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2012 the original author or authors.
+ *    Copyright 2009-2014 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -40,17 +40,18 @@ public class DefaultSqlSession implements SqlSession {
   private Configuration configuration;
   private Executor executor;
 
+  private boolean autoCommit;
   private boolean dirty;
-
-  @Deprecated
+  
   public DefaultSqlSession(Configuration configuration, Executor executor, boolean autoCommit) {
-    this(configuration, executor);
-  }
-
-  public DefaultSqlSession(Configuration configuration, Executor executor) {
     this.configuration = configuration;
     this.executor = executor;
     this.dirty = false;
+    this.autoCommit = autoCommit;
+  }
+
+  public DefaultSqlSession(Configuration configuration, Executor executor) {
+    this(configuration, executor, false);
   }
 
   public <T> T selectOne(String statement) {
@@ -231,7 +232,7 @@ public void clearCache() {
   }
 
   private boolean isCommitOrRollbackRequired(boolean force) {
-    return dirty || force;
+    return (!autoCommit && dirty) || force;
   }
 
   private Object wrapCollection(final Object object) {

src/main/java/org/apache/ibatis/session/defaults/DefaultSqlSessionFactory.java

@@ -81,8 +81,8 @@ private SqlSession openSessionFromDataSource(ExecutorType execType, TransactionI
       final Environment environment = configuration.getEnvironment();
       final TransactionFactory transactionFactory = getTransactionFactoryFromEnvironment(environment);
       tx = transactionFactory.newTransaction(environment.getDataSource(), level, autoCommit);
-      final Executor executor = configuration.newExecutor(tx, execType, autoCommit);
-      return new DefaultSqlSession(configuration, executor);
+      final Executor executor = configuration.newExecutor(tx, execType);
+      return new DefaultSqlSession(configuration, executor, autoCommit);
     } catch (Exception e) {
       closeTransaction(tx); // may have fetched a connection so lets call close()
       throw ExceptionFactory.wrapException("Error opening session.  Cause: " + e, e);
@@ -93,11 +93,19 @@ private SqlSession openSessionFromDataSource(ExecutorType execType, TransactionI
 
   private SqlSession openSessionFromConnection(ExecutorType execType, Connection connection) {
     try {
+      boolean autoCommit;
+      try {
+        autoCommit = connection.getAutoCommit();
+      } catch (SQLException e) {
+        // Failover to true, as most poor drivers
+        // or databases won't support transactions
+        autoCommit = true;
+      }      
       final Environment environment = configuration.getEnvironment();
       final TransactionFactory transactionFactory = getTransactionFactoryFromEnvironment(environment);
       final Transaction tx = transactionFactory.newTransaction(connection);
-      final Executor executor = configuration.newExecutor(tx, execType, connection.getAutoCommit());
-      return new DefaultSqlSession(configuration, executor);
+      final Executor executor = configuration.newExecutor(tx, execType);
+      return new DefaultSqlSession(configuration, executor, autoCommit);
     } catch (Exception e) {
       throw ExceptionFactory.wrapException("Error opening session.  Cause: " + e, e);
     } finally {

src/test/java/org/apache/ibatis/submitted/cache/CacheTest.java

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2012 the original author or authors.
+ *    Copyright 2009-2014 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -18,13 +18,12 @@
 import java.io.Reader;
 import java.sql.Connection;
 
-import junit.framework.Assert;
-
 import org.apache.ibatis.io.Resources;
 import org.apache.ibatis.jdbc.ScriptRunner;
 import org.apache.ibatis.session.SqlSession;
 import org.apache.ibatis.session.SqlSessionFactory;
 import org.apache.ibatis.session.SqlSessionFactoryBuilder;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;