details: https://code.openbravo.com/erp/devel/pi/rev/2c0b8265b9b1
changeset: 32514:2c0b8265b9b1
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Fri Jul 28 11:04:38 2017 +0200
summary: fixed 36359: can get stale cached WS user context
When invoking WS, user context is cached in order to prevent its (heavy)
creation on every request. This cache was not properly invalidated.
Changes to improve this invalidation:
1. Purge cache before trying to retrieve elements, not afterwards.
2. Cache elements already had a valid period of 30 minutes, but this period
was restarted everytime the element was used. Now the period starts when
the context is cached and it never gets reset.
3. In addition to this expiration time, ADUser.updated was checked when
purging
cache, and if it was modified since it was cached, it was removed. This
implementation had some problems, that are now solved:
* ADUser is not the only (nor even the main) entity that when modified
can change permissions in cached contexts, therefore modifications in
any other entity (ie. Organization) were not triggering cache
invalidataion.
A new EventObserver taking care of all entities that participate in
the
permissions definition has been created to invalidate cache.
* For every WS request ADUser.updated for every cached context was
checked
querying DB, adding some performance overhead (specially when many
contexts
where already cached). As now invalidation occurs in memory, it is not
necessary to query DB anymore.
This mechanism provides a fully accurate cache when working in a single node.
In case of clustered environments, it is possible that caches kept in a node
different than the one modifying permissions to continue returning stale
contexts,
but, as per topic 2, this is restricted to, at most, 30 minutes.
details: https://code.openbravo.com/erp/devel/pi/rev/93c5e55ef8fb
changeset: 32515:93c5e55ef8fb
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Thu Jul 27 08:38:09 2017 +0200
summary: [wsah] refactor to split in methods
details: https://code.openbravo.com/erp/devel/pi/rev/87a29aed13a1
changeset: 32516:87a29aed13a1
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Thu Jul 27 08:52:27 2017 +0200
summary: [wsah] added some log
details: https://code.openbravo.com/erp/devel/pi/rev/a3e04cd55526
changeset: 32517:a3e04cd55526
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Thu Jul 27 09:41:55 2017 +0200
summary: [wsah] added adcs.getWindow
details: https://code.openbravo.com/erp/devel/pi/rev/dd2f17bee637
changeset: 32518:dd2f17bee637
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Thu Jul 27 09:42:22 2017 +0200
summary: [adcs] get window from adcs, don't iterate over all roles to get
current one
details: https://code.openbravo.com/erp/devel/pi/rev/8325bece5e17
changeset: 32519:8325bece5e17
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Thu Jul 27 10:11:15 2017 +0200
summary: [wsah] prevent more queries + dal initialization using adcs
details: https://code.openbravo.com/erp/devel/pi/rev/ffd6bed38f34
changeset: 32520:ffd6bed38f34
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Thu Jul 27 10:40:46 2017 +0200
summary: [wsah] minor code clean up
details: https://code.openbravo.com/erp/devel/pi/rev/3ec690db55fe
changeset: 32521:3ec690db55fe
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Mon Jul 31 09:00:49 2017 +0200
summary: fixed bug 36172: WindowSettingsActionHandler underperformance
Several changes in WindowSettingsActionHandler performance which improve both
processing time and heap memory consumption:
- Use ADCS to query window/tab definition in order to prevent DB queries
- Optimized DAL queries so that only relevant records are queries from DB
and
loaded in memory
diffstat:
modules/org.openbravo.client.application/src/org/openbravo/client/application/WindowSettingsActionHandler.java
| 401 +++++----
modules/org.openbravo.client.application/src/org/openbravo/client/application/window/ApplicationDictionaryCachedStructures.java
| 39 +-
src/org/openbravo/service/UserCtxCacheObserver.java
| 66 +
src/org/openbravo/service/web/UserContextCache.java
| 54 +-
4 files changed, 352 insertions(+), 208 deletions(-)
diffs (truncated from 752 to 300 lines):
diff -r c5f112181318 -r 3ec690db55fe
modules/org.openbravo.client.application/src/org/openbravo/client/application/WindowSettingsActionHandler.java
---
a/modules/org.openbravo.client.application/src/org/openbravo/client/application/WindowSettingsActionHandler.java
Thu Jul 20 17:54:57 2017 -0400
+++
b/modules/org.openbravo.client.application/src/org/openbravo/client/application/WindowSettingsActionHandler.java
Mon Jul 31 09:00:49 2017 +0200
@@ -28,15 +28,17 @@
import javax.enterprise.inject.Any;
import javax.enterprise.inject.Instance;
import javax.inject.Inject;
+import javax.servlet.ServletException;
-import org.apache.log4j.Logger;
import org.codehaus.jettison.json.JSONArray;
+import org.codehaus.jettison.json.JSONException;
import org.codehaus.jettison.json.JSONObject;
import org.openbravo.base.exception.OBException;
import org.openbravo.base.model.Entity;
import org.openbravo.base.model.ModelProvider;
import org.openbravo.base.model.Property;
import org.openbravo.client.application.personalization.PersonalizationHandler;
+import
org.openbravo.client.application.window.ApplicationDictionaryCachedStructures;
import org.openbravo.client.kernel.BaseActionHandler;
import org.openbravo.client.kernel.KernelUtils;
import org.openbravo.client.kernel.StaticResourceComponent;
@@ -47,11 +49,12 @@
import org.openbravo.erpCommon.utility.PropertyException;
import org.openbravo.model.ad.access.FieldAccess;
import org.openbravo.model.ad.access.TabAccess;
-import org.openbravo.model.ad.access.WindowAccess;
import org.openbravo.model.ad.ui.Field;
import org.openbravo.model.ad.ui.Tab;
import org.openbravo.model.ad.ui.Window;
import org.openbravo.service.db.DalConnectionProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Computes different settings which may be user/role specific for a certain
window.
@@ -61,206 +64,254 @@
*/
@ApplicationScoped
public class WindowSettingsActionHandler extends BaseActionHandler {
- private static final Logger log4j =
Logger.getLogger(WindowSettingsActionHandler.class);
+ private static final Logger log =
LoggerFactory.getLogger(WindowSettingsActionHandler.class);
public static final String EXTRA_CALLBACKS = "extraCallbacks";
@Inject
private PersonalizationHandler personalizationHandler;
@Inject
+ private ApplicationDictionaryCachedStructures adcs;
+
+ @Inject
@Any
private Instance<ExtraWindowSettingsInjector> extraSettings;
+ @Override
protected JSONObject execute(Map<String, Object> parameters, String data) {
+ long t = System.currentTimeMillis();
+ final String windowId = (String) parameters.get("windowId");
try {
OBContext.setAdminMode();
- final String windowId = (String) parameters.get("windowId");
- final Window window = OBDal.getInstance().get(Window.class, windowId);
- final String roleId = OBContext.getOBContext().getRole().getId();
- final DalConnectionProvider dalConnectionProvider = new
DalConnectionProvider(false);
- final JSONObject jsonUIPattern = new JSONObject();
- final String windowType = window.getWindowType();
- for (Tab tab : window.getADTabList()) {
- final boolean readOnlyAccess =
org.openbravo.erpCommon.utility.WindowAccessData
- .hasReadOnlyAccess(dalConnectionProvider, roleId, tab.getId());
- String uiPattern = readOnlyAccess ? "RO" : tab.getUIPattern();
- // window should be read only when is assigned with a table defined as
a view
- if (!"RO".equals(uiPattern) && ("T".equals(windowType) ||
"M".equals(windowType))
- && tab.getTable().isView()) {
- log4j.warn("Tab \"" + tab.getName()
- + "\" is set to read only because is assigned with a table
defined as a view.");
- uiPattern = "RO";
- }
- jsonUIPattern.put(tab.getId(), uiPattern);
- }
+ final Window window = adcs.getWindow(windowId);
+
final JSONObject json = new JSONObject();
- json.put("uiPattern", jsonUIPattern);
- final String autoSaveStr = Preferences.getPreferenceValue("Autosave",
false, OBContext
- .getOBContext().getCurrentClient(),
OBContext.getOBContext().getCurrentOrganization(),
- OBContext.getOBContext().getUser(),
OBContext.getOBContext().getRole(), window);
- json.put("autoSave", Preferences.YES.equals(autoSaveStr));
+ json.put("uiPattern", getUIPattern(window));
+ json.put("autoSave", getBooleanPreference(window, "Autosave", true));
try {
json.put("personalization",
personalizationHandler.getPersonalizationForWindow(window));
- } catch (Throwable t) {
+ } catch (Throwable e) {
// be robust about errors in the personalization settings
- log4j.error("Error for window " + window, t);
+ log.error("Error for window: " + window + " - role: " +
OBContext.getOBContext().getRole(),
+ e);
}
- final String showConfirmationStr =
Preferences.getPreferenceValue("ShowConfirmationDefault",
- false, OBContext.getOBContext().getCurrentClient(),
OBContext.getOBContext()
- .getCurrentOrganization(), OBContext.getOBContext().getUser(),
OBContext
- .getOBContext().getRole(), window);
- json.put("showAutoSaveConfirmation",
Preferences.YES.equals(showConfirmationStr));
-
- // Field Level Roles
- final JSONArray tabs = new JSONArray();
- json.put("tabs", tabs);
- for (WindowAccess winAccess : window.getADWindowAccessList()) {
- if (winAccess.isActive() &&
winAccess.getRole().getId().equals(roleId)) {
- for (TabAccess tabAccess : winAccess.getADTabAccessList()) {
- if (tabAccess.isActive()) {
- boolean tabEditable = tabAccess.isEditableField();
- final Entity entity =
ModelProvider.getInstance().getEntityByTableId(
- tabAccess.getTab().getTable().getId());
- final JSONObject jTab = new JSONObject();
- tabs.put(jTab);
- jTab.put("tabId", tabAccess.getTab().getId());
- jTab.put("updatable", tabEditable);
- final JSONObject jFields = new JSONObject();
- jTab.put("fields", jFields);
- final Set<String> fields = new TreeSet<String>();
- for (Field field : tabAccess.getTab().getADFieldList()) {
- if (!field.isReadOnly() && !field.isShownInStatusBar()
- && field.getColumn().isUpdatable()) {
- final Property property = KernelUtils.getProperty(entity,
field);
- if (property != null) {
- fields.add(property.getName());
- }
- }
- }
- for (FieldAccess fieldAccess : tabAccess.getADFieldAccessList())
{
- if (fieldAccess.isActive()) {
- final Property property = KernelUtils.getProperty(entity,
fieldAccess.getField());
- if (property != null) {
- final String name = KernelUtils.getProperty(entity,
fieldAccess.getField())
- .getName();
- if (fields.contains(name)) {
- jFields.put(name, fieldAccess.isEditableField());
- fields.remove(name);
- }
- }
- }
- }
- for (String name : fields) {
- jFields.put(name, tabEditable);
- }
- }
- }
- }
- }
-
- // processes can be secured in 3 ways:
- // - Secured preference is set: explicit grant is required
- // - Process is marked as requiresExplicitAccessPermission: explicit
grant is required
- // - None of the above: permission is inherited from window
- boolean securedProcess = false;
- try {
- securedProcess =
Preferences.YES.equals(Preferences.getPreferenceValue("SecuredProcess",
- true, OBContext.getOBContext().getCurrentClient(),
OBContext.getOBContext()
- .getCurrentOrganization(), OBContext.getOBContext().getUser(),
OBContext
- .getOBContext().getRole(), window));
- } catch (PropertyException e) {
- // do nothing, property is not set so securedProcess is false
- }
-
- String restrictedProcessesQry = " as f where tab.window = :window and
tab.active = true and (";
-
- restrictedProcessesQry += "(column.oBUIAPPProcess is not null";
- if (!securedProcess) {
- // not secured, restrict only those that require explicit permission
- // subquery required to prevent inner join due to compound path check
- // (process.requiresExplicitAccessPermission)
- restrictedProcessesQry += " and exists (select 1 from OBUIAPP_Process
p where p = f.column.oBUIAPPProcess and requiresExplicitAccessPermission =
true) ";
- }
- restrictedProcessesQry += " and not exists (select 1 from " //
- + " OBUIAPP_Process_Access a"
- + " where a.obuiappProcess = f.column.oBUIAPPProcess"
- + " and a.role.id = :role and a.active=true))"//
-
- + " or (column.process is not null ";
-
- if (!securedProcess) {
- // not secured, restrict only those that require explicit permission
- // subquery required to prevent inner join due to compound path check
- // (process.requiresExplicitAccessPermission)
- restrictedProcessesQry += " and exists (select 1 from ADProcess p
where p = f.column.process and requiresExplicitAccessPermission = true) ";
- }
-
- restrictedProcessesQry += " and not exists (select 1 from
ADProcessAccess a where a.process = f.column.process and "
- + " a.role.id = :role and a.active=true))";
-
- restrictedProcessesQry += ") order by f.tab";
-
- OBQuery<Field> q = OBDal.getInstance().createQuery(Field.class,
restrictedProcessesQry);
- q.setNamedParameter("window", window);
- q.setNamedParameter("role", OBContext.getOBContext().getRole().getId());
-
- final JSONArray processes = new JSONArray();
- json.put("notAccessibleProcesses", processes);
- Tab tab = null;
- JSONObject t;
- JSONArray ps = null;
- for (Field f : q.list()) {
- if (tab == null || !tab.getId().equals(f.getTab().getId())) {
- t = new JSONObject();
- tab = f.getTab();
- ps = new JSONArray();
- t.put("tabId", tab.getId());
- t.put("processes", ps);
- processes.put(t);
- }
- ps.put(KernelUtils.getProperty(f).getName());
- }
-
- JSONObject extraSettingsJson = new JSONObject();
- json.put("extraSettings", extraSettingsJson);
- JSONArray extraCallbacks = new JSONArray();
-
- // Add the extraSettings injected
- for (ExtraWindowSettingsInjector nextSetting : extraSettings) {
- Map<String, Object> settingsToAdd =
nextSetting.doAddSetting(parameters, json);
- for (Entry<String, Object> setting : settingsToAdd.entrySet()) {
- String settingKey = setting.getKey();
- Object settingValue = setting.getValue();
- if (settingKey.equals(EXTRA_CALLBACKS)) {
- if (settingValue instanceof List) {
- for (Object callbackExtra : (List<?>) settingValue) {
- if (callbackExtra instanceof String) {
- extraCallbacks.put(callbackExtra);
- } else {
- log4j.warn("You are trying to set a wrong instance of
extraCallbacks");
- }
- }
- } else if (settingValue instanceof String) {
- extraCallbacks.put(settingValue);
- } else {
- log4j.warn("You are trying to set a wrong instance of
extraCallbacks");
- }
- } else {
- extraSettingsJson.put(settingKey, settingValue);
- }
- }
- }
- json.put("extraCallbacks", extraCallbacks);
+ json.put("showAutoSaveConfirmation",
+ getBooleanPreference(window, "ShowConfirmationDefault", false));
+ json.put("tabs", getFieldLevelRoles(window));
+ json.put("notAccessibleProcesses", getNotAccessibleProcesses(window));
+ setExtraSettings(parameters, json);
return json;
} catch (Exception e) {
+ log.error("Error for window: " + windowId + " - role: " +
OBContext.getOBContext().getRole(),
+ e);
throw new OBException(e);
} finally {
OBContext.restorePreviousMode();
// clear anything we have in session as there's no change to make faster
flush
OBDal.getInstance().getSession().clear();
+ log.debug("window: {} - role: {} - took: {}ms", new Object[] { windowId,
+ OBContext.getOBContext().getRole(), System.currentTimeMillis() - t
});
}
}
+
+ private JSONObject getUIPattern(Window window) throws ServletException,
JSONException {
+ final String roleId = OBContext.getOBContext().getRole().getId();
+ final JSONObject jsonUIPattern = new JSONObject();
+ final String windowType = window.getWindowType();
+ DalConnectionProvider cp = new DalConnectionProvider(false);
+ for (Tab tab : window.getADTabList()) {
+ final boolean readOnlyAccess =
org.openbravo.erpCommon.utility.WindowAccessData
+ .hasReadOnlyAccess(cp, roleId, tab.getId());
+ String uiPattern = readOnlyAccess ? "RO" : tab.getUIPattern();
+ // window should be read only when is assigned with a table defined as a
view
+ if (!"RO".equals(uiPattern) && ("T".equals(windowType) ||
"M".equals(windowType))
+ && tab.getTable().isView()) {
+ log.warn("Tab {} is set to read only because is assigned with a table
defined as a view.",
+ tab);
+ uiPattern = "RO";
+ }
+ jsonUIPattern.put(tab.getId(), uiPattern);
+ }
+ return jsonUIPattern;
+ }
+
+ private boolean getBooleanPreference(Window window, String preference,
boolean defaultValue) {
+ try {
+ String prefValue = Preferences.getPreferenceValue(preference, false,
OBContext.getOBContext()
+ .getCurrentClient(),
OBContext.getOBContext().getCurrentOrganization(), OBContext
+ .getOBContext().getUser(), OBContext.getOBContext().getRole(),
window);
+ return Preferences.YES.equals(prefValue);
+ } catch (PropertyException ignore) {
+ return defaultValue;
+ }
+ }
+
+ private JSONArray getFieldLevelRoles(Window window) throws JSONException {
+ final String roleId = OBContext.getOBContext().getRole().getId();
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openbravo-commits mailing list
Openbravo-commits@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbravo-commits
