From 2b4be85656349fd9228fc2eeb63204abaaaa8d21 Mon Sep 17 00:00:00 2001 From: Manuel Rego Casasnovas Date: Mon, 10 Dec 2012 09:08:00 +0100 Subject: [PATCH 1/3] Revert "Bug #1590: Fix problem calling several times the same method in OrderModel" This reverts commit 7ca0d43883d6ddc78f3346695867b2a94d5461d8. --- .../main/java/org/zkoss/ganttz/TabsRegistry.java | 15 +-------------- .../java/org/zkoss/ganttz/extensions/ITab.java | 2 -- .../org/zkoss/ganttz/extensions/TabProxy.java | 5 ----- .../web/planner/tabs/CreatedOnDemandTab.java | 15 --------------- .../tabs/MultipleTabsPlannerController.java | 2 +- .../web/planner/tabs/OrdersTabCreator.java | 7 ++----- .../libreplan/web/planner/tabs/TabOnModeType.java | 9 --------- 7 files changed, 4 insertions(+), 51 deletions(-) diff --git a/ganttzk/src/main/java/org/zkoss/ganttz/TabsRegistry.java b/ganttzk/src/main/java/org/zkoss/ganttz/TabsRegistry.java index 23db9e97b..e9b98b2ef 100644 --- a/ganttzk/src/main/java/org/zkoss/ganttz/TabsRegistry.java +++ b/ganttzk/src/main/java/org/zkoss/ganttz/TabsRegistry.java @@ -65,23 +65,10 @@ public class TabsRegistry { show(tab, DO_NOTHING); } - public void showWithoutAfterCreate(ITab tab) { - show(tab, DO_NOTHING, false); - } - public void show(ITab tab, IBeforeShowAction beforeShowAction) { - show(tab, beforeShowAction, true); - } - - private void show(ITab tab, IBeforeShowAction beforeShowAction, - boolean afterCreate) { hideAllExcept(tab); beforeShowAction.doAction(); - if (afterCreate) { - tab.show(); - } else { - tab.showWithoutAfterCreate(); - } + tab.show(); parent.invalidate(); activateMenuIfRegistered(tab); } diff --git a/ganttzk/src/main/java/org/zkoss/ganttz/extensions/ITab.java b/ganttzk/src/main/java/org/zkoss/ganttz/extensions/ITab.java index d822d2fa5..7645a8047 100644 --- a/ganttzk/src/main/java/org/zkoss/ganttz/extensions/ITab.java +++ b/ganttzk/src/main/java/org/zkoss/ganttz/extensions/ITab.java @@ -37,8 +37,6 @@ public interface ITab { void show(); - void showWithoutAfterCreate(); - void hide(); } diff --git a/ganttzk/src/main/java/org/zkoss/ganttz/extensions/TabProxy.java b/ganttzk/src/main/java/org/zkoss/ganttz/extensions/TabProxy.java index 096d92fa5..2fbd04ce7 100644 --- a/ganttzk/src/main/java/org/zkoss/ganttz/extensions/TabProxy.java +++ b/ganttzk/src/main/java/org/zkoss/ganttz/extensions/TabProxy.java @@ -53,11 +53,6 @@ public class TabProxy implements ITab { proxiedTab.show(); } - @Override - public void showWithoutAfterCreate() { - proxiedTab.showWithoutAfterCreate(); - } - @Override public String getCssClass() { return proxiedTab.getCssClass(); diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/CreatedOnDemandTab.java b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/CreatedOnDemandTab.java index b7ef835fd..ebba95f83 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/CreatedOnDemandTab.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/CreatedOnDemandTab.java @@ -72,21 +72,9 @@ public class CreatedOnDemandTab implements ITab { @Override public void show() { - show(true); - } - - @Override - public void showWithoutAfterCreate() { - show(false); - } - - private void show(boolean afterCreate) { beforeShowAction(); if (component == null) { component = componentCreator.create(parent); - if (afterCreate) { - afterCreateAction(component); - } } component.setParent(parent); afterShowAction(); @@ -102,9 +90,6 @@ public class CreatedOnDemandTab implements ITab { protected void beforeShowAction() { } - protected void afterCreateAction(Component component) { - } - protected void afterShowAction() { } diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java index 824245410..9cc20d977 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java @@ -472,7 +472,7 @@ public class MultipleTabsPlannerController implements Composer, @Override public void goToOrdersList() { // ordersTab.show(); - getTabsRegistry().showWithoutAfterCreate(ordersTab); + getTabsRegistry().show(ordersTab); } public void goToCreateForm() { diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java index 6b822bede..15ecc5a39 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java @@ -68,6 +68,8 @@ public class OrdersTabCreator { args.put("orderController", setupOrderCrudController()); result = Executions.createComponents("/orders/_ordersTab.zul", parent, args); + Util.createBindingsFor(result); + Util.reloadBindings(result); return result; } @@ -108,11 +110,6 @@ public class OrdersTabCreator { } } - @Override - protected void afterCreateAction(org.zkoss.zk.ui.Component component) { - Util.createBindingsFor(component); - } - @Override protected void afterShowAction() { orderCRUDController.goToList(); diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/TabOnModeType.java b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/TabOnModeType.java index f56f08168..6f92e1880 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/TabOnModeType.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/TabOnModeType.java @@ -140,13 +140,4 @@ public class TabOnModeType implements ITab { } } - @Override - public void showWithoutAfterCreate() { - beingShown = true; - ITab currentTab = getCurrentTab(); - if (currentTab != null) { - currentTab.showWithoutAfterCreate(); - } - } - } From f06a44d6e92e6e61d938fed5b888b5526cccf6a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93scar=20Gonz=C3=A1lez=20Fern=C3=A1ndez?= Date: Mon, 10 Dec 2012 19:58:09 +0100 Subject: [PATCH 2/3] Avoid some redundant loads of bindings in the same request Sometimes reloadBindings is called several times in the same request. Now only the first call would force the reload and the others would be ignored. For example, when switching to Projects List reloadBindings is called after the tab being created and another time when calling org.libreplan.web.orders.OrderCRUDController.goToList(). Now the second call is ignored. --- .../java/org/libreplan/web/common/Util.java | 91 ++++++++++++++++++- .../web/orders/OrderCRUDController.java | 3 +- .../web/planner/tabs/OrdersTabCreator.java | 3 +- 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java b/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java index 70b573eb9..127bff48c 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java @@ -27,9 +27,12 @@ import java.io.IOException; import java.math.BigDecimal; import java.text.DateFormat; import java.text.DecimalFormat; +import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import javax.servlet.http.HttpServletResponse; @@ -85,18 +88,103 @@ public class Util { private static final String[] DECIMAL_FORMAT_SPECIAL_CHARS = { "0", ",", ".", "\u2030", "%", "#", ";", "-" }; + private static final String RELOADED_COMPONENTS_ATTR = Util.class.getName() + + ":" + "reloaded"; + private Util() { } + /** + * Forces to reload the bindings of the provided components if there is an + * associated {@link DataBinder}. + * + * @param toReload + * the components to reload + */ public static void reloadBindings(Component... toReload) { + reloadBindings(ReloadStrategy.FORCE, toReload); + } + + public enum ReloadStrategy { + /** + * If the {@link DataBinder} exists the bindings are reloaded no matter + * what. + */ + FORCE, + /** + * Once the bindings for a component have been manually loaded in one + * request, subsequent calls for reload the bindings of the same + * component or descendants using this strategy are ignored. + */ + ONE_PER_REQUEST; + + public static boolean isForced(ReloadStrategy reloadStrategy) { + return reloadStrategy == ReloadStrategy.FORCE; + } + } + + /** + * Reload the bindings of the provided components if there is an associated + * {@link DataBinder} and the {@link ReloadStrategy} allows it. + * + * @param toReload + * the components to reload + */ + public static void reloadBindings(ReloadStrategy reloadStrategy, + Component... toReload) { + reloadBindings(ReloadStrategy.isForced(reloadStrategy), toReload); + } + + private static void reloadBindings(boolean forceReload, + Component... toReload) { for (Component reload : toReload) { DataBinder binder = Util.getBinder(reload); - if (binder != null) { + if (binder != null + && (forceReload || notReloadedInThisRequest(reload))) { binder.loadComponent(reload); + markAsReloadedForThisRequest(reload); } } } + private static boolean notReloadedInThisRequest(Component reload) { + return !getReloadedComponents(reload).contains(reload); + } + + private static Set getReloadedComponents(Component component) { + Execution execution = component.getDesktop().getExecution(); + @SuppressWarnings("unchecked") + Set result = (Set) execution + .getAttribute(RELOADED_COMPONENTS_ATTR); + if (result == null) { + result = new HashSet(); + execution.setAttribute(RELOADED_COMPONENTS_ATTR, result); + } + return result; + } + + private static void markAsReloadedForThisRequest(Component component) { + Set reloadedComponents = getReloadedComponents(component); + reloadedComponents.add(component); + reloadedComponents.addAll(getAllDescendants(component)); + } + + private static void markAsNotReloadedForThisRequest(Component component) { + Set reloadedComponents = getReloadedComponents(component); + reloadedComponents.remove(component); + reloadedComponents.removeAll(getAllDescendants(component)); + } + + @SuppressWarnings("unchecked") + private static List getAllDescendants(Component component) { + List result = new ArrayList(); + for (Component each : (List) component.getChildren()) { + result.add(each); + result.addAll(getAllDescendants(each)); + } + return result; + } + public static void saveBindings(Component... toReload) { for (Component reload : toReload) { DataBinder binder = Util.getBinder(reload); @@ -113,6 +201,7 @@ public class Util { public static void createBindingsFor(org.zkoss.zk.ui.Component result) { AnnotateDataBinder binder = new AnnotateDataBinder(result, true); result.setVariable("binder", binder, true); + markAsNotReloadedForThisRequest(result); } /** diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/orders/OrderCRUDController.java b/libreplan-webapp/src/main/java/org/libreplan/web/orders/OrderCRUDController.java index ab9434b69..a34a7d3e2 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/orders/OrderCRUDController.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/orders/OrderCRUDController.java @@ -55,6 +55,7 @@ import org.libreplan.web.common.Level; import org.libreplan.web.common.MessagesForUser; import org.libreplan.web.common.OnlyOneVisible; import org.libreplan.web.common.Util; +import org.libreplan.web.common.Util.ReloadStrategy; import org.libreplan.web.common.components.bandboxsearch.BandboxMultipleSearch; import org.libreplan.web.common.components.bandboxsearch.BandboxSearch; import org.libreplan.web.common.components.finders.FilterPair; @@ -799,7 +800,7 @@ public class OrderCRUDController extends GenericForwardComposer { private void showWindow(Window window) { getVisibility().showOnly(window); - Util.reloadBindings(window); + Util.reloadBindings(ReloadStrategy.ONE_PER_REQUEST, window); } public void reloadHoursGroupOrder() { diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java index 15ecc5a39..f0a08883a 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/OrdersTabCreator.java @@ -28,6 +28,7 @@ import java.util.HashMap; import java.util.Map; import org.libreplan.web.common.Util; +import org.libreplan.web.common.Util.ReloadStrategy; import org.libreplan.web.orders.OrderCRUDController; import org.libreplan.web.planner.order.IOrderPlanningGate; import org.libreplan.web.planner.tabs.CreatedOnDemandTab.IComponentCreator; @@ -69,7 +70,7 @@ public class OrdersTabCreator { result = Executions.createComponents("/orders/_ordersTab.zul", parent, args); Util.createBindingsFor(result); - Util.reloadBindings(result); + Util.reloadBindings(ReloadStrategy.ONE_PER_REQUEST, result); return result; } From c8129cd292dcbb854bc7a7f58983aa42ca841f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93scar=20Gonz=C3=A1lez=20Fern=C3=A1ndez?= Date: Mon, 10 Dec 2012 20:45:14 +0100 Subject: [PATCH 3/3] Bug #1590: Avoid repeated calls to goToOrdersList A mechanism for ignoring in a scope the calls to createBindings has been added. When accessing from the entry point the page is been created and the AnnotatedDataBinder created automatically will track the created tab. So in these cases we must ignore createBindings calls. --- .../java/org/libreplan/web/common/Util.java | 18 ++++++++++++++++++ .../common/entrypoints/EntryPointsHandler.java | 14 ++++++++++---- .../tabs/MultipleTabsPlannerController.java | 1 - 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java b/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java index 127bff48c..9d5add8b0 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/common/Util.java @@ -198,7 +198,25 @@ public class Util { return (DataBinder) component.getVariable("binder", false); } + private static final ThreadLocal ignoreCreateBindings = new ThreadLocal() { + protected Boolean initialValue() { + return false; + }; + }; + + public static void executeIgnoringCreationOfBindings(Runnable action) { + try { + ignoreCreateBindings.set(true); + action.run(); + } finally { + ignoreCreateBindings.set(false); + } + } + public static void createBindingsFor(org.zkoss.zk.ui.Component result) { + if (ignoreCreateBindings.get()) { + return; + } AnnotateDataBinder binder = new AnnotateDataBinder(result, true); result.setVariable("binder", binder, true); markAsNotReloadedForThisRequest(result); diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/common/entrypoints/EntryPointsHandler.java b/libreplan-webapp/src/main/java/org/libreplan/web/common/entrypoints/EntryPointsHandler.java index f190f8b41..e2a586442 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/common/entrypoints/EntryPointsHandler.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/common/entrypoints/EntryPointsHandler.java @@ -39,6 +39,7 @@ import javax.servlet.http.HttpServletRequest; import org.apache.commons.lang.Validate; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.libreplan.web.common.Util; import org.libreplan.web.common.converters.IConverter; import org.libreplan.web.common.converters.IConverterFactory; import org.zkoss.zk.ui.Execution; @@ -283,20 +284,25 @@ public class EntryPointsHandler { return applyIfMatches(controller, matrixParams); } - private boolean applyIfMatches(S controller, + private boolean applyIfMatches(final S controller, Map matrixParams) { flagAlreadyExecutedInThisRequest(); Set matrixParamsNames = matrixParams.keySet(); for (Entry entry : metadata.entrySet()) { - EntryPointMetadata entryPointMetadata = entry.getValue(); + final EntryPointMetadata entryPointMetadata = entry.getValue(); EntryPoint entryPointAnnotation = entryPointMetadata.annotation; HashSet requiredParams = new HashSet(Arrays .asList(entryPointAnnotation.value())); if (matrixParamsNames.equals(requiredParams)) { - Object[] arguments = retrieveArguments(matrixParams, + final Object[] arguments = retrieveArguments(matrixParams, entryPointAnnotation, entryPointMetadata.method .getParameterTypes()); - callMethod(controller, entryPointMetadata.method, arguments); + Util.executeIgnoringCreationOfBindings(new Runnable() { + public void run() { + callMethod(controller, entryPointMetadata.method, + arguments); + } + }); return true; } } diff --git a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java index 9cc20d977..1f5784e77 100644 --- a/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java +++ b/libreplan-webapp/src/main/java/org/libreplan/web/planner/tabs/MultipleTabsPlannerController.java @@ -471,7 +471,6 @@ public class MultipleTabsPlannerController implements Composer, @Override public void goToOrdersList() { - // ordersTab.show(); getTabsRegistry().show(ordersTab); }