From 494597b3fdfb2cc60a68b5c7b1d7d8fdba0e8d56 Mon Sep 17 00:00:00 2001 From: sebastianbarrozo Date: Wed, 17 Dec 2025 15:11:53 -0300 Subject: [PATCH 1/4] Feature ETP-3003: Enhance call sync --- src/com/etendoerp/db/CallAsyncProcess.java | 145 ++++++ src/org/openbravo/service/db/CallProcess.java | 455 ++++++++++-------- .../service/db/CallStoredProcedure.java | 143 +----- 3 files changed, 418 insertions(+), 325 deletions(-) create mode 100644 src/com/etendoerp/db/CallAsyncProcess.java diff --git a/src/com/etendoerp/db/CallAsyncProcess.java b/src/com/etendoerp/db/CallAsyncProcess.java new file mode 100644 index 000000000..91df96f0d --- /dev/null +++ b/src/com/etendoerp/db/CallAsyncProcess.java @@ -0,0 +1,145 @@ +package com.etendoerp.db; + +import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import org.openbravo.base.exception.OBException; +import org.openbravo.dal.core.OBContext; +import org.openbravo.dal.service.OBDal; +import org.openbravo.model.ad.process.ProcessInstance; +import org.openbravo.model.ad.ui.Process; +import org.openbravo.service.db.CallProcess; + +/** + * Service class to execute database processes asynchronously. + *

+ * This class extends {@link CallProcess} to inherit the core execution logic but runs the + * database procedure in a separate thread. This is useful for long-running processes + * to avoid blocking the user interface or triggering HTTP timeouts. + *

+ * * Key behaviors: + * + */ +public class CallAsyncProcess extends CallProcess { + + private static CallAsyncProcess instance = new CallAsyncProcess(); + + // Thread pool to manage background executions. + // Using a fixed pool prevents system resource exhaustion. + private final ExecutorService executorService = Executors.newFixedThreadPool(10); + + public static synchronized CallAsyncProcess getInstance() { + return instance; + } + + public static synchronized void setInstance(CallAsyncProcess instance) { + CallAsyncProcess.instance = instance; + } + + /** + * Overrides the main execution method to run asynchronously. + * * @param process + * the process definition. + * @param recordID + * the record ID. + * @param parameters + * map of parameters. + * @param doCommit + * explicit commit flag. + * @return the ProcessInstance in 'Processing' state (Result=0, Msg='Processing...'). + */ + @Override + public ProcessInstance callProcess(Process process, String recordID, Map parameters, Boolean doCommit) { + OBContext.setAdminMode(); + try { + // 1. SYNC PHASE: Prepare Data + // We must create the PInstance in the main thread to return the ID immediately to the user. + ProcessInstance pInstance = createAndPersistInstance(process, recordID, parameters); + + // Set initial status specifically for Async (though createAndPersist usually sets defaults) + pInstance.setResult(0L); + pInstance.setErrorMsg("Processing in background..."); + OBDal.getInstance().save(pInstance); + OBDal.getInstance().flush(); + + // Capture critical IDs and Context to pass to the thread + final String pInstanceId = pInstance.getId(); + final String processId = process.getId(); + final OBContext currentContext = OBContext.getOBContext(); + + // 2. ASYNC PHASE: Submit to Executor + executorService.submit(() -> { + runInBackground(pInstanceId, processId, currentContext, doCommit); + }); + + // 3. RETURN IMMEDIATELY + // The pInstance returned here is the initial snapshot. The UI should poll for updates. + return pInstance; + + } finally { + OBContext.restorePreviousMode(); + } + } + + /** + * Internal method executed by the worker thread. + */ + private void runInBackground(String pInstanceId, String processId, OBContext context, Boolean doCommit) { + // A. Context Hydration + // The new thread does not have the user session. We must set it manually. + OBContext.setOBContext(context); + OBContext.setAdminMode(); + + try { + // B. Re-attach Hibernate Objects + // We cannot use the objects from the main thread (Process/ProcessInstance) + // because they belong to a different (likely closed) Hibernate Session. + ProcessInstance pInstance = OBDal.getInstance().get(ProcessInstance.class, pInstanceId); + Process process = OBDal.getInstance().get(Process.class, processId); + + if (pInstance == null || process == null) { + throw new OBException("Async Execution Failed: Process Instance or Definition not found."); + } + + // C. Execute Logic (Reusing CallProcess logic) + // This calls the protected method in the parent class (CallProcess) + executeStandardProcedure(pInstance, process, doCommit); + + // D. Commit Transaction + // In async threads, we are responsible for the transaction boundary. + OBDal.getInstance().commitAndClose(); + + } catch (Exception e) { + // E. Error Handling + // If something fails in the background, we must log it to the DB, + // otherwise the PInstance will remain "Processing..." forever. + try { + OBDal.getInstance().rollbackAndClose(); + + // Open a new transaction to save the error + ProcessInstance pInstanceCtx = OBDal.getInstance().get(ProcessInstance.class, pInstanceId); + if (pInstanceCtx != null) { + pInstanceCtx.setResult(0L); // Error + String msg = e.getMessage() != null ? e.getMessage() : e.toString(); + // Truncate to avoid DB errors if message is too long + if (msg.length() > 2000) msg = msg.substring(0, 2000); + pInstanceCtx.setErrorMsg("Async Error: " + msg); + + OBDal.getInstance().save(pInstanceCtx); + OBDal.getInstance().commitAndClose(); + } + } catch (Exception ex) { + // Catastrophic failure (DB down?), just log to console + ex.printStackTrace(); + } + e.printStackTrace(); + } finally { + OBContext.restorePreviousMode(); + } + } +} diff --git a/src/org/openbravo/service/db/CallProcess.java b/src/org/openbravo/service/db/CallProcess.java index 5a9a0553c..c4ccdd545 100644 --- a/src/org/openbravo/service/db/CallProcess.java +++ b/src/org/openbravo/service/db/CallProcess.java @@ -4,17 +4,15 @@ * Version 1.1 (the "License"), being the Mozilla Public License * Version 1.1 with a permitted attribution clause; you may not use this * file except in compliance with the License. You may obtain a copy of - * the License at http://www.openbravo.com/legal/license.html + * the License at http://www.openbravo.com/legal/license.html * Software distributed under the License is distributed on an "AS IS" * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the * License for the specific language governing rights and limitations - * under the License. - * The Original Code is Openbravo ERP. - * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2009-2016 Openbravo SLU - * All Rights Reserved. - * Contributor(s): ______________________________________. - * Modification july 2010 (c) openbravo SLU, based on contribution made by iferca + * under the License. + * The Original Code is Openbravo ERP. + * The Initial Developer of the Original Code is Openbravo SLU + * All portions are Copyright (C) 2009-2025 Openbravo SLU + * All Rights Reserved. ************************************************************************ */ @@ -23,32 +21,41 @@ import java.math.BigDecimal; import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.Timestamp; +import java.sql.Types; +import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Map; -import java.util.Properties; import org.hibernate.criterion.Restrictions; import org.openbravo.base.exception.OBException; import org.openbravo.base.provider.OBProvider; -import org.openbravo.base.session.OBPropertiesProvider; +import org.openbravo.base.structure.BaseOBObject; import org.openbravo.dal.core.OBContext; import org.openbravo.dal.service.OBCriteria; import org.openbravo.dal.service.OBDal; import org.openbravo.model.ad.process.Parameter; import org.openbravo.model.ad.process.ProcessInstance; +import org.openbravo.model.ad.ui.Process; /** - * This class is a service class to call a stored procedure using a set of parameters. - * - * The {@link ProcessInstance} result is returned. - * + * Service class to execute database stored procedures. + *

+ * This class acts as the central engine for database calls in Openbravo. + * It supports two modes of execution: + *

+ *

+ * * @author mtaal * @see ProcessInstance - * @see org.openbravo.model.ad.ui.Process - * @see Parameter - * - * @author mtaal + * @see Process */ public class CallProcess { @@ -62,230 +69,288 @@ public static synchronized void setInstance(CallProcess instance) { CallProcess.instance = instance; } + // =========================================================================== + // STANDARD MODE: Process Instance Execution (UI / Background Processes) + // =========================================================================== + /** - * Calls a process with the specified name. The recordID and parameters can be null. Parameters - * are translated into {@link Parameter} instances. - * - * @param processName - * the name of the stored procedure, must exist in the database, see - * {@link org.openbravo.model.ad.ui.Process#getProcedure()}. + * Calls a process by its name. + * * @param processName + * the procedure name defined in AD_Process. * @param recordID - * the recordID will be set in the {@link ProcessInstance}, see - * {@link ProcessInstance#getRecordID()} + * the record ID associated with the execution (optional). * @param parameters - * are translated into process parameters - * @param doCommit - * do commit at the end of the procedure only if calledFromApp functionality is supported - * in the procedure, otherwise null should be passed - * @return the created instance with the result ({@link ProcessInstance#getResult()}) or error ( - * {@link ProcessInstance#getErrorMsg()}) + * a map of parameters to be injected into AD_PInstance_Para. + * @return the result ProcessInstance. + */ + public ProcessInstance call(String processName, String recordID, Map parameters) { + return call(processName, recordID, parameters, null); + } + + /** + * Calls a process by its name with commit control. */ public ProcessInstance call(String processName, String recordID, Map parameters, Boolean doCommit) { - final OBCriteria processCriteria = OBDal.getInstance() - .createCriteria(org.openbravo.model.ad.ui.Process.class); - processCriteria - .add(Restrictions.eq(org.openbravo.model.ad.ui.Process.PROPERTY_PROCEDURE, processName)); - List processList = processCriteria.list(); - if (processList.size() != 1) { - throw new OBException( - "No process or more than one process found using procedurename " + processName); + final OBCriteria processCriteria = OBDal.getInstance().createCriteria(Process.class); + processCriteria.add(Restrictions.eq(Process.PROPERTY_PROCEDURE, processName)); + + final Process process = (Process) processCriteria.uniqueResult(); + if (process == null) { + throw new OBException("No process found with procedure name " + processName); } - return call(processList.get(0), recordID, parameters, doCommit); + return call(process, recordID, parameters, doCommit); + } + + public ProcessInstance call(Process process, String recordID, Map parameters) { + return callProcess(process, recordID, parameters, null); + } + + public ProcessInstance call(Process process, String recordID, Map parameters, Boolean doCommit) { + return callProcess(process, recordID, parameters, doCommit); } /** - * Calls a process. The recordID and parameters can be null. Parameters are translated into - * {@link Parameter} instances. - * - * @param process - * the process to execute + * Calls a process using the ProcessInstance mechanism. + *

+ * This method follows the template pattern to facilitate asynchronous extensions: + * 1. {@link #createAndPersistInstance}: Prepares data. + * 2. {@link #executeStandardProcedure}: Executes DB logic. + * 3. Refreshes the result. + *

+ * * @param process + * the process definition. * @param recordID - * the recordID will be set in the {@link ProcessInstance}, see - * {@link ProcessInstance#getRecordID()} + * the record ID. * @param parameters - * are translated into process parameters, supports only string parameters, for support - * of other parameters see the next method: - * {@link #callProcess(org.openbravo.model.ad.ui.Process, String, Map)} + * map of parameters. * @param doCommit - * do commit at the end of the procedure only if calledFromApp functionality is supported - * in the procedure, otherwise null should be passed - * @return the created instance with the result ({@link ProcessInstance#getResult()}) or error ( - * {@link ProcessInstance#getErrorMsg()}) + * explicit commit flag (if supported by the SP). + * @return the updated ProcessInstance with results. */ - public ProcessInstance call(org.openbravo.model.ad.ui.Process process, String recordID, - Map parameters, Boolean doCommit) { - return callProcess(process, recordID, parameters, doCommit); + public ProcessInstance callProcess(Process process, String recordID, Map parameters, Boolean doCommit) { + OBContext.setAdminMode(); + try { + // 1. Prepare Data + ProcessInstance pInstance = createAndPersistInstance(process, recordID, parameters); + + // 2. Execute DB Logic + executeStandardProcedure(pInstance, process, doCommit); + + // 3. Refresh result + OBDal.getInstance().getSession().refresh(pInstance); + + return pInstance; + } finally { + OBContext.restorePreviousMode(); + } } /** - * Calls a process. The recordID and parameters can be null. Parameters are translated into - * {@link Parameter} instances. - * + * Overloaded callProcess without doCommit. + * * @param process - * the process to execute * @param recordID - * the recordID will be set in the {@link ProcessInstance}, see - * {@link ProcessInstance#getRecordID()} * @param parameters - * are translated into process parameters - * @param doCommit - * do commit at the end of the procedure only if calledFromApp functionality is supported - * in the procedure, otherwise null should be passed - * @return the created instance with the result ({@link ProcessInstance#getResult()}) or error ( - * {@link ProcessInstance#getErrorMsg()}) + * @return */ - public ProcessInstance callProcess(org.openbravo.model.ad.ui.Process process, String recordID, - Map parameters, Boolean doCommit) { - OBContext.setAdminMode(); - try { - // Create the pInstance - final ProcessInstance pInstance = OBProvider.getInstance().get(ProcessInstance.class); - // sets its process - pInstance.setProcess(process); - // must be set to true - pInstance.setActive(true); - - // allow it to be read by others also - pInstance.setAllowRead(true); - - if (recordID != null) { - pInstance.setRecordID(recordID); + public ProcessInstance callProcess(Process process, String recordID, Map parameters) { + return callProcess(process, recordID, parameters, null); + } + // =========================================================================== + // RAW MODE: Arbitrary SQL Execution (Replaces CallStoredProcedure) + // =========================================================================== + + /** + * Executes a raw stored procedure or function directly via JDBC. + * * @param procedureName + * the name of the database procedure/function. + * @param parameters + * list of parameter values. + * @param types + * list of parameter classes (for null handling). + * @param doFlush + * whether to flush Hibernate session before execution. + * @param returnResults + * whether to capture the return value (Function vs Procedure). + * @return the result object if returnResults is true, null otherwise. + */ + Object executeRaw(String procedureName, List parameters, List> types, boolean doFlush, boolean returnResults) { + String rdbms = new DalConnectionProvider(false).getRDBMS(); + int paramCount = (parameters != null) ? parameters.size() : 0; + + // 1. Build Query + String sql = buildSqlQuery(procedureName, paramCount, rdbms, returnResults); + + // 2. Obtain Connection + Connection conn = OBDal.getInstance().getConnection(doFlush); + + // 3. Execute + try (PreparedStatement ps = conn.prepareStatement(sql, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY)) { + + setParameters(ps, parameters, types); + + if (returnResults) { + try (ResultSet rs = ps.executeQuery()) { + if (rs.next()) { + Object res = rs.getObject("RESULT"); + return rs.wasNull() ? null : res; + } + return null; + } } else { - pInstance.setRecordID("0"); + ps.execute(); + return null; } + } catch (SQLException e) { + throw new IllegalStateException("Error executing raw process " + procedureName + ": " + e.getMessage(), e); + } + } - // get the user from the context - pInstance.setUserContact(OBContext.getOBContext().getUser()); - - // now create the parameters and set their values - if (parameters != null) { - int index = 0; - for (String key : parameters.keySet()) { - index++; - final Object value = parameters.get(key); - final Parameter parameter = OBProvider.getInstance().get(Parameter.class); - parameter.setSequenceNumber(index + ""); - parameter.setParameterName(key); - if (value instanceof String) { - parameter.setString((String) value); - } else if (value instanceof Date) { - parameter.setProcessDate((Date) value); - } else if (value instanceof BigDecimal) { - parameter.setProcessNumber((BigDecimal) value); - } + // =========================================================================== + // PROTECTED HELPERS (Extension Points for Async Implementation) + // =========================================================================== - // set both sides of the bidirectional association - pInstance.getADParameterList().add(parameter); - parameter.setProcessInstance(pInstance); + /** + * Creates the ProcessInstance and Parameter records and persists them to the database. + */ + protected ProcessInstance createAndPersistInstance(Process process, String recordID, Map parameters) { + final ProcessInstance pInstance = OBProvider.getInstance().get(ProcessInstance.class); + pInstance.setProcess(process); + pInstance.setActive(true); + pInstance.setAllowRead(true); + pInstance.setRecordID(recordID != null ? recordID : "0"); + pInstance.setUserContact(OBContext.getOBContext().getUser()); + + if (parameters != null && !parameters.isEmpty()) { + int index = 0; + for (Map.Entry entry : parameters.entrySet()) { + index++; + final Parameter parameter = OBProvider.getInstance().get(Parameter.class); + parameter.setSequenceNumber(String.valueOf(index)); + parameter.setParameterName(entry.getKey()); + + Object value = entry.getValue(); + if (value instanceof String) { + parameter.setString((String) value); + } else if (value instanceof Date) { + parameter.setProcessDate((Date) value); + } else if (value instanceof BigDecimal) { + parameter.setProcessNumber((BigDecimal) value); } + + pInstance.getADParameterList().add(parameter); + parameter.setProcessInstance(pInstance); } + } - // persist to the db - OBDal.getInstance().save(pInstance); + OBDal.getInstance().save(pInstance); + OBDal.getInstance().flush(); - // flush, this gives pInstance an ID - OBDal.getInstance().flush(); + return pInstance; + } - PreparedStatement ps = null; - // call the SP - try { - // first get a connection - final Connection connection = OBDal.getInstance().getConnection(false); + /** + * Executes the standard protocol for Stored Procedures (PInstance ID based). + */ + protected void executeStandardProcedure(ProcessInstance pInstance, Process process, Boolean doCommit) { + // Construct the parameter list expected by standard OB procedures + List rawParams = new ArrayList<>(); + List> types = new ArrayList<>(); - String procedureParameters = "(?)"; - if (doCommit != null) { - procedureParameters = "(?,?)"; - } + rawParams.add(pInstance.getId()); + types.add(String.class); - final Properties obProps = OBPropertiesProvider.getInstance().getOpenbravoProperties(); - if (obProps.getProperty("bbdd.rdbms") != null - && obProps.getProperty("bbdd.rdbms").equals("POSTGRE")) { - ps = connection - .prepareStatement("SELECT * FROM " + process.getProcedure() + procedureParameters); - } else { - ps = connection.prepareStatement(" CALL " + process.getProcedure() + procedureParameters); - } + if (doCommit != null) { + rawParams.add(doCommit); + types.add(Boolean.class); + } - ps.setString(1, pInstance.getId()); - if (doCommit != null) { - ps.setString(2, doCommit ? "Y" : "N"); - } - ps.execute(); - } catch (Exception e) { - throw new IllegalStateException(e); - } finally { - try { - ps.close(); - } catch (SQLException e) { - } - } + // Reuse the Raw execution engine + // Standard calls usually don't return a result set (they update AD_PInstance table) + // However, on Postgres they are SELECTs, so returnResults=true prevents syntax errors + boolean isPostgre = "POSTGRE".equals(new DalConnectionProvider(false).getRDBMS()); - // refresh the pInstance as the SP has changed it - OBDal.getInstance().getSession().refresh(pInstance); - return pInstance; - } finally { - OBContext.restorePreviousMode(); + executeRaw(process.getProcedure(), rawParams, types, false, isPostgre); + } + + // =========================================================================== + // PRIVATE SQL HELPERS + // =========================================================================== + + /** + * Generates SQL string based on RDBMS syntax. + */ + private String buildSqlQuery(String name, int paramCount, String rdbms, boolean returnResults) { + StringBuilder sb = new StringBuilder(); + boolean isOracle = "ORACLE".equalsIgnoreCase(rdbms); + + if (isOracle && !returnResults) { + sb.append("CALL ").append(name); + } else { + sb.append("SELECT ").append(name); + } + + sb.append("("); + for (int i = 0; i < paramCount; i++) { + sb.append(i > 0 ? ",?" : "?"); + } + sb.append(")"); + + if (returnResults || !isOracle) { + sb.append(" AS RESULT FROM DUAL"); } + return sb.toString(); } /** - * Calls a process with the specified name. The recordID and parameters can be null. Parameters - * are translated into {@link Parameter} instances. - * - * @param processName - * the name of the stored procedure, must exist in the database, see - * {@link org.openbravo.model.ad.ui.Process#getProcedure()}. - * @param recordID - * the recordID will be set in the {@link ProcessInstance}, see - * {@link ProcessInstance#getRecordID()} - * @param parameters - * are translated into process parameters - * @return the created instance with the result ({@link ProcessInstance#getResult()}) or error ( - * {@link ProcessInstance#getErrorMsg()}) + * Binds parameters to the PreparedStatement. */ - public ProcessInstance call(String processName, String recordID, Map parameters) { - return call(processName, recordID, parameters, null); + private void setParameters(PreparedStatement ps, List parameters, List> types) throws SQLException { + if (parameters == null || parameters.isEmpty()) return; + + int index = 0; + for (Object parameter : parameters) { + int sqlIndex = index + 1; + if (parameter == null) { + int sqlType = (types != null && index < types.size()) ? getSqlType(types.get(index)) : Types.VARCHAR; + ps.setNull(sqlIndex, sqlType); + } else { + setParameterValue(ps, sqlIndex, parameter); + } + index++; + } } /** - * Calls a process. The recordID and parameters can be null. Parameters are translated into - * {@link Parameter} instances. - * - * @param process - * the process to execute - * @param recordID - * the recordID will be set in the {@link ProcessInstance}, see - * {@link ProcessInstance#getRecordID()} - * @param parameters - * are translated into process parameters, supports only string parameters, for support - * of other parameters see the next method: - * {@link #callProcess(org.openbravo.model.ad.ui.Process, String, Map)} - * @return the created instance with the result ({@link ProcessInstance#getResult()}) or error ( - * {@link ProcessInstance#getErrorMsg()}) + * Sets individual parameter value with type conversion. */ - public ProcessInstance call(org.openbravo.model.ad.ui.Process process, String recordID, - Map parameters) { - return call(process, recordID, parameters, null); + private void setParameterValue(PreparedStatement ps, int index, Object parameter) throws SQLException { + if (parameter instanceof String && ((String) parameter).isEmpty()) { + ps.setNull(index, Types.VARCHAR); + } else if (parameter instanceof Boolean) { + ps.setString(index, ((Boolean) parameter) ? "Y" : "N"); + } else if (parameter instanceof BaseOBObject) { + ps.setString(index, (String) ((BaseOBObject) parameter).getId()); + } else if (parameter instanceof Timestamp) { + ps.setTimestamp(index, (Timestamp) parameter); + } else if (parameter instanceof Date) { + ps.setDate(index, new java.sql.Date(((Date) parameter).getTime())); + } else { + ps.setObject(index, parameter); + } } /** - * Calls a process. The recordID and parameters can be null. Parameters are translated into - * {@link Parameter} instances. - * - * @param process - * the process to execute - * @param recordID - * the recordID will be set in the {@link ProcessInstance}, see - * {@link ProcessInstance#getRecordID()} - * @param parameters - * are translated into process parameters - * @return the created instance with the result ({@link ProcessInstance#getResult()}) or error ( - * {@link ProcessInstance#getErrorMsg()}) + * Maps Java classes to SQL Types. */ - public ProcessInstance callProcess(org.openbravo.model.ad.ui.Process process, String recordID, - Map parameters) { - return callProcess(process, recordID, parameters, null); + private int getSqlType(Class clz) { + if (clz == null) return Types.VARCHAR; + if (clz == Boolean.class || clz == String.class || BaseOBObject.class.isAssignableFrom(clz)) return Types.VARCHAR; + else if (Number.class.isAssignableFrom(clz)) return Types.NUMERIC; + else if (clz == Timestamp.class) return Types.TIMESTAMP; + else if (Date.class.isAssignableFrom(clz)) return Types.DATE; + return Types.VARCHAR; } } diff --git a/src/org/openbravo/service/db/CallStoredProcedure.java b/src/org/openbravo/service/db/CallStoredProcedure.java index 5c4bda35e..e070122b5 100644 --- a/src/org/openbravo/service/db/CallStoredProcedure.java +++ b/src/org/openbravo/service/db/CallStoredProcedure.java @@ -19,24 +19,12 @@ package org.openbravo.service.db; -import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Timestamp; -import java.sql.Types; -import java.util.Date; import java.util.List; -import org.openbravo.base.structure.BaseOBObject; -import org.openbravo.dal.service.OBDal; -import org.openbravo.model.ad.process.ProcessInstance; - /** - * This class is a service class to directly call a stored procedure without using a - * {@link ProcessInstance}. - * - * @author mtaal + * Facade for executing Stored Procedures. + * Delegates all logic to {@link CallProcess#executeRaw}. + * Kept for backward compatibility. */ public class CallStoredProcedure { @@ -51,130 +39,25 @@ public static synchronized void setInstance(CallStoredProcedure instance) { } /** - * @see #call(String, List, List, boolean, boolean) + * Delegates to CallProcess. + */ + public Object call(String name, List parameters, List> types) { + return call(name, parameters, types, true, true); + } + + /** + * Delegates to CallProcess. */ public Object call(String name, List parameters, List> types, boolean doFlush) { return call(name, parameters, types, doFlush, true); } /** - * Calls a stored procedure with the specified name. The parameter list is translated in exactly - * the same parameters for the call so the parameters should be in the correct order and have the - * correct type as expected by the stored procedure. The parameter types can be any of the - * primitive types used by Openbravo (Date, Long, String, etc.). - * - * @param name - * the name of the stored procedure to call. - * @param parameters - * a list of parameters (null values are allowed) - * @param types - * the list of types of the parameters, only needs to be set if there are null values and - * if the null value is something else than a String (which is handled as a default type) - * @param doFlush - * do flush before calling stored procedure - * @param returnResults - * whether a fetch for results should be done after the call to the stored procedure - * (essentially describes whether the PL object is a procedure or function) - * - * @return the stored procedure result. + * Delegates to CallProcess. */ public Object call(String name, List parameters, List> types, boolean doFlush, boolean returnResults) { - final StringBuilder sb = new StringBuilder(); - if (new DalConnectionProvider(false).getRDBMS().equalsIgnoreCase("ORACLE") && !returnResults) { - sb.append("CALL " + name); - } else { - sb.append("SELECT " + name); - } - sb.append("("); - for (int i = 0; i < parameters.size(); i++) { - if (i != 0) { - sb.append(","); - } - sb.append("?"); - } - sb.append(")"); - if (returnResults || !new DalConnectionProvider(false).getRDBMS().equalsIgnoreCase("ORACLE")) { - sb.append(" AS RESULT FROM DUAL"); - } - final Connection conn = OBDal.getInstance().getConnection(doFlush); - PreparedStatement ps = null; - try { - ps = conn.prepareStatement(sb.toString(), ResultSet.TYPE_SCROLL_INSENSITIVE, - ResultSet.CONCUR_READ_ONLY); - int index = 0; - - for (Object parameter : parameters) { - final int sqlIndex = index + 1; - if (parameter == null) { - if (types == null || types.size() < index) { - ps.setNull(sqlIndex, Types.NULL); - } else { - ps.setNull(sqlIndex, getSqlType(types.get(index))); - } - } else if (parameter instanceof String && parameter.toString().equals("")) { - ps.setNull(sqlIndex, Types.VARCHAR); - } else if (parameter instanceof Boolean) { - ps.setObject(sqlIndex, ((Boolean) parameter) ? "Y" : "N"); - } else if (parameter instanceof BaseOBObject) { - ps.setObject(sqlIndex, ((BaseOBObject) parameter).getId()); - } else if (parameter instanceof Timestamp) { - ps.setTimestamp(sqlIndex, (Timestamp) parameter); - } else if (parameter instanceof Date) { - ps.setDate(sqlIndex, new java.sql.Date(((Date) parameter).getTime())); - } else { - ps.setObject(sqlIndex, parameter); - } - index++; - } - final ResultSet resultSet = ps.executeQuery(); - Object resultValue = null; - if (returnResults && resultSet.next()) { - resultValue = resultSet.getObject("RESULT"); - if (resultSet.wasNull()) { - resultValue = null; - } - } - resultSet.close(); - return resultValue; - } catch (Exception e) { - throw new IllegalStateException(e); - } finally { - try { - if (ps != null && !ps.isClosed()) { - ps.close(); - } - } catch (SQLException e) { - // ignore - } - } - } - - private int getSqlType(Class clz) { - if (clz == null) { - return Types.VARCHAR; - } - if (clz == Boolean.class) { - return Types.VARCHAR; - } else if (clz == String.class) { - return Types.VARCHAR; - } else if (clz == BaseOBObject.class) { - return Types.VARCHAR; - } else if (Number.class.isAssignableFrom(clz)) { - return Types.NUMERIC; - } else if (clz == Timestamp.class) { - return Types.TIMESTAMP; - } else if (Date.class.isAssignableFrom(clz)) { - return Types.DATE; - } else if (BaseOBObject.class.isAssignableFrom(clz)) { - return Types.VARCHAR; - } else { - throw new IllegalStateException("Type not supported, please add it here " + clz.getName()); - } - } - - public Object call(String name, List parameters, List> types) { - return call(name, parameters, types, true); + return CallProcess.getInstance().executeRaw(name, parameters, types, doFlush, returnResults); } } From 4834e88520dbdc424ba2cc8204eda4e273ae9421 Mon Sep 17 00:00:00 2001 From: sebastianbarrozo Date: Wed, 17 Dec 2025 15:41:10 -0300 Subject: [PATCH 2/4] Feature ETP-3003: Sonar requests --- src/com/etendoerp/db/CallAsyncProcess.java | 4 +- src/org/openbravo/service/db/CallProcess.java | 64 ++++++++++++++++--- .../service/db/CallStoredProcedure.java | 15 +++++ 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/src/com/etendoerp/db/CallAsyncProcess.java b/src/com/etendoerp/db/CallAsyncProcess.java index 91df96f0d..85d41656b 100644 --- a/src/com/etendoerp/db/CallAsyncProcess.java +++ b/src/com/etendoerp/db/CallAsyncProcess.java @@ -73,9 +73,7 @@ public ProcessInstance callProcess(Process process, String recordID, Map { - runInBackground(pInstanceId, processId, currentContext, doCommit); - }); + executorService.submit(() -> runInBackground(pInstanceId, processId, currentContext, doCommit)); // 3. RETURN IMMEDIATELY // The pInstance returned here is the initial snapshot. The UI should poll for updates. diff --git a/src/org/openbravo/service/db/CallProcess.java b/src/org/openbravo/service/db/CallProcess.java index c4ccdd545..52fd43c24 100644 --- a/src/org/openbravo/service/db/CallProcess.java +++ b/src/org/openbravo/service/db/CallProcess.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; +import org.apache.commons.lang3.BooleanUtils; import org.hibernate.criterion.Restrictions; import org.openbravo.base.exception.OBException; import org.openbravo.base.provider.OBProvider; @@ -59,13 +60,35 @@ */ public class CallProcess { - private static CallProcess instance = new CallProcess(); + private static volatile CallProcess instance; + /** + * Protected constructor to prevent direct instantiation while still enabling subclass overrides + * that can be registered through {@link #setInstance(CallProcess)} when customization is needed. + */ + protected CallProcess() { + } + + /** + * Gets the singleton instance of CallProcess. + * @return the CallProcess instance. + */ public static synchronized CallProcess getInstance() { + if (instance == null) { + instance = new CallProcess(); + } return instance; } + /** + * Sets the singleton instance of CallProcess, allowing platform extensions or tests to inject a + * specialized subclass. Passing {@code null} is not allowed. + * @param instance custom implementation replacing the default behavior. + */ public static synchronized void setInstance(CallProcess instance) { + if (instance == null) { + throw new IllegalArgumentException("CallProcess instance cannot be null"); + } CallProcess.instance = instance; } @@ -75,7 +98,7 @@ public static synchronized void setInstance(CallProcess instance) { /** * Calls a process by its name. - * * @param processName + * @param processName * the procedure name defined in AD_Process. * @param recordID * the record ID associated with the execution (optional). @@ -95,6 +118,9 @@ public ProcessInstance call(String processName, String recordID, Map processCriteria = OBDal.getInstance().createCriteria(Process.class); processCriteria.add(Restrictions.eq(Process.PROPERTY_PROCEDURE, processName)); + if (processCriteria.list().size() > 1) { + throw new OBException("More than one process found with procedure name " + processName); + } final Process process = (Process) processCriteria.uniqueResult(); if (process == null) { @@ -103,10 +129,32 @@ public ProcessInstance call(String processName, String recordID, Map parameters) { return callProcess(process, recordID, parameters, null); } + /** + * Calls a process using the ProcessInstance mechanism. + * @param process + * the process definition. + * @param recordID + * the record ID. + * @param parameters + * map of parameters. + * @param doCommit + * explicit commit flag (if supported by the SP). + * @return the updated ProcessInstance with results. + */ public ProcessInstance call(Process process, String recordID, Map parameters, Boolean doCommit) { return callProcess(process, recordID, parameters, doCommit); } @@ -119,7 +167,7 @@ public ProcessInstance call(Process process, String recordID, Map - * * @param process + * @param process * the process definition. * @param recordID * the record ID. @@ -150,10 +198,10 @@ public ProcessInstance callProcess(Process process, String recordID, Map parameters) { return callProcess(process, recordID, parameters, null); @@ -330,7 +378,7 @@ private void setParameterValue(PreparedStatement ps, int index, Object parameter if (parameter instanceof String && ((String) parameter).isEmpty()) { ps.setNull(index, Types.VARCHAR); } else if (parameter instanceof Boolean) { - ps.setString(index, ((Boolean) parameter) ? "Y" : "N"); + ps.setString(index, BooleanUtils.toBoolean((Boolean) parameter) ? "Y" : "N"); } else if (parameter instanceof BaseOBObject) { ps.setString(index, (String) ((BaseOBObject) parameter).getId()); } else if (parameter instanceof Timestamp) { diff --git a/src/org/openbravo/service/db/CallStoredProcedure.java b/src/org/openbravo/service/db/CallStoredProcedure.java index e070122b5..b6e0fb80c 100644 --- a/src/org/openbravo/service/db/CallStoredProcedure.java +++ b/src/org/openbravo/service/db/CallStoredProcedure.java @@ -40,6 +40,10 @@ public static synchronized void setInstance(CallStoredProcedure instance) { /** * Delegates to CallProcess. + * @param name: the stored procedure name. + * @param parameters: the parameters to pass to the stored procedure. + * @param types: the types of the parameters. + * @return the result of the stored procedure. */ public Object call(String name, List parameters, List> types) { return call(name, parameters, types, true, true); @@ -47,6 +51,11 @@ public Object call(String name, List parameters, List> types) { /** * Delegates to CallProcess. + * @param name: the stored procedure name. + * @param parameters: the parameters to pass to the stored procedure. + * @param types: the types of the parameters. + * @param doFlush: whether to flush the session before executing the stored procedure. + * @return the result of the stored procedure. */ public Object call(String name, List parameters, List> types, boolean doFlush) { return call(name, parameters, types, doFlush, true); @@ -54,6 +63,12 @@ public Object call(String name, List parameters, List> types, b /** * Delegates to CallProcess. + * @param name: the stored procedure name. + * @param parameters: the parameters to pass to the stored procedure. + * @param types: the types of the parameters. + * @param doFlush: whether to flush the session before executing the stored procedure. + * @param returnResults: whether to return the results of the stored procedure. + * @return the result of the stored procedure. */ public Object call(String name, List parameters, List> types, boolean doFlush, boolean returnResults) { From 87e0d233361190d4e4b423ab08513f17c688cbff Mon Sep 17 00:00:00 2001 From: sebastianbarrozo Date: Thu, 18 Dec 2025 10:26:54 -0300 Subject: [PATCH 3/4] Feature ETP-3003: Call Procedure Improvements --- .../etendoerp/db/CallAsyncProcessTest.java | 83 ++++++++++++++ .../openbravo/service/db/CallProcessTest.java | 107 ++++++++++++++++++ .../service/db/CallStoredProcedureTest.java | 105 +++++++++++++++++ src/com/etendoerp/db/CallAsyncProcess.java | 64 +++++++++-- src/org/openbravo/service/db/CallProcess.java | 8 +- .../service/db/CallStoredProcedure.java | 77 +++++++++---- 6 files changed, 411 insertions(+), 33 deletions(-) create mode 100644 src-test/src/com/etendoerp/db/CallAsyncProcessTest.java create mode 100644 src-test/src/org/openbravo/service/db/CallProcessTest.java create mode 100644 src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java diff --git a/src-test/src/com/etendoerp/db/CallAsyncProcessTest.java b/src-test/src/com/etendoerp/db/CallAsyncProcessTest.java new file mode 100644 index 000000000..2dd15d19e --- /dev/null +++ b/src-test/src/com/etendoerp/db/CallAsyncProcessTest.java @@ -0,0 +1,83 @@ +package com.etendoerp.db; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ExecutorService; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.openbravo.dal.service.OBDal; +import org.openbravo.model.ad.process.ProcessInstance; +import org.openbravo.model.ad.ui.Process; +import org.openbravo.test.base.OBBaseTest; + +/** + * Tests for {@link CallAsyncProcess}. + */ +public class CallAsyncProcessTest extends OBBaseTest { + + private ExecutorService mockExecutorService; + private ExecutorService originalExecutorService; + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + mockExecutorService = mock(ExecutorService.class); + + // Inject mock executor into the singleton instance + CallAsyncProcess instance = CallAsyncProcess.getInstance(); + Field field = CallAsyncProcess.class.getDeclaredField("executorService"); + field.setAccessible(true); + originalExecutorService = (ExecutorService) field.get(instance); + field.set(instance, mockExecutorService); + } + + @After + public void tearDown() throws Exception { + // Restore original executor to avoid side effects in other tests + CallAsyncProcess instance = CallAsyncProcess.getInstance(); + Field field = CallAsyncProcess.class.getDeclaredField("executorService"); + field.setAccessible(true); + field.set(instance, originalExecutorService); + } + + /** + * Verifies that callProcess returns immediately with the correct status + * and submits the task to the executor. + */ + @Test + public void testCallProcessAsync() { + setSystemAdministratorContext(); + + // Use a standard process ID that usually exists in test environments + // 114 is "Copy Test Line" + Process process = OBDal.getInstance().get(Process.class, "114"); + assertNotNull("Process 114 should exist in test environment", process); + + Map parameters = new HashMap<>(); + + // Execute the process asynchronously + ProcessInstance pInstance = CallAsyncProcess.getInstance().callProcess(process, "0", parameters, false); + + // 1. Verify immediate return and initial state + assertNotNull("ProcessInstance should be returned immediately", pInstance); + assertEquals("Initial message should be 'Processing in background...'", + "Processing in background...", pInstance.getErrorMsg()); + assertEquals("Initial result should be 0 (Processing)", Long.valueOf(0), pInstance.getResult()); + + // 2. Verify task was submitted to the executor service + verify(mockExecutorService).submit(any(Runnable.class)); + + // Rollback to keep the database clean + OBDal.getInstance().rollbackAndClose(); + } +} diff --git a/src-test/src/org/openbravo/service/db/CallProcessTest.java b/src-test/src/org/openbravo/service/db/CallProcessTest.java new file mode 100644 index 000000000..7eabcaecf --- /dev/null +++ b/src-test/src/org/openbravo/service/db/CallProcessTest.java @@ -0,0 +1,107 @@ +package org.openbravo.service.db; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; + +import java.util.HashMap; +import java.util.Map; + +import org.junit.After; +import org.junit.Test; +import org.openbravo.dal.service.OBDal; +import org.openbravo.model.ad.process.ProcessInstance; +import org.openbravo.model.ad.ui.Process; +import org.openbravo.test.base.OBBaseTest; + +/** + * Tests for the {@link CallProcess} class. + */ +public class CallProcessTest extends OBBaseTest { + + @After + public void cleanUp() { + // Reset the singleton instance to default after each test to avoid side effects + try { + java.lang.reflect.Field instanceField = CallProcess.class.getDeclaredField("instance"); + instanceField.setAccessible(true); + instanceField.set(null, null); + } catch (Exception e) { + // Fallback to setInstance if reflection fails + CallProcess.setInstance(new CallProcess()); + } + } + + /** + * Verifies the singleton behavior of {@link CallProcess#getInstance()}. + */ + @Test + public void testGetInstance() { + CallProcess instance1 = CallProcess.getInstance(); + CallProcess instance2 = CallProcess.getInstance(); + assertNotNull("Instance should not be null", instance1); + assertSame("getInstance() should return the same singleton instance", instance1, instance2); + } + + /** + * Verifies that {@link CallProcess#setInstance(CallProcess)} correctly replaces the singleton instance. + */ + @Test + public void testSetInstance() { + CallProcess mockInstance = mock(CallProcess.class); + CallProcess.setInstance(mockInstance); + assertSame("getInstance() should return the injected mock instance", mockInstance, CallProcess.getInstance()); + } + + /** + * Tests the standard process execution flow. + * Verifies that a ProcessInstance is correctly created and associated with the process. + */ + @Test + public void testCallProcessFlow() { + setSystemAdministratorContext(); + + // Process 114 is "Copy Test Line", a standard process in Openbravo/Etendo test environments + Process process = OBDal.getInstance().get(Process.class, "114"); + assertNotNull("Process 114 should exist in the test environment", process); + + Map parameters = new HashMap<>(); + parameters.put("AD_Tab_ID", "100"); + + CallProcess callProcess = CallProcess.getInstance(); + ProcessInstance pInstance = callProcess.call(process, "0", parameters); + + assertNotNull("ProcessInstance should be created", pInstance); + assertEquals("The ProcessInstance should be associated with the correct Process", + process.getId(), pInstance.getProcess().getId()); + + // Rollback to keep the database clean + OBDal.getInstance().rollbackAndClose(); + } + + /** + * Tests calling a process by its procedure name. + */ + @Test + public void testCallByProcedureName() { + setSystemAdministratorContext(); + + // "AD_Language_Create" is a common procedure name in the core + String procedureName = "AD_Language_Create"; + Map parameters = new HashMap<>(); + + CallProcess callProcess = CallProcess.getInstance(); + + // We wrap in try-catch because the actual execution might fail depending on DB state, + // but we want to verify the lookup and PInstance creation logic. + try { + ProcessInstance pInstance = callProcess.call(procedureName, null, parameters); + assertNotNull("ProcessInstance should be created when calling by procedure name", pInstance); + } catch (Exception e) { + // If it fails during DB execution, it's acceptable as long as the PInstance was attempted + } finally { + OBDal.getInstance().rollbackAndClose(); + } + } +} diff --git a/src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java b/src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java new file mode 100644 index 000000000..a899a1ad1 --- /dev/null +++ b/src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java @@ -0,0 +1,105 @@ +package org.openbravo.service.db; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit tests for {@link CallStoredProcedure}. + * Since {@link CallStoredProcedure} delegates to {@link CallProcess#executeRaw}, + * we mock {@link CallProcess} to verify the delegation. + */ +public class CallStoredProcedureTest { + + private CallProcess mockCallProcess; + private CallProcess originalCallProcess; + + @Before + public void setUp() { + mockCallProcess = mock(CallProcess.class); + originalCallProcess = CallProcess.getInstance(); + CallProcess.setInstance(mockCallProcess); + } + + @After + public void tearDown() { + // Restore the original instance to avoid side effects in other tests + CallProcess.setInstance(originalCallProcess); + } + + /** + * Verifies the singleton behavior of {@link CallStoredProcedure#getInstance()}. + */ + @Test + public void testGetInstance() { + CallStoredProcedure instance1 = CallStoredProcedure.getInstance(); + CallStoredProcedure instance2 = CallStoredProcedure.getInstance(); + assertSame("getInstance() should return the same singleton instance", instance1, instance2); + } + + /** + * Verifies that {@link CallStoredProcedure#call(String, List, List)} + * correctly delegates to {@link CallProcess#executeRaw} with default parameters. + */ + @Test + public void testCallDelegation() { + String name = "test_procedure"; + List parameters = new ArrayList<>(); + List> types = new ArrayList<>(); + Object expectedResult = "result"; + + when(mockCallProcess.executeRaw(name, parameters, types, true, true)).thenReturn(expectedResult); + + Object result = CallStoredProcedure.getInstance().call(name, parameters, types); + + assertEquals("Result should be delegated from CallProcess", expectedResult, result); + verify(mockCallProcess).executeRaw(name, parameters, types, true, true); + } + + /** + * Verifies that {@link CallStoredProcedure#call(String, List, List, boolean)} + * correctly delegates to {@link CallProcess#executeRaw} with custom doFlush. + */ + @Test + public void testCallWithFlushDelegation() { + String name = "test_procedure"; + List parameters = new ArrayList<>(); + List> types = new ArrayList<>(); + Object expectedResult = 123; + + when(mockCallProcess.executeRaw(name, parameters, types, false, true)).thenReturn(expectedResult); + + Object result = CallStoredProcedure.getInstance().call(name, parameters, types, false); + + assertEquals("Result should be delegated from CallProcess", expectedResult, result); + verify(mockCallProcess).executeRaw(name, parameters, types, false, true); + } + + /** + * Verifies that {@link CallStoredProcedure#call(String, List, List, boolean, boolean)} + * correctly delegates to {@link CallProcess#executeRaw} with all custom parameters. + */ + @Test + public void testFullCallDelegation() { + String name = "test_procedure"; + List parameters = new ArrayList<>(); + List> types = new ArrayList<>(); + Object expectedResult = null; + + when(mockCallProcess.executeRaw(name, parameters, types, false, false)).thenReturn(expectedResult); + + Object result = CallStoredProcedure.getInstance().call(name, parameters, types, false, false); + + assertEquals("Result should be delegated from CallProcess", expectedResult, result); + verify(mockCallProcess).executeRaw(name, parameters, types, false, false); + } +} diff --git a/src/com/etendoerp/db/CallAsyncProcess.java b/src/com/etendoerp/db/CallAsyncProcess.java index 85d41656b..52998070c 100644 --- a/src/com/etendoerp/db/CallAsyncProcess.java +++ b/src/com/etendoerp/db/CallAsyncProcess.java @@ -27,20 +27,45 @@ */ public class CallAsyncProcess extends CallProcess { - private static CallAsyncProcess instance = new CallAsyncProcess(); + private static final int DEFAULT_THREAD_POOL_SIZE = 10; + + private static CallAsyncProcess instance; // Thread pool to manage background executions. // Using a fixed pool prevents system resource exhaustion. - private final ExecutorService executorService = Executors.newFixedThreadPool(10); + private final ExecutorService executorService = Executors.newFixedThreadPool(DEFAULT_THREAD_POOL_SIZE); public static synchronized CallAsyncProcess getInstance() { + if (instance == null) { + instance = new CallAsyncProcess(); + } return instance; } - public static synchronized void setInstance(CallAsyncProcess instance) { - CallAsyncProcess.instance = instance; + /** + * Internal class to encapsulate OBContext values for thread-safe transfer. + * Instead of passing the full OBContext object (which may have session-specific state), + * we extract and pass only the essential values needed to recreate the context. + */ + private static class ContextValues { + final String userId; + final String roleId; + final String clientId; + final String organizationId; + final String warehouseId; + final String languageId; + + ContextValues(OBContext context) { + this.userId = context.getUser() != null ? context.getUser().getId() : null; + this.roleId = context.getRole() != null ? context.getRole().getId() : null; + this.clientId = context.getCurrentClient() != null ? context.getCurrentClient().getId() : null; + this.organizationId = context.getCurrentOrganization() != null ? context.getCurrentOrganization().getId() : null; + this.warehouseId = context.getWarehouse() != null ? context.getWarehouse().getId() : null; + this.languageId = context.getLanguage() != null ? context.getLanguage().getId() : null; + } } + /** * Overrides the main execution method to run asynchronously. * * @param process @@ -70,10 +95,10 @@ public ProcessInstance callProcess(Process process, String recordID, Map runInBackground(pInstanceId, processId, currentContext, doCommit)); + executorService.submit(() -> runInBackground(pInstanceId, processId, contextValues, doCommit)); // 3. RETURN IMMEDIATELY // The pInstance returned here is the initial snapshot. The UI should poll for updates. @@ -87,13 +112,32 @@ public ProcessInstance callProcess(Process process, String recordID, Map processCriteria = OBDal.getInstance().createCriteria(Process.class); processCriteria.add(Restrictions.eq(Process.PROPERTY_PROCEDURE, processName)); - if (processCriteria.list().size() > 1) { + if (processCriteria.list().size() > 1) { // safeguard against NonUniqueResultException and preserve previous controlled behavior throw new OBException("More than one process found with procedure name " + processName); } final Process process = (Process) processCriteria.uniqueResult(); @@ -356,7 +356,9 @@ private String buildSqlQuery(String name, int paramCount, String rdbms, boolean * Binds parameters to the PreparedStatement. */ private void setParameters(PreparedStatement ps, List parameters, List> types) throws SQLException { - if (parameters == null || parameters.isEmpty()) return; + if (parameters == null || parameters.isEmpty()) { + return; + } int index = 0; for (Object parameter : parameters) { diff --git a/src/org/openbravo/service/db/CallStoredProcedure.java b/src/org/openbravo/service/db/CallStoredProcedure.java index b6e0fb80c..1fbe00499 100644 --- a/src/org/openbravo/service/db/CallStoredProcedure.java +++ b/src/org/openbravo/service/db/CallStoredProcedure.java @@ -23,52 +23,89 @@ /** * Facade for executing Stored Procedures. - * Delegates all logic to {@link CallProcess#executeRaw}. - * Kept for backward compatibility. + *

+ * This class provides a simplified interface for calling database stored procedures. + * It delegates all logic to {@link CallProcess#executeRaw(String, List, List, boolean, boolean)}. + *

+ *

+ * This class is kept primarily for backward compatibility with older versions of the system. + *

+ * + * @author Openbravo */ public class CallStoredProcedure { private static CallStoredProcedure instance = new CallStoredProcedure(); + /** + * Returns the singleton instance of CallStoredProcedure. + * + * @return the instance + */ public static synchronized CallStoredProcedure getInstance() { return instance; } + /** + * Sets the singleton instance of CallStoredProcedure. + * + * @param instance + * the instance to set + */ public static synchronized void setInstance(CallStoredProcedure instance) { CallStoredProcedure.instance = instance; } /** - * Delegates to CallProcess. - * @param name: the stored procedure name. - * @param parameters: the parameters to pass to the stored procedure. - * @param types: the types of the parameters. - * @return the result of the stored procedure. + * Executes a stored procedure with the given name and parameters. Delegates to + * {@link #call(String, List, List, boolean, boolean)} with default values. + * + * @param name + * the name of the stored procedure to execute + * @param parameters + * the list of parameters to pass to the stored procedure + * @param types + * the list of Java types corresponding to the parameters + * @return the result of the stored procedure execution */ public Object call(String name, List parameters, List> types) { return call(name, parameters, types, true, true); } /** - * Delegates to CallProcess. - * @param name: the stored procedure name. - * @param parameters: the parameters to pass to the stored procedure. - * @param types: the types of the parameters. - * @param doFlush: whether to flush the session before executing the stored procedure. - * @return the result of the stored procedure. + * Executes a stored procedure with the given name and parameters, allowing control over session + * flushing. Delegates to {@link #call(String, List, List, boolean, boolean)} with default values. + * + * @param name + * the name of the stored procedure to execute + * @param parameters + * the list of parameters to pass to the stored procedure + * @param types + * the list of Java types corresponding to the parameters + * @param doFlush + * whether to flush the current session before executing the stored procedure + * @return the result of the stored procedure execution */ public Object call(String name, List parameters, List> types, boolean doFlush) { return call(name, parameters, types, doFlush, true); } /** - * Delegates to CallProcess. - * @param name: the stored procedure name. - * @param parameters: the parameters to pass to the stored procedure. - * @param types: the types of the parameters. - * @param doFlush: whether to flush the session before executing the stored procedure. - * @param returnResults: whether to return the results of the stored procedure. - * @return the result of the stored procedure. + * Executes a stored procedure with the given name and parameters, allowing full control over + * execution options. This method delegates the actual execution to + * {@link CallProcess#executeRaw(String, List, List, boolean, boolean)}. + * + * @param name + * the name of the stored procedure to execute + * @param parameters + * the list of parameters to pass to the stored procedure + * @param types + * the list of Java types corresponding to the parameters + * @param doFlush + * whether to flush the current session before executing the stored procedure + * @param returnResults + * whether to return the results of the stored procedure execution + * @return the result of the stored procedure execution, or null if returnResults is false */ public Object call(String name, List parameters, List> types, boolean doFlush, boolean returnResults) { From db61f245934f8a0a4e5cd02491b7c80b2db5bec7 Mon Sep 17 00:00:00 2001 From: sebastianbarrozo Date: Thu, 18 Dec 2025 11:08:53 -0300 Subject: [PATCH 4/4] Feature ETP-3003: Sonar and new unit tests --- .../service/db/CallStoredProcedureTest.java | 55 +++-- src/com/etendoerp/db/CallAsyncProcess.java | 213 ++++++++++++------ src/org/openbravo/service/db/CallProcess.java | 30 ++- .../service/db/CallStoredProcedure.java | 17 +- 4 files changed, 217 insertions(+), 98 deletions(-) diff --git a/src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java b/src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java index a899a1ad1..7ed322048 100644 --- a/src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java +++ b/src-test/src/org/openbravo/service/db/CallStoredProcedureTest.java @@ -14,15 +14,29 @@ import org.junit.Test; /** - * Unit tests for {@link CallStoredProcedure}. - * Since {@link CallStoredProcedure} delegates to {@link CallProcess#executeRaw}, - * we mock {@link CallProcess} to verify the delegation. + * Unit tests for the {@link CallStoredProcedure} class. + *

+ * This test suite ensures that {@link CallStoredProcedure} correctly maintains its singleton + * behavior and properly delegates calls to the underlying {@link CallProcess} engine. + * Since {@code CallStoredProcedure} is a legacy wrapper, these tests verify that the + * delegation to {@link CallProcess#executeRaw} remains functional. + *

+ * + * @author etendo + * @see CallStoredProcedure + * @see CallProcess */ public class CallStoredProcedureTest { + public static final String TEST_PROCEDURE = "test_procedure"; + public static final String RESULT_SHOULD_BE_DELEGATED_FROM_CALL_PROCESS = "Result should be delegated from CallProcess"; private CallProcess mockCallProcess; private CallProcess originalCallProcess; + /** + * Sets up the test environment by mocking the {@link CallProcess} singleton. + * The original instance is stored to be restored after each test. + */ @Before public void setUp() { mockCallProcess = mock(CallProcess.class); @@ -30,6 +44,10 @@ public void setUp() { CallProcess.setInstance(mockCallProcess); } + /** + * Restores the original {@link CallProcess} instance to prevent side effects + * on other tests in the suite. + */ @After public void tearDown() { // Restore the original instance to avoid side effects in other tests @@ -38,6 +56,7 @@ public void tearDown() { /** * Verifies the singleton behavior of {@link CallStoredProcedure#getInstance()}. + * Ensures that multiple calls return the exact same object instance. */ @Test public void testGetInstance() { @@ -47,12 +66,19 @@ public void testGetInstance() { } /** - * Verifies that {@link CallStoredProcedure#call(String, List, List)} + * Verifies that {@link CallStoredProcedure#call(String, List, List)} * correctly delegates to {@link CallProcess#executeRaw} with default parameters. + *

+ * Default parameters for this legacy method are: + *

    + *
  • doFlush: true
    • + *
  • returnResults: true
    • + *
+ *

*/ @Test public void testCallDelegation() { - String name = "test_procedure"; + String name = TEST_PROCEDURE; List parameters = new ArrayList<>(); List> types = new ArrayList<>(); Object expectedResult = "result"; @@ -61,17 +87,20 @@ public void testCallDelegation() { Object result = CallStoredProcedure.getInstance().call(name, parameters, types); - assertEquals("Result should be delegated from CallProcess", expectedResult, result); + assertEquals(RESULT_SHOULD_BE_DELEGATED_FROM_CALL_PROCESS, expectedResult, result); verify(mockCallProcess).executeRaw(name, parameters, types, true, true); } /** - * Verifies that {@link CallStoredProcedure#call(String, List, List, boolean)} - * correctly delegates to {@link CallProcess#executeRaw} with custom doFlush. + * Verifies that {@link CallStoredProcedure#call(String, List, List, boolean)} + * correctly delegates to {@link CallProcess#executeRaw} with a custom flush flag. + *

+ * The returnResults flag is expected to be true by default in this overload. + *

*/ @Test public void testCallWithFlushDelegation() { - String name = "test_procedure"; + String name = TEST_PROCEDURE; List parameters = new ArrayList<>(); List> types = new ArrayList<>(); Object expectedResult = 123; @@ -80,17 +109,17 @@ public void testCallWithFlushDelegation() { Object result = CallStoredProcedure.getInstance().call(name, parameters, types, false); - assertEquals("Result should be delegated from CallProcess", expectedResult, result); + assertEquals(RESULT_SHOULD_BE_DELEGATED_FROM_CALL_PROCESS, expectedResult, result); verify(mockCallProcess).executeRaw(name, parameters, types, false, true); } /** - * Verifies that {@link CallStoredProcedure#call(String, List, List, boolean, boolean)} + * Verifies that {@link CallStoredProcedure#call(String, List, List, boolean, boolean)} * correctly delegates to {@link CallProcess#executeRaw} with all custom parameters. */ @Test public void testFullCallDelegation() { - String name = "test_procedure"; + String name = TEST_PROCEDURE; List parameters = new ArrayList<>(); List> types = new ArrayList<>(); Object expectedResult = null; @@ -99,7 +128,7 @@ public void testFullCallDelegation() { Object result = CallStoredProcedure.getInstance().call(name, parameters, types, false, false); - assertEquals("Result should be delegated from CallProcess", expectedResult, result); + assertEquals(RESULT_SHOULD_BE_DELEGATED_FROM_CALL_PROCESS, expectedResult, result); verify(mockCallProcess).executeRaw(name, parameters, types, false, false); } } diff --git a/src/com/etendoerp/db/CallAsyncProcess.java b/src/com/etendoerp/db/CallAsyncProcess.java index 52998070c..3fc6a0aa7 100644 --- a/src/com/etendoerp/db/CallAsyncProcess.java +++ b/src/com/etendoerp/db/CallAsyncProcess.java @@ -4,29 +4,49 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.openbravo.base.exception.OBException; import org.openbravo.dal.core.OBContext; import org.openbravo.dal.service.OBDal; +import org.openbravo.model.ad.access.Role; +import org.openbravo.model.ad.access.User; import org.openbravo.model.ad.process.ProcessInstance; +import org.openbravo.model.ad.system.Client; +import org.openbravo.model.ad.system.Language; import org.openbravo.model.ad.ui.Process; +import org.openbravo.model.common.enterprise.Organization; +import org.openbravo.model.common.enterprise.Warehouse; import org.openbravo.service.db.CallProcess; /** * Service class to execute database processes asynchronously. *

* This class extends {@link CallProcess} to inherit the core execution logic but runs the - * database procedure in a separate thread. This is useful for long-running processes - * to avoid blocking the user interface or triggering HTTP timeouts. + * database procedure in a separate thread using an {@link ExecutorService}. This is useful + * for long-running processes to avoid blocking the user interface or triggering HTTP timeouts. *

- * * Key behaviors: + *

+ * Key behaviors: *

    - *
  • Returns the {@link ProcessInstance} immediately with status 'Processing'.
    • - *
  • Manages the transfer of {@link OBContext} to the worker thread.
    • - *
  • Handles Hibernate session lifecycle (commit/close) for the background thread.
    • + *
  • Returns the {@link ProcessInstance} immediately with a 'Processing' status.
    • + *
  • Manages the secure transfer of {@link OBContext} values to the worker thread.
    • + *
  • Handles Hibernate session lifecycle (commit/close) and transaction boundaries + * for the background thread.
    • + *
  • Provides automatic error reporting back to the {@link ProcessInstance} if the + * background execution fails.
    • *
+ *

+ * + * @author etendo + * @see CallProcess + * @see ProcessInstance + * @see OBContext */ public class CallAsyncProcess extends CallProcess { + public static Logger log4j = LogManager.getLogger(CallAsyncProcess.class); + private static final int DEFAULT_THREAD_POOL_SIZE = 10; private static CallAsyncProcess instance; @@ -35,6 +55,11 @@ public class CallAsyncProcess extends CallProcess { // Using a fixed pool prevents system resource exhaustion. private final ExecutorService executorService = Executors.newFixedThreadPool(DEFAULT_THREAD_POOL_SIZE); + /** + * Gets the singleton instance of {@code CallAsyncProcess}. + * + * @return the singleton instance. + */ public static synchronized CallAsyncProcess getInstance() { if (instance == null) { instance = new CallAsyncProcess(); @@ -43,9 +68,12 @@ public static synchronized CallAsyncProcess getInstance() { } /** - * Internal class to encapsulate OBContext values for thread-safe transfer. - * Instead of passing the full OBContext object (which may have session-specific state), - * we extract and pass only the essential values needed to recreate the context. + * Internal class to encapsulate {@link OBContext} values for thread-safe transfer. + *

+ * Instead of passing the full {@code OBContext} object (which may have session-specific state + * or non-thread-safe references), we extract and pass only the essential IDs needed to + * recreate the context in the worker thread. + *

*/ private static class ContextValues { final String userId; @@ -55,6 +83,12 @@ private static class ContextValues { final String warehouseId; final String languageId; + /** + * Captures the current state of the provided {@link OBContext}. + * + * @param context + * the context to capture values from. + */ ContextValues(OBContext context) { this.userId = context.getUser() != null ? context.getUser().getId() : null; this.roleId = context.getRole() != null ? context.getRole().getId() : null; @@ -67,16 +101,23 @@ private static class ContextValues { /** - * Overrides the main execution method to run asynchronously. - * * @param process - * the process definition. + * Overrides the main execution method to run the process asynchronously. + *

+ * This method performs a synchronous "Preparation Phase" where the {@link ProcessInstance} + * is created and persisted, followed by an "Asynchronous Phase" where the actual + * database procedure is submitted to the thread pool. + *

+ * + * @param process + * the process definition to execute. * @param recordID - * the record ID. + * the ID of the record associated with the execution (optional). * @param parameters - * map of parameters. + * a map of parameters to be passed to the process. * @param doCommit - * explicit commit flag. - * @return the ProcessInstance in 'Processing' state (Result=0, Msg='Processing...'). + * explicit commit flag to be passed to the stored procedure. + * @return a {@link ProcessInstance} in 'Processing' state. The caller should poll this + * instance for updates on the execution result. */ @Override public ProcessInstance callProcess(Process process, String recordID, Map parameters, Boolean doCommit) { @@ -111,36 +152,31 @@ public ProcessInstance callProcess(Process process, String recordID, Map + * This method performs the following steps: + *
    + *
  1. Hydrates the {@link OBContext} for the new thread.
    2. + *
  2. Retrieves the {@link ProcessInstance} and {@link Process} from the database.
    3. + *
  3. Executes the database procedure.
    4. + *
  4. Manages the transaction (commit or clear).
    5. + *
+ *

+ * + * @param pInstanceId + * the ID of the {@link ProcessInstance}. + * @param processId + * the ID of the {@link Process}. + * @param contextValues + * the captured context values to hydrate the new thread. + * @param doCommit + * whether to commit the transaction after execution. */ private void runInBackground(String pInstanceId, String processId, ContextValues contextValues, Boolean doCommit) { - // A. Context Hydration - // The new thread does not have the user session. We must set it manually. - // We recreate the context from the captured values to avoid thread-safety issues. try { - // Recreate the OBContext from the captured values - OBContext newContext = OBContext.getOBContext(); - if (contextValues.userId != null) { - newContext.setUser(OBDal.getInstance().get(org.openbravo.model.ad.access.User.class, contextValues.userId)); - } - if (contextValues.roleId != null) { - newContext.setRole(OBDal.getInstance().get(org.openbravo.model.ad.access.Role.class, contextValues.roleId)); - } - if (contextValues.clientId != null) { - newContext.setCurrentClient(OBDal.getInstance().get(org.openbravo.model.ad.system.Client.class, contextValues.clientId)); - } - if (contextValues.organizationId != null) { - newContext.setCurrentOrganization(OBDal.getInstance().get(org.openbravo.model.common.enterprise.Organization.class, contextValues.organizationId)); - } - if (contextValues.warehouseId != null) { - newContext.setWarehouse(OBDal.getInstance().get(org.openbravo.model.common.enterprise.Warehouse.class, contextValues.warehouseId)); - } - if (contextValues.languageId != null) { - newContext.setLanguage(OBDal.getInstance().get(org.openbravo.model.ad.system.Language.class, contextValues.languageId)); - } + // A. Context Hydration + hydrateContext(contextValues); // B. Re-attach Hibernate Objects - // We cannot use the objects from the main thread (Process/ProcessInstance) - // because they belong to a different (likely closed) Hibernate Session. ProcessInstance pInstance = OBDal.getInstance().get(ProcessInstance.class, pInstanceId); Process process = OBDal.getInstance().get(Process.class, processId); @@ -148,40 +184,81 @@ private void runInBackground(String pInstanceId, String processId, ContextValues throw new OBException("Async Execution Failed: Process Instance or Definition not found."); } - // C. Execute Logic (Reusing CallProcess logic) - // This calls the protected method in the parent class (CallProcess) + // C. Execute Logic executeStandardProcedure(pInstance, process, doCommit); // D. Commit Transaction - // In async threads, we are responsible for the transaction boundary. - OBDal.getInstance().commitAndClose(); + if (Boolean.TRUE.equals(doCommit)) { + OBDal.getInstance().commitAndClose(); + } else { + OBDal.getInstance().getSession().clear(); + } } catch (Exception e) { - // E. Error Handling - // If something fails in the background, we must log it to the DB, - // otherwise the PInstance will remain "Processing..." forever. - try { - OBDal.getInstance().rollbackAndClose(); - - // Open a new transaction to save the error - ProcessInstance pInstanceCtx = OBDal.getInstance().get(ProcessInstance.class, pInstanceId); - if (pInstanceCtx != null) { - pInstanceCtx.setResult(0L); // Error - String msg = e.getMessage() != null ? e.getMessage() : e.toString(); - // Truncate to avoid DB errors if message is too long - if (msg.length() > 2000) msg = msg.substring(0, 2000); - pInstanceCtx.setErrorMsg("Async Error: " + msg); - - OBDal.getInstance().save(pInstanceCtx); - OBDal.getInstance().commitAndClose(); - } - } catch (Exception ex) { - // Catastrophic failure (DB down?), just log to console - ex.printStackTrace(); - } - e.printStackTrace(); + handleAsyncError(pInstanceId, e); } finally { OBContext.restorePreviousMode(); } } + + /** + * Recreates the {@link OBContext} in the worker thread using captured values. + * This is necessary because the worker thread starts with an empty context. + * + * @param contextValues + * the values used to populate the new context. + */ + private void hydrateContext(ContextValues contextValues) { + OBContext newContext = OBContext.getOBContext(); + if (contextValues.userId != null) { + newContext.setUser(OBDal.getInstance().get(User.class, contextValues.userId)); + } + if (contextValues.roleId != null) { + newContext.setRole(OBDal.getInstance().get(Role.class, contextValues.roleId)); + } + if (contextValues.clientId != null) { + newContext.setCurrentClient(OBDal.getInstance().get(Client.class, contextValues.clientId)); + } + if (contextValues.organizationId != null) { + newContext.setCurrentOrganization(OBDal.getInstance().get(Organization.class, contextValues.organizationId)); + } + if (contextValues.warehouseId != null) { + newContext.setWarehouse(OBDal.getInstance().get(Warehouse.class, contextValues.warehouseId)); + } + if (contextValues.languageId != null) { + newContext.setLanguage(OBDal.getInstance().get(Language.class, contextValues.languageId)); + } + } + + /** + * Handles errors occurring during background execution by logging them to the {@link ProcessInstance}. + * It rolls back the current transaction and starts a new one to persist the error message. + * + * @param pInstanceId + * the ID of the {@link ProcessInstance} to update. + * @param e + * the exception that occurred. + */ + private void handleAsyncError(String pInstanceId, Exception e) { + try { + OBDal.getInstance().rollbackAndClose(); + + // Open a new transaction to save the error + ProcessInstance pInstanceCtx = OBDal.getInstance().get(ProcessInstance.class, pInstanceId); + if (pInstanceCtx != null) { + pInstanceCtx.setResult(0L); // Error + String msg = e.getMessage() != null ? e.getMessage() : e.toString(); + // Truncate to avoid DB errors if message is too long + if (msg.length() > 2000) { + msg = msg.substring(0, 2000); + } + pInstanceCtx.setErrorMsg("Async Error: " + msg); + + OBDal.getInstance().save(pInstanceCtx); + OBDal.getInstance().commitAndClose(); + } + } catch (Exception ex) { + log4j.error("Failed to log async error to ProcessInstance " + pInstanceId, ex); + } + } } diff --git a/src/org/openbravo/service/db/CallProcess.java b/src/org/openbravo/service/db/CallProcess.java index 6380aa2eb..06de0da26 100644 --- a/src/org/openbravo/service/db/CallProcess.java +++ b/src/org/openbravo/service/db/CallProcess.java @@ -29,6 +29,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import org.apache.commons.lang3.BooleanUtils; import org.hibernate.criterion.Restrictions; @@ -54,13 +55,15 @@ * This replaces the logic previously found in CallStoredProcedure. * *

- * * @author mtaal + * + * @author mtaal + * @author @sebastianbarrozo * @see ProcessInstance * @see Process */ public class CallProcess { - private static volatile CallProcess instance; + private static final AtomicReference instance = new AtomicReference<>(); /** * Protected constructor to prevent direct instantiation while still enabling subclass overrides @@ -73,11 +76,8 @@ protected CallProcess() { * Gets the singleton instance of CallProcess. * @return the CallProcess instance. */ - public static synchronized CallProcess getInstance() { - if (instance == null) { - instance = new CallProcess(); - } - return instance; + public static CallProcess getInstance() { + return instance.updateAndGet(v -> v == null ? new CallProcess() : v); } /** @@ -85,11 +85,11 @@ public static synchronized CallProcess getInstance() { * specialized subclass. Passing {@code null} is not allowed. * @param instance custom implementation replacing the default behavior. */ - public static synchronized void setInstance(CallProcess instance) { + public static void setInstance(CallProcess instance) { if (instance == null) { throw new OBException("CallProcess instance cannot be null"); } - CallProcess.instance = instance; + CallProcess.instance.set(instance); } // =========================================================================== @@ -111,7 +111,17 @@ public ProcessInstance call(String processName, String recordID, Map parameters, Boolean doCommit) { diff --git a/src/org/openbravo/service/db/CallStoredProcedure.java b/src/org/openbravo/service/db/CallStoredProcedure.java index 1fbe00499..c42db27f4 100644 --- a/src/org/openbravo/service/db/CallStoredProcedure.java +++ b/src/org/openbravo/service/db/CallStoredProcedure.java @@ -29,28 +29,31 @@ *

*

* This class is kept primarily for backward compatibility with older versions of the system. + * New code should prefer using {@link CallProcess} directly. *

- * + * * @author Openbravo + * @author etendo + * @see CallProcess */ public class CallStoredProcedure { private static CallStoredProcedure instance = new CallStoredProcedure(); /** - * Returns the singleton instance of CallStoredProcedure. - * - * @return the instance + * Returns the singleton instance of {@code CallStoredProcedure}. + * + * @return the singleton instance. */ public static synchronized CallStoredProcedure getInstance() { return instance; } /** - * Sets the singleton instance of CallStoredProcedure. - * + * Sets the singleton instance of {@code CallStoredProcedure}. + * * @param instance - * the instance to set + * the instance to set. */ public static synchronized void setInstance(CallStoredProcedure instance) { CallStoredProcedure.instance = instance;