dev@javaserverfaces.java.net

Re: Seeking Review: 140-FacesContextScope

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Mon, 09 Jun 2008 08:57:08 -0700

Go ahead and check this in, but I will be making some changes:

 - getAttributes() implementation will be moved to FacesContext.java
   proper to avoid the proliferation of the impl dependency hacks
   (unless you saw some reason not to).
 - RequestStateManager will be removed in favor of leveraging
   the FacesContext attributes Map.



Ed Burns wrote:
> r= PENDING
>
> SECTION: API Changes
>
> M jsf-api/src/javax/faces/context/FacesContext.java
>
> - getAttributes() map.
>
> M jsf-api/src/javax/faces/component/UIComponent.java
> M jsf-api/src/javax/faces/component/UIComponentBase.java
> M jsf-api/src/javax/faces/webapp/UIComponentClassicTagBase.java
>
> - Leverage the facesContext attributes map instead of the request scope
> wherever possible.
>
> SECTION: Mojarra changes
>
> M jsf-ri/src/com/sun/faces/context/FacesContextImpl.java
> M jsf-ri/src/com/sun/faces/config/InitFacesContext.java
>
> - Implement the getAttributes() contract.
>
> M jsf-ri/src/com/sun/faces/renderkit/html_basic/StylesheetRenderer.java
> M jsf-ri/src/com/sun/faces/renderkit/html_basic/CommandLinkRenderer.java
> M jsf-ri/src/com/sun/faces/renderkit/html_basic/ScriptRenderer.java
>
> - Leverage the facesContext attributes map instead of the request scope
> wherever possible.
>
> M jsf-ri/src/com/sun/faces/el/ImplicitObjectELResolver.java
> M jsf-ri/src/com/sun/faces/el/ELConstants.java
>
> - Add "component" implicit object, instead of relying on scoped
> attribute lookup.
>
> M lib/jsf-extensions-test-time.jar
>
> - new test-time jar.
>
> SECTION: Diffs
>
> Index: jsf-api/src/javax/faces/context/FacesContext.java
> ===================================================================
> --- jsf-api/src/javax/faces/context/FacesContext.java (revision 4881)
> +++ jsf-api/src/javax/faces/context/FacesContext.java (working copy)
> @@ -93,8 +93,70 @@
> * this instance has been released
> */
> public abstract Application getApplication();
> +
> +
> + /**
> + * <p class="changed_added_2_0">Return a mutable <code>Map</code>
> + * representing the attributes associated wth this
> + * <code>FacesContext</code> instance. This <code>Map</code> is
> + * useful to store attributes that you want to go out of scope when the
> + * Faces lifecycle for the current request ends, which is not always the same
> + * as the request ending, especially in the case of Servlet filters
> + * that are invoked <strong>after</strong> the Faces lifecycle for this
> + * request completes. Accessing this <code>Map</code> does not cause any
> + * events to fire, as is the case with the other maps: for request, session, and
> + * application scope.</p>
> + *
> + * <div class="changed_added_2_0">
> + *
> + * <p>The <code>Map</code> returned by this method is not associated with
> + * the request. If you would like to get or set request attributes,
> + * see {_at_link ExternalContext#getRequestMap}. The
> + * returned implementation must support all of the
> + * standard and optional <code>Map</code> methods, plus support the
> + * following additional requirements:</p>
> + *
> + * <ul>
>
> + * <li><p>The Map implementation must implement the
> + * <code>java.io.Serializable</code> interface.</p></li>
>
> + * <li><p>Any attempt to add a <code>null</code> key or value must throw a
> + * <code>NullPointerException</code>.</p></li>
> +
> + * <li><p>The <code>Map</code> must be cleared when the {_at_link #release}
> + * is called.</p></li>
> + *
> + * </ul>
> + *
> + * <p>The default implementation throws
> + * <code>UnsupportedOperationException</code> and is provided
> + * for the sole purpose of not breaking existing applications that extend
> + * this class.</p>
> + *
> + * </div>
> + *
> + * @throws IllegalStateException if this method is called after
> + * this instance has been released
> + *
> + * @since 2.0
> + */
> +
> + public Map<Object, Object> getAttributes() {
> +
> + Map m = (Map) getExternalContext().getRequestMap().get("com.sun.faces.util.RequestStateManager");
> + if (m != null) {
> + FacesContext impl = (FacesContext) m.get("com.sun.faces.FacesContextImpl");
> + if (impl != null) {
> + return impl.getAttributes();
> + } else {
> + throw new UnsupportedOperationException();
> + }
> + }
> + throw new UnsupportedOperationException();
> + }
> +
> +
> /**
> * <p>Return an <code>Iterator</code> over the client identifiers for
> * which at least one {_at_link javax.faces.application.FacesMessage} has been queued. If there are no
> Index: jsf-api/src/javax/faces/component/UIComponent.java
> ===================================================================
> --- jsf-api/src/javax/faces/component/UIComponent.java (revision 4881)
> +++ jsf-api/src/javax/faces/component/UIComponent.java (working copy)
> @@ -1047,9 +1047,9 @@
> */
> protected void pushComponentToEL(FacesContext context) {
>
> - Map<String,Object> requestMap = context.getExternalContext().getRequestMap();
> - if (requestMap != null) {
> - previouslyPushed = (UIComponent) requestMap.put("component", this);
> + Map<Object,Object> contextMap = context.getAttributes();
> + if (contextMap != null) {
> + previouslyPushed = (UIComponent) contextMap.put("component", this);
> }
>
> }
> @@ -1063,12 +1063,12 @@
> */
> protected void popComponentFromEL(FacesContext context) {
>
> - Map<String,Object> requestMap = context.getExternalContext().getRequestMap();
> - if (requestMap != null) {
> + Map<Object,Object> contextMap = context.getAttributes();
> + if (contextMap != null) {
> if (previouslyPushed != null) {
> - requestMap.put("component", previouslyPushed);
> + contextMap.put("component", previouslyPushed);
> } else {
> - requestMap.remove("component");
> + contextMap.remove("component");
> }
> }
>
> @@ -1089,8 +1089,8 @@
> public static UIComponent getCurrentComponent() {
>
> FacesContext context = FacesContext.getCurrentInstance();
> - Map<String, Object> requestMap = context.getExternalContext().getRequestMap();
> - return (UIComponent) requestMap.get("component");
> + Map<Object, Object> contextMap = context.getAttributes();
> + return (UIComponent) contextMap.get("component");
>
> }
>
> Index: jsf-api/src/javax/faces/component/UIComponentBase.java
> ===================================================================
> --- jsf-api/src/javax/faces/component/UIComponentBase.java (revision 4881)
> +++ jsf-api/src/javax/faces/component/UIComponentBase.java (working copy)
> @@ -2058,22 +2058,22 @@
> private static final String IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME = "com.sun.faces.IS_POSTBACK_AND_RESTORE_VIEW";
>
> private void clearPostbackAndRestoreViewCache(FacesContext context) {
> - Map<String, Object> requestMap = context.getExternalContext().getRequestMap();
> - requestMap.remove(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME);
> + Map<Object, Object> contextMap = context.getAttributes();
> + contextMap.remove(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME);
>
> }
>
> private boolean isPostbackAndRestoreView(FacesContext context) {
> boolean result = false;
> - Map<String, Object> requestMap = context.getExternalContext().getRequestMap();
> - if (requestMap.containsKey(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME)) {
> - result = Boolean.TRUE == requestMap.get(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME) ? true : false;
> + Map<Object, Object> contextMap = context.getAttributes();
> + if (contextMap.containsKey(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME)) {
> + result = Boolean.TRUE == contextMap.get(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME) ? true : false;
> }
> else {
> result = getResponseStateManager(context,
> context.getViewRoot().getRenderKitId()).isPostback(context) &&
> context.getCurrentPhaseId().equals(PhaseId.RESTORE_VIEW);
> - requestMap.put(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME,
> + contextMap.put(IS_POSTBACK_AND_RESTORE_VIEW_REQUEST_ATTR_NAME,
> result ? Boolean.TRUE : Boolean.FALSE);
>
> }
> Index: jsf-api/src/javax/faces/webapp/UIComponentClassicTagBase.java
> ===================================================================
> --- jsf-api/src/javax/faces/webapp/UIComponentClassicTagBase.java (revision 4881)
> +++ jsf-api/src/javax/faces/webapp/UIComponentClassicTagBase.java (working copy)
> @@ -154,8 +154,8 @@
>
> // ------------------------------------------------------ Manifest Constants
> /**
> - * <p>The request scope attribute under which a component tag stack
> - * for the current request will be maintained.</p>
> + * <p>The facesContext scope attribute under which a component tag stack
> + * for the current facesContext will be maintained.</p>
> */
> private static final String COMPONENT_TAG_STACK_ATTR =
> "javax.faces.webapp.COMPONENT_TAG_STACK";
> @@ -207,7 +207,7 @@
> UIViewRoot.UNIQUE_ID_PREFIX + '_';
>
> /**
> - * Used to store the previousJspId Map in requestScope
> + * Used to store the previousJspId Map in facesContextScope
> */
> private static final String PREVIOUS_JSP_ID_SET =
> "javax.faces.webapp.PREVIOUS_JSP_ID_SET";
> @@ -223,7 +223,7 @@
> "javax.faces.webapp.PAGECONTEXT_MARKER";
>
> /**
> - * This is a <code>request</code> scoped attribute which contains
> + * This is a <code>facesContext</code> scoped attribute which contains
> * an AtomicInteger which we use to increment the PageContext
> * count.
> */
> @@ -686,7 +686,7 @@
> public static UIComponentClassicTagBase getParentUIComponentClassicTagBase(PageContext context) {
>
> FacesContext facesContext = getFacesContext(context);
> - List list = (List) facesContext.getExternalContext().getRequestMap()
> + List list = (List) facesContext.getAttributes()
> .get(COMPONENT_TAG_STACK_ATTR);
>
> if (list != null) {
> @@ -749,9 +749,9 @@
> * stack, deleting the stack if this was the last entry.</p>
> */
> private void popUIComponentClassicTagBase() {
> - Map<String, Object> requestMap =
> - context.getExternalContext().getRequestMap();
> - List list = (List) requestMap.get(COMPONENT_TAG_STACK_ATTR);
> + Map<Object, Object> contextMap =
> + context.getAttributes();
> + List list = (List) contextMap.get(COMPONENT_TAG_STACK_ATTR);
>
> // if an exception occurred in a nested tag,
> //there could be a few tags left in the stack.
> @@ -761,7 +761,7 @@
> uic = (UIComponentClassicTagBase) list.get(idx);
> list.remove(idx);
> if (idx < 1) {
> - requestMap.remove(COMPONENT_TAG_STACK_ATTR);
> + contextMap.remove(COMPONENT_TAG_STACK_ATTR);
> list = null;
> }
> }
> @@ -774,14 +774,14 @@
> */
> private void pushUIComponentClassicTagBase() {
>
> - Map<String,Object> requestMap =
> - context.getExternalContext().getRequestMap();
> + Map<Object,Object> contextMap =
> + context.getAttributes();
> List<UIComponentClassicTagBase> list = TypedCollections.dynamicallyCastList((List)
> - requestMap.get(COMPONENT_TAG_STACK_ATTR), UIComponentClassicTagBase.class);
> + contextMap.get(COMPONENT_TAG_STACK_ATTR), UIComponentClassicTagBase.class);
> if (list == null) {
> //noinspection CollectionWithoutInitialCapacity
> list = new ArrayList<UIComponentClassicTagBase>();
> - requestMap.put(COMPONENT_TAG_STACK_ATTR, list);
> + contextMap.put(COMPONENT_TAG_STACK_ATTR, list);
> }
> list.add(this);
>
> @@ -1103,16 +1103,16 @@
> }
>
> parentTag = getParentUIComponentClassicTagBase(pageContext);
> - Map<String,Object> requestMap = context.getExternalContext().getRequestMap();
> + Map<Object,Object> contextMap = context.getAttributes();
> Map<String,UIComponentTagBase> componentIds;
> if (parentTag == null) {
> // create the map if we're the top level UIComponentTag
> //noinspection CollectionWithoutInitialCapacity
> componentIds = new HashMap<String,UIComponentTagBase>();
> - requestMap.put(GLOBAL_ID_VIEW, componentIds);
> + contextMap.put(GLOBAL_ID_VIEW, componentIds);
> } else {
> componentIds = TypedCollections.dynamicallyCastMap((Map)
> - requestMap.get(GLOBAL_ID_VIEW), String.class, UIComponentTagBase.class);
> + contextMap.get(GLOBAL_ID_VIEW), String.class, UIComponentTagBase.class);
> }
>
> // If we're not inside of a facet, and if we are inside of a
> @@ -1539,14 +1539,14 @@
> * @return String <code>id</code> with a counter appended to it.
> */
> private String generateIncrementedId (String componentId) {
> - Map<String,Object> requestMap = getFacesContext().getExternalContext().getRequestMap();
> - Integer serialNum = (Integer) requestMap.get(componentId);
> + Map<Object,Object> contextMap = getFacesContext().getAttributes();
> + Integer serialNum = (Integer) contextMap.get(componentId);
> if (null == serialNum) {
> serialNum = 1;
> } else {
> serialNum = serialNum.intValue() + 1;
> }
> - requestMap.put(componentId, serialNum);
> + contextMap.put(componentId, serialNum);
> componentId = componentId + UNIQUE_ID_PREFIX + serialNum.intValue();
> return componentId;
> }
> @@ -1669,12 +1669,12 @@
> // to check the ID after the tag has been used
> this.jspId = null;
>
> - Map<String,Object> reqMap =
> - getFacesContext().getExternalContext().getRequestMap();
> - AtomicInteger aInt = (AtomicInteger) reqMap.get(JAVAX_FACES_PAGECONTEXT_COUNTER);
> + Map<Object,Object> contextMap =
> + getFacesContext().getAttributes();
> + AtomicInteger aInt = (AtomicInteger) contextMap.get(JAVAX_FACES_PAGECONTEXT_COUNTER);
> if (aInt == null) {
> aInt = new AtomicInteger();
> - reqMap.put(JAVAX_FACES_PAGECONTEXT_COUNTER, aInt);
> + contextMap.put(JAVAX_FACES_PAGECONTEXT_COUNTER, aInt);
> }
>
> Integer pcId = (Integer)
> Index: jsf-ri/src/com/sun/faces/context/FacesContextImpl.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/context/FacesContextImpl.java (revision 4881)
> +++ jsf-ri/src/com/sun/faces/context/FacesContextImpl.java (working copy)
> @@ -71,6 +71,7 @@
> import com.sun.faces.util.FacesLogger;
> import com.sun.faces.util.RequestStateManager;
> import com.sun.faces.util.Util;
> +import java.util.HashMap;
>
> public class FacesContextImpl extends FacesContext {
>
> @@ -91,6 +92,7 @@
> private Severity maxSeverity;
> private boolean renderResponse = false;
> private boolean responseComplete = false;
> + private Map<Object, Object> attributes = null;
>
> /**
> * Store mapping of clientId to ArrayList of FacesMessage
> @@ -141,7 +143,21 @@
> return application;
> }
>
> + @Override
> + public Map<Object, Object> getAttributes() {
> +
> + assertNotReleased();
> +
> + if (null == attributes) {
> + attributes = new HashMap<Object,Object>();
> + }
> +
> + return attributes;
> + }
> +
> +
>
> +
> /**
> * @see javax.faces.context.FacesContext#getELContext()
> */
> @@ -392,6 +408,11 @@
> renderResponse = false;
> responseComplete = false;
> viewRoot = null;
> +
> + if (null != attributes) {
> + attributes.clear();
> + attributes = null;
> + }
>
> // PENDING(edburns): write testcase that verifies that release
> // actually works. This will be important to keep working as
> Index: jsf-ri/src/com/sun/faces/renderkit/html_basic/StylesheetRenderer.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/renderkit/html_basic/StylesheetRenderer.java (revision 4881)
> +++ jsf-ri/src/com/sun/faces/renderkit/html_basic/StylesheetRenderer.java (working copy)
> @@ -95,17 +95,17 @@
> throws IOException {
>
> Map<String,Object> attributes = component.getAttributes();
> - Map<String, Object> requestMap = context.getExternalContext().getRequestMap();
> + Map<Object, Object> contextMap = context.getAttributes();
>
> String name = (String) attributes.get("name");
> String library = (String) attributes.get("library");
> String key = name + library;
>
> // Ensure this stylesheet is not rendered more than once per request
> - if (requestMap.containsKey(key)) {
> + if (contextMap.containsKey(key)) {
> return;
> }
> - requestMap.put(key, Boolean.TRUE);
> + contextMap.put(key, Boolean.TRUE);
>
> Resource resource = context.getApplication().getResourceHandler()
> .createResource(name, library);
> Index: jsf-ri/src/com/sun/faces/renderkit/html_basic/CommandLinkRenderer.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/renderkit/html_basic/CommandLinkRenderer.java (revision 4881)
> +++ jsf-ri/src/com/sun/faces/renderkit/html_basic/CommandLinkRenderer.java (working copy)
> @@ -297,7 +297,7 @@
> */
> private static boolean hasScriptBeenRendered(FacesContext context) {
>
> - return (context.getExternalContext().getRequestMap()
> + return (context.getAttributes()
> .get(SCRIPT_STATE) != null);
>
> }
> @@ -311,7 +311,7 @@
> */
> private static void setScriptAsRendered(FacesContext context) {
>
> - context.getExternalContext().getRequestMap()
> + context.getAttributes()
> .put(SCRIPT_STATE, Boolean.TRUE);
>
> }
> Index: jsf-ri/src/com/sun/faces/renderkit/html_basic/ScriptRenderer.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/renderkit/html_basic/ScriptRenderer.java (revision 4881)
> +++ jsf-ri/src/com/sun/faces/renderkit/html_basic/ScriptRenderer.java (working copy)
> @@ -106,7 +106,7 @@
> throws IOException {
>
> Map<String,Object> attributes = component.getAttributes();
> - Map<String, Object> requestMap = context.getExternalContext().getRequestMap();
> + Map<Object, Object> contextMap = context.getAttributes();
>
> String name = (String) attributes.get("name");
> String library = (String) attributes.get("library");
> @@ -114,10 +114,10 @@
> String key = name + library;
>
> // Ensure this script is not rendered more than once per request
> - if (requestMap.containsKey(key)) {
> + if (contextMap.containsKey(key)) {
> return;
> }
> - requestMap.put(key, Boolean.TRUE);
> + contextMap.put(key, Boolean.TRUE);
>
> Resource resource = context.getApplication().getResourceHandler()
> .createResource(name, library);
> Index: jsf-ri/src/com/sun/faces/el/ImplicitObjectELResolver.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/el/ImplicitObjectELResolver.java (revision 4881)
> +++ jsf-ri/src/com/sun/faces/el/ImplicitObjectELResolver.java (working copy)
> @@ -60,7 +60,7 @@
> public class ImplicitObjectELResolver extends ELResolver implements ELConstants{
>
> static final String[] IMPLICIT_OBJECTS = new String[] {
> - "application", "applicationScope", "cookie", "facesContext",
> + "application", "applicationScope", "component", "cookie", "facesContext",
> "header", "headerValues", "initParam", "param", "paramValues",
> "request", "requestScope", "resource", "session", "sessionScope",
> "view", "viewScope" };
> @@ -95,6 +95,9 @@
> case APPLICATION_SCOPE:
> context.setPropertyResolved(true);
> return extCtx.getApplicationMap();
> + case COMPONENT:
> + context.setPropertyResolved(true);
> + return facesContext.getAttributes().get("component");
> case COOKIE:
> context.setPropertyResolved(true);
> return extCtx.getRequestCookieMap();
> Index: jsf-ri/src/com/sun/faces/el/ELConstants.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/el/ELConstants.java (revision 4881)
> +++ jsf-ri/src/com/sun/faces/el/ELConstants.java (working copy)
> @@ -47,32 +47,34 @@
>
> public static final int APPLICATION_SCOPE = 1;
>
> - public static final int COOKIE = 2;
> + public static final int COMPONENT = 2;
>
> - public static final int FACES_CONTEXT = 3;
> + public static final int COOKIE = 3;
>
> - public static final int HEADER = 4;
> + public static final int FACES_CONTEXT = 4;
>
> - public static final int HEADER_VALUES = 5;
> + public static final int HEADER = 5;
>
> - public static final int INIT_PARAM = 6;
> + public static final int HEADER_VALUES = 6;
>
> - public static final int PARAM = 7;
> + public static final int INIT_PARAM = 7;
>
> - public static final int PARAM_VALUES = 8;
> + public static final int PARAM = 8;
>
> - public static final int REQUEST = 9;
> + public static final int PARAM_VALUES = 9;
>
> - public static final int REQUEST_SCOPE = 10;
> + public static final int REQUEST = 10;
>
> - public static final int RESOURCE = 11;
> + public static final int REQUEST_SCOPE = 11;
>
> - public static final int SESSION = 12;
> + public static final int RESOURCE = 12;
>
> - public static final int SESSION_SCOPE = 13;
> + public static final int SESSION = 13;
>
> - public static final int VIEW = 14;
> + public static final int SESSION_SCOPE = 14;
>
> - public static final int VIEW_SCOPE = 15;
> + public static final int VIEW = 15;
> +
> + public static final int VIEW_SCOPE = 16;
>
> }
> Index: jsf-ri/src/com/sun/faces/config/InitFacesContext.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/config/InitFacesContext.java (revision 4881)
> +++ jsf-ri/src/com/sun/faces/config/InitFacesContext.java (working copy)
> @@ -70,6 +70,7 @@
> private ExternalContext ec;
> private UIViewRoot viewRoot;
> private FacesContext orig;
> + private Map<Object,Object> attributes;
>
> public InitFacesContext(ServletContext sc) {
> ec = new ServletContextAdapter(sc);
> @@ -82,6 +83,17 @@
> FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY);
> return factory.getApplication();
> }
> +
> + @Override
> + public Map<Object, Object> getAttributes() {
> +
> + if (null == attributes) {
> + attributes = new HashMap<Object,Object>();
> + }
> +
> + return attributes;
> + }
> +
>
> public Iterator<String> getClientIdsWithMessages() {
> List<String> list = Collections.emptyList();
> @@ -143,6 +155,10 @@
>
> public void release() {
> setCurrentInstance(orig);
> + if (null != attributes) {
> + attributes.clear();
> + attributes = null;
> + }
> }
>
> public void renderResponse() { }
>