midsuit
/
core-jgi
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

IDEMPIERE-4653 Improve timeout handling of window tab (#539) 

* IDEMPIERE-4653 Improve timeout handling of window tab

* Fixed a problem with IDEMPIERE-4130 that was not using role max query records
* GridTable was throwing NPEs because the arrays were disposed before closing
* Lowered log level on MRole to avoid clogging the console (I was testing with INFO level)
* Added more points of control for the timeout exception to give meaningful errors to the user and allow refining the query without closing the Find dialog
* Implemented a control to avoid repeating a slow initial query every time the dialog is open
* Added dialog when the find criteria doesn't return rows and allows the user to refine the query
* When the initial number of records is unknown, show a question mark instead of the count

* * Fix wrong role level pushed in first commit
This commit is contained in:
Carlos Ruiz 2021-01-20 14:45:33 +01:00 committed by GitHub
parent e1ba21360f
commit 24ccb86f16
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 66 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
org.adempiere.base/src/org/compiere/model/GridTab.java
View File

@ -3437,7 +3437,7 @@ public class GridTab implements DataStatusListener, Evaluatee, Serializable
// minimum between AD_Tab.MaxQueryRecords and AD_Role.MaxQueryRecords // minimum between AD_Tab.MaxQueryRecords and AD_Role.MaxQueryRecords
int roleMaxQueryRecords = MRole.getDefault().getMaxQueryRecords(); int roleMaxQueryRecords = MRole.getDefault().getMaxQueryRecords();
int tabMaxQueryRecords = m_vo.MaxQueryRecords; int tabMaxQueryRecords = m_vo.MaxQueryRecords;
if (roleMaxQueryRecords > 0 && roleMaxQueryRecords < tabMaxQueryRecords) if (roleMaxQueryRecords > 0 && (roleMaxQueryRecords < tabMaxQueryRecords || tabMaxQueryRecords == 0))
tabMaxQueryRecords = roleMaxQueryRecords; tabMaxQueryRecords = roleMaxQueryRecords;
return tabMaxQueryRecords; return tabMaxQueryRecords;
} }

11
org.adempiere.base/src/org/compiere/model/GridTable.java
View File

@ -785,21 +785,22 @@ public class GridTable extends AbstractTableModel
if (m_buffer != null) if (m_buffer != null)
{ {
m_buffer.clear(); m_buffer.clear();
m_buffer = null;
} }
if (m_sort != null) if (m_sort != null)
{ {
m_sort.clear(); m_sort.clear();
m_sort = null;
} }
if (m_virtualBuffer != null) if (m_virtualBuffer != null)
{ {
m_virtualBuffer.clear(); m_virtualBuffer.clear();
m_virtualBuffer = null;
} }
if (finalCall) if (finalCall) {
dispose(); dispose();
m_buffer = null;
m_sort = null;
m_virtualBuffer = null;
}
// Fields are disposed from MTab // Fields are disposed from MTab
log.fine(""); log.fine("");
@ -3698,7 +3699,7 @@ public class GridTable extends AbstractTableModel
} }
} // while(rs.next()) } // while(rs.next())
} }
catch (SQLException e) catch (Exception e)
{ {
log.log(Level.SEVERE, "run", e); log.log(Level.SEVERE, "run", e);
} }

2
org.adempiere.base/src/org/compiere/model/MRole.java
View File

@ -124,7 +124,7 @@ public final class MRole extends X_AD_Role implements ImmutablePOSupport
*/ */
public synchronized static MRole get (Properties ctx, int AD_Role_ID, int AD_User_ID, boolean reload) public synchronized static MRole get (Properties ctx, int AD_Role_ID, int AD_User_ID, boolean reload)
{ {
if (s_log.isLoggable(Level.INFO)) s_log.info("AD_Role_ID=" + AD_Role_ID + ", AD_User_ID=" + AD_User_ID + ", reload=" + reload); if (s_log.isLoggable(Level.CONFIG)) s_log.config("AD_Role_ID=" + AD_Role_ID + ", AD_User_ID=" + AD_User_ID + ", reload=" + reload);
String key = AD_Role_ID + "_" + AD_User_ID; String key = AD_Role_ID + "_" + AD_User_ID;
MRole role = (MRole)s_roles.get (key, e -> new MRole(ctx, e)); MRole role = (MRole)s_roles.get (key, e -> new MRole(ctx, e));
if (role == null || reload) if (role == null || reload)

2
org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/adwindow/ADTabpanel.java
View File

@ -1172,7 +1172,7 @@ DataStatusListener, IADTabpanel, IdSpace, IFieldEditorContainer
{ {
if (DBException.isTimeout(e)) if (DBException.isTimeout(e))
{ {
FDialog.error(windowNo, GridTable.LOAD_TIMEOUT_ERROR_MESSAGE); throw e;
} }
else else
{ {

22
org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/adwindow/AbstractADWindowContent.java
View File

@ -21,6 +21,7 @@ import static org.compiere.model.MSysConfig.ZK_GRID_AFTER_FIND;
import static org.compiere.model.SystemIDs.PROCESS_AD_CHANGELOG_REDO; import static org.compiere.model.SystemIDs.PROCESS_AD_CHANGELOG_REDO;
import static org.compiere.model.SystemIDs.PROCESS_AD_CHANGELOG_UNDO; import static org.compiere.model.SystemIDs.PROCESS_AD_CHANGELOG_UNDO;
import java.sql.SQLException;
import java.text.MessageFormat; import java.text.MessageFormat;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
@ -660,7 +661,13 @@ public abstract class AbstractADWindowContent extends AbstractUIPart implements
fTabPanel.createUI(); fTabPanel.createUI();
if (!m_queryInitiating) if (!m_queryInitiating)
{ {
initFirstTabpanel(); try {
initFirstTabpanel();
} catch (Exception e) {
if (DBException.isTimeout(e)) {
FDialog.error(curWindowNo, GridTable.LOAD_TIMEOUT_ERROR_MESSAGE);
}
}
} }
} }
@ -2994,7 +3001,18 @@ public abstract class AbstractADWindowContent extends AbstractUIPart implements
if (query != null) { if (query != null) {
m_onlyCurrentRows = false; m_onlyCurrentRows = false;
adTabbox.getSelectedGridTab().setQuery(query); adTabbox.getSelectedGridTab().setQuery(query);
adTabbox.getSelectedTabpanel().query(m_onlyCurrentRows, m_onlyCurrentDays, MRole.getDefault().getMaxQueryRecords()); // autoSize try {
adTabbox.getSelectedTabpanel().query(m_onlyCurrentRows, m_onlyCurrentDays, MRole.getDefault().getMaxQueryRecords()); // autoSize
} catch (Exception e) {
if ( e.getCause() != null
&& e.getCause() instanceof SQLException
&& DB.getDatabase().isQueryTimeout((SQLException)e.getCause())) {
// ignore, is captured somewhere else
return;
} else {
throw new DBException(e);
}
}
} }
adTabbox.getSelectedGridTab().dataRefresh(false); adTabbox.getSelectedGridTab().dataRefresh(false);

61
org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/window/FindWindow.java
View File

@ -135,7 +135,7 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
/** /**
* *
*/ */
private static final long serialVersionUID = 3968142284124286827L; private static final long serialVersionUID = -5087378621976257241L;
private static final String FIND_ROW_EDITOR = "find.row.editor"; private static final String FIND_ROW_EDITOR = "find.row.editor";
@ -190,6 +190,8 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
private static final CLogger log = CLogger.getCLogger(FindWindow.class); private static final CLogger log = CLogger.getCLogger(FindWindow.class);
/** Number of records */ /** Number of records */
private int m_total; private int m_total;
/** Initial slow query */
private boolean initialSlowQuery = false;
private PreparedStatement m_pstmt; private PreparedStatement m_pstmt;
// //
/** List of WEditors */ /** List of WEditors */
@ -247,6 +249,8 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
private static final String ON_POST_VISIBLE_ATTR = "onPostVisible.Event.Posted"; private static final String ON_POST_VISIBLE_ATTR = "onPostVisible.Event.Posted";
private static final int COUNTING_RECORDS_TIMED_OUT = -255;
/** START DEVCOFFEE **/ /** START DEVCOFFEE **/
private StatusBarPanel statusBar = new StatusBarPanel(); private StatusBarPanel statusBar = new StatusBarPanel();
/** END DEVCOFFEE **/ /** END DEVCOFFEE **/
@ -315,7 +319,7 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
initFind(); initFind();
initFindAdvanced(); initFindAdvanced();
if (m_total < m_minRecords) if (m_total != COUNTING_RECORDS_TIMED_OUT && m_total < m_minRecords)
{ {
return false; return false;
} }
@ -350,7 +354,7 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
m_minRecords = minRecords; m_minRecords = minRecords;
m_total = getNoOfRecords(null, false); m_total = getNoOfRecords(null, false);
if (m_total < m_minRecords) if (m_total != COUNTING_RECORDS_TIMED_OUT && m_total < m_minRecords)
{ {
return false; return false;
} }
@ -1479,15 +1483,10 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
{ {
fQueryName.setSelectedIndex(0); fQueryName.setSelectedIndex(0);
cmd_ok_Simple(); cmd_ok_Simple();
if (advancedPanel != null) {
advancedPanel.getItems().clear();
}
dispose();
} }
else if ("btnOkAdv".equals(btn.getName())) else if ("btnOkAdv".equals(btn.getName()))
{ {
cmd_ok_Advanced(); cmd_ok_Advanced();
dispose();
} }
else if("btnCancel".equals(btn.getName())) else if("btnCancel".equals(btn.getName()))
{ {
@ -1524,12 +1523,10 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
if (winLookupRecord.equals(event.getTarget())) if (winLookupRecord.equals(event.getTarget()))
{ {
cmd_ok_Simple(); cmd_ok_Simple();
dispose();
} }
else if (winAdvanced.equals(event.getTarget())) else if (winAdvanced.equals(event.getTarget()))
{ {
cmd_ok_Advanced(); cmd_ok_Advanced();
dispose();
} }
// Check simple panel fields // Check simple panel fields
for (int i = 0; i < m_sEditors.size(); i++) for (int i = 0; i < m_sEditors.size(); i++)
@ -1538,13 +1535,11 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
if (editor.getComponent() == event.getTarget()) if (editor.getComponent() == event.getTarget())
{ {
cmd_ok_Simple(); cmd_ok_Simple();
dispose();
} }
WEditor editorTo = (WEditor)m_sEditorsTo.get(i); WEditor editorTo = (WEditor)m_sEditorsTo.get(i);
if (editorTo != null && editor.getComponent() == event.getTarget()) if (editorTo != null && editor.getComponent() == event.getTarget())
{ {
cmd_ok_Simple(); cmd_ok_Simple();
dispose();
} }
} }
} }
@ -2354,8 +2349,16 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
cmd_saveSimple(false, false); cmd_saveSimple(false, false);
// Test for no records // Test for no records
if (getNoOfRecords(m_query, true) != 0) if (getNoOfRecords(m_query, true) != 0) {
dispose(); if (m_total == COUNTING_RECORDS_TIMED_OUT) {
FDialog.error(m_targetWindowNo, "InfoQueryTimeOutError");
} else {
if (advancedPanel != null) {
advancedPanel.getItems().clear();
}
dispose();
}
}
} // cmd_ok_Simple } // cmd_ok_Simple
@ -2428,8 +2431,13 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
addHistoryRestriction(historyCombo.getSelectedItem()); addHistoryRestriction(historyCombo.getSelectedItem());
} }
if (getNoOfRecords(m_query, true) != 0) if (getNoOfRecords(m_query, true) != 0) {
dispose(); if (m_total == COUNTING_RECORDS_TIMED_OUT) {
FDialog.error(m_targetWindowNo, "InfoQueryTimeOutError");
} else {
dispose();
}
}
} // cmd_ok_Advanced } // cmd_ok_Advanced
/** /**
@ -2443,13 +2451,15 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
/** /**
* Get the number of records of target tab * Get the number of records of target tab
* @param query where clause for target tab * @param query where clause for target tab
* @param alertZeroRecords show dialog if there are no records * @param alertRecords show dialog if there are no records or there are more records than allowed for role/tab
* @return number of selected records; * @return number of selected records;
* if the results are more then allowed this method will return 0 * if the results are more then allowed this method will return 0
**/ **/
private int getNoOfRecords (MQuery query, boolean alertZeroRecords) private int getNoOfRecords (MQuery query, boolean alertRecords)
{ {
if (log.isLoggable(Level.CONFIG)) log.config("" + query); if (log.isLoggable(Level.CONFIG)) log.config("" + query);
if (initialSlowQuery && (query == null || query.getRestrictionCount() == 0))
return COUNTING_RECORDS_TIMED_OUT;
StringBuilder sql = new StringBuilder("SELECT COUNT(*) FROM "); StringBuilder sql = new StringBuilder("SELECT COUNT(*) FROM ");
sql.append(m_tableName); sql.append(m_tableName);
boolean hasWhere = false; boolean hasWhere = false;
@ -2492,7 +2502,10 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
{ {
if (DB.getDatabase().isQueryTimeout(e)) if (DB.getDatabase().isQueryTimeout(e))
{ {
throw new DBException(Msg.getMsg(Env.getCtx(), GridTable.LOAD_TIMEOUT_ERROR_MESSAGE)); m_total = COUNTING_RECORDS_TIMED_OUT; // unknown
if (query == null) {
initialSlowQuery = true;
}
} }
else else
{ {
@ -2506,10 +2519,10 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
stmt = null; stmt = null;
} }
// No Records // No Records
/* if (m_total == 0 && alertZeroRecords) if (m_total == 0 && alertRecords)
FDialog.warn(m_targetWindowNo, this, "FindZeroRecords");*/ FDialog.warn(m_targetWindowNo, this, "FindZeroRecords", null);
// More then allowed // More then allowed
if (m_gridTab != null && query != null && m_gridTab.isQueryMax(m_total)) if (m_gridTab != null && alertRecords && m_total != COUNTING_RECORDS_TIMED_OUT && m_gridTab.isQueryMax(m_total))
{ {
FDialog.error(m_targetWindowNo, this, "FindOverMax", FDialog.error(m_targetWindowNo, this, "FindOverMax",
m_total + " > " + m_gridTab.getMaxQueryRecords()); m_total + " > " + m_gridTab.getMaxQueryRecords());
@ -2700,7 +2713,7 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
*/ */
public MQuery getQuery() public MQuery getQuery()
{ {
if (m_gridTab != null && m_gridTab.isQueryMax(getTotalRecords()) && !m_isCancel) if (m_gridTab != null && m_total != COUNTING_RECORDS_TIMED_OUT && m_gridTab.isQueryMax(m_total) && !m_isCancel)
{ {
m_query = MQuery.getNoRecordQuery (m_tableName, false); m_query = MQuery.getNoRecordQuery (m_tableName, false);
m_total = 0; m_total = 0;
@ -2867,7 +2880,7 @@ public class FindWindow extends Window implements EventListener<Event>, ValueCha
*/ */
private void setStatusDB (int currentCount) private void setStatusDB (int currentCount)
{ {
StringBuilder text = new StringBuilder(" ").append(Msg.getMsg(Env.getCtx(), "Records")).append(" = ").append(m_total).append(" "); StringBuilder text = new StringBuilder(" ").append(Msg.getMsg(Env.getCtx(), "Records")).append(" = ").append(m_total == COUNTING_RECORDS_TIMED_OUT ? "?" : m_total).append(" ");
statusBar.setStatusDB(text.toString()); statusBar.setStatusDB(text.toString());
} // setDtatusDB } // setDtatusDB
/** END DEVCOFFEE **/ /** END DEVCOFFEE **/