Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ public void apply(FaceletContext ctx, UIComponent parent) throws IOException {

boolean replaying = replayCount != NO_REPLAY;

if (replaying && srcVE != null) {
IterationBaseCache.put(ctx.getFacesContext(), srcVE, src);
}

Integer sO = begin != null ? s : null;
Integer eO = end != null ? e : null;
Integer mO = step != null ? m : null;
Expand Down Expand Up @@ -280,10 +284,7 @@ public void apply(FaceletContext ctx, UIComponent parent) throws IOException {
int decisionsBeforeBody = 0;
int unreproducibleMarksBeforeBody = 0;
if (indexable) {
recordDecisions(ctx, srcVE, src, s, e, m, t);
if (replaying) {
recordIterationCount(ctx, srcVE, s, e, m, replayCount);
}
recordDecisions(ctx, srcVE, src, s, e, m, t, replayCount);
stateKey = ITERATION_STATE + System.identityHashCode(parent) + ':' + tag.getLocation();
range = new int[] { s, e, m };
if (facesContext.getCurrentPhaseId() == PhaseId.RESTORE_VIEW) {
Expand Down Expand Up @@ -558,9 +559,13 @@ private RestoredItemValueExpression restoredVarExpr(ValueExpression srcVE, Objec
* iterate the same way and is performed when it would not (see {@code refreshTransientBuildOnPSS}). The item
* sequence takes a decision of its own, since it is a comparison over every element rather than a single value;
* the body's own decisions stand beside it, each captured with this iteration's index-based var expression and
* therefore reading its own element live.
* therefore reading its own element live. A replaying build iterates the saved count, which the items may no longer
* yield. The decision therefore also checks that the current items yield the saved count. If they do not, the render
* time re-apply runs. A begin/end range needs no such check: its count follows from the range, which is recorded
* already.
*/
private void recordDecisions(FaceletContext ctx, ValueExpression srcVE, Object src, int begin, int end, int step, boolean tranzient) {
private void recordDecisions(FaceletContext ctx, ValueExpression srcVE, Object src, int begin, int end, int step, boolean tranzient,
int replayCount) {
recordBuildTimeDecision(ctx, this.begin, Integer.class, begin);
recordBuildTimeDecision(ctx, this.end, Integer.class, end);
recordBuildTimeDecision(ctx, this.step, Integer.class, step);
Expand All @@ -571,20 +576,15 @@ private void recordDecisions(FaceletContext ctx, ValueExpression srcVE, Object s
if (srcVE != null) {
ELContext elContext = ctx.getFacesContext().getELContext();
List<Object> recordedItems = snapshotElements(src);
recordBuildTimeDecision(ctx, () -> sameItems(recordedItems, srcVE.getValue(elContext)), Boolean.TRUE);
recordBuildTimeDecision(ctx, () -> {
Object currentItems = srcVE.getValue(elContext);
return sameItems(recordedItems, currentItems) && reproducesCount(currentItems, begin, end, step, replayCount);
}, Boolean.TRUE);
}
}

/**
* Records that this build produced the iterations the build which rendered the view produced rather than the ones
* the items hold now, so that the render time re-apply which would produce another count is performed rather than
* skipped. A begin/end range needs none of this: its count follows from the range, which is recorded already.
*/
private void recordIterationCount(FaceletContext ctx, ValueExpression srcVE, int begin, int end, int step, int count) {
if (srcVE != null) {
ELContext elContext = ctx.getFacesContext().getELContext();
recordBuildTimeDecision(ctx, () -> iterationCount(srcVE.getValue(elContext), begin, end, step), count);
}
private static boolean reproducesCount(Object items, int begin, int end, int step, int replayCount) {
return replayCount == NO_REPLAY || iterationCount(items, begin, end, step) == replayCount;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,7 @@ static Object getValue(ELContext elContext, ValueExpression orig) {
return orig.getValue(elContext);
}

Map<Object, Object> attributes = context.getAttributes();
IterationBaseCache cache = (IterationBaseCache) attributes.get(KEY);
if (cache == null || cache.phase != phase) {
cache = new IterationBaseCache(phase);
attributes.put(KEY, cache);
}

IterationBaseCache cache = forPhase(context, phase);
Object base = cache.bases.get(orig);
if (base == null) {
base = orig.getValue(elContext);
Expand All @@ -79,4 +73,23 @@ static Object getValue(ELContext elContext, ValueExpression orig) {
}
return base == NULL ? null : base;
}

/**
* Holds the value of {@code orig} which the caller has just resolved, for the rest of the current phase. Callers
* must only use this while restoring a view, where nothing can replace the items before the rows read them. A
* {@code preRenderView} listener can replace them between the build and the rendering.
*/
static void put(FacesContext context, ValueExpression orig, Object base) {
forPhase(context, context.getCurrentPhaseId()).bases.put(orig, base == null ? NULL : base);
}

private static IterationBaseCache forPhase(FacesContext context, PhaseId phase) {
Map<Object, Object> attributes = context.getAttributes();
IterationBaseCache cache = (IterationBaseCache) attributes.get(KEY);
if (cache == null || cache.phase != phase) {
cache = new IterationBaseCache(phase);
attributes.put(KEY, cache);
}
return cache;
}
}
19 changes: 8 additions & 11 deletions impl/src/main/java/jakarta/faces/component/UIComponentBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -265,15 +265,11 @@ static boolean isStandardComponent(UIComponent component) {
transient boolean restoredFullState;

/**
* Encode-scoped cache of {@link #isRendered()}. Within a single component encode the renderer's
* {@code shouldEncode}, {@code encodeChildren} and {@code encodeEnd} all re-consult {@code isRendered()},
* each otherwise re-reading {@code rendered} from the StateHelper. {@link #encodeBegin} computes it once and
* holds it here for the {@code encodeBegin}..{@code encodeEnd} window (covering both the {@code encodeAll} and
* renderer {@code encodeRecursive} paths, which both flow through those methods); {@link #encodeEnd} clears it.
* {@code rendered} cannot change mid-encode, so the cached value is authoritative for that window. It is
* recomputed fresh on every {@code encodeBegin}, so an encode that aborts before {@code encodeEnd} cannot leave
* a stale value in effect. {@code null} means "not currently encoding" — reads fall through to the StateHelper.
* Transient: never part of view state.
* Encode-scoped cache of {@link #isRendered()}, filled by {@code encodeBegin} and cleared by {@code encodeEnd}.
* Within a single component encode the renderer's {@code shouldEncode}, {@code encodeChildren} and {@code encodeEnd}
* all re-consult {@code isRendered()}, each otherwise re-reading {@code rendered} from the StateHelper. An encode
* which aborts before {@code encodeEnd} leaves the value behind. {@code null} means "not currently encoding": reads
* fall through to the StateHelper. Transient: never part of view state.
*/
private transient Boolean cachedIsRendered;

Expand Down Expand Up @@ -666,8 +662,9 @@ public void encodeBegin(FacesContext context) throws IOException {
pushComponentToEL(context, null);

// Memoize rendered for this component's encode pass: the renderer's shouldEncode, encodeChildren and
// encodeEnd re-consult isRendered() and would otherwise re-read the StateHelper each time. Recomputed fresh
// here (rather than relying on a prior clear) so an encode that aborted before encodeEnd leaves no stale value.
// encodeEnd re-consult isRendered() and would otherwise re-read the StateHelper each time.
// This evaluation cannot reuse the check in encodeAll(), which ran before pushComponentToEL: #{component} is the
// parent there and this component here, so both can answer differently.
cachedIsRendered = isRendered();
if (!cachedIsRendered) {
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright (c) 2026 Contributors to the Eclipse Foundation.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
* version 2 with the GNU Classpath Exception, which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
*/

package com.sun.faces.facelets.tag.jstl.core;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.List;

import org.junit.jupiter.api.Test;

import com.sun.faces.junit.JUnitFacesTestCaseBase;

import jakarta.el.ELContext;
import jakarta.el.ValueExpression;
import jakarta.faces.event.PhaseId;

class IterationBaseCacheTest extends JUnitFacesTestCaseBase {

private final ValueExpression items = mock(ValueExpression.class);
private final ELContext elContext = mock(ELContext.class);

@Test
void aValueAlreadyResolvedIsNotResolvedAgainInTheSamePhase() {
facesContext.setCurrentPhaseId(PhaseId.RESTORE_VIEW);
List<String> resolved = List.of("a");

IterationBaseCache.put(facesContext, items, resolved);

assertEquals(resolved, IterationBaseCache.getValue(elContext, items));
verify(items, never()).getValue(any(ELContext.class));
}

@Test
void aResolvedNullIsNotResolvedAgainInTheSamePhase() {
facesContext.setCurrentPhaseId(PhaseId.RESTORE_VIEW);

IterationBaseCache.put(facesContext, items, null);

assertNull(IterationBaseCache.getValue(elContext, items));
verify(items, never()).getValue(any(ELContext.class));
}

@Test
void aValueResolvedInAnEarlierPhaseIsResolvedAgain() {
facesContext.setCurrentPhaseId(PhaseId.RESTORE_VIEW);
IterationBaseCache.put(facesContext, items, List.of("old"));
List<String> current = List.of("new");
when(items.getValue(elContext)).thenReturn(current);

facesContext.setCurrentPhaseId(PhaseId.RENDER_RESPONSE);

assertEquals(current, IterationBaseCache.getValue(elContext, items));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright (c) 2026 Contributors to the Eclipse Foundation.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
* version 2 with the GNU Classpath Exception, which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
*/

package jakarta.faces.component;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.List;

import org.junit.jupiter.api.Test;

import com.sun.faces.junit.JUnitFacesTestCaseBase;

import jakarta.el.ELContext;
import jakarta.el.ValueExpression;
import jakarta.faces.context.FacesContext;
import jakarta.faces.render.Renderer;

/**
* An expression in the <code>rendered</code> attribute can read <code>#{component}</code>, which is the parent during the
* check of {@link UIComponent#encodeAll(FacesContext)} and the component itself during
* {@link UIComponentBase#encodeBegin(FacesContext)}. Both evaluations count, so neither may reuse the result of the other.
* A cache of {@link UIComponent#isRendered()} in {@link UIComponent#encodeAll(FacesContext)} therefore fails this test.
*/
class UIComponentBaseEncodeAllTest extends JUnitFacesTestCaseBase {

@Test
@SuppressWarnings("unchecked")
void encodeBeginEvaluatesRenderedAgainWithTheComponentOnTheElStack() throws Exception {
Renderer<UIComponent> renderer = mock(Renderer.class);
UIOutput component = new UIOutput() {
@Override
protected Renderer<?> getRenderer(FacesContext context) {
return renderer;
}
};
UIOutput parent = new UIOutput();
List<UIComponent> currentComponents = new ArrayList<>();
ValueExpression rendered = mock(ValueExpression.class);
when(rendered.getValue(any(ELContext.class))).thenAnswer(invocation -> {
UIComponent current = UIComponent.getCurrentComponent(facesContext);
currentComponents.add(current);
return current != component;
});
component.setRendererType("test");
component.setValueExpression("rendered", rendered);

parent.pushComponentToEL(facesContext, null);
component.encodeAll(facesContext);

assertEquals(List.of(parent, component), currentComponents);
verify(renderer, never()).encodeBegin(facesContext, component);
}

}
Loading