[JAVASERVERFACES-2283] System Events Example App: Dynamic Components Created: 06/Jan/12  Updated: 02/Nov/12  Resolved: 24/Jul/12

Status: Closed
Project: javaserverfaces
Component/s: None
Affects Version/s: 2.0.7, 2.1.6
Fix Version/s: 2.1.9, 2.2.0-m03

Type: Bug Priority: Major
Reporter: Manfred Riem Assignee: Manfred Riem
Resolution: Works as designed Votes: 6
Labels: None
Remaining Estimate: 0 minutes
Time Spent: 15 minutes
Original Estimate: Not Specified

Attachments: Zip Archive addressbook-faces2.zip     Java Archive File metawidget-all.jar    
Issue Links:
Related
is related to JAVASERVERFACES_SPEC_PUBLIC-1124 Clarify UIViewRoot.createUniqueId Closed
Tags: dynamic

 Description   

When using partial state saving, dynamic UISelectItem / UISelectItems are not restored upon postback. We need to investigate if UISelectItem and UISelectItems should be added to the list of dynamic actions in the first place. Note get the test application from issue 1826, the addressbook-faces2.war attachment.



 Comments   
Comment by kennardconsulting [ 14/Jan/12 ]

With respect to the latest JAR (javax.faces-2.1.7-20120112.181559-11.jar), this issue now appears to work in some cases. But not in others:

1. Go to http://localhost:8080/addressbook-faces2
2. Click 'Homer Simpson'
3. Click 'Edit'
4. Click 'Save'

Note the Gender UISelectItem has lost its values. Also you will see the field ordering has changed significantly.

Comment by kennardconsulting [ 23/Jan/12 ]

Manfred,

If you must close JAVASERVERFACES-1826 because you "wanted to make sure that these changes would get into the code base as there are a number of other issues dependent on this issue", can you at least mark issue JAVASERVERFACES-2283 as 'major' priority (just like 1826 was?)

I must stress that the issue as a whole (i.e. that dynamic component manipulation works in Mojarra) is not resolved. And it goes beyond just UISelectItems.

Regards,

Richard.

Comment by kennardconsulting [ 31/Jan/12 ]

The overarching problem is that partial state saving in Mojarra is broken for dynamic components. I've been whinging about this for many months across many issues (JAVASERVERFACES-1383, JAVASERVERFACES-1402, JAVASERVERFACES-1313, JAVASERVERFACES-1717). Others have noticed it too (JAVASERVERFACES-2231, JAVASERVERFACES-1414, JAVASERVERFACES-1972). It's a complicated issue with many symptoms and side effects. JAVASERVERFACES-1826 was my attempt to help you guys by providing a number of test cases.

However issue 1826 could never hope to cover all edge cases of your particular implementation

What has happened is that you've finished JAVASERVERFACES-1826 and closed it, and spun off one of the remaining issues into JAVASERVERFACES-2283. This new issue is marked minor. But it is not minor! It is still blocking anything from working reliably.

The issue of dynamic components in Mojarra is not solved

Could you please make sure this is understood? At the very least, I would want to see JAVASERVERFACES-2283 given a priority of 'high' (same as JAVASERVERFACES-1826 used to be). I think it should be renamed too. What has happened is we've moved on from 'here are a bunch of little acid tests, please make sure they work' to 'here is a small web app, please make sure it works'. And the small web app does not work. It has a number of problems, from missing UISelectItems to scrambled component ordering, which are not present under MyFaces.

I'm worried we are going to lose momentum. Even though much has been accomplished, all of it is for nothing unless everything works.

Comment by rogerk [ 13/Feb/12 ]

Change title and priority.

Comment by kennardconsulting [ 16/Mar/12 ]

Hi guys,

Has anybody had chance to look at this issue? Maybe to at least confirm they can reproduce it?

Regards,

Richard.

Comment by ova2 [ 13/Apr/12 ]

I agree, the issue with dynamic components is not solved. See also my issue http://java.net/jira/browse/JAVASERVERFACES-2383

Comment by kennardconsulting [ 08/May/12 ]

Can someone please comment on the timeline for this issue?

Comment by kennardconsulting [ 24/May/12 ]

Buoyed by the progress on JAVASERVERFACES-1414, I have revisited and updated the test application for this issue (attached).

Still no luck with javax.faces-2.1.9-20120524.180105-1.jar or javax.faces-2.2.0-20120518.034337-98.jar I'm afraid. Works fine with MyFaces.

Comment by Manfred Riem [ 22/Jun/12 ]

I tried to use the attached project, but I can't seem to build it. It appears a dependency is missing?

Comment by kennardconsulting [ 23/Jun/12 ]

Hi Manfred,

I just tried this again and it seems okay to me? I...

1. Downloaded and unzipped the attached addressbook-faces2.zip
2. Opened Eclipse (with m2e installed)
3. Used Import... Existing Maven Projects
4. Published to Tomcat 7.0.25

...and it ran okay? What dependency are you seeing missing?

Note the attachment includes libraries in a WEB-INF/lib. It also includes MyFaces (so that you can see it working). You will need to manually swap in Mojarra.

Regards,

Richard.

Comment by Manfred Riem [ 26/Jun/12 ]

This is what I get:

[compiler:compile]
-------------------------------------------------------------
COMPILATION ERROR :
-------------------------------------------------------------
org/metawidget/example/shared/addressbook/util/ExampleUtils.java:[23,26] error: package org.metawidget.util does not exist
org/metawidget/example/shared/addressbook/model/Contact.java:[19,38] error: package org.metawidget.inspector does not exist
org/metawidget/example/shared/addressbook/model/Contact.java:[26,42] error: package org.metawidget.inspector.annotation does not exist

Comment by kennardconsulting [ 27/Jun/12 ]

Hi Manfred,

Thanks for trying to reproduce this issue. Sorry for the confusion!

The JAR you are missing is located inside the src/main/webapp/WEB-INF/lib of the ZIP. So you need that folder on your CLASSPATH. If you use Eclipse (with m2e installed) this all happens automatically. You can set up a server in the Servers tab and deploy the project to it automatically. You won't have to run Maven directly.

I did it this way (rather than putting all dependencies in the pom.xml) because I had to modify metawidget-all.jar slightly. The normal metawidget-all.jar has an explicit check to warn users and stop it running with Mojarra (until bug 2283 is fixed).

Regards,

Richard.

Comment by Manfred Riem [ 03/Jul/12 ]

Is it possible to remove those dependencies? I want to make sure we can integrate this test into our test harness so we don't experience any regressions on this!

Comment by kennardconsulting [ 04/Jul/12 ]

Manfred,

Apologies for the confusion.

I did go through an exercise removing dependencies and simulating scenarios in order to produce regression tests. I simulated 8 scenarios that way (see JAVASERVERFACES-1826). The problem is my simulated scenarios cannot anticipate all implementation details of your particular Mojarra implementation (they were sufficient for the MyFaces team).

So the next logical step is to give you the small app itself so you can determine what other cases are failing. Note the small app works in the same way as the JAVASERVERFACES-1826 scenarios, and works fine under MyFaces.

I'm hopeful you can use this app to see what further regression tests you need to simulate.

Regards,

Richard.

Comment by Manfred Riem [ 18/Jul/12 ]

After working to make it compilable I now get the following stacktrace when I run your application:

javax.faces.FacesException: Cannot add the same component twice: form:j_id3
at com.sun.faces.context.StateContext$AddRemoveListener.handleAddRemoveWithAutoPrune(StateContext.java:476)
at com.sun.faces.context.StateContext$AddRemoveListener.handleAdd(StateContext.java:422)
at com.sun.faces.context.StateContext$AddRemoveListener.processEvent(StateContext.java:338)
at javax.faces.event.SystemEvent.processListener(SystemEvent.java:106)
at com.sun.faces.application.ApplicationImpl.processListenersAccountingForAdds(ApplicationImpl.java:2217)
at com.sun.faces.application.ApplicationImpl.invokeViewListenersFor(ApplicationImpl.java:2038)
at com.sun.faces.application.ApplicationImpl.publishEvent(ApplicationImpl.java:291)
at com.sun.faces.application.ApplicationImpl.publishEvent(ApplicationImpl.java:246)
at javax.faces.component.UIComponentBase.publishAfterViewEvents(UIComponentBase.java:2201)
at javax.faces.component.UIComponentBase.doPostAddProcessing(UIComponentBase.java:1883)
at javax.faces.component.UIComponentBase.setParent(UIComponentBase.java:400)
at org.metawidget.faces.renderkit.html.HtmlLayoutRenderer.createLabel(HtmlLayoutRenderer.java:175)
at org.metawidget.faces.renderkit.html.HtmlLayoutRenderer.layoutLabel(HtmlLayoutRenderer.java:147)
at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.layoutLabel(HtmlTableLayoutRenderer.java:456)
at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.layoutBeforeChild(HtmlTableLayoutRenderer.java:355)
at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.encodeChildren(HtmlTableLayoutRenderer.java:288)
at javax.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:845)
at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779)
at javax.faces.render.Renderer.encodeChildren(Renderer.java:168)
at javax.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:845)
at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779)
at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1782)
at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1782)
at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:424)
at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:125)
at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:121)
at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101)
at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:139)
at javax.faces.webapp.FacesServlet.service(FacesServlet.java:594)

Comment by kennardconsulting [ 19/Jul/12 ]

Manfred,

Thanks for taking the time to get the test working. I know this isn't your preferred approach! However the advantage is we no longer have a 'moving target' for our test case. This is important because the errors I see vary from implementation to implementation. For example:

MyFaces 2.0.7: works, no problems
Mojarra 2.1.9-20120524.180105-1: gets the 'Cannot add the same component twice: form:j_id3' error you describe
Mojarra 2.2.0-20120518.034337-98: almost works, but reshuffles field ordering (as described in steps 1-4 at top of this JIRA)

I hope this helps you debug it.

Regards,

Richard.

Comment by Manfred Riem [ 19/Jul/12 ]

OK these are the results after testing each version. Note that version 2.1.9 and upwards, and 2.2.0-m03 and upwards all signal that your code is trying to add the same component twice. The listener specifically guards against that since allowing it would violate the JSF specification.

Results per version
2.1.6 - search throws NullPointerException
2.1.7 - search shows not a valid option
2.1.8 - search shows not a valid option
2.1.9 - cannot add component twice
2.1.10 - cannot add component twice
2.1.11 - cannot add component twice
2.1.12-20120719.084854-6 - cannot add component twice

2.2.0-m01 - throws IllegalStateException
2.2.0-m02 - search throws NullPointerException
2.2.0-m03 - cannot add component twice
2.2.0-m04 - cannot add component twice
2.2.0-20120719.080026-131 - cannot add component twice

Can you investigate in your code how/why it is adding a component twice?

Comment by kennardconsulting [ 20/Jul/12 ]

Manfred,

Thanks again for your time on this!

I don't believe it is adding a component twice. I would base this on:

a) works fine in MyFaces 2.0.3 and above
b) works in Mojarra 2.2.0-20120518.034337-98.jar, apart form some re-ordering problems
c) we've seen 'duplicate component' error before when doing JAVASERVERFACES-1826, and it was invariably a bug in the ViewState code.

However, I'm very used to being wrong! So I've created a new version of metawidget-all.jar (attached) that outputs some System.out's for you. You'll see that the offending j_id3 is actually a UISelectItem:

Adding javax.faces.component.html.HtmlInputText@203ba002 with value binding #

{contactSearch.current.firstname}

and id contactSearchCurrentFirstname
Adding javax.faces.component.html.HtmlInputText@777490ad with value binding #

{contactSearch.current.surname}

and id contactSearchCurrentSurname
Adding javax.faces.component.UISelectItem@3cdc904a with id j_id2
Adding javax.faces.component.UISelectItem@a32087b with id j_id3
Adding javax.faces.component.UISelectItem@5acac877 with id j_id4
Adding javax.faces.component.html.HtmlSelectOneMenu@4271c5bc with value binding #

{contactSearch.current.type}

and id contactSearchCurrentType
Adding org.metawidget.faces.component.UIStub@ab245dc with value binding #

{contactSearch.current}

and id j_id5
Adding org.metawidget.faces.component.UIStub@3e087400 with value binding #

{contactSearch.results}

and id j_id6

It's not a 'duplicate' as such (it has a different id, and different itemValue) but there are multiple UISelectItems. Given UISelectItems has been a problem area, perhaps this is a clue?

Comment by kennardconsulting [ 20/Jul/12 ]

Version of Metawidget with extra debug

Comment by Manfred Riem [ 20/Jul/12 ]

Debugging through it now (thanks for telling me what the component type was) shows that the UISelectItem has the same id set as the label it is trying to add. I believe the problem stems from this block of code:

private void addSelectItem( UIComponent component, Object value, String label, UIMetawidget metawidget ) {

FacesContext context = FacesContext.getCurrentInstance();
Application application = context.getApplication();

UISelectItem selectItem = (UISelectItem) application.createComponent( UISelectItem.COMPONENT_TYPE );
selectItem.setId( context.getViewRoot().createUniqueId() );

It looks like you are assigning an id to the UISelectItem when you add it to the component tree. However you are asking the UIViewRoot for a unique id, whereas you should be asking the closest UINamingContainer for a unique id (which in this case is UIForm).

Can you please change that and try the application again?

Comment by ova2 [ 21/Jul/12 ]

Hi Manfred,

"It looks like you are assigning an id to the UISelectItem when you add it to the component tree. However you are asking the UIViewRoot for a unique id, whereas you should be asking the closest UINamingContainer for a unique id".

Really? I'm a component developer too and I always ask the UIViewRoot for an unique Id. Never had problems with that. Believe me, I did this many times and everything is fine until now for both impls. - Mojarra and MyFaces. I think context.getViewRoot().createUniqueId() is a quite proper approach. I did the same e.g. for UIParameter and UISelectItems (never tried UISelectItem, but UISelectItems is ok). Just one problem - I couldn't create nested components programmatically (issue about non unique Ids). For instance, if you try to create nested components (component + its child) in a TagHandler inside public void onComponentPopulated (...), you will face a "non unique Ids" exception. The best approach is to append some string to the original Id. Something like

public void onComponentPopulated(FaceletContext ctx, UIComponent c, UIComponent parent) {
if (!isNew(parent))

{ return; }

Application app = ctx.getFacesContext().getApplication();
RefObjectTypeInput refObjectTypeInput = (RefObjectTypeInput) c;
String id = refObjectTypeInput.getId();

// label
HtmlOutputText label = (HtmlOutputText) app.createComponent(HtmlOutputText.COMPONENT_TYPE);
label.setId(id + "_label");
...
refObjectTypeInput.getChildren().add(label);
}

This works smoothly.

Comment by kennardconsulting [ 22/Jul/12 ]

Manfred,

Thanks so much for diving into the Metawidget source code! I really appreciate you going the extra mile. I tried your suggestion and am delighted to report it makes an enormous difference. I'm still running tests but so far everything looks perfect!

However I agree, Ova2, this is surprising advice. It appears that being able to call createUniqueId from something other than UIViewRoot is new to JSF2? It's part of the new UniqueIdVendor interface?

Isn't this, therefore, a change in the spec that causes an incompatibility between JSF1 and JSF2? Because you're saying that whilst in JSF1 UIViewRoot.createUniqueId was sufficient for uniqueness, in JSF2 it is not?

Manfred: Can you please double-check UIViewRoot.createUniqueId shouldn't remain 'completely, heavyweight unique' whilst the new UniqueIdVendor offers some kind of 'local, more efficient, uniqueness'? The JavaDoc for UniqueIdVendor says "reduce the amount of id generation variance... helpful for improved state saving". The phrasing ('reduce', 'helpful', 'improved') seems to imply it is just an optimization rather than a requirement?

Also, the JSF 2.0 spec only ever talks about UIViewRoot.createUniqueId as far as I can see?

Comment by kennardconsulting [ 23/Jul/12 ]

Manfred,

After further testing, it appears there are some points of contention between how MyFaces and Mojarra handle this. There seem to be cases, when using MyFaces 2 with RichFaces Ajax processing, that UIForm.createUniqueId will just return 'j_id3' (or whatever) without also incrementing the global value returned by UIViewRoot.createUniqueId. This leads to duplicate id conflicts.

So I guess one of the teams has an incorrect implementation? Before I harrass the MyFaces guys, can you please double-check the spec permits UIViewRoot.createUniqueId to create non-unique Ids (in cases where there is a nested component that implements UniqueIdVendor)?

Comment by Ed Burns [ 23/Jul/12 ]

Hello Richard and Manfred,

Thanks to you both for helping to make JSF more robust. While it is
true that one can often get away with using UIViewRoot.createUniqueId()
as your one-stop-shop for unique ids anywhere in your app, the fact that
this works is dependent on the particular arrangement of components in a
view. Let's take a look at UIViewRoot.createUniqueId(), from JSF 1.2
[1].

public java.lang.String createUniqueId()

Generate an identifier for a component. The identifier will be
prefixed with UNIQUE_ID_PREFIX, and will be unique within this
UIViewRoot.

And let's also take a look at UIComponent.setId(), also from JSF 1.2
[2].

public abstract void setId(java.lang.String id)

[...]

Component identifiers must also obey the following semantic
restrictions (note that this restriction is NOT enforced by the
setId() implementation):

The specified identifier must be unique among all the components
(including facets) that are descendents of the nearest ancestor
UIComponent that is a NamingContainer, or within the scope of the
entire component tree if there is no such ancestor that is a
NamingContainer.

Manfred recommended this approach: If you must call setId() with an auto
generated value, always use the closest ancestor naming container to get
that value. Given the above constraints, this is the only way to
guarantee that the auto generated values will not collide.

I will add the following clarification to UIViewRoot.createUniqueId().

Previous text:

The identifier will be prefixed with UNIQUE_ID_PREFIX, and will be
unique within this UIViewRoot.

New text:

The identifier will be prefixed with UNIQUE_ID_PREFIX, and will be
unique within the non-NamingContainer children of UIViewRoot.

Ed

[1] https://maven.java.net/service/local/repositories/releases/archive/com/sun/faces/jsf-api/1.2/jsf-api-1.2-javadoc.jar/!/javax/faces/component/UIViewRoot.html#createUniqueId%28%29

[2] https://maven.java.net/service/local/repositories/releases/archive/com/sun/faces/jsf-api/1.2/jsf-api-1.2-javadoc.jar/!/javax/faces/component/UIComponent.html#setId%28java.lang.String%29

Comment by kennardconsulting [ 23/Jul/12 ]

Okay, I'm very happy with that.

How about we change the name of this issue to something like 'UIViewRoot.createUniqueId does not always create unique ids' (which, as it turned out, was the cause of all this) and then the resolution is simply a doc clarification?

So it appears that JAVASERVERFACES-1826 really did resolve the dynamic components issue, after all. Brilliant! Thank you guys so much for sticking with me and getting this working. I apologise for my impatient demeanor at times.

I'm still doing futher testing, but so far it is looking perfect. I'll open new issues as they arise and as I start porting more apps. But for now you can consider this issue, and JAVASERVERFACES-1826, closed.

Metawidget finally works properly on Mojarra 2!!!

Comment by Manfred Riem [ 24/Jul/12 ]

Closing as "Works as designed" but showing when the issue was really fixed as a consequence of fixing #1826. Note the requested JavaDoc clarification is being tracked as a spec issue.

Comment by arjan tijms [ 24/Jul/12 ]

After further testing, it appears there are some points of contention between how MyFaces and Mojarra handle this. There seem to be cases, when using MyFaces 2 with RichFaces Ajax processing, that UIForm.createUniqueId will just return 'j_id3' (or whatever) without also incrementing the global value returned by UIViewRoot.createUniqueId. This leads to duplicate id conflicts.

With the new insight, is this issue now also resolved? Or do you use UIViewroot for MyFaces and UIForm for Mojarra now?

Comment by Manfred Riem [ 24/Jul/12 ]

You should use the nearest UINamingContainer to get a truly unique id within the scope of a UINamingContainer. So you would walk up the tree, get the first UINamingContainer and ask it for a unique id. That should work in both MyFaces and Mojarra.

Comment by arjan tijms [ 24/Jul/12 ]

I hear you, that should work, but I'm still concerned about the statement kennardconsulting made that MyFaces in combination with RichFaces has a problem with that approach. If this is indeed true, then it's of course a MyFaces issue and neither a JSF Spec or Mojarra problem.

Comment by kennardconsulting [ 25/Jul/12 ]

I kind of dodged the issue, since I needed something that would work on JSF1 and JSF2, and didn't want to chase people any further. I'll leave that fight for somebody else

My 'dodge' is here (FacesUtils.createUniqueId):

https://github.com/metawidget/metawidget/blob/master/modules/faces/core/src/main/java/org/metawidget/faces/FacesUtils.java

It seems to work for my purposes, but I'd appreciate anybody's feedback if this is considered 'unsafe'.

Generated at Sun Feb 26 04:58:26 UTC 2017 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.