dev@javaserverfaces.java.net

[1200-PhaseListenerCompositeComponent] Seeking Review

From: Ed Burns <Ed.Burns_at_Sun.COM>
Date: Wed, 15 Jul 2009 14:32:43 -0700

<>>>>> On Tue, 14 Jul 2009 15:21:41 -0700, Ryan Lubke <Ryan.Lubke_at_Sun.COM> said:

RL> Hi Ed,
RL> Could you please take over implementation issue 1200 while I'm on
RL> vacation? I've done some basic root-cause analysis and have updated the
RL> issue with my findings.

Ryan's findings made it simple to fix this bug.

Can someone please review my change-bundle at
<https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1200>, and
included here for convenience.

Issue: 1200

As Ryan described in the bug, our normal trick of using
ComponentHandler.isNew(parent) to tell whether or not a TagHandler
should actually be applied does not work in the case of composite
components. Another approach is needed.


SECTION: Modified Files
----------------------------
M jsf-ri/src/com/sun/faces/application/view/FaceletViewHandlingStrategy.java

- create a static getter: isBuildingMetadata(FacesContext context) to
replace the need to manually lookup the FacesContext attr.

M jsf-ri/src/com/sun/faces/facelets/tag/jsf/core/PhaseListenerHandler.java

- Handle the composite component case.

M jsf-ri/src/com/sun/faces/facelets/tag/composite/InterfaceHandler.java

- Use the new isBuildingMetadata() static method

SECTION: Diffs
----------------------------
Index: jsf-ri/src/com/sun/faces/facelets/tag/jsf/core/PhaseListenerHandler.java
===================================================================
--- jsf-ri/src/com/sun/faces/facelets/tag/jsf/core/PhaseListenerHandler.java (revision 7554)
+++ jsf-ri/src/com/sun/faces/facelets/tag/jsf/core/PhaseListenerHandler.java (working copy)
@@ -51,6 +51,7 @@
 
 package com.sun.faces.facelets.tag.jsf.core;
 
+import com.sun.faces.application.view.FaceletViewHandlingStrategy;
 import com.sun.faces.facelets.tag.TagHandlerImpl;
 import com.sun.faces.facelets.tag.jsf.ComponentSupport;
 import com.sun.faces.facelets.util.ReflectionUtil;
@@ -66,6 +67,8 @@
 import javax.faces.view.facelets.*;
 import java.io.IOException;
 import java.io.Serializable;
+import java.util.HashMap;
+import java.util.Map;
 
 public class PhaseListenerHandler extends TagHandlerImpl {
 
@@ -158,7 +161,11 @@
 
     public void apply(FaceletContext ctx, UIComponent parent)
           throws IOException {
- if (ComponentHandler.isNew(parent)) {
+ FacesContext context = ctx.getFacesContext();
+ if (ComponentHandler.isNew(parent) ||
+ (FaceletViewHandlingStrategy.isBuildingMetadata(context) &&
+ !compositeComponentHasPhaseListenerForHandler(context, parent,
+ this))) {
             UIViewRoot root = ComponentSupport.getViewRoot(ctx, parent);
             if (root == null) {
                 throw new TagException(this.tag, "UIViewRoot not available");
@@ -173,4 +180,38 @@
             root.addPhaseListener(pl);
         }
     }
+
+
+
+ private boolean compositeComponentHasPhaseListenerForHandler(FacesContext context,
+ UIComponent parent, TagHandler tagHandler) {
+ boolean result = false;
+
+ // This method uses the the UIComponent parent argument as a key in
+ // contextAttrs. The value for such a key is a Map<TagHandler, Boolean>.
+ // If there is a non-null entry in this inner map for argument
+ // TagHandler tagHandler, we know that the composite component parent
+ // has already had this specific phaseListener tag applied to it.
+ Map<Object, Object> contextAttrs = context.getAttributes();
+
+ // If this composite component instance has already this tag
+ // instance applied...
+ if (contextAttrs.containsKey(parent) &&
+ ((Map<TagHandler, Boolean>)contextAttrs.get(parent)).containsKey(tagHandler)) {
+ // return false.
+ result = true;
+ } else {
+ // Otherwise return true and store the result for future queries
+ // on this request.
+ result = false;
+ Map<TagHandler, Boolean> innerMap;
+ if (null == (innerMap = (Map<TagHandler, Boolean>)contextAttrs.get(parent))) {
+ innerMap = new HashMap<TagHandler, Boolean>();
+ contextAttrs.put(parent, innerMap);
+ }
+ innerMap.put(tagHandler, Boolean.TRUE);
+ }
+
+ return result;
+ }
 }
Index: jsf-ri/src/com/sun/faces/facelets/tag/composite/InterfaceHandler.java
===================================================================
--- jsf-ri/src/com/sun/faces/facelets/tag/composite/InterfaceHandler.java (revision 7554)
+++ jsf-ri/src/com/sun/faces/facelets/tag/composite/InterfaceHandler.java (working copy)
@@ -83,7 +83,7 @@
         FacesContext context = ctx.getFacesContext();
         // only process if it's been created
         // Do not process if we're simply building metadata
- if (context.getAttributes().containsKey(FaceletViewHandlingStrategy.IS_BUILDING_METADATA)) {
+ if (FaceletViewHandlingStrategy.isBuildingMetadata(context)) {
             imbueComponentWithMetadata(ctx, parent);
             this.nextHandler.apply(ctx, parent);
         }
Index: jsf-ri/src/com/sun/faces/application/view/FaceletViewHandlingStrategy.java
===================================================================
--- jsf-ri/src/com/sun/faces/application/view/FaceletViewHandlingStrategy.java (revision 7554)
+++ jsf-ri/src/com/sun/faces/application/view/FaceletViewHandlingStrategy.java (working copy)
@@ -136,6 +136,13 @@
 
     }
 
+ // ------------------------------------------------------------ Constructors
+
+ public static boolean isBuildingMetadata(FacesContext context) {
+ boolean result = context.getAttributes().containsKey(FaceletViewHandlingStrategy.IS_BUILDING_METADATA);
+ return result;
+ }
+
     // ------------------------------------ Methods from ViewDeclarationLanguage
 
     @Override





-- 
| ed.burns_at_sun.com  | office: 408 884 9519 OR x31640
| homepage:         | http://ridingthecrest.com/