[ICE-7185] ice:dataTable getters called for unrendered panelTabs - ICEsoft JIRA Issue Tracker

Details

Type: Improvement
Status: Closed
Priority: Major
Resolution: Fixed
Affects Version/s: EE-2.0.0.GA
Fix Version/s: 2.1-Beta, EE-2.0.0.GA_P01
Component/s: ICE-Components
Labels:
None
Environment:
-

Assignee Priority:
P1

Description

Consider the following source code (BeanA and BeanB source is the same):

@SessionScoped
public class BeanA implements Serializable
{
@PostConstruct
public void init(){
System.out.println("A Created ");
}

public List<String> getA(){
return Arrays.asList("A", "A");
}
}

and the view:

<ice:panelTabSet>
<ice:panelTab label="1">
</ice:panelTab>
<ice:panelTab label="2">
<ice:dataTable value="#{beanB.b}" var="_b">
<ice:column>
<ice:outputText value="#{_b}" />
</ice:column>
</ice:dataTable>
</ice:panelTab>
<ice:panelTab label="3">
<ice:dataTable value="#{beanA.a}" var="_a">
<ice:column>
<ice:outputText value="#{_a}" />
</ice:column>
</ice:dataTable>
</ice:panelTab>
</ice:panelTabSet>

If you navigate to tab 2, the first bean´s init method will be called ("B Created"). Then navigate to tab 3 and you will see the same for A ("A created"). If you swap out the h:dataTable/h:column and replace it with it´s ice equivalents and retry the above test, both B and A are created at the same time when you click on tab 2 (("B Created" & "A Created"). Having all getters called can hurt performance since they may contain expensive logic such as DB accesses, etc..

Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

Hide
sc10442.war

05/Sep/11 5:43 AM

7.70 MB

Tyler Johnson
META-INF/MANIFEST.MF 0.0 kB

ICEfacesPage1.xhtml 1 kB

WEB-INF/classes/BeanA.java 0.5 kB

WEB-INF/classes/BeanB.java 0.5 kB

WEB-INF/classes/BeanA.class 1 kB

WEB-INF/classes/BeanB.class 1 kB

WEB-INF/faces-config.xml 0.3 kB

WEB-INF/lib/icefaces-ee-ext.jar 56 kB

WEB-INF/lib/icefaces-ee.jar 206 kB

WEB-INF/lib/jsf-api.jar 593 kB

WEB-INF/lib/jsf-impl.jar 1.78 MB

WEB-INF/lib/commons-logging.jar 52 kB

WEB-INF/lib/icefaces-ee-compat.jar 2.67 MB

WEB-INF/lib/jxl.jar 708 kB

WEB-INF/.../krysalis-jCharts-1.0.0-alpha-1.jar 151 kB

WEB-INF/lib/icefaces-ee-ace.jar 2.39 MB

WEB-INF/web.xml 3 kB

index.jsp 0.1 kB

Download Zip
Show

sc10442.war

05/Sep/11 5:43 AM

7.70 MB

Tyler Johnson

screenshot-01.png

185 kB

07/Sep/11 5:02 PM

Issue Links

is duplicated by

ICE-7314 Backing bean called on children of non-rendered elements

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Tyler Johnson added a comment - 05/Sep/11 7:16 AM

Mark Collette said:

Ok, basically the problem is that we've got an incomplete implementation of visitTree, which assumes that it needs to iterate over the dataTable rows, when it should not since we're only doing state restoration, that causes the side effect of loading the dataTable's DataModel and thus loading the beans. Nils has already solved this in the ACE dataTable by adding this guard:

public boolean visitTree(VisitContext context, VisitCallback callback) {
boolean ret = false;
if (this.isVisitable(context)) {
boolean visitRows = requiresRowIteration(context); // *** Here ***

int savedIndex = -1;
if (visitRows)

{ // *** Here *** savedIndex = getRowIndex(); setRowIndex(-1); }

...
if (visitRows) setRowIndex(savedIndex); // *** Here ***
...
}

private boolean requiresRowIteration(VisitContext ctx) {
try

{ // Use JSF 2.1 hints if available return !ctx.getHints().contains(VisitHint.SKIP_ITERATION); }

catch (NoSuchFieldError e)

{ FacesContext fctx = FacesContext.getCurrentInstance(); return (!PhaseId.RESTORE_VIEW.equals(fctx.getCurrentPhaseId())); }

}

Show

Tyler Johnson added a comment - 05/Sep/11 7:16 AM Mark Collette said: Ok, basically the problem is that we've got an incomplete implementation of visitTree, which assumes that it needs to iterate over the dataTable rows, when it should not since we're only doing state restoration, that causes the side effect of loading the dataTable's DataModel and thus loading the beans. Nils has already solved this in the ACE dataTable by adding this guard: public boolean visitTree(VisitContext context, VisitCallback callback) { boolean ret = false; if (this.isVisitable(context)) { boolean visitRows = requiresRowIteration(context); // *** Here *** int savedIndex = -1; if (visitRows) { // *** Here *** savedIndex = getRowIndex(); setRowIndex(-1); } ... if (visitRows) setRowIndex(savedIndex); // *** Here *** ... } private boolean requiresRowIteration(VisitContext ctx) { try { // Use JSF 2.1 hints if available return !ctx.getHints().contains(VisitHint.SKIP_ITERATION); } catch (NoSuchFieldError e) { FacesContext fctx = FacesContext.getCurrentInstance(); return (!PhaseId.RESTORE_VIEW.equals(fctx.getCurrentPhaseId())); } }

Hide

Permalink

yip.ng added a comment - 07/Sep/11 4:57 PM - edited

The above suggested code changes were already in UIData originally. We just removed them in UISeries and introduced the bug. These can be reinstated by copying visitTree() from UISeries into HtmlDataTable and reinstating the original code.

However, this copying produces other problems. Four methods used by visitTree() are private to UISeries. See screenshot 1. Do we then copy them as well? (And others used by them, and so on.) Two of them (doVisitChildren and visitFacets) can be reverted similarly back to the UIData versions with checking of visitRows. The other two (visitColumnFacets and visitColumnsAndRows) have more non-trivial changes to the corresponding versions in UIData (but differently named) and need more understanding.

Or do we want to make changes directly in UISeries?

And with these changes, the code is reverting back to be more and more like the orignal UIData visitTree logic now.

Show

yip.ng added a comment - 07/Sep/11 4:57 PM - edited The above suggested code changes were already in UIData originally. We just removed them in UISeries and introduced the bug. These can be reinstated by copying visitTree() from UISeries into HtmlDataTable and reinstating the original code. However, this copying produces other problems. Four methods used by visitTree() are private to UISeries. See screenshot 1. Do we then copy them as well? (And others used by them, and so on.) Two of them (doVisitChildren and visitFacets) can be reverted similarly back to the UIData versions with checking of visitRows. The other two (visitColumnFacets and visitColumnsAndRows) have more non-trivial changes to the corresponding versions in UIData (but differently named) and need more understanding. Or do we want to make changes directly in UISeries? And with these changes, the code is reverting back to be more and more like the orignal UIData visitTree logic now.

Hide

Permalink

Ken Fyten added a comment - 07/Sep/11 5:21 PM

What was the justification for making these UISeries changes originally?

Show

Ken Fyten added a comment - 07/Sep/11 5:21 PM What was the justification for making these UISeries changes originally?

Hide

Permalink

yip.ng added a comment - 07/Sep/11 5:59 PM

~~ICE-5815~~. According to my comments there, visitTree() and related methods were just copied from UIData. Different version of UIData than now (Mojarra 2.1.2)?

Show

yip.ng added a comment - 07/Sep/11 5:59 PM ICE-5815 . According to my comments there, visitTree() and related methods were just copied from UIData. Different version of UIData than now (Mojarra 2.1.2)?

Hide

Permalink

Ken Fyten added a comment - 07/Sep/11 6:02 PM

Then I'd suggest we use the current code from Mojarra 2.1.2 that should resolve this issue.

Show

Ken Fyten added a comment - 07/Sep/11 6:02 PM Then I'd suggest we use the current code from Mojarra 2.1.2 that should resolve this issue.

Hide

Permalink

yip.ng added a comment - 09/Sep/11 3:24 PM - edited

Looks like old UIData code was copied from Mojarra 2.0.2. Suggested fix (by Mark above) started to appear in 2.0.3.

Checked to make sure no other changes made to copied code since ~~ICE-5815~~.

Re-copied code from latest Mojarra version (2.1.3). Fix automatically included and this JIRA's issue automatically solved. Tested with Tyler-supplied test app., transplanted into compat showcase.

Re-applied fix for ~~ICE-5815~~. Tested with test case mentioned in ~~ICE-5790~~: http://server:8888/svn/repo/qa/trunk/Regression-Icefaces2/Nightly/ICE-3057

Revision: 25440

Modified : /icefaces2/trunk/icefaces/compat/components/src/main/java/com/icesoft/faces/component/panelseries/UISeries.java

Revision: 25441

Modified : /icefaces2/branches/icefaces-2.0.x-maintenance/icefaces/compat/components/src/main/java/com/icesoft/faces/component/panelseries/UISeries.java

Show

yip.ng added a comment - 09/Sep/11 3:24 PM - edited Looks like old UIData code was copied from Mojarra 2.0.2. Suggested fix (by Mark above) started to appear in 2.0.3. Checked to make sure no other changes made to copied code since ICE-5815 . Re-copied code from latest Mojarra version (2.1.3). Fix automatically included and this JIRA's issue automatically solved. Tested with Tyler-supplied test app., transplanted into compat showcase. Re-applied fix for ICE-5815 . Tested with test case mentioned in ICE-5790 : http://server:8888/svn/repo/qa/trunk/Regression-Icefaces2/Nightly/ICE-3057 Revision: 25440 Modified : /icefaces2/trunk/icefaces/compat/components/src/main/java/com/icesoft/faces/component/panelseries/UISeries.java Revision: 25441 Modified : /icefaces2/branches/icefaces-2.0.x-maintenance/icefaces/compat/components/src/main/java/com/icesoft/faces/component/panelseries/UISeries.java

Hide

Permalink

Nicklas Karlsson added a comment - 12/Sep/11 12:42 AM

Do you think there could be a related issue with the rendering of the tree component? I see getters being called for a tree that's on another tab (and having rendered evaluated to false at that point)

Show

Nicklas Karlsson added a comment - 12/Sep/11 12:42 AM Do you think there could be a related issue with the rendering of the tree component? I see getters being called for a tree that's on another tab (and having rendered evaluated to false at that point)

Hide

Permalink

Ken Fyten added a comment - 13/Sep/11 2:40 PM

Nicklas, since panelTabSet uses UISeries for dynamic tabs, it's possible this fix might help you also. If not, please create a new JIRA.

Show

Ken Fyten added a comment - 13/Sep/11 2:40 PM Nicklas, since panelTabSet uses UISeries for dynamic tabs, it's possible this fix might help you also. If not, please create a new JIRA.

Hide

Permalink

Nicklas Karlsson added a comment - 06/Oct/11 3:05 AM

It appears as there has been a regression regarding f:ajax usage introduced with this issues.

Given a bean like


@Named 
@SessionScoped 
public class Bean implements Serializable 
{ 
	private String s; 

	public void change(ValueChangeEvent e) 
	{ 
		System.out.println(e); 
	} 

	public String getS() 
	{ 
		return s; 
	} 

	public void setS(String s) 
	{ 
		this.s = s; 
	} 
}

and a view like

<?xml version="1.0" encoding="UTF-8"?> 
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" 
	"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> 
<html xmlns="http://www.w3.org/1999/xhtml" 
	xmlns:h="http://java.sun.com/jsf/html" 
	xmlns:f="http://java.sun.com/jsf/core" 
	xmlns:ice="http://www.icesoft.com/icefaces/component" 
	xmlns:ui="http://java.sun.com/jsf/facelets" > 
<f:view contentType="text/html"> 
<h:head> 
	<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /> 
	<ice:outputStyle href="./xmlhttp/css/rime/rime.css"/> 
	<title>ICEfaces</title> 
</h:head> 
<h:body> 
	<h:form> 
		<ice:panelTabSet> 
			<ice:panelTab label="tab"> 
				<h:selectOneMenu value="#{bean.s}" valueChangeListener="#{bean.change}"> 
					<f:selectItem noSelectionOption="true" itemLabel=""/> 
					<f:selectItem itemValue="a"/> 
					<f:selectItem itemValue="b"/> 
					<f:ajax event="change" execute="@this" render="@form"/> 
				</h:selectOneMenu> 
			</ice:panelTab> 
		</ice:panelTabSet> 
	</h:form> 
</h:body> 
</f:view> 
</html>

the eventlistener is never fired, values are not updated in the model etc.

Moving the f:selectOneMenu out of the panelTabSet makes it work again.

Show

Nicklas Karlsson added a comment - 06/Oct/11 3:05 AM It appears as there has been a regression regarding f:ajax usage introduced with this issues. Given a bean like @Named @SessionScoped public class Bean implements Serializable { private String s; public void change(ValueChangeEvent e) { System .out.println(e); } public String getS() { return s; } public void setS( String s) { this .s = s; } } and a view like <?xml version= "1.0" encoding= "UTF-8" ?> <!DOCTYPE html PUBLIC "- //W3C//DTD XHTML 1.0 Transitional//EN" "http: //www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd" > <html xmlns= "http: //www.w3.org/1999/xhtml" xmlns:h= "http: //java.sun.com/jsf/html" xmlns:f= "http: //java.sun.com/jsf/core" xmlns:ice= "http: //www.icesoft.com/icefaces/component" xmlns:ui= "http: //java.sun.com/jsf/facelets" > <f:view contentType= "text/html" > <h:head> <meta http-equiv= "Content-Type" content= "text/html; charset=UTF-8" /> <ice:outputStyle href= "./xmlhttp/css/rime/rime.css" /> <title>ICEfaces</title> </h:head> <h:body> <h:form> <ice:panelTabSet> <ice:panelTab label= "tab" > <h:selectOneMenu value= "#{bean.s}" valueChangeListener= "#{bean.change}" > <f:selectItem noSelectionOption= " true " itemLabel=""/> <f:selectItem itemValue= "a" /> <f:selectItem itemValue= "b" /> <f:ajax event= "change" execute= "@ this " render= "@form" /> </h:selectOneMenu> </ice:panelTab> </ice:panelTabSet> </h:form> </h:body> </f:view> </html> the eventlistener is never fired, values are not updated in the model etc. Moving the f:selectOneMenu out of the panelTabSet makes it work again.

People

Assignee:

Mark Collette

Reporter:

Tyler Johnson

Votes:

0 Vote for this issue

Watchers:

2 Start watching this issue

Dates

Created:

05/Sep/11 5:26 AM

Updated:

17/Feb/12 5:16 PM

Resolved:

09/Sep/11 4:12 PM