IDEMPIERE-5138 Storage Cleanup Fixes (#1117)

* IDEMPIERE-5138 Storage Cleanup Fixes

* IDEMPIERE-5138 Storage Cleanup Fixes

org.adempiere.base.process/src/org/compiere/process/StorageCleanup.java

@@ -22,12 +22,14 @@ import java.sql.ResultSet;
 import java.util.ArrayList;
 import java.util.logging.Level;
 
+import org.compiere.model.MAttributeSetInstance;
 import org.compiere.model.MMovement;
 import org.compiere.model.MMovementLine;
 import org.compiere.model.MRefList;
 import org.compiere.model.MStorageOnHand;
 import org.compiere.util.DB;
 import org.compiere.util.Env;
+import org.compiere.util.Util;
 
 /**
  * 	StorageCleanup
@@ -67,18 +69,27 @@ public class StorageCleanup extends SvrProcess
 	protected String doIt () throws Exception
 	{
 		log.info("");
-		//	Clean up empty Storage
+		//	Clean up empty Storage with no asi
 		String sql = "DELETE FROM M_StorageOnHand "
-			+ "WHERE QtyOnHand = 0"
+			+ "WHERE QtyOnHand = 0 AND M_AttributeSetInstance_ID=0 "
 			+ " AND Created < getDate()-3";
-		int no = DB.executeUpdate(sql, get_TrxName());
+		int no = DB.executeUpdateEx(sql, get_TrxName());
 		if (log.isLoggable(Level.INFO)) log.info("Delete Empty #" + no);
 
+		//	Clean up empty Storage with asi but not using serial/lot
+		sql = "DELETE FROM M_StorageOnHand "
+			+ "WHERE QtyOnHand = 0 AND M_AttributeSetInstance_ID > 0 "
+			+ " AND Created < getDate()-3"
+			+ " AND EXISTS (SELECT 1 FROM M_AttributeSetInstance a WHERE a.M_AttributeSetInstance_ID=M_StorageOnHand.M_AttributeSetInstance_ID"
+			+ " AND a.Lot IS NULL AND a.SerNo IS NULL) ";
+		no = DB.executeUpdateEx(sql, get_TrxName());
+		if (log.isLoggable(Level.INFO)) log.info("Delete Empty #" + no);
+			
 		//	Clean up empty Reservation Storage
 		sql = "DELETE FROM M_StorageReservation "
 			+ "WHERE Qty = 0"
 			+ " AND Created < getDate()-3";
-		no = DB.executeUpdate(sql, get_TrxName());
+		no = DB.executeUpdateEx(sql, get_TrxName());
 		if (log.isLoggable(Level.INFO)) log.info("Delete Empty #" + no);
 
 		//
@@ -97,14 +108,12 @@ public class StorageCleanup extends SvrProcess
 				+ " AND s.M_Product_ID=sw.M_Product_ID"
 				+ " AND s.M_Locator_ID=sl.M_Locator_ID"
 				+ " AND sl.M_Warehouse_ID=swl.M_Warehouse_ID)";
-		PreparedStatement pstmt = null;
-		ResultSet rs = null;
 		int lines = 0;
-		try
+		try (PreparedStatement pstmt = DB.prepareStatement (sql, get_TrxName()))
 		{
-			pstmt = DB.prepareStatement (sql, get_TrxName());
+			
 			pstmt.setInt(1, Env.getAD_Client_ID(getCtx()));
-			rs = pstmt.executeQuery ();
+			ResultSet rs = pstmt.executeQuery ();
 			while (rs.next ())
 			{
 				lines += move (new MStorageOnHand(getCtx(), rs, get_TrxName()));
@@ -114,11 +123,6 @@ public class StorageCleanup extends SvrProcess
 		{
 			log.log (Level.SEVERE, sql, e);
 		}
-		finally
-		{
-			DB.close(rs, pstmt);
-			rs = null; pstmt = null;
-		}
 		StringBuilder msgreturn = new StringBuilder("#").append(lines);
 		return msgreturn.toString();
 	}	//	doIt
@@ -133,20 +137,42 @@ public class StorageCleanup extends SvrProcess
 		if (log.isLoggable(Level.INFO)) log.info(target.toString());
 		BigDecimal qty = target.getQtyOnHand().negate();
 
-		//	Create Movement
+		MMovement mh = null;
-		MMovement mh = new MMovement (getCtx(), 0, get_TrxName());
+		MAttributeSetInstance targetASI = null;
-		mh.setAD_Org_ID(target.getAD_Org_ID());
+		if (target.getM_AttributeSetInstance_ID() > 0)
-		mh.setC_DocType_ID(p_C_DocType_ID);
+		{
-		mh.setDescription(getName());
+			targetASI = new MAttributeSetInstance(Env.getCtx(), target.getM_AttributeSetInstance_ID(), get_TrxName());
-		if (!mh.save())
+		}
-			return 0;
-
 		int lines = 0;
 		MStorageOnHand[] sources = getSources(target.getM_Product_ID(), target.getM_Locator_ID());
 		for (int i = 0; i < sources.length; i++)
 		{
 			MStorageOnHand source = sources[i];
+			//check serno and lot
+			if (source.getM_AttributeSetInstance_ID() > 0)
+			{
+				MAttributeSetInstance asi = new MAttributeSetInstance(Env.getCtx(), source.getM_AttributeSetInstance_ID(), get_TrxName());
+				if (!Util.isEmpty(asi.getSerNo(), true))
+				{
+					if (targetASI == null || !asi.getSerNo().equals(targetASI.getSerNo()))
+						continue;
+				}
+				if (!Util.isEmpty(asi.getLot(), true))
+				{
+					if (targetASI == null || !asi.getLot().equals(targetASI.getLot()))
+						continue;
+				}
+			}
 			
+			if (mh == null)
+			{
+				// Create Movement
+				mh = new MMovement (getCtx(), 0, get_TrxName());
+				mh.setAD_Org_ID(target.getAD_Org_ID());
+				mh.setC_DocType_ID(p_C_DocType_ID);
+				mh.setDescription(getName());
+				mh.saveEx();
+			}
 			//	Movement Line
 			MMovementLine ml = new MMovementLine(mh);
 			ml.setM_Product_ID(target.getM_Product_ID());
@@ -163,8 +189,7 @@ public class StorageCleanup extends SvrProcess
 			//
 			lines++;
 			ml.setLine(lines*10);
-			if (!ml.save())
+			ml.saveEx();
-				return 0;
 			
 			qty = qty.subtract(qtyMove);
 			if (qty.signum() <= 0)
@@ -172,16 +197,18 @@ public class StorageCleanup extends SvrProcess
 		}	//	for all movements
 		
 		//	Process
-		if (!mh.processIt(MMovement.ACTION_Complete)) {
+		if (mh != null) {
-			log.warning("Movement Process Failed: " + mh + " - " + mh.getProcessMsg());
+			if (!mh.processIt(MMovement.ACTION_Complete)) {
-			throw new IllegalStateException("Movement Process Failed: " + mh + " - " + mh.getProcessMsg());
+				log.warning("Movement Process Failed: " + mh + " - " + mh.getProcessMsg());
-			
+				throw new IllegalStateException("Movement Process Failed: " + mh + " - " + mh.getProcessMsg());
+				
+			}
+			mh.saveEx();
+			StringBuilder msglog= new StringBuilder("@M_Movement_ID@ ").append(mh.getDocumentNo()).append(" (") 
+					.append(MRefList.get(getCtx(), MMovement.DOCSTATUS_AD_Reference_ID, 
+							mh.getDocStatus(), get_TrxName())).append(")");
+			addLog(0, null, new BigDecimal(lines), msglog.toString());
 		}
-		mh.saveEx();
-		StringBuilder msglog= new StringBuilder("@M_Movement_ID@ ").append(mh.getDocumentNo()).append(" (") 
-				.append(MRefList.get(getCtx(), MMovement.DOCSTATUS_AD_Reference_ID, 
-						mh.getDocStatus(), get_TrxName())).append(")");
-		addLog(0, null, new BigDecimal(lines), msglog.toString());
 
 		eliminateReservation(target);
 		return lines;

org.idempiere.test/src/org/idempiere/test/model/MStorageOnHandTest.java

@@ -25,16 +25,24 @@
 package org.idempiere.test.model;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.math.BigDecimal;
 import java.sql.Timestamp;
+import java.util.List;
 
 import org.compiere.model.MAttributeSet;
 import org.compiere.model.MAttributeSetInstance;
 import org.compiere.model.MLocator;
+import org.compiere.model.MPInstance;
+import org.compiere.model.MPInstancePara;
+import org.compiere.model.MProcess;
 import org.compiere.model.MProduct;
 import org.compiere.model.MStorageOnHand;
+import org.compiere.model.Query;
+import org.compiere.process.ProcessInfo;
+import org.compiere.process.ServerProcessCtl;
 import org.compiere.util.CacheMgt;
 import org.compiere.util.DB;
 import org.compiere.util.Env;
@@ -55,6 +63,8 @@ public class MStorageOnHandTest extends AbstractTestCase {
 	private static final int TAX_CATEGORY_STANDARD_ID = 107;
 	private static final int CHEMICALS_CATEGORY_ID = 109;
 	private static final int UOM_EACH_ID = 100;
+	private static final int STORAGE_CLEANUP_PROCESS_ID = 325;
+	private static final int MATERIAL_MOVEMENT_DOCTYPE_ID = 143;
 	
 	public MStorageOnHandTest() {
 	}
@@ -193,4 +203,171 @@ public class MStorageOnHandTest extends AbstractTestCase {
 		M_Locator_ID = MStorageOnHand.getM_Locator_ID(hqLocator.getM_Warehouse_ID(), product.get_ID(), -1, new BigDecimal("1"), getTrxName());
 		assertEquals(hqLocator1.get_ID(), M_Locator_ID);
 	}
+	
+	@Test
+	public void testStorageCleanUp() {
+		MProduct product = new MProduct(Env.getCtx(), 0, getTrxName());
+		product.setName("testStorageCleanUp");
+		product.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+		product.setIsStocked(true);
+		product.setProductType(MProduct.PRODUCTTYPE_Item);
+		product.setC_UOM_ID(UOM_EACH_ID);
+		product.setM_Product_Category_ID(CHEMICALS_CATEGORY_ID);
+		product.setC_TaxCategory_ID(TAX_CATEGORY_STANDARD_ID);
+		product.saveEx();
+		
+		Timestamp today = TimeUtil.getDay(null);
+		MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product.get_ID(), 0, new BigDecimal("2"), today, getTrxName());
+		Query query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=?", getTrxName());
+		int count = query.setParameters(product.get_ID()).count();
+		assertEquals(1, count);
+		
+		MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product.get_ID(), 0, new BigDecimal("-2"), today, getTrxName());
+		DB.executeUpdateEx("UPDATE M_StorageOnHand SET Created=? WHERE M_Product_ID=?", new Object[] {TimeUtil.addDays(today, -7), product.get_ID()}, getTrxName());
+		query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=?", getTrxName());
+		count = query.setParameters(product.get_ID()).count();
+		assertEquals(1, count);
+		
+		//movement get product from cache
+		MProduct product1 = new MProduct(Env.getCtx(), 0, null);
+		product1.setName("testStorageCleanUp#1");
+		product1.setValue(product1.getName());
+		product1.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+		product1.setIsStocked(true);
+		product1.setProductType(MProduct.PRODUCTTYPE_Item);
+		product1.setC_UOM_ID(UOM_EACH_ID);
+		product1.setM_Product_Category_ID(CHEMICALS_CATEGORY_ID);
+		product1.setC_TaxCategory_ID(TAX_CATEGORY_STANDARD_ID);
+		product1.saveEx();
+		
+		try {
+			MAttributeSetInstance asi1 = new MAttributeSetInstance(Env.getCtx(), 0, getTrxName());
+			asi1.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+			asi1.setLot("Lot1");
+			asi1.saveEx();		
+			MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product1.get_ID(), asi1.get_ID(), new BigDecimal("-1"), today, getTrxName());		
+			MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product1.get_ID(), 0, new BigDecimal("1"), today, getTrxName());
+			
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			MStorageOnHand onhand = query.setParameters(product1.get_ID(), asi1.get_ID()).first();
+			assertNotNull(onhand);
+			assertEquals(-1, onhand.getQtyOnHand().intValue());
+			
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			onhand = query.setParameters(product1.get_ID(), 0).first();
+			assertNotNull(onhand);
+			assertEquals(1, onhand.getQtyOnHand().intValue());
+			
+			MProduct product2 = new MProduct(Env.getCtx(), 0, getTrxName());
+			product2.setName("testStorageCleanUp#2");
+			product2.setValue(product2.getName());
+			product2.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+			product2.setIsStocked(true);
+			product2.setProductType(MProduct.PRODUCTTYPE_Item);
+			product2.setC_UOM_ID(UOM_EACH_ID);
+			product2.setM_Product_Category_ID(CHEMICALS_CATEGORY_ID);
+			product2.setC_TaxCategory_ID(TAX_CATEGORY_STANDARD_ID);
+			product2.saveEx();
+			
+			MAttributeSetInstance asi2 = new MAttributeSetInstance(Env.getCtx(), 0, getTrxName());
+			asi2.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+			asi2.setLot("LotX");
+			asi2.saveEx();		
+			MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product2.get_ID(), asi2.get_ID(), new BigDecimal("-1"), today, getTrxName());
+			
+			MAttributeSetInstance asi3 = new MAttributeSetInstance(Env.getCtx(), 0, getTrxName());
+			asi3.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+			asi3.setLot("LotY");
+			asi3.saveEx();
+			MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product2.get_ID(), asi3.get_ID(), new BigDecimal("1"), today, getTrxName());
+			
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			onhand = query.setParameters(product2.get_ID(), asi2.get_ID()).first();
+			assertNotNull(onhand);
+			assertEquals(-1, onhand.getQtyOnHand().intValue());
+			
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			onhand = query.setParameters(product2.get_ID(), asi3.get_ID()).first();
+			assertNotNull(onhand);
+			assertEquals(1, onhand.getQtyOnHand().intValue());
+			
+			MProduct product3 = new MProduct(Env.getCtx(), 0, getTrxName());
+			product3.setName("testStorageCleanUp#3");
+			product3.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+			product3.setIsStocked(true);
+			product3.setProductType(MProduct.PRODUCTTYPE_Item);
+			product3.setC_UOM_ID(UOM_EACH_ID);
+			product3.setM_Product_Category_ID(CHEMICALS_CATEGORY_ID);
+			product3.setC_TaxCategory_ID(TAX_CATEGORY_STANDARD_ID);
+			product3.saveEx();
+			
+			MAttributeSetInstance asi4 = new MAttributeSetInstance(Env.getCtx(), 0, getTrxName());
+			asi4.setM_AttributeSet_ID(FERTILIZER_LOT_ATTRIBUTESET_ID);
+			asi4.setSerNo(product3.getName()+"SerialNo#1");
+			asi4.saveEx();
+			MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product3.get_ID(), asi4.get_ID(), new BigDecimal("1"), today, getTrxName());
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=?", getTrxName());
+			count = query.setParameters(product3.get_ID()).count();
+			assertEquals(1, count);
+			
+			MStorageOnHand.add(Env.getCtx(), HQ_LOCATOR_ID, product3.get_ID(), asi4.get_ID(), new BigDecimal("-1"), today, getTrxName());
+			DB.executeUpdateEx("UPDATE M_StorageOnHand SET Created=? WHERE M_Product_ID=?", new Object[] {TimeUtil.addDays(today, -7), product3.get_ID()}, getTrxName());
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=?", getTrxName());
+			count = query.setParameters(product3.get_ID()).count();
+			assertEquals(1, count);
+			
+			MPInstance instance = new MPInstance(Env.getCtx(), STORAGE_CLEANUP_PROCESS_ID, 0);
+			instance.saveEx();
+			MPInstancePara para = new MPInstancePara(instance, 10);
+			para.setParameterName("C_DocType_ID");
+			para.setP_Number(MATERIAL_MOVEMENT_DOCTYPE_ID);
+			para.saveEx();
+			
+			MProcess process = MProcess.get(Env.getCtx(), STORAGE_CLEANUP_PROCESS_ID);
+			ProcessInfo pi = new ProcessInfo(process.getName(), process.get_ID());
+			pi.setAD_PInstance_ID(instance.get_ID());
+			pi.setAD_Client_ID(getAD_Client_ID());
+			pi.setAD_User_ID(getAD_User_ID());
+			pi.setTransactionName(getTrxName());
+			ServerProcessCtl.process(pi, getTrx(), false);
+			assertFalse(pi.isError(), pi.getSummary());
+			
+			//check 0 stock removed
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=?", getTrxName());
+			count = query.setParameters(product.get_ID()).count();
+			assertEquals(0, count);
+			
+			//check 0 stock with serno not removed
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=?", getTrxName());
+			List<MStorageOnHand> onhands = query.setParameters(product3.get_ID()).list();
+			assertEquals(1, onhands.size());
+			assertEquals(0, onhands.get(0).getQtyOnHand().intValue());
+			assertEquals(asi4.get_ID(), onhands.get(0).getM_AttributeSetInstance_ID());
+			
+			//check -1 and 1 consolidated
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			onhand = query.setParameters(product1.get_ID(), asi1.get_ID()).first();
+			assertNotNull(onhand);
+			assertEquals(0, onhand.getQtyOnHand().intValue());
+			
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			onhand = query.setParameters(product1.get_ID(), 0).first();
+			assertNotNull(onhand);
+			assertEquals(0, onhand.getQtyOnHand().intValue());
+			
+			//check -1 and 1 not consolidated due to different lot
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			onhand = query.setParameters(product2.get_ID(), asi2.get_ID()).first();
+			assertNotNull(onhand);
+			assertEquals(-1, onhand.getQtyOnHand().intValue());
+			
+			query = new Query(Env.getCtx(), MStorageOnHand.Table_Name, "M_Product_ID=? AND M_AttributeSetInstance_ID=?", getTrxName());
+			onhand = query.setParameters(product2.get_ID(), asi3.get_ID()).first();
+			assertNotNull(onhand);
+			assertEquals(1, onhand.getQtyOnHand().intValue());
+		} finally {
+			getTrx().rollback();
+			product1.deleteEx(true);
+		}
+	}
 }