From 377c749c7747084e48f230900ff4b7bca123e1c7 Mon Sep 17 00:00:00 2001 From: Bauke Scholtz Date: Sun, 26 Nov 2023 14:39:17 -0400 Subject: [PATCH] Remove jakarta.faces.visit.SKIP_ITERATION https://github.com/eclipse-ee4j/mojarra/issues/5355 --- .../FaceletFullStateManagementStrategy.java | 141 ++++++++---------- ...FaceletPartialStateManagementStrategy.java | 110 +++++++------- .../application/view/FormOmittedChecker.java | 31 ++-- .../sun/faces/lifecycle/RestoreViewPhase.java | 18 +-- 4 files changed, 128 insertions(+), 172 deletions(-) diff --git a/impl/src/main/java/com/sun/faces/application/view/FaceletFullStateManagementStrategy.java b/impl/src/main/java/com/sun/faces/application/view/FaceletFullStateManagementStrategy.java index 96b7b8be92..4b50125348 100644 --- a/impl/src/main/java/com/sun/faces/application/view/FaceletFullStateManagementStrategy.java +++ b/impl/src/main/java/com/sun/faces/application/view/FaceletFullStateManagementStrategy.java @@ -82,7 +82,7 @@ public class FaceletFullStateManagementStrategy extends StateManagementStrategy /** * Stores the skip hint. */ - private static final String SKIP_ITERATION_HINT = "jakarta.faces.visit.SKIP_ITERATION"; + private static final Set SKIP_ITERATION_HINT = EnumSet.of(SKIP_ITERATION); /** * Stores the class map. @@ -185,41 +185,34 @@ private UIComponent locateComponentByClientId(final FacesContext context, final final List found = new ArrayList<>(); UIComponent result = null; - try { - context.getAttributes().put(SKIP_ITERATION_HINT, true); - Set hints = EnumSet.of(VisitHint.SKIP_ITERATION); - - VisitContext visitContext = VisitContext.createVisitContext(context, null, hints); - subTree.visitTree(visitContext, (visitContext1, component) -> { - VisitResult result1 = ACCEPT; - if (component.getClientId(visitContext1.getFacesContext()).equals(clientId)) { - /* - * If the client id matches up we have found our match. - */ - found.add(component); - result1 = COMPLETE; - } else if (component instanceof UIForm) { - /* - * If the component is a UIForm and it is prepending its id then we can short circuit out of here if the the client id - * of the component we are trying to find does not begin with the id of the UIForm. - */ - UIForm form = (UIForm) component; - if (form.isPrependId() && !clientId.startsWith(form.getClientId(visitContext1.getFacesContext()))) { - result1 = REJECT; - } - } else if (component instanceof NamingContainer && !clientId.startsWith(component.getClientId(visitContext1.getFacesContext()))) { - /* - * If the component is a naming container then assume it is prepending its id so if our client id we are looking for - * does not start with the naming container id we can skip visiting this tree. - */ + VisitContext visitContext = VisitContext.createVisitContext(context, null, SKIP_ITERATION_HINT); + subTree.visitTree(visitContext, (visitContext1, component) -> { + VisitResult result1 = ACCEPT; + if (component.getClientId(visitContext1.getFacesContext()).equals(clientId)) { + /* + * If the client id matches up we have found our match. + */ + found.add(component); + result1 = COMPLETE; + } else if (component instanceof UIForm) { + /* + * If the component is a UIForm and it is prepending its id then we can short circuit out of here if the the client id + * of the component we are trying to find does not begin with the id of the UIForm. + */ + UIForm form = (UIForm) component; + if (form.isPrependId() && !clientId.startsWith(form.getClientId(visitContext1.getFacesContext()))) { result1 = REJECT; } + } else if (component instanceof NamingContainer && !clientId.startsWith(component.getClientId(visitContext1.getFacesContext()))) { + /* + * If the component is a naming container then assume it is prepending its id so if our client id we are looking for + * does not start with the naming container id we can skip visiting this tree. + */ + result1 = REJECT; + } - return result1; - }); - } finally { - context.getAttributes().remove(SKIP_ITERATION_HINT); - } + return result1; + }); if (!found.isEmpty()) { result = found.get(0); @@ -311,36 +304,30 @@ private void restoreComponentState(final FacesContext context, final HashMap hints = EnumSet.of(SKIP_ITERATION); - VisitContext visitContext = VisitContext.createVisitContext(context, null, hints); + VisitContext visitContext = VisitContext.createVisitContext(context, null, SKIP_ITERATION_HINT); - viewRoot.visitTree(visitContext, (visitContext1, component) -> { - VisitResult result = ACCEPT; + viewRoot.visitTree(visitContext, (visitContext1, component) -> { + VisitResult result = ACCEPT; - String clientId = component.getClientId(context); - Object stateObj = state.get(clientId); + String clientId = component.getClientId(context); + Object stateObj = state.get(clientId); - if (stateObj != null && !stateContext.componentAddedDynamically(component)) { - boolean restoreStateNow = true; - if (stateObj instanceof StateHolderSaver) { - restoreStateNow = !((StateHolderSaver) stateObj).componentAddedDynamically(); - } - if (restoreStateNow) { - try { - component.restoreState(context, stateObj); - } catch (Exception e) { - throw new FacesException(e); - } + if (stateObj != null && !stateContext.componentAddedDynamically(component)) { + boolean restoreStateNow = true; + if (stateObj instanceof StateHolderSaver) { + restoreStateNow = !((StateHolderSaver) stateObj).componentAddedDynamically(); + } + if (restoreStateNow) { + try { + component.restoreState(context, stateObj); + } catch (Exception e) { + throw new FacesException(e); } } + } - return result; - }); - } finally { - context.getAttributes().remove(SKIP_ITERATION_HINT); - } + return result; + }); } /** @@ -592,33 +579,27 @@ private Object saveComponentState(FacesContext context) { final UIViewRoot viewRoot = context.getViewRoot(); final FacesContext finalContext = context; - context.getAttributes().put(SKIP_ITERATION_HINT, true); - Set hints = EnumSet.of(SKIP_ITERATION); - VisitContext visitContext = VisitContext.createVisitContext(context, null, hints); + VisitContext visitContext = VisitContext.createVisitContext(context, null, SKIP_ITERATION_HINT); - try { - viewRoot.visitTree(visitContext, (context1, component) -> { - VisitResult result = ACCEPT; - Object stateObj; - if (!component.isTransient()) { - if (stateContext.componentAddedDynamically(component)) { - component.getAttributes().put(DYNAMIC_COMPONENT, new Integer(getProperChildIndex(component))); - stateObj = new StateHolderSaver(finalContext, component); - } else { - stateObj = component.saveState(finalContext); - } - if (stateObj != null) { - stateMap.put(component.getClientId(finalContext), stateObj); - } + viewRoot.visitTree(visitContext, (context1, component) -> { + VisitResult result = ACCEPT; + Object stateObj; + if (!component.isTransient()) { + if (stateContext.componentAddedDynamically(component)) { + component.getAttributes().put(DYNAMIC_COMPONENT, new Integer(getProperChildIndex(component))); + stateObj = new StateHolderSaver(finalContext, component); } else { - result = REJECT; + stateObj = component.saveState(finalContext); } + if (stateObj != null) { + stateMap.put(component.getClientId(finalContext), stateObj); + } + } else { + result = REJECT; + } - return result; - }); - } finally { - context.getAttributes().remove(SKIP_ITERATION_HINT); - } + return result; + }); return stateMap; } diff --git a/impl/src/main/java/com/sun/faces/application/view/FaceletPartialStateManagementStrategy.java b/impl/src/main/java/com/sun/faces/application/view/FaceletPartialStateManagementStrategy.java index 29e763978f..6faf35945e 100644 --- a/impl/src/main/java/com/sun/faces/application/view/FaceletPartialStateManagementStrategy.java +++ b/impl/src/main/java/com/sun/faces/application/view/FaceletPartialStateManagementStrategy.java @@ -68,10 +68,16 @@ public class FaceletPartialStateManagementStrategy extends StateManagementStrate * Stores the logger. */ private static final Logger LOGGER = FacesLogger.APPLICATION_VIEW.getLogger(); + /** * Stores the skip hint. */ - private static final String SKIP_ITERATION_HINT = "jakarta.faces.visit.SKIP_ITERATION"; + private static final Set SKIP_ITERATION_HINT = EnumSet.of(SKIP_ITERATION); + + /** + * Stores the skip and lifecycle hints. + */ + private static final Set SKIP_ITERATION_AND_EXECUTE_LIFECYCLE_HINTS = EnumSet.of(VisitHint.SKIP_ITERATION, VisitHint.EXECUTE_LIFECYCLE); /** * Constructor. @@ -102,41 +108,34 @@ private UIComponent locateComponentByClientId(final FacesContext context, final final List found = new ArrayList<>(); UIComponent result = null; - try { - context.getAttributes().put(SKIP_ITERATION_HINT, true); - Set hints = EnumSet.of(SKIP_ITERATION); - - VisitContext visitContext = VisitContext.createVisitContext(context, null, hints); - subTree.visitTree(visitContext, (visitContext1, component) -> { - VisitResult result1 = ACCEPT; - if (component.getClientId(visitContext1.getFacesContext()).equals(clientId)) { - /* - * If the client id matches up we have found our match. - */ - found.add(component); - result1 = COMPLETE; - } else if (component instanceof UIForm) { - /* - * If the component is a UIForm and it is prepending its id then we can short circuit out of here if the the client id - * of the component we are trying to find does not begin with the id of the UIForm. - */ - UIForm form = (UIForm) component; - if (form.isPrependId() && !clientId.startsWith(form.getClientId(visitContext1.getFacesContext()))) { - result1 = REJECT; - } - } else if (component instanceof NamingContainer && !clientId.startsWith(component.getClientId(visitContext1.getFacesContext()))) { - /* - * If the component is a naming container then assume it is prepending its id so if our client id we are looking for - * does not start with the naming container id we can skip visiting this tree. - */ + VisitContext visitContext = VisitContext.createVisitContext(context, null, SKIP_ITERATION_HINT); + subTree.visitTree(visitContext, (visitContext1, component) -> { + VisitResult result1 = ACCEPT; + if (component.getClientId(visitContext1.getFacesContext()).equals(clientId)) { + /* + * If the client id matches up we have found our match. + */ + found.add(component); + result1 = COMPLETE; + } else if (component instanceof UIForm) { + /* + * If the component is a UIForm and it is prepending its id then we can short circuit out of here if the the client id + * of the component we are trying to find does not begin with the id of the UIForm. + */ + UIForm form = (UIForm) component; + if (form.isPrependId() && !clientId.startsWith(form.getClientId(visitContext1.getFacesContext()))) { result1 = REJECT; } + } else if (component instanceof NamingContainer && !clientId.startsWith(component.getClientId(visitContext1.getFacesContext()))) { + /* + * If the component is a naming container then assume it is prepending its id so if our client id we are looking for + * does not start with the naming container id we can skip visiting this tree. + */ + result1 = REJECT; + } - return result1; - }); - } finally { - context.getAttributes().remove(SKIP_ITERATION_HINT); - } + return result1; + }); if (!found.isEmpty()) { result = found.get(0); @@ -343,9 +342,7 @@ public UIViewRoot restoreView(FacesContext context, String viewId, String render try { stateContext.setTrackViewModifications(false); - context.getAttributes().put(SKIP_ITERATION_HINT, true); - Set hints = EnumSet.of(VisitHint.SKIP_ITERATION, VisitHint.EXECUTE_LIFECYCLE); - VisitContext visitContext = VisitContext.createVisitContext(context, null, hints); + VisitContext visitContext = VisitContext.createVisitContext(context, null, SKIP_ITERATION_AND_EXECUTE_LIFECYCLE_HINTS); viewRoot.visitTree(visitContext, (context1, target) -> { VisitResult result = VisitResult.ACCEPT; String cid = target.getClientId(context1.getFacesContext()); @@ -370,7 +367,6 @@ public UIViewRoot restoreView(FacesContext context, String viewId, String render restoreDynamicActions(context, stateContext, state); } finally { stateContext.setTrackViewModifications(true); - context.getAttributes().remove(SKIP_ITERATION_HINT); } } else { viewRoot = null; @@ -436,33 +432,27 @@ public Object saveView(FacesContext context) { final Map stateMap = new HashMap<>(); final StateContext stateContext = StateContext.getStateContext(context); - context.getAttributes().put(SKIP_ITERATION_HINT, true); - Set hints = EnumSet.of(VisitHint.SKIP_ITERATION); - VisitContext visitContext = VisitContext.createVisitContext(context, null, hints); + VisitContext visitContext = VisitContext.createVisitContext(context, null, SKIP_ITERATION_HINT); final FacesContext finalContext = context; - try { - viewRoot.visitTree(visitContext, (context1, target) -> { - VisitResult result = VisitResult.ACCEPT; - Object stateObj; - if (!target.isTransient()) { - if (stateContext.componentAddedDynamically(target)) { - target.getAttributes().put(DYNAMIC_COMPONENT, target.getParent().getChildren().indexOf(target)); - stateObj = new StateHolderSaver(finalContext, target); - } else { - stateObj = target.saveState(context1.getFacesContext()); - } - if (stateObj != null) { - stateMap.put(target.getClientId(context1.getFacesContext()), stateObj); - } + viewRoot.visitTree(visitContext, (context1, target) -> { + VisitResult result = VisitResult.ACCEPT; + Object stateObj; + if (!target.isTransient()) { + if (stateContext.componentAddedDynamically(target)) { + target.getAttributes().put(DYNAMIC_COMPONENT, target.getParent().getChildren().indexOf(target)); + stateObj = new StateHolderSaver(finalContext, target); } else { - return VisitResult.REJECT; + stateObj = target.saveState(context1.getFacesContext()); } - return result; - }); - } finally { - context.getAttributes().remove(SKIP_ITERATION_HINT); - } + if (stateObj != null) { + stateMap.put(target.getClientId(context1.getFacesContext()), stateObj); + } + } else { + return VisitResult.REJECT; + } + return result; + }); saveDynamicActions(context, stateContext, stateMap); StateContext.release(context); diff --git a/impl/src/main/java/com/sun/faces/application/view/FormOmittedChecker.java b/impl/src/main/java/com/sun/faces/application/view/FormOmittedChecker.java index 1375e70e9d..2463b52c39 100644 --- a/impl/src/main/java/com/sun/faces/application/view/FormOmittedChecker.java +++ b/impl/src/main/java/com/sun/faces/application/view/FormOmittedChecker.java @@ -16,6 +16,8 @@ package com.sun.faces.application.view; +import static jakarta.faces.component.visit.VisitHint.SKIP_ITERATION; + import java.util.EnumSet; import java.util.List; import java.util.Set; @@ -42,7 +44,7 @@ class FormOmittedChecker { /** * Stores the skip hint. */ - private static String SKIP_ITERATION_HINT = "jakarta.faces.visit.SKIP_ITERATION"; + private static final Set SKIP_ITERATION_HINT = EnumSet.of(SKIP_ITERATION); /** * Constructor. @@ -61,24 +63,17 @@ public static void check(FacesContext context) { List children = viewRoot.getChildren(); for (UIComponent child : children) { - try { - context.getAttributes().put(SKIP_ITERATION_HINT, true); - Set hints = EnumSet.of(VisitHint.SKIP_ITERATION); - - VisitContext visitContext = VisitContext.createVisitContext(context, null, hints); - child.visitTree(visitContext, (visitContext1, component) -> { - VisitResult result = VisitResult.ACCEPT; + VisitContext visitContext = VisitContext.createVisitContext(context, null, SKIP_ITERATION_HINT); + child.visitTree(visitContext, (visitContext1, component) -> { + VisitResult result = VisitResult.ACCEPT; - if (isForm(component)) { - result = VisitResult.REJECT; - } else if (isInNeedOfForm(component)) { - addFormOmittedMessage(finalContext, component); - } - return result; - }); - } finally { - context.getAttributes().remove(SKIP_ITERATION_HINT); - } + if (isForm(component)) { + result = VisitResult.REJECT; + } else if (isInNeedOfForm(component)) { + addFormOmittedMessage(finalContext, component); + } + return result; + }); } } diff --git a/impl/src/main/java/com/sun/faces/lifecycle/RestoreViewPhase.java b/impl/src/main/java/com/sun/faces/lifecycle/RestoreViewPhase.java index a869c08382..2e335c0190 100644 --- a/impl/src/main/java/com/sun/faces/lifecycle/RestoreViewPhase.java +++ b/impl/src/main/java/com/sun/faces/lifecycle/RestoreViewPhase.java @@ -25,6 +25,7 @@ import static com.sun.faces.util.MessageUtils.getExceptionMessageString; import static com.sun.faces.util.Util.getViewHandler; import static com.sun.faces.util.Util.isOneOf; +import static jakarta.faces.component.visit.VisitHint.SKIP_ITERATION; import static jakarta.faces.event.PhaseId.RESTORE_VIEW; import static jakarta.faces.render.ResponseStateManager.NON_POSTBACK_VIEW_TOKEN_PARAM; import static jakarta.faces.view.ViewMetadata.hasMetadata; @@ -80,7 +81,7 @@ public class RestoreViewPhase extends Phase { private static final String WEBAPP_ERROR_PAGE_MARKER = "jakarta.servlet.error.message"; private static final Logger LOGGER = FacesLogger.LIFECYCLE.getLogger(); - private static String SKIP_ITERATION_HINT = "jakarta.faces.visit.SKIP_ITERATION"; + private static final Set SKIP_ITERATION_HINT = EnumSet.of(SKIP_ITERATION); // ---------------------------------------------------------- Public Methods @@ -101,7 +102,7 @@ public void doPhase(FacesContext context, Lifecycle lifecycle, ListIterator - * + * * POSTCONDITION: The facesContext has been initialized with a tree. */ @@ -382,14 +383,8 @@ private void deliverPostRestoreStateEvent(FacesContext facesContext) throws Face UIViewRoot root = facesContext.getViewRoot(); PostRestoreStateEvent postRestoreStateEvent = new PostRestoreStateEvent(root); try { - // PENDING: This is included for those component frameworks that don't utilize the - // new VisitHint(s) yet - but still wish to know that they should be non-iterating - // during state saving. It should be removed at some point. - facesContext.getAttributes().put(SKIP_ITERATION_HINT, true); facesContext.getApplication().publishEvent(facesContext, PostRestoreStateEvent.class, root); - - Set hints = EnumSet.of(VisitHint.SKIP_ITERATION); - VisitContext visitContext = VisitContext.createVisitContext(facesContext, null, hints); + VisitContext visitContext = VisitContext.createVisitContext(facesContext, null, SKIP_ITERATION_HINT); root.visitTree(visitContext, (context, target) -> { postRestoreStateEvent.setComponent(target); target.processEvent(postRestoreStateEvent); @@ -401,11 +396,6 @@ private void deliverPostRestoreStateEvent(FacesContext facesContext) throws Face .publishEvent( facesContext, ExceptionQueuedEvent.class, new ExceptionQueuedEventContext(facesContext, e, null, PhaseId.RESTORE_VIEW)); - } finally { - // PENDING: This is included for those component frameworks that don't utilize the - // new VisitHint(s) yet - but still wish to know that they should be non-iterating - // during state saving. It should be removed at some point. - facesContext.getAttributes().remove(SKIP_ITERATION_HINT); } }