IDEMPIERE-4406 Performance: PO Cache should not always reset all entr… (#212)

* IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record

PO update - reset cache by record id

* IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record

refine unit test

* IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record

add cache reset fix for delete

* IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record

Fix exception when cache is empty
Expose hidden cache reset exception

org.adempiere.base/src/org/compiere/model/PO.java

@@ -2384,7 +2384,7 @@ public abstract class PO
 			m_createNew = false;
 		}
 		if (!newRecord) {
-			CacheMgt.get().reset(p_info.getTableName());
+			CacheMgt.get().reset(p_info.getTableName(), get_ID());
 			MRecentItem.clearLabel(p_info.getAD_Table_ID(), get_ID());
 		} else if (get_ID() > 0 && success)
 			CacheMgt.get().newRecord(p_info.getTableName(), get_ID());
@@ -3542,7 +3542,7 @@ public abstract class PO
 				int size = p_info.getColumnCount();
 				m_oldValues = new Object[size];
 				m_newValues = new Object[size];
-				CacheMgt.get().reset(p_info.getTableName());
+				CacheMgt.get().reset(p_info.getTableName(), Record_ID);
 			}
 		}
 		finally

org.adempiere.base/src/org/compiere/util/CCache.java

@@ -22,6 +22,7 @@ import java.io.Serializable;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
@@ -428,11 +429,17 @@ public class CCache<K,V> implements CacheInterface, Map<K, V>, Serializable
 		if (recordId <= 0)
 			return reset();
 
+		Iterator<K> iterator = cache.keySet().iterator();
+		K firstKey = iterator.hasNext() ? iterator.next() : null;
+		if (firstKey != null && firstKey instanceof Integer) {
 			if (!nullList.isEmpty()) {
 				if (nullList.remove(recordId)) return 1;
 			}
 			V removed = cache.remove(recordId);
 			return removed != null ? 1 : 0;
+		} else {
+			return reset();
+		}
 	}
 
 	@Override

org.adempiere.base/src/org/compiere/util/CacheInterface.java

@@ -31,7 +31,8 @@ public interface CacheInterface
 	public int reset();
 	
 	/**
-	 *	Reset Cache
+	 *	Reset Cache by record id
+	 *  @param recordId
 	 *	@return number of items reset
 	 */
 	public int reset(int recordId);

org.adempiere.base/src/org/compiere/util/CacheMgt.java

@@ -176,9 +176,15 @@ public class CacheMgt
 						total += i.get();
 					}
 				} catch (InterruptedException e) {
-					e.printStackTrace();
+					throw new RuntimeException(e);
 				} catch (ExecutionException e) {
-					e.printStackTrace();
+					if (e.getCause() != null)
+						if (e.getCause() instanceof RuntimeException)
+							throw (RuntimeException)e.getCause();
+						else
+							throw new RuntimeException(e.getCause());
+					else
+						throw new RuntimeException(e);
 				}
 				return total;
 			} else {
@@ -263,9 +269,9 @@ public class CacheMgt
 	}
 
 	/**
-	 * @return
+	 * @return cache instances
 	 */
-	protected synchronized CacheInterface[] getInstancesAsArray() {
+	public synchronized CacheInterface[] getInstancesAsArray() {
 		return m_instances.toArray(new CacheInterface[0]);
 	}
 	

org.idempiere.test/src/org/idempiere/test/performance/CacheTest.java

@@ -24,15 +24,20 @@
  **********************************************************************/
 package org.idempiere.test.performance;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import org.compiere.model.I_AD_Table;
 import org.compiere.model.MColumn;
 import org.compiere.model.MOrder;
+import org.compiere.model.MProduct;
 import org.compiere.model.MRefTable;
 import org.compiere.model.MTable;
 import org.compiere.model.MWarehouse;
 import org.compiere.model.MZoomCondition;
+import org.compiere.util.CCache;
+import org.compiere.util.CacheInterface;
 import org.compiere.util.CacheMgt;
 import org.compiere.util.Env;
 import org.idempiere.test.AbstractTestCase;
@@ -80,4 +85,90 @@ public class CacheTest extends AbstractTestCase {
 		table2 = refTable.getAD_Table();
 		assertTrue(table == table2);
 	}
+	
+	@SuppressWarnings({"unchecked", "rawtypes"})
+	@Test
+	public void testPOCacheAfterUpdate() {
+		int mulch = 137;
+		int oak = 123;
+		//init cache
+		MProduct p1 = MProduct.get(Env.getCtx(), mulch);
+		CCache<Integer, MProduct> pc = null;
+		CacheInterface[] cis = CacheMgt.get().getInstancesAsArray();
+		//find product cache instance
+		for(CacheInterface ci : cis) {
+			if (ci instanceof CCache<?, ?>) {				
+				CCache ccache = (CCache) ci;
+				if (ccache.getName().equals(ccache.getTableName()) && ccache.getTableName().equals(MProduct.Table_Name)) {
+					if (ccache.containsKey(mulch)) {
+						pc = ccache;
+						break;
+					}
+				}
+			}
+		}
+		
+		if (pc == null)
+			fail("Product cache instance missing");
+		
+		//second get, hit should increase
+		long hit = pc.getHit();
+		p1 = MProduct.get(Env.getCtx(), mulch);
+		assertEquals(mulch, p1.getM_Product_ID());
+		assertTrue(pc.getHit() > hit, "Second get of product Mulch, cache hit should increase");
+		
+		//first get for p2, miss should increase
+		long miss = pc.getMiss();
+		MProduct p2 = MProduct.get(Env.getCtx(), oak);
+		assertEquals(oak, p2.getM_Product_ID());
+		assertTrue(pc.getMiss() > miss, "First get of product Oak, cache miss should increase");
+		
+		//second get for p2, hit should increase
+		hit = pc.getHit();
+		p2 = MProduct.get(Env.getCtx(), oak);
+		assertEquals(oak, p2.getM_Product_ID());
+		assertTrue(pc.getHit() > hit, "Second get of product Oak, cache hit should increase");
+		
+		p2.set_TrxName(getTrxName());
+		p2.setDescription("Test Update @ " + System.currentTimeMillis());
+		p2.saveEx();
+		
+		//get after p2 update, miss should increase
+		miss = pc.getMiss();
+		p2 = MProduct.get(Env.getCtx(), oak);
+		assertEquals(oak, p2.getM_Product_ID());
+		assertTrue(pc.getMiss() > miss, "Get of product Oak after update of product Oak, cache miss should increase");
+		
+		//cache for p1 not effected by p2 update, hit should increase
+		hit = pc.getHit();
+		p1 = MProduct.get(Env.getCtx(), mulch);
+		assertEquals(mulch, p1.getM_Product_ID());
+		assertTrue(pc.getHit() > hit, "Get of product Mulch after update of product Oak, cache hit should increase");
+		
+		//create p3 to test delete
+		MProduct p3 = new MProduct(Env.getCtx(), 0, getTrxName());
+		String name = "Test@"+System.currentTimeMillis();
+		p3.setValue(name);
+		p3.setName(name);
+		p3.setM_Product_Category_ID(p1.getM_Product_Category_ID());
+		p3.setC_UOM_ID(p1.getC_UOM_ID());
+		p3.setC_TaxCategory_ID(p1.getC_TaxCategory_ID());
+		p3.saveEx();
+		
+		p3.deleteEx(true);
+		
+		//cache for p2 not effected by p3 delete, hit should increase
+		hit = pc.getHit();
+		p2 = MProduct.get(Env.getCtx(), oak);
+		assertEquals(oak, p2.getM_Product_ID());
+		assertTrue(pc.getHit() > hit, "Get of product Oak after delete of product Mulch, cache hit should increase");
+		
+		//test update when cache is empty
+		CacheMgt.get().reset();
+		p2.set_TrxName(getTrxName());
+		p2.setDescription("Test1@"+System.currentTimeMillis());
+		p2.saveEx();
+		
+		rollback();
+	}
 }