IDEMPIERE-4268 Fix recursive call to PO (#424)

* IDEMPIERE-4268 Fix recursive call to PO

Found a case where PO.checkValidClient fails, and then because of log.severe it tries to create a record with MIssue, and the PO.checkValidClient fails in MIssue, and is called recursively until it fails with a "Trx.run: Transaction timeout."

* IDEMPIERE-4268

- Improve error message
- Fix problem with valid cross-tenant reads in MRegion and MWorkflow
This commit is contained in:
Carlos Ruiz 2020-11-30 21:36:51 +01:00 committed by GitHub
parent 5d236de30c
commit 3a9b932df4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 42 deletions

View File

@ -16,27 +16,25 @@
*****************************************************************************/ *****************************************************************************/
package org.compiere.model; package org.compiere.model;
import static org.compiere.model.SystemIDs.COUNTRY_JAPAN;
import java.io.Serializable; import java.io.Serializable;
import java.sql.ResultSet; import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.text.Collator; import java.text.Collator;
import java.util.Arrays; import java.util.Arrays;
import java.util.Comparator; import java.util.Comparator;
import java.util.List;
import java.util.Properties; import java.util.Properties;
import java.util.logging.Level; import java.util.logging.Level;
import org.compiere.Adempiere; import org.compiere.Adempiere;
import org.compiere.util.CLogger; import org.compiere.util.CLogger;
import org.compiere.util.DB;
import org.compiere.util.Env; import org.compiere.util.Env;
import org.idempiere.cache.ImmutablePOSupport;
import org.idempiere.cache.ImmutablePOCache; import org.idempiere.cache.ImmutablePOCache;
import org.idempiere.cache.ImmutablePOSupport;
import static org.compiere.model.SystemIDs.*;
/** /**
* Localtion Region Model (Value Object) * Location Region Model (Value Object)
* *
* @author Jorg Janke * @author Jorg Janke
* @version $Id: MRegion.java,v 1.3 2006/07/30 00:58:36 jjanke Exp $ * @version $Id: MRegion.java,v 1.3 2006/07/30 00:58:36 jjanke Exp $
@ -47,7 +45,7 @@ public class MRegion extends X_C_Region
/** /**
* *
*/ */
private static final long serialVersionUID = 5309795002353895615L; private static final long serialVersionUID = -752467671346696325L;
/** /**
* Load Regions (cached) * Load Regions (cached)
@ -55,31 +53,26 @@ public class MRegion extends X_C_Region
private static void loadAllRegions () private static void loadAllRegions ()
{ {
s_regions.clear(); s_regions.clear();
String sql = "SELECT * FROM C_Region WHERE IsActive='Y'"; List<MRegion> regions;
Statement stmt = null; int cid = Env.getAD_Client_ID(Env.getCtx());
ResultSet rs = null; try {
try if (cid > 0) {
{ // forced potential cross tenant read - requires System client in context
stmt = DB.createStatement(); Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
rs = stmt.executeQuery(sql); }
while(rs.next()) regions = new Query(Env.getCtx(), Table_Name, "", null)
{ .setOnlyActiveRecords(true)
MRegion r = new MRegion (Env.getCtx(), rs, null); .list();
r.markImmutable(); } finally {
s_regions.put(String.valueOf(r.getC_Region_ID()), r); if (cid > 0) {
if (r.isDefault()) Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
s_default = r;
} }
} }
catch (SQLException e) for (MRegion r : regions) {
{ r.markImmutable();
s_log.log(Level.SEVERE, sql, e); s_regions.put(r.getC_Region_ID(), r);
} if (r.isDefault())
finally s_default = r;
{
DB.close(rs, stmt);
rs = null;
stmt = null;
} }
if (s_log.isLoggable(Level.FINE)) s_log.fine(s_regions.size() + " - default=" + s_default); if (s_log.isLoggable(Level.FINE)) s_log.fine(s_regions.size() + " - default=" + s_default);
} // loadAllRegions } // loadAllRegions
@ -103,14 +96,13 @@ public class MRegion extends X_C_Region
{ {
if (s_regions.size() == 0) if (s_regions.size() == 0)
loadAllRegions(); loadAllRegions();
String key = String.valueOf(C_Region_ID); MRegion r = s_regions.get(ctx, C_Region_ID, e -> new MRegion(ctx, e));
MRegion r = s_regions.get(ctx, key, e -> new MRegion(ctx, e));
if (r != null) if (r != null)
return r; return r;
r = new MRegion (ctx, C_Region_ID, null); r = new MRegion (ctx, C_Region_ID, null);
if (r.getC_Region_ID() == C_Region_ID) if (r.getC_Region_ID() == C_Region_ID)
{ {
s_regions.put(key, r, e -> new MRegion(Env.getCtx(), e)); s_regions.put(C_Region_ID, r, e -> new MRegion(Env.getCtx(), e));
return r; return r;
} }
return null; return null;
@ -190,7 +182,7 @@ public class MRegion extends X_C_Region
} // getRegions } // getRegions
/** Region Cache */ /** Region Cache */
private static ImmutablePOCache<String,MRegion> s_regions = new ImmutablePOCache<String,MRegion>(Table_Name, Table_Name, 100, 0, false, 0); private static ImmutablePOCache<Integer,MRegion> s_regions = new ImmutablePOCache<Integer,MRegion>(Table_Name, Table_Name, 100, 0, false, 0);
/** Default Region */ /** Default Region */
private static MRegion s_default = null; private static MRegion s_default = null;
/** Static Logger */ /** Static Logger */

View File

@ -4986,8 +4986,13 @@ public abstract class PO
int poClientID = getAD_Client_ID(); int poClientID = getAD_Client_ID();
if (poClientID != envClientID && if (poClientID != envClientID &&
(poClientID != 0 || writing)) { (poClientID != 0 || writing)) {
log.severe("Table="+get_TableName()+" Record_ID="+get_ID()+" Env.AD_Client_ID="+envClientID+" PO.AD_Client_ID="+poClientID); log.warning("Table="+get_TableName()
String message = "Cross tenant PO request detected from session " +" Record_ID="+get_ID()
+" Env.AD_Client_ID="+envClientID
+" PO.AD_Client_ID="+poClientID
+" writing="+writing
+" Session="+Env.getContext(getCtx(), "#AD_Session_ID"));
String message = "Cross tenant PO " + (writing ? "writing" : "reading") + " request detected from session "
+ Env.getContext(getCtx(), "#AD_Session_ID") + " for table " + get_TableName() + Env.getContext(getCtx(), "#AD_Session_ID") + " for table " + get_TableName()
+ " Record_ID=" + get_ID(); + " Record_ID=" + get_ID();
throw new AdempiereException(message); throw new AdempiereException(message);

View File

@ -130,11 +130,23 @@ public class MWorkflow extends X_AD_Workflow implements ImmutablePOSupport
if (s_cacheDocValue.isReset()) if (s_cacheDocValue.isReset())
{ {
final String whereClause = "WorkflowType=? AND IsValid=?"; final String whereClause = "WorkflowType=? AND IsValid=?";
List<MWorkflow> workflows = new Query(ctx, Table_Name, whereClause, trxName) List<MWorkflow> workflows;
.setParameters(new Object[]{WORKFLOWTYPE_DocumentValue, true}) int cid = Env.getAD_Client_ID(Env.getCtx());
.setOnlyActiveRecords(true) try {
.setOrderBy("AD_Client_ID, AD_Table_ID") if (cid > 0) {
.list(); // forced potential cross tenant read - requires System client in context
Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
}
workflows = new Query(ctx, Table_Name, whereClause, trxName)
.setParameters(new Object[]{WORKFLOWTYPE_DocumentValue, true})
.setOnlyActiveRecords(true)
.setOrderBy("AD_Client_ID, AD_Table_ID")
.list();
} finally {
if (cid > 0) {
Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
}
}
ArrayList<MWorkflow> list = new ArrayList<MWorkflow>(); ArrayList<MWorkflow> list = new ArrayList<MWorkflow>();
String oldKey = ""; String oldKey = "";
String newKey = null; String newKey = null;